Make the surface layer kind-agnostic#642
Open
jeremybower wants to merge 13 commits into
Open
Conversation
|
Thanks, this now meets the contribution policy. I've cleared the |
github-actions
Bot
added
the
invalid
jeremybower
changed the title
jeremybower
force-pushed
the
feature/agnostic-surface-actions
branch
from
6188acf to
7fe9ec7
Compare
github-actions
Bot
removed
the
invalid
Generalize the per-tab `SplitTree` from holding `GhosttySurfaceView`
leaves to a content-agnostic `SurfaceView` base, so the tree, focus,
occlusion, drag/drop, progress, and AX machinery no longer assume the
terminal is the only kind of pane a split can hold.
- Add `SurfaceView`: an `NSView, Identifiable` base whose abstract
`content: SurfaceContent` is overridden by each concrete leaf. The
leaf *is* the real content view (first responder, drag source, AX
node), so there is no wrapper box. `id` and the default frame move up
from `GhosttySurfaceView` into the base `init(id:)`.
- Add the `SurfaceContent` enum (`case terminal(GhosttySurfaceView)`
today). Kind-specific code routes through a `switch` on `content`, so
adding a leaf kind breaks the build at every site that must handle it
instead of silently failing an `as?` downcast.
- `GhosttySurfaceView` now subclasses `SurfaceView`, overrides `content`
to return `.terminal(self)`, and drops its own `id` / frame setup.
- Retype `SplitTree<GhosttySurfaceView>` to `SplitTree<SurfaceView>`
throughout `WorktreeTerminalState` and `TerminalSplitTreeView`
(trees map, tab creation, layout capture/restore, split direction
mapping, drop zones, AX panes). Terminal-only call sites unwrap via
`switch leaf.content { case .terminal(let surface): ... }`.
- Add `focusLeaf(_:in:)` to route leaf focus through `content` while the
internal `surfaces` map and `focusSurface` stay concretely terminal.
- Add the `SurfaceView.terminalForTesting` helper and update the
terminal test suites to create/inspect leaves through it.
Move the "reclaim firstResponder when my view re-attaches to a window" machinery out of GhosttySurfaceView and into the SurfaceView base so any future leaf subclass inherits it. Behavior-preserving for the terminal. The base now owns onFocusChange/shouldClaimFocus, hasBeenInWindow, pendingFocusClaim, and the viewDidMoveToWindow reclaim logic, parameterized by two overridable points (focusResponder, claimsFocusOnFirstMount) and a subtree-aware steal-guard (surfaceAncestor(of:)). The detach-time focus-loss hook routes through an overridable surfaceDidDetachFromWindow(), which GhosttySurfaceView overrides to call focusDidChange(false). GhosttySurfaceView keeps the defaults (it is its own first responder, claims only on re-attach) so the terminal behaves identically.
Replace the split-pane drag-and-drop with an all-AppKit implementation that sits above the split-tree leaves, so concrete SurfaceViews carry no rearrange code — a future web-view leaf inherits it for free. - SurfaceDragCoordinator (@observable, owned by WorktreeTerminalState) holds the in-flight drag and forwards a landed drop to performSplitOperation. - SurfaceDragHandleView is an NSDraggingSource: it paints the OS floating drag image, and its willBegin/endedAt callbacks toggle the coordinator's drag-in-flight state — a reliable signal (including cancel) that SwiftUI .onDrag lacks. - SurfaceDropCatcherView is a real AppKit NSView mounted in front of each pane for the pane's lifetime, registered solely for the private surface drag type. It is mounted persistently — not only during a drag — so its drag-type registration is in place before a session starts; AppKit acquires its drop-target set at session start and can miss a target that registers mid-drag. It stays click-through (hitTest) until a drag is in flight so it never intercepts normal clicks, then wins the drag even over a greedy child view (e.g. a future WKWebView) — while a content drop (a Finder file) carries a disjoint type and falls through to the surface below. Drops resolve through the tab's SplitTree<SurfaceView> + focusLeaf, so the rearrange is content-agnostic end to end. The terminal's own file/text drop is unchanged. Unit tests cover the coordinator lifecycle, payload parsing, and the tree mutation; the live AppKit drag was verified manually in the dev app.
createTab/createTabWithInput fuse into one createTab carrying a SurfaceSpec; splitSurface's raw input moves into the same payload. Which kind of surface to create is now data on the neutral creation verbs instead of verb spelling, so a future kind adds a spec case rather than new commands.
TerminalClient.Command keeps only neutral lifecycle verbs; scripts, binding actions, and scrollback search move into TerminalSurfaceCommand behind a single Command.terminal(Worktree, _) arm the manager dispatches to a terminal-kind handler. Event mirrors it: taskStatusChanged, blockingScriptCompleted, setupScriptConsumed, agentHookEventReceived, and the has-any-surface aggregate ride Event.terminal(_). The exhaustive switch on the wrapper arm is the registration point for a future kind. On the reducer side, AppFeature handles the wrapper through a single .terminalEvent(.terminal(_)) arm that delegates to handleTerminalSurfaceEvent; neutral lifecycle / projection arms keep their own arms. A future kind adds a sibling handler behind its own wrapper arm.
WorktreeTerminalState's generic core (tree / tab / focus / occlusion / zoom / persistence scheduling) no longer declares terminal-kind state. The Ghostty view map, zmx launch/session bookkeeping, blocking-script tracking, setup-script flag, environment injection, and agent-OSC notification dedupe now live in TerminalSurfaceStore, together with the launch-resolution and environment helpers that only touch that state. Private accessors keep the orchestration bodies unchanged, so the diff is the ownership move, not a rewrite. Per-kind stores are keyed by surface/tab ID and empty when the kind is absent, so additional kinds don't multiply per-worktree state.
TerminalLayoutSnapshot's leaf becomes enum SurfaceSnapshot with a terminal case carrying the old payload (now TerminalSurfaceSnapshot). The terminal case still encodes the legacy flat shape, so layouts round-trip with older builds; a future kind writes a `kind` tag this build rejects, and the now-lossy tabs decode drops just the affected tab instead of the whole layout. Restore and the leaf walkers dispatch on the kind exhaustively. Because the lossy decode is positional, the persisted selectedTabIndex stays in the original tabs array's coordinates, so dropping an unknown-kind tab that precedes the selection would restore the wrong tab. Decode therefore reports dropped indices (a new decodeLossyArrayWithDroppedIndices helper alongside decodeLossyArrayIfPresent) and shifts the index left by the number of dropped tabs before it.
Mechanical rename, no behavior change: TerminalClient → SurfaceClient (with the terminalClient dependency and the .terminalEvent action following), WorktreeTerminalManager → WorktreeSurfaceManager, and WorktreeTerminalState → WorktreeSurfaceState — the old per-surface WorktreeSurfaceState observable becomes SurfaceIndicatorState to free the name. Docs updated to match. Tab-layer names (TerminalTabID, TerminalTabManager, TerminalLayoutSnapshot, the tab/split views) stay terminal-flavored until tab creation is kind-dispatched end to end;
handleTerminalSurfaceCommand hoisted state(for:) above the switch, so .stopRunScript / .stopScript minted a WorktreeSurfaceState for a worktree that had none before stopBlockingScripts could take its stateIfExists path. The lookup now happens inside each arm, only where creating state is legitimate.
createTab and splitSurface were the same operation — materialize a new leaf from a SurfaceSpec — differing only in where the leaf lands. SurfaceClient.Command.create(worktree, spec:placement:id:) replaces both; SurfacePlacement says .tab or .adjacent(tabID:anchor:direction:), so a future kind composes with every placement without new verbs. The .adjacent direction is a neutral 4-way enum mapped onto Ghostty's split primitive at the manager boundary. The old splitSurface payload carried the 2-way persisted SplitDirection with the placement hardcoded (vertical → down, else right), which made left/up splits unexpressible through the neutral verb. The persisted SplitDirection and the CLI wire format are untouched: snapshots keep encoding axis + child order, and deeplink h/v still map to right/down at the handler. Per-arm behavior is preserved: tab creation stays async for the setup-script read, a failed adjacent placement with an explicit id still emits surfaceCreationFailed, and .adjacent still selects the target tab first.
The File menu's split arm and the tab-bar split buttons went through the terminal-kind binding channel (performBindingAction with a new_split string), whose Ghostty round-trip contributes nothing for splits — Ghostty parses the string and bounces it straight back via bridge.onSplitAction. Both now dispatch .create with an .adjacent placement, resolving the anchor from selectedTabID / selectedSurfaceID at dispatch, which also gives menu splits the full 4-way range through the neutral verb. The input path is untouched: Ghostty-originated keystrokes on a focused terminal still arrive via bridge.onSplitAction so custom Ghostty keybindings keep working, ghosttyBinding stays for mirroring those bindings as menu shortcut displays, and performBindingAction remains for genuinely Ghostty-owned verbs. Bindings are the input path; verbs are the dispatch path for everything the app initiates.
restoreFromSnapshot and createRestorationSplit each unwrapped the persisted SurfaceSnapshot to its terminal payload and called createSurface with the same argument shape. A private makeSurface(from:) is now the single kind dispatch for snapshot restore, and the restore walk (restoreLayoutNode / createRestorationSplit anchors) is typed against SurfaceView, so a future snapshot kind registers in exactly one place. No behavior change.
The terminal-kind command handler and its stop helper lived as private funcs on the neutral WorktreeSurfaceManager, so each new kind would have grown the shared file by a handler's worth of kind code. They now live on TerminalSurfaceManager, a kind-level coordinator the core routes to without interpreting the payload; a future kind adds a sibling manager and one dispatch arm. One coordinator serves every worktree: commands arrive addressed by Worktree, and per-worktree resolution stays behind the core's state(for:) / stateIfExists(for:) split so the stop arms keep acting on existing state without minting any. forceEmitProjection widens to internal for the stale-mirror reconcile path.
jeremybower
force-pushed
the
feature/agnostic-surface-actions
branch
from
7fe9ec7 to
89c90f4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #641
Summary
Foundation track of #500, superseding #536 (whose three commits are the base of this branch). Makes the tab/split/lifecycle layer speak surface-neutral verbs so a surface kind other than a terminal is a first-class citizen, per the bar agreed in #500: the shared layer never enumerates kinds, absent kinds cost nothing per worktree, and a third kind touches only its own module plus registration cases.
No user-visible change — terminal behavior stays identical, and the PR is reviewable purely as "terminal behavior unchanged + is this the right seam."
Commit-by-commit (each stands alone):
SurfaceViewwith a kind-typedcontentenum;GhosttySurfaceViewis one concrete kind.SurfaceClient.Command/Eventkeep neutral lifecycle verbs; scripts, binding actions, and search ride a singleterminal(...)arm, mirrored by one delegate arm inAppFeature.selectedTabIndexremapped past dropped tabs.Scope calls worth flagging for review
TerminalSurfaceStore; the tree/tab/focus orchestration bodies stay on the shared class, reading the store through private accessors. The exhaustiveSurfaceContentswitches there are the sanctioned registration points — moving the orchestration itself would have turned a reviewable ownership move into a rewrite.SurfaceClient,WorktreeSurfaceManager,WorktreeSurfaceState,SurfaceIndicatorState). Tab-layer names (TerminalTabID,TerminalTabManager,TerminalLayoutSnapshot, tab/split views) stay terminal-flavored until tab creation is kind-dispatched end to end; renaming them now would drown the diff.TerminalSurfaceStoreeagerly. Accepted: the empty store is a handful of empty dictionaries, and every worktree hosts terminal surfaces today. If a kind ever becomes genuinely optional per worktree, make its store lazy then.Type of change
ready)How was this tested?
Rebased on current
mainand verified end to end:make checkpasses (format + lint)make testpassesNotes: the full suite is green except
PullRequestMergeQueueStatusTests/surfacesPositionAndEstimatedTime, which fails onmaintoo under non-US English locales and is fixed by #632. Three unrelated tests flaked once under full parallel load and pass deterministically in isolation.Checklist
Closes #above.ready.