Conversation
SurfacePool was fully wired but permanently disabled behind programaSurfacePoolEnabled, a UserDefaults flag with no writer anywhere in the repo (verified via repo-wide grep including tests_v2/, CLI/, daemon/). The claim()/warmIfNeeded() calls always no-op, so this collapses each call site to the only path that ever ran: - TabManager.swift: drop the canUsePool/claim branch, call makeWorkspaceForCreation directly (the only reachable path). - AppDelegate.swift: drop the teardownAll() call at app termination. - GhosttyTerminalView.swift: drop the dead pre-warm DispatchQueue.main.async block after app init. Also removes the pbxproj entries for the deleted file. Refs #89.
Move TabItemView and its exclusive helper-view cluster (SidebarWorkspaceDescriptionText, SidebarMarkdownRenderer, SidebarMetadataRows/EntryRow/MarkdownBlocks/BlockRow) out of ContentView.swift into a new file, verbatim. Access-level widening: TabItemView was file-private (private struct); widened to internal (dropped the private modifier) because VerticalTabsSidebar, which stays in ContentView.swift, constructs it at its ForEach call site. No other behavior change. Equatable conformance, precomputed let parameters, and the .equatable() call site are untouched. Refs #94.
…move) Move the FileDropOverlayView NSView class out of ContentView.swift into its own file, verbatim (no access-level changes needed — it was already internal). Refs #94.
…anical move) Move the command-palette fuzzy-search engine (switcher search metadata/indexer, fuzzy matcher, search corpus entry/result types, search engine) out of ContentView.swift into its own file, verbatim. Zero coupling to ContentView state. Refs #94.
TerminalSurface.desiredFocusState and GhosttyNSView.desiredFocus both track intended focus, and could diverge silently. The concrete bug: clearSuppressReparentFocus() called recordExternalFocusState(true) unconditionally, then (on the immediate-apply branch) called reassertTerminalSurfaceFocus() -> setFocus(true). setFocus's dedup guard (focused != desiredFocusState) saw no change, because recordExternalFocusState had already set desiredFocusState = true moments earlier, so the real ghostty_surface_set_focus push was silently skipped. A surface could end up looking focused in the UI (bonsplit selection, desiredFocus true) while the Ghostty renderer never actually received focus, after a programmatic split/reparent. Fix: recordExternalFocusState(true) is now only called on the branches that don't reach reassertTerminalSurfaceFocus (deferred: window not key, or hidden/tiny geometry). The immediate-apply branch lets setFocus own desiredFocusState as its sole writer, so the dedup guard sees the real transition and pushes ghostty_surface_set_focus. Each branch now has exactly one writer of desiredFocusState for its transition. No change to forceRefresh or keyDown paths. Refs #86.
…eHostedView WindowTerminalPortal.synchronizeHostedView tripled a near-identical hide-and-schedule-transient-recovery block across the missingAnchorOrWindow, anchorWindowMismatch, and hostBoundsNotReady guards. Only the hostBoundsNotReady copy had the production-build fallback (calling scheduleDeferredFullSynchronizeAll() when Self.transientRecoveryEnabled is false) — the other two copies would silently drop the hide with no follow-up resync scheduled in that configuration, and a genuinely-transient condition could leave a hosted view stuck hidden. Extracted hideHostedViewSchedulingRecovery(hostedId:entry:hostedView:reason:) with the fallback included, and call it from all three sites. Behavior is now consistent across all three guards; each caller's own distinct diagnostic dlog (anchorWindowMismatch's anchorWindow=, hostBoundsNotReady's portal.sync.defer) is kept as a separate pre-log alongside the shared generic 'portal.hidden value=1 reason=X' log the helper emits, so a couple of call sites may emit one additional DEBUG-only log line per hide compared to before; no functional/production behavior change beyond fixing the missing fallback for the two previously-unprotected reasons. Refs #85.
Mechanical eviction of ~2,950 zero-dependency-on-AppDelegate free-standing types into their own files: - MainWindowHostingView.swift: MainWindowHostingView (SwiftUI hosting view) - TypingProfiler.swift: ProgramaTypingTiming, ProgramaMainRunLoopStallMonitor, ProgramaMainThreadTurnProfiler (all #if DEBUG) - TerminalDirectoryOpener.swift: FinderServicePathResolver, TerminalDirectoryOpenTarget - VSCodeIntegration.swift: VSCodeServeWebURLBuilder, VSCodeCLILaunchConfiguration(Builder), VSCodeServeWebController, ServeWebOutputCollector, ServeWebPortStore - WorkspaceShortcutMapper.swift: WorkspaceShortcutMapper - CLIInstaller.swift: ProgramaCLIPathInstaller - MenuBarIconRenderer.swift: MenuBarExtraController, notification menu snapshot/formatting types, MenuBarIconRenderer and its settings - WindowSwizzles.swift: the NSWindow/NSApplication method-swizzle implementations and their supporting file-scoped state Access-level widening required for cross-file visibility (no other code changes): - AppDelegate.recordTypingActivity(): fileprivate -> internal. Called from NSWindow.programa_sendEvent(_:), now in WindowSwizzles.swift. - extension NSWindow / extension NSApplication (swizzle methods): private -> internal. AppDelegate.installWindowResponderSwizzles() references programa_makeFirstResponder/programa_sendEvent/programa_performKeyEquivalent/ programa_applicationSendEvent via #selector(...) from AppDelegate.swift. - programaFirstResponderGuardCurrentEventOverride/HitViewOverride: private -> internal. AppDelegate.setWindowFirstResponderGuardTesting(...) and .clearWindowFirstResponderGuardTesting() write these directly. private extension AppDelegate (handleThemesReloadNotification) stays in AppDelegate.swift since it extends AppDelegate itself, not a zero-dependency type. Refs #95.
…nect configureRemoteConnection and disconnectRemoteConnection each carried a drifted copy of the remote-state reset block. configureRemoteConnection never cleared the per-panel bookkeeping fields (activeRemoteTerminalSurfaceIds, activeRemoteTerminalSessionCount, pendingRemoteSurfaceTTYName/SurfaceId, pendingRemoteSurfacePortKickReason/SurfaceId) that disconnectRemoteConnection did, so reconfiguring an already-connected workspace to a new destination left stale per-panel remote-session bookkeeping behind — including causing seedInitialRemoteTerminalSessionIfNeeded to skip seeding the new session because activeRemoteTerminalSurfaceIds wasn't actually empty. Extract resetRemoteState(), the union of both blocks, and call it from both paths. configureRemoteConnection now captures pendingRemoteForegroundAuthToken before the reset (since the reset nulls it) and seeds the initial terminal session after the reset instead of before, so seeding sees a clean slate. Refs #83.
… scp upload Three pieces of logic were copy-pasted across WorkspaceRemoteSession.swift, WorkspaceRemoteDaemon.swift, and TerminalSSHSessionDetector.swift: - shellSingleQuoted: byte-identical POSIX single-quoting helper, present 4 times (twice in WorkspaceRemoteDaemon.swift alone — one copy was entirely dead code with no call sites). Consolidated into RemoteSSHConnectionPolicy.shellSingleQuoted. - The SSH connection-policy flag set (ConnectTimeout/ServerAliveInterval/ ServerAliveCountMax keepalive triple, the StrictHostKeyChecking=accept-new default, the BatchMode=yes/ControlMaster=no pairing) and the underlying -o key=value option parsing (optionKey/hasOptionKey/normalizedOptions/ backgroundOptions/optionValue) were each defined 2-3 times with matching behavior. Consolidated into RemoteSSHConnectionPolicy; each call site still assembles its own full argument list (they legitimately differ in jump-host/ agent-forwarding/compression handling and port-flag spelling) but the identical fragments now have one definition, spliced in at the same position so argument order is unchanged. - The scp-upload-with-cancel-cleanup control flow (upload each file, checking for cancellation, cleaning up whatever was already uploaded and rethrowing on any failure) was independently implemented for the ad-hoc detected-SSH path (TerminalSSHSessionDetector.uploadDroppedFilesSync) and the daemon-relay path (WorkspaceRemoteSession.uploadDroppedFilesLocked). Extracted performSCPUploadWithCancelCleanup, parameterized by a per-item upload closure; each call site still decides whether to record a file's remote path before or after the transfer completes, since that differed between the two existing implementations and isn't part of this dedup. New files RemoteSSHConnectionPolicy.swift and RemoteSCPUpload.swift are registered in GhosttyTabs.xcodeproj/project.pbxproj (main app target only — these are only used by main-app-only files). Refs #92.
handleCustomShortcut(event:) was a single 966-line function with ~55 sequential if-checks. Documents the discovered precedence in a comment (palette > browser/terminal pre-checks > app-shortcut) and extracts the lowest-precedence phase -- the flat table of ~55 matchConfiguredShortcut / numberedConfiguredShortcutDigit / matchConfiguredDirectionalShortcut / matchTabShortcut checks -- into handleConfiguredAppShortcutActions(event: commandPaletteTargetWindow:hasFocusedAddressBarInShortcutContext:). This is pure tail-call code motion: the extracted function is byte-identical to the original tail of handleCustomShortcut (same statements, same order, same early returns), so evaluation order and behavior are unchanged. The higher-precedence palette and browser/terminal pre-check phases are left grouped in place per the conservative fallback (group into sub-functions without changing evaluation order) rather than risking a full reorder of a typing-latency-sensitive, tightly state-coupled dispatch chain. Refs #95.
…moves TabItemView.swift, FileDropOverlayView.swift, and CommandPaletteSearchEngine.swift now live in their own files, so file-private symbols they depend on (or that depend on them) that remain in ContentView.swift need at least internal visibility: - SidebarDragAutoScrollController, SidebarTabItemSettingsSnapshot, SidebarTabDragPayload, BonsplitTabDragPayload, SidebarBonsplitTabDropDelegate, SidebarTabDropDelegate, MiddleClickCapture, MiddleClickCaptureView — used by the extracted TabItemView. - coloredCircleImage, debugCommandPaletteTextPreview, the Color(hex:) initializer — top-level helpers used by the extracted TabItemView. FileDropOverlayView.swift needs `import Bonsplit` for dlog() (per CLAUDE.md's dlog-needs-bonsplit-import note). No behavior changes — access-level widening only. Refs #94.
WorkspaceRemoteDaemon.swift (2226 lines) held nine top-level type declarations covering unrelated concerns: RPC pending-call bookkeeping, SSH batch-command argument building, the daemon RPC client, HTTP request/response rewriting for the loopback proxy alias host, the local proxy tunnel plus its broker, and the CLI relay server. Split into six files along those seams, moving each type verbatim (no code changes): - WorkspaceRemoteDaemonPendingCallRegistry.swift - WorkspaceRemoteSSHBatchCommandBuilder.swift - WorkspaceRemoteDaemonRPCClient.swift - WorkspaceRemoteLoopbackHTTPRewriting.swift (RemoteLoopbackHTTPRequestRewriter, RemoteLoopbackHTTPRequestStreamRewriter, RemoteLoopbackHTTPResponseRewriter — kept together, they're the cohesive HTTP-rewriting subsystem) - WorkspaceRemoteProxyBroker.swift (WorkspaceRemoteDaemonProxyTunnel + WorkspaceRemoteProxyBroker — kept together since the tunnel is declared private and is only used by the broker; splitting them would have required widening its access level, which this move-only pass avoids) - WorkspaceRemoteCLIRelayServer.swift No access-level changes were needed — every moved type was already internal by default, aside from the private WorkspaceRemoteDaemonProxyTunnel which stayed paired with its only caller. project.pbxproj updated: old WorkspaceRemoteDaemon.swift entries replaced with the six new files (main app target only). Refs #98.
…nalView.swift GhosttyTerminalView.swift (9,490 lines) hosts 6+ subsystems; apply the existing +Extension convention (+Keyboard, +Mouse, +DragDrop, +Accessibility) to two more, clearly self-contained ones: - GhosttyTerminalView+IME.swift: the GhosttyNSView: NSTextInputClient conformance (marked-text/composition state, committed-text delivery, character-coordinate queries for system input methods). Moved verbatim. - GhosttyTerminalView+SwiftUIWrapper.swift: the GhosttyTerminalView NSViewRepresentable struct, its Coordinator, and the private HostContainerView it manages — the SwiftUI/AppKit bridge. Moved verbatim. Both are pure file moves; the only code change is widening GhosttyNSView.imePointOverrideForTesting from private to private(set) so the moved IME extension (a #if DEBUG testing hook, firstRect override) can still read it — it is written only inside GhosttyNSView's own declaration, which stays in GhosttyTerminalView.swift, so private(set) preserves the single-writer invariant. GhosttyTerminalView.swift drops from 9,490 to 8,592 lines. Registered both new files in project.pbxproj (NRGV0001-NRGV0004). Refs #97.
…onary Replace 7 parallel [UUID: _] dictionaries (visibility, pendingOpen, recentRequestAt, escapeSuppression set, escapeSuppressionStartedAt, selection, snapshot) with a single CommandPaletteWindowState struct stored in one commandPaletteStateByWindowId: [UUID: CommandPaletteWindowState] dictionary. Adds three small helpers (commandPaletteState(for:), updateCommandPaletteState(for:_:), teardownCommandPaletteState(for:)) so the two previously-duplicated 7-line teardown blocks (window context discard, window close) collapse to a single teardownCommandPaletteState(for:) call each. Every read/write site is a mechanical per-field translation with no behavior change (removeValue(forKey:) on a field-specific dictionary maps to setting that field back to its default; Set.contains maps to an Optional Bool comparison, etc). Refs #95.
…ls.swift Consolidate the drag-and-drop domain (DragOverlayRoutingPolicy, drag payload enums, drop delegates, drop edge/indicator/planner, auto-scroll planner, drag lifecycle/failsafe types) and the sidebar visual chrome (footer/help-menu/ scrim/blur/material/tint/preset enums, visual-effect/backdrop views, NSColor extension), both previously fragmented across several non-contiguous locations in ContentView.swift, into two new files. Done together in one pass since both extractions needed the same verification cycle. Access-level widening (dropped `private`, no other behavior change): - SidebarDragFailsafeMonitor, SidebarExternalDropOverlay — constructed from VerticalTabsSidebar / ContentView, which stay in ContentView.swift. - SidebarFooter, SidebarTopScrim, SidebarScrollViewResolver, SidebarScrollViewResolverView, SidebarEmptyArea, ClearScrollBackground, DraggableFolderIcon, TitlebarLeadingInsetReader, SidebarTrailingBorder, SidebarBackdrop — same reason. The fileDropOverlayKey/commandPaletteWindowOverlayKey/ tmuxWorkspacePaneWindowOverlayKey associated-object keys and their overlay container identifiers stay in ContentView.swift (they back WindowCommandPaletteOverlayController / WindowTmuxWorkspacePaneOverlayController, not the drag-drop domain) — they only transited SidebarDragDrop.swift briefly during the mechanical move and were moved back. Refs #94.
…ttySurfaceScrollView GhosttySurfaceScrollView (~2,780 lines) is one of the god-view subsystems called out by Nuclear Review #97. Extract its debug-only render/frame introspection (debugRenderStats, debugCopyIOSurfaceCGImage, debugSampleIOSurface + their DebugRenderStats/DebugFrameSample structs; entirely #if DEBUG) into GhosttyTerminalView+RenderStats.swift as a same-type extension, matching the existing +Extension convention. Pure move; three access-level widenings were required for cross-file visibility from the moved extension (each noted inline at the declaration): - GhosttySurfaceScrollView.surfaceView: private let -> internal let (immutable, so this only grants read access). - GhosttySurfaceScrollView.isActive: private var -> private(set) var (write stays confined to the class's own declaration in GhosttyTerminalView.swift; the moved extension only reads it). - GhosttySurfaceScrollView.contentsKey(for:) and .updatePresentStats(...): private static func -> internal static func (called, not just read, from the moved extension). Refs #97.
… concern WorkspaceRemoteSession.swift (2582 lines) held a single class, WorkspaceRemoteSessionController, whose ~100 methods spanned five unrelated concerns. Split along those seams into extension files, moving every method verbatim: - WorkspaceRemoteSessionController+ConnectionOrchestration.swift: connection-attempt/reverse-relay/proxy-lease orchestration and status publishing (stopAllLocked...stopReverseRelayViaControlMasterLocked, plus the bootstrapRemoteTTYRetry* constants). - WorkspaceRemoteSessionController+ProcessExecution.swift: ssh/scp argument assembly and the underlying Process execution primitive (sshCommonArguments...runProcess). - WorkspaceRemoteSessionController+DaemonInstall.swift: remote daemon bootstrap/build/download/upload and dropped-file upload (bootstrapDaemonLocked...helloRemoteDaemonLocked, plus the remotePlatformProbe* markers and remoteDaemonManifestInfoKey). - WorkspaceRemoteSessionController+ScriptBuilders.swift: debug logging, remote shell script builders, and process/PID utilities (debugLog...shouldEscalateProxyErrorToBootstrap, plus cachedRemoteDaemonSourceFingerprint). - WorkspaceRemoteSessionController+PortScanning.swift: remote listening-port scan scheduling, polling, and script builders (updateRemotePortScanTTYs...remoteAllPortsScanScript). WorkspaceRemoteSession.swift itself now holds only the class declaration, its nested types, stored properties, init, and the start/stop/ uploadDroppedFiles entry points. Access-level widening: every member of WorkspaceRemoteSessionController (nested types, stored properties, and ~100 methods) was declared private, which in Swift restricts visibility to the declaring file. To split the class into extensions across files, all of these were widened from private to the Swift default internal access level -- still module-scoped, not exposed outside the app target. This is a blanket, mechanical widening across the whole type rather than an itemized one (the type's entire private surface needed it to support the split), verified behavior-identical by a non-blank-line multiset diff against the pre-split file before committing. project.pbxproj updated: five new extension files registered (main app target only). Refs #98.
…tate ownership Move all 34 command-palette-only @State properties plus the 2 @AppStorage settings (commandPaletteQuery, commandPaletteMode, search corpus/results, rename/workspace-description drafts, focus-restore targets, usage history, etc.) off ContentView and onto a new CommandPaletteController: ObservableObject, as @Published/@AppStorage properties. ContentView keeps exactly one `@StateObject private var commandPaletteController`. ContentView's existing (unqualified) call sites — the ~4000 lines of palette orchestration (registerCommandPaletteHandlers, commandPaletteCommandContributions, rename/workspace-description flows, focus-restore, usage-history persistence, the view body) — are preserved unchanged: each former @State name becomes a thin computed proxy on ContentView that forwards get/set to the controller (`private var commandPaletteQuery: String { get { commandPaletteController. commandPaletteQuery } nonmutating set { ... } }`). This is a real ownership transfer (the controller is the only place these values are stored; ContentView holds no @State backing them anymore), done without touching the orchestration method bodies, which keeps this change mechanically verifiable. 5 properties that were bound via SwiftUI's $name projection at their call sites (commandPaletteQuery, commandPaletteRenameDraft, commandPaletteWorkspaceDescriptionDraft, commandPaletteWorkspaceDescriptionHeight, commandPaletteShouldFocusWorkspaceDescriptionEditor) get a matching `nameBinding: Binding<T>` proxy instead, since computed properties can't project a $-binding; call sites were updated to use . The 2 @focusstate properties (isCommandPaletteSearchFocused, isCommandPaletteRenameFocused) stay on ContentView — @focusstate is a SwiftUI View-only property wrapper and cannot be hosted on an ObservableObject. This is the actual reason the file's previous "CV1" extraction attempt called a full split impossible; it wasn't, once the @Published-storage-plus-proxy shape is used instead of moving @State verbatim. Access-level widening (dropped `private`, no other behavior change) for nested types now referenced from CommandPaletteController.swift (a different file): CommandPaletteMode, CommandPaletteRestoreFocusTarget, CommandPaletteTextSelectionBehavior, CommandPaletteMultilineTextEditorRepresentable, CommandPaletteRenameTarget, CommandPaletteWorkspaceDescriptionTarget (enum-case associated types must be at least as visible as the enum), and their AppKit backing views CommandPaletteMultilineTextEditorView / CommandPaletteMultilineTextView (NSViewRepresentable makeNSView/updateNSView must be at least as visible as the representable struct). The ~4000-line orchestration block and the ContentView+CommandPalette.swift pure helpers were NOT physically relocated onto the controller in this pass — they continue to compile and run correctly against the controller through the proxies, but remain textually in ContentView.swift/ContentView+CommandPalette.swift. Physically moving that method block is left for a follow-up; the state ownership change (the core ask) is complete and verified. Refs #88.
…ce.swift
Workspace.swift (7430 lines) held the Workspace class plus several
already-separate top-level extension blocks. Extracted three of those
existing seams into their own files, moving content verbatim:
- Workspace+Persistence.swift: session snapshot/restore
(sessionSnapshot, restoreSessionSnapshot, and their layout/panel
helpers) -- was a standalone `extension Workspace { ... }` block.
- Workspace+Layout.swift: programa.json custom layout application
(applyCustomLayout and its tree/pane helpers) -- was a standalone
`extension Workspace { ... }` block.
- Workspace+Bonsplit.swift: the `extension Workspace: BonsplitDelegate`
conformance.
Workspace.swift drops from 7430 to 5626 lines; it now holds the class
declaration, its nested types, stored properties, and the methods that
don't yet have a dedicated extension file.
Access-level widening (required for cross-file visibility, since Swift's
`private` is file-scoped even across extensions of the same type):
- Blanket: every direct member of `final class Workspace` (nested types,
stored properties, methods) was widened from `private` to the Swift
default `internal`, mirroring the same treatment already applied to
WorkspaceRemoteSessionController -- the class's entire private surface
needed it to support extraction, so this was done as one mechanical
pass rather than itemized per-symbol.
- Individually noted, since they were outside that blanket sweep:
- `SessionPaneRestoreEntry` (a private top-level struct, not a class
member) -- used by the extracted persistence helpers.
- `panels`, `panelCustomTitles`, `pinnedPanelIds`, `manualUnreadPanelIds`,
`tmuxLayoutSnapshot` -- `private(set)` published properties whose
setters are called from Workspace+Bonsplit.swift; widened to plain
internal-settable `@Published var`.
- `manualUnreadClearDelayAfterFocusFlash` -- a `nonisolated private
static let` read from Workspace+Bonsplit.swift.
- `applyTabSelection`, `beginNonFocusSplitFocusReassert`,
`clearNonFocusSplitFocusReassert`, `markExplicitFocusIntent`,
`matchesPendingNonFocusSplitFocusReassert` -- private methods
declared inside the BonsplitDelegate extension itself but called
from the remaining Workspace.swift class body.
Verified behavior-identical by a non-blank-line multiset diff between
the pre-split file and the four post-split files (normalizing only the
access-modifier changes above); the only remaining differences were the
added file-header comments, the per-file import blocks every new Swift
file needs, and two now-dangling "// MARK:" section comments that were
dropped since the content they marked moved to dedicated files.
project.pbxproj updated: three new files registered (main app target
only).
The remaining seams called out in #98 for Workspace.swift -- theming,
remote-connection glue, surface creation, and focus/geometry -- are not
yet extracted; they live inside the still-5626-line class body rather
than as pre-existing standalone extension blocks, so pulling them out
safely needs the same per-symbol cross-reference verification done here,
at a larger scale. Left for a follow-up pass; noted in issuesPartial.
Refs #98.
22 tasks
2 tasks
Member
Author
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.
What this does
Fixes the stale-bookkeeping bug when reconfiguring a connected remote workspace, and consolidates the copy-pasted shell-quoting, SSH policy, and scp-upload logic.
Summary
#98's Workspace.swift split is partial; the issue stays open.
Fixes #83. Fixes #92. Refs #98.
Test Plan