diff --git a/Docs/LSP.md b/Docs/LSP.md index 4004d7a..56c17a9 100644 --- a/Docs/LSP.md +++ b/Docs/LSP.md @@ -21,7 +21,8 @@ conversation with the server, and the join between the two. language. Owns the handshake, the open-document set and the version counters. - **`LSP/LanguageServerRegistry.swift`** — language → binary, one server per (root, language), started lazily the first time a matching file is opened and - prewarmed from the dashboard. + prewarmed from the dashboard. It is also what stops one again: see + **Servers do not stay forever** below. - **`LSP/LanguageServerConfiguration.swift`**, **`LanguageServerOptions.swift`**, **`ManagedLanguageServers.swift`** — which servers exist, their flags, and the ones the app can fetch for you. @@ -137,6 +138,17 @@ been fine; they were not why. ## Also worth knowing +- **Servers do not stay forever.** A server holding no documents for fifteen + minutes is shut down and forgotten by the registry — the node ones are a few + hundred megabytes each, sourcekit-lsp rather more, and a dashboard prewarms up + to `prewarmLimit` of them before a single file has been opened. Nothing used + to stop one short of removing its repository from the sidebar, so a day across + eight repositories left every server of all eight resident. `LanguageService` + records `idleSince` when its open-document count reaches zero and tells the + registry; there is no sweep on a clock, because a server can only become idle + at the moment its last document closes. Opening a file in that language starts + a fresh one, which is what already happens the first time — `shutdown()` + clears the remembered handshake so it can. - **Large files are excluded on purpose.** See `OpenDocument.largeFileNote`. `didOpen` sends the whole text, and the answer to a 3.5 MB minified bundle is a diagnostic per line of something nobody is editing. No coordinator is even diff --git a/Docs/Terminal.md b/Docs/Terminal.md index b79c703..bf55477 100644 --- a/Docs/Terminal.md +++ b/Docs/Terminal.md @@ -45,6 +45,24 @@ sixteen ANSI slots a terminal needs. ## Gotchas learned the hard way +- **Ghostty's callbacks arrive at frame rate, and a hop each is too many.** A + shell painting a spinner — every `claude` mid-turn — asks to render every + frame whether or not anyone is looking at that tab, and wakes the app several + times per frame besides. One `Task { @MainActor … }` per request meant the + cost of a window grew with each terminal left running in it. Both are gathered + on ghostty's own thread instead (`RenderQueue`, `Latch` at the foot of + `GhosttyRuntime.swift`), and one hop serves everything that arrived while it + was waiting its turn. The latch is cleared at the *start* of the scheduled + work, so a request landing mid-drain schedules the next pass rather than being + swallowed by this one. +- **A tab title is not published on every rename, for the same reason.** `claude` + paints its spinner into the OSC title, and each name used to be a write to an + observable property — which redraws the terminals list, the tab bar and the + viewer header. `TerminalSession.titleArrived` splits what a name *means* from + the text of it: `isWorking` (what raises the "finished" banner) is read at + once and only written when it changes, while the text lands on a 200 ms tick. + The last name of a burst is always published, so a tab never settles showing + half a spinner. - `ghostty_surface_text` is a **paste** (`completeClipboardPaste` in core). Under bracketed paste, a pasted `\n` never executes a command — that is why `TerminalSession.send` turns a trailing newline into `pressEnter()`, a diff --git a/README.md b/README.md index bece3f3..69b0f1f 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,7 @@ missing ones, so you never have to work out which command it wanted. | --- | --- | | A real terminal | The Ghostty engine, rooted at the repo. As many shells as you like. | | Ask Claude (⇧⌘L) | The real Claude Code CLI in a floating panel — every flag, slash command and plugin works. Drag it, size it from any edge or corner, or fold it down to a bar on the dock along the bottom of the editor. One click on the bar brings it straight back, with no wait: a folded conversation keeps its terminal. | +| Park it (⇧⌘→) | While you are typing in a conversation, one key puts it back in the bottom-right corner of the editor, at the size you gave it. Anywhere else — the editor, a comment box — ⇧⌘→ still selects to the end of the line. | | Many at once | As many conversations as you like, each named after what it is about, all on screen together. Fold the ones you are not reading down to the dock; when there are more bars than the editor is wide, the dock scrolls sideways, and dragging a bar moves it along the row. | | Resume anything | Past conversations listed from disk — including ones you started in your own shell. | | It tells you | A Notification Centre banner when a shell wants you back; click it to land on that tab. | @@ -185,6 +186,7 @@ everything macOS binds) are not on offer there. | ⌃⇥ | Switch repository | | ⌘R | Refresh all repositories | | ⇧⌘L | Ask Claude about the selected repo, in a panel over the window | +| ⇧⌘→ | Park the conversation you are typing in, bottom-right of the editor | | ⌃⌘T / ⌘T / ⇧⌘T | Repo terminal · new tab · home terminal | | ⌃` | Show the repo's terminal, and the same key to leave it | | ⌃⌘P | Create PR with Claude Code | diff --git a/Sources/Workspace/LSP/LanguageServerRegistry.swift b/Sources/Workspace/LSP/LanguageServerRegistry.swift index 9737d94..96b17de 100644 --- a/Sources/Workspace/LSP/LanguageServerRegistry.swift +++ b/Sources/Workspace/LSP/LanguageServerRegistry.swift @@ -63,7 +63,7 @@ final class LanguageServerRegistry { guard let entry = catalog.entry(forFile: url) ?? catalog.entry(for: language) else { return nil } let key = Self.key(root: root, language: entry.language) if let existing = services[key] { return existing } - let service = LanguageService(definition: Self.definition(for: entry), root: root) + let service = makeService(Self.definition(for: entry), root: root) services[key] = service return service } @@ -140,7 +140,7 @@ final class LanguageServerRegistry { let key = Self.key(root: root, language: entry.language) let service = services[key] - ?? LanguageService(definition: Self.definition(for: entry), root: root) + ?? makeService(Self.definition(for: entry), root: root) services[key] = service // Deliberately not awaited in turn: sourcekit-lsp can spend the // best part of a minute on its handshake, and the servers behind it @@ -163,7 +163,7 @@ final class LanguageServerRegistry { }) else { return nil } let key = Self.key(root: root, language: entry.language) if let existing = services[key] { return existing } - let service = LanguageService(definition: Self.definition(for: entry), root: root) + let service = makeService(Self.definition(for: entry), root: root) services[key] = service return service } @@ -217,6 +217,67 @@ final class LanguageServerRegistry { } } + // MARK: - Letting idle servers go + + /// How long a server may hold no documents before it is stopped. + /// + /// Nothing used to stop one short of removing its repository from the + /// sidebar, and a server is not a cheap thing to leave sitting there: the + /// node ones are a few hundred megabytes each, sourcekit-lsp rather more, + /// and a repository prewarms up to ``prewarmLimit`` of them the first time + /// its dashboard is looked at. A working day across eight repositories left + /// every server of all eight resident, which is the machine slowing down + /// rather than the app — but it is the app that is blamed for it. + /// + /// Long enough that reading two files in a language with a break between + /// them does not pay the handshake twice, and reading one file in a + /// repository visited by accident does not hold a server all afternoon. + private static let idleLimit: TimeInterval = 15 * 60 + + /// The one wait in flight. There is no sweep on a clock: a server can only + /// become idle by having its last document closed, so the wait is put on + /// when that happens and taken off when it is served. + @ObservationIgnored private var idleSweep: Task? + + /// Every service is made here, so every one of them reports going idle. + private func makeService(_ definition: LanguageServerDefinition, root: URL) -> LanguageService { + let service = LanguageService(definition: definition, root: root) + service.onIdle = { [weak self] in self?.scheduleIdleSweep() } + return service + } + + private func scheduleIdleSweep(in delay: TimeInterval = LanguageServerRegistry.idleLimit) { + guard idleSweep == nil else { return } + idleSweep = Task { [weak self] in + try? await Task.sleep(for: .seconds(delay)) + guard !Task.isCancelled, let self else { return } + shutdownIdleServices() + } + } + + /// Stops everything that has been idle long enough, and puts the wait back + /// on for whatever is idle but not yet due. + private func shutdownIdleServices() { + idleSweep = nil + let now = Date() + var soonest: TimeInterval? + + for (key, service) in services { + guard let idleSince = service.idleSince else { continue } + let idle = now.timeIntervalSince(idleSince) + if idle >= Self.idleLimit { + service.shutdown() + services[key] = nil + } else { + soonest = min(soonest ?? .greatestFiniteMagnitude, Self.idleLimit - idle) + } + } + + // A server that went idle while this wait was running has its own left + // to serve; scheduling for the nearest keeps one wait for all of them. + if let soonest { scheduleIdleSweep(in: soonest) } + } + private static func key(root: URL, language: String) -> String { "\(root.path)|\(language)" } diff --git a/Sources/Workspace/LSP/LanguageService.swift b/Sources/Workspace/LSP/LanguageService.swift index 6ca56e9..8765b0f 100644 --- a/Sources/Workspace/LSP/LanguageService.swift +++ b/Sources/Workspace/LSP/LanguageService.swift @@ -39,9 +39,32 @@ final class LanguageService { /// coordinator to decide whether it may send ranges or must send the file. private(set) var syncKind: LSP.SyncKind = .full + /// Since when this server has been holding no documents at all, or nil + /// while it is holding one. + /// + /// A language server is an expensive thing to leave running — a few hundred + /// megabytes each for the node ones, more for sourcekit-lsp — and nothing + /// used to stop one until the repository it belongs to was removed from the + /// sidebar. A day of looking at eight repositories left every server of all + /// eight resident. ``LanguageServerRegistry`` reads this and stops the ones + /// that have been idle long enough; opening a file in that language starts + /// a fresh one, which is what already happens the first time. + private(set) var idleSince: Date? { + didSet { if idleSince != nil { onIdle?() } } + } + + /// Told when this server stops holding documents, so the registry can put + /// a wait on it rather than sweeping every server on a clock. + @ObservationIgnored var onIdle: (() -> Void)? + @ObservationIgnored private let connection: LSPConnection @ObservationIgnored private var versions: [String: Int] = [:] - @ObservationIgnored private var openCount: [String: Int] = [:] + @ObservationIgnored private var openCount: [String: Int] = [:] { + didSet { + guard openCount.isEmpty != oldValue.isEmpty else { return } + idleSince = openCount.isEmpty ? Date() : nil + } + } @ObservationIgnored private var diagnosticObservers: [ObjectIdentifier: (String, [LSP.Diagnostic]) -> Void] = [:] @ObservationIgnored private var startTask: Task? @ObservationIgnored private var settings: LSP.Value? @@ -121,6 +144,9 @@ final class LanguageService { ) } status = .running + // A prewarmed server holds nothing yet, so its idle clock + // starts here. `open` clears it the moment a file arrives. + if openCount.isEmpty, idleSince == nil { idleSince = Date() } return true } catch { status = .failed(error.localizedDescription) @@ -134,8 +160,18 @@ final class LanguageService { } func shutdown() { + // Before the resets below, so emptying the documents does not report a + // server that is going away as one that has just gone idle. + onIdle = nil Task { await connection.stop() } status = .notStarted + // The handshake is what `startIfNeeded` remembers; without this a + // service stopped for being idle could never be started again. + startTask = nil + openCount = [:] + versions = [:] + diagnostics = [:] + idleSince = nil } // MARK: - Document sync diff --git a/Sources/Workspace/Models/KeyboardShortcuts.swift b/Sources/Workspace/Models/KeyboardShortcuts.swift index 1514e34..28eb5e1 100644 --- a/Sources/Workspace/Models/KeyboardShortcuts.swift +++ b/Sources/Workspace/Models/KeyboardShortcuts.swift @@ -121,6 +121,7 @@ enum ShortcutAction: String, CaseIterable, Identifiable, Sendable { case openTerminal case newTerminalTab case newHomeTerminal + case parkChat case submit @@ -145,6 +146,7 @@ enum ShortcutAction: String, CaseIterable, Identifiable, Sendable { case .openTerminal: "Open Terminal" case .newTerminalTab: "New Terminal Tab" case .newHomeTerminal: "New Terminal in Home" + case .parkChat: "Park Conversation" case .submit: "Post / Commit" } } @@ -169,6 +171,7 @@ enum ShortcutAction: String, CaseIterable, Identifiable, Sendable { case .openTerminal: "Show the selected repository's terminal" case .newTerminalTab: "Another shell beside the current one" case .newHomeTerminal: "A shell that belongs to no repository" + case .parkChat: "Put the conversation you are typing in down in the bottom-right corner of the viewer" case .submit: "Post the comment you typed, or commit the staged files" } } @@ -183,7 +186,7 @@ enum ShortcutAction: String, CaseIterable, Identifiable, Sendable { .editor case .goToFile, .goBack, .goForward: .go - case .askClaude, .toggleTerminal, .openTerminal, .newTerminalTab, .newHomeTerminal: + case .askClaude, .toggleTerminal, .openTerminal, .newTerminalTab, .newHomeTerminal, .parkChat: .terminal case .submit: .writing @@ -212,6 +215,16 @@ enum ShortcutAction: String, CaseIterable, Identifiable, Sendable { case .openTerminal: KeyChord("t", [.control, .command]) case .newTerminalTab: KeyChord("t", [.command]) case .newHomeTerminal: KeyChord("t", [.command, .shift]) + // ⇧ is on it because ⌘→ alone is already spoken for: it is "end of + // line" in every text view on the Mac, and on a keyboard where the + // arrows double as ⌥→/⌘→ the system takes it first — an app that binds + // it is one whose key sometimes fires and sometimes does not. ⇧⌘→ is + // "select to end of line", which belongs to a *text view* rather than to + // the system, and that is why it is reachable here: this is not a menu + // item, so the monitor only takes it while a conversation has the + // keyboard (see ``ChatPanelOverlay``) and the editor, a comment box and + // a commit message all keep the selection. + case .parkChat: KeyChord.rightArrow([.command, .shift]) case .submit: KeyChord.return([.command]) } } @@ -260,9 +273,15 @@ struct KeyChord: Hashable, Codable, Sendable { /// The placeholder written to disk for an action bound to nothing. static let unbound = KeyChord("", []) - /// The two keys the defaults use that type no character of their own. + /// The keys the defaults use that type no character of their own. An arrow + /// is one of AppKit's private-use codes rather than a character, which is + /// exactly what `charactersIgnoringModifiers` reports for it — so a recorded + /// arrow and this one are the same string. static func tab(_ modifiers: KeyModifiers) -> KeyChord { KeyChord("\t", modifiers) } static func `return`(_ modifiers: KeyModifiers) -> KeyChord { KeyChord("\r", modifiers) } + static func rightArrow(_ modifiers: KeyModifiers) -> KeyChord { + KeyChord(character(NSRightArrowFunctionKey), modifiers) + } /// SwiftUI's side of it. Nil when the stored key is not a single character, /// which only a hand-edited preference could produce. diff --git a/Sources/Workspace/Models/PullRequestBuild.swift b/Sources/Workspace/Models/PullRequestBuild.swift index c834a4a..d3b0b67 100644 --- a/Sources/Workspace/Models/PullRequestBuild.swift +++ b/Sources/Workspace/Models/PullRequestBuild.swift @@ -33,6 +33,19 @@ struct PullRequestBuild: Identifiable, Sendable, Hashable { } } + /// Whether this is a verdict rather than a stage on the way to one. + /// A run that has settled will not change again, so there is nothing + /// left to learn by asking the host about it a second time — see + /// `PullRequestDetailView.watchBuilds`. `unknown` is deliberately not + /// settled: it is a word this app failed to read, not a host saying + /// the run is over. + var isSettled: Bool { + switch self { + case .passed, .failed, .cancelled, .skipped: true + case .running, .pending, .unknown: false + } + } + /// Every host spells these differently, and one host spells them /// differently in two places — Cloud reports a pipeline's outcome in /// `state.result.name` but a commit status's in `state`. Matching on diff --git a/Sources/Workspace/Models/TerminalSession.swift b/Sources/Workspace/Models/TerminalSession.swift index d743eb8..d8ca90f 100644 --- a/Sources/Workspace/Models/TerminalSession.swift +++ b/Sources/Workspace/Models/TerminalSession.swift @@ -104,21 +104,7 @@ final class TerminalSession: Identifiable { self.title = title self.view = GhosttySurfaceView() view.onTitleChange = { [weak self] title in - guard let self else { return } - let wasWorking = isWorking - self.title = title - isWorking = Self.readsAsBusy(title) - titleChanges += 1 - // `claude` renames the tab as it starts work on a prompt, which is - // exactly when a conversation that had no transcript may have got - // one — so a rename is the cue to go looking again. - watchForClaudeName() - // The spinner going away is the turn ending: whatever was being - // waited for is done, and the person who set it going is very - // likely somewhere else by now. - if wasWorking, !isWorking { - raiseAttention("Finished — waiting for you") - } + self?.titleArrived(title) } view.onClose = { [weak self] in self?.isWorking = false @@ -135,6 +121,64 @@ final class TerminalSession: Identifiable { } } + /// The newest name the shell has given itself, published or not. + @ObservationIgnored private var latestTitle: String? + /// The wait that will publish it; see ``titleArrived``. + @ObservationIgnored private var titlePublish: Task? + /// How long a burst of renames is allowed to run before the tab catches + /// up. Short enough to read as immediate, long enough that a spinner is + /// several frames rather than several redraws. + private static let titleInterval = Duration.milliseconds(200) + + /// A terminal renames itself far faster than anyone can read. + /// + /// `claude` paints its spinner **into the tab title**, so a running turn is + /// a new name every frame — and every one of those used to be a write to an + /// observable property, which redraws the terminals list, the tab bar and + /// the viewer header. With several conversations going at once that was the + /// whole window redrawing continuously for a character nobody can follow, + /// and it got worse with every tab left running. + /// + /// So the two halves are separated. What a name *means* is read straight + /// away — `isWorking` is what puts a banner up when a turn ends, and it + /// only changes at the two ends of one, so writing it on the change alone + /// costs nothing. The text itself is published on a slow tick, and the last + /// name of a burst is always published, so a tab never settles showing a + /// frame of the spinner. + private func titleArrived(_ title: String) { + // A program re-announcing the name it already has is not a rename. + guard title != latestTitle else { return } + latestTitle = title + + let wasWorking = isWorking + let working = Self.readsAsBusy(title) + if working != wasWorking { isWorking = working } + titleChanges += 1 + // `claude` renames the tab as it starts work on a prompt, which is + // exactly when a conversation that had no transcript may have got one — + // so a rename is the cue to go looking again. + watchForClaudeName() + // The spinner going away is the turn ending: whatever was being waited + // for is done, and the person who set it going is very likely somewhere + // else by now. + if wasWorking, !working { + raiseAttention("Finished — waiting for you") + } + + guard titlePublish == nil else { return } + titlePublish = Task { [weak self] in + // `try?` here would swallow the cancellation and publish anyway, + // which is `terminate()` cancelling this and getting one last name + // written over the tab it is closing. + do { try await Task.sleep(for: Self.titleInterval) } catch { return } + guard let self else { return } + // Cleared before the write, so a rename arriving in the same turn + // starts the next wait rather than being dropped. + titlePublish = nil + if let latestTitle, latestTitle != self.title { self.title = latestTitle } + } + } + /// Whether a tab title reads as "the program in here is busy". /// /// There is no other way to know. The app drives a real terminal, so the @@ -311,6 +355,8 @@ final class TerminalSession: Identifiable { isWorking = false nameWatch?.cancel() nameWatch = nil + titlePublish?.cancel() + titlePublish = nil claudeStartupWatch?.cancel() claudeStartupWatch = nil view.close() diff --git a/Sources/Workspace/Models/WorkspaceStore.swift b/Sources/Workspace/Models/WorkspaceStore.swift index 6c7c9dc..7ca3d0e 100644 --- a/Sources/Workspace/Models/WorkspaceStore.swift +++ b/Sources/Workspace/Models/WorkspaceStore.swift @@ -426,13 +426,55 @@ final class WorkspaceStore { /// Reads every repository again. `quietly` is the sweep on a clock: the same /// reads, with no spinner put up for them — see `Project.refresh`. + /// + /// **A few at a time.** One read is a handful of processes — `git status`, + /// `git log`, `gh pr list` — and starting one task per repository meant a + /// sidebar of a dozen fired forty-odd of them at once, every five minutes. + /// They land on the cooperative pool, which has a thread per core, so the + /// sweep was a hitch across the whole window rather than work happening + /// behind it. The numbers are no less fresh for arriving a second later. func refreshAll(quietly: Bool = false) { lastAutoRefresh = Date() - for project in projects { - Task { await project.refresh(quietly: quietly) } + let due = projects + // Cancelled *and waited for*. Cancellation here only stops the sweep + // adding more repositories: `Project.refresh` checks nothing, so the + // ones already in flight run their `git` and `gh` to the end whatever + // this says. Starting the replacement on top of them would put twice + // the width on the pool and read those repositories twice, which is + // the doubling this whole task was added to stop. + let previous = refreshSweep + previous?.cancel() + refreshSweep = Task { + await previous?.value + // A third sweep may have arrived while this one waited, and it is + // the newer answer. + guard !Task.isCancelled else { return } + await withTaskGroup(of: Void.self) { group in + var next = due.makeIterator() + var running = 0 + while running < Self.refreshWidth, let project = next.next() { + group.addTask { await project.refresh(quietly: quietly) } + running += 1 + } + // One in, one out: the group never holds more than the width. + while await group.next() != nil { + guard !Task.isCancelled, let project = next.next() else { continue } + group.addTask { await project.refresh(quietly: quietly) } + } + } } } + /// How many repositories are read at once. Three keeps the sweep off the + /// pool's last threads on a four-core machine while still being several + /// times faster than reading them one after another. + private static let refreshWidth = 3 + + /// The sweep in flight, so a second one replaces it rather than doubling + /// up — a hand refresh landing on top of the five-minute tick used to run + /// every repository twice. Replacing means waiting for it; see `refreshAll`. + @ObservationIgnored private var refreshSweep: Task? + // MARK: - Refreshing on a clock /// How often every repository in the sidebar is read again — the branch it @@ -468,15 +510,31 @@ final class WorkspaceStore { startReloadingOnReturningToFront() autoRefreshTask = Task { [weak self] in - // The loop runs a minute at a time and decides by the clock, so a - // sweep the user asked for by hand pushes the next one out too. + // The loop decides by the clock rather than by its own rhythm, so a + // sweep the user asked for by hand pushes the next one out too — it + // sleeps for whatever is left of the interval rather than waking + // every minute to work out that there is nothing to do yet. while !Task.isCancelled { - try? await Task.sleep(for: .seconds(60)) - guard !Task.isCancelled, let self else { return } - let due = Date().timeIntervalSince(lastAutoRefresh) >= Self.autoRefreshInterval - guard due || missedAutoRefresh else { continue } + guard let self else { return } + let waited = Date().timeIntervalSince(lastAutoRefresh) + // A second short is due: without the slack, a sleep that lands + // a hair early goes round again for a fraction of a second. + let remaining = Self.autoRefreshInterval - waited - 1 + if remaining > 0, !missedAutoRefresh { + try? await Task.sleep(for: .seconds(remaining)) + guard !Task.isCancelled else { return } + // The clock is asked again rather than trusted: a hand + // refresh during the sleep moved the deadline, and the Mac + // waking from sleep moved it the other way. + continue + } guard NSApp.isActive else { + // Behind something else: nothing goes out, and the one tick + // it owes is paid when the app comes back — checked a + // minute at a time, which is cheap next to a sweep. missedAutoRefresh = true + try? await Task.sleep(for: .seconds(60)) + guard !Task.isCancelled else { return } continue } missedAutoRefresh = false @@ -2594,6 +2652,29 @@ final class WorkspaceStore { panel.remember() } + /// Puts a conversation down in the bottom-right corner of the **centre + /// pane** — the corner a chat opens in, and the one part of the window + /// nothing is ever read in. + /// + /// The panel keeps its size: this is the panel being moved out of the way of + /// the code, not resized, and a reader who made theirs tall meant it. The + /// corner is the viewer's rather than the window's for the reason the dock's + /// is (see ``chatDockRegion``): the window's belongs to the navigator. + /// + /// Only ever asked for by hand — the key is caught while the conversation + /// has the keyboard — so a panel dragged somewhere on purpose stays there + /// until the reader says otherwise. + func parkChatPanel(_ panel: ChatPanel) { + let parked = panel.frame.parked(in: chatDockRegion, within: chatPanelBounds) + guard parked != panel.frame else { return } + withAnimation(Self.chatPanelMotion) { + panel.frame = parked + } + // The same corner next time, and next launch: parking one is as much a + // statement about where this repository's chat belongs as dragging it. + panel.remember() + } + /// The overlay has been laid out: the window's size, and with it any panel /// the window just got too small for. A folded panel is clamped the same /// way, since what is being kept in bounds is where it will come *back* to; @@ -3356,6 +3437,23 @@ struct ChatPanelFrame: Equatable, Codable { return result } + /// The same frame, moved down into the bottom-right corner of `region` — + /// the centre pane — with the app's usual gap left around it. + /// + /// The size is untouched. What is being asked for is the panel *out of the + /// way*, and a conversation that also changed shape on the way there would + /// have to be sized again every time. + /// + /// Clamped to the window like every other placement, which is what answers a + /// panel taller than the pane it is being put in: it lands on the corner it + /// can reach rather than half off the top of the window. + func parked(in region: CGRect, within bounds: CGSize) -> ChatPanelFrame { + var result = self + result.x = region.maxX - width - Self.margin + result.y = region.maxY - height - Self.margin + return result.clamped(to: bounds) + } + /// The frame a pull on one edge or corner leaves, with every edge that is /// not being pulled left exactly where it was. /// diff --git a/Sources/Workspace/Support/TerminalFocus.swift b/Sources/Workspace/Support/TerminalFocus.swift index f9a370b..6ddde3d 100644 --- a/Sources/Workspace/Support/TerminalFocus.swift +++ b/Sources/Workspace/Support/TerminalFocus.swift @@ -59,7 +59,14 @@ enum TerminalFocus { /// Whether the keys are going into a terminal right now. static func isHeldByTerminal(in window: NSWindow) -> Bool { - surface(enclosing: window.firstResponder as? NSView) != nil + focused(in: window) != nil + } + + /// The terminal the keys are going into right now, if they are going into + /// one. What a key meant for a *conversation* — rather than for the window — + /// is recognised by: the panel it belongs to is the one holding this surface. + static func focused(in window: NSWindow) -> GhosttySurfaceView? { + surface(enclosing: window.firstResponder as? NSView) } /// The terminal a point in the window lands in, if it lands in one. diff --git a/Sources/Workspace/Terminal/GhosttyRuntime.swift b/Sources/Workspace/Terminal/GhosttyRuntime.swift index e7aabf5..fe00b6f 100644 --- a/Sources/Workspace/Terminal/GhosttyRuntime.swift +++ b/Sources/Workspace/Terminal/GhosttyRuntime.swift @@ -45,8 +45,17 @@ final class GhosttyRuntime { runtime.userdata = nil runtime.supports_selection_clipboard = false + // Ghostty wakes us whenever it has work, which under a program painting + // at frame rate is many times per frame. One tick answers all of them, + // so only the first wakeup of a batch pays for a hop. runtime.wakeup_cb = { _ in - Task { @MainActor in GhosttyRuntime.shared.tick() } + guard GhosttyRuntime.pendingTick.claim() else { return } + Task { @MainActor in + // Cleared first: a wakeup arriving during the tick is about + // work this tick has not seen, and has to schedule its own. + GhosttyRuntime.pendingTick.clear() + GhosttyRuntime.shared.tick() + } } runtime.action_cb = { _, target, action in @@ -214,8 +223,18 @@ final class GhosttyRuntime { let surfaceBits = Int(bitPattern: surface) switch action.tag { + // Gathered rather than hopped one at a time; see ``pendingRenders``. case GHOSTTY_ACTION_RENDER: - onView(surfaceBits) { view in view.needsDisplay = true } + guard pendingRenders.add(surfaceBits) else { return true } + Task { @MainActor in + for bits in pendingRenders.drain() { + guard let surface = UnsafeMutableRawPointer(bitPattern: bits), + let userdata = ghostty_surface_userdata(surface) else { continue } + Unmanaged.fromOpaque(userdata) + .takeUnretainedValue() + .needsDisplay = true + } + } return true case GHOSTTY_ACTION_SET_TITLE: @@ -273,6 +292,21 @@ final class GhosttyRuntime { } } + // MARK: - Coalescing + + /// The surfaces waiting to be marked for redraw. + /// + /// Ghostty asks to render whenever a surface is dirty, and a shell running + /// a spinner — every `claude` mid-turn — is dirty every frame, whether or + /// not anyone has that tab on screen. A hop per request meant the cost of + /// a window grew with each terminal left running in it, which is the shape + /// of an app that gets slower the longer it is open. The requests are + /// gathered on ghostty's own thread instead, and one hop marks everything + /// that arrived while it was waiting its turn. + private nonisolated static let pendingRenders = RenderQueue() + /// The same for `wakeup_cb`, which has nothing to gather. + private nonisolated static let pendingTick = Latch() + /// Finds the view that owns a surface and runs `body` on the main actor. private nonisolated static func onView( _ surfaceBits: Int, @@ -285,3 +319,59 @@ final class GhosttyRuntime { } } } + +/// A one-shot flag shared across threads: the first caller to ``claim`` owes +/// the main-actor hop, and everyone arriving behind it is already covered by +/// the hop that one is about to make. +private final class Latch: @unchecked Sendable { + private let lock = NSLock() + private var isSet = false + + /// True when this caller is the one that has to schedule the work. + func claim() -> Bool { + lock.lock() + defer { lock.unlock() } + guard !isSet else { return false } + isSet = true + return true + } + + /// Called at the *start* of the scheduled work, so anything arriving while + /// it runs schedules a fresh pass rather than being swallowed by this one. + func clear() { + lock.lock() + isSet = false + lock.unlock() + } +} + +/// The surfaces that have asked to be redrawn since the last drain, and one +/// latch saying whether a drain is already on its way. +private final class RenderQueue: @unchecked Sendable { + private let lock = NSLock() + private var pending: Set = [] + private var isScheduled = false + + /// Records a surface, and returns true only for the request that has to + /// schedule the drain. + func add(_ surfaceBits: Int) -> Bool { + lock.lock() + defer { lock.unlock() } + pending.insert(surfaceBits) + guard !isScheduled else { return false } + isScheduled = true + return true + } + + /// Everything that arrived since the last drain. Taking the batch and + /// releasing the latch is one step, so a request landing mid-drain is + /// either in this batch or schedules the next one — never neither. + func drain() -> Set { + lock.lock() + defer { lock.unlock() } + isScheduled = false + let batch = pending + pending.removeAll(keepingCapacity: true) + return batch + } +} diff --git a/Sources/Workspace/Views/ChatPanelOverlay.swift b/Sources/Workspace/Views/ChatPanelOverlay.swift index e9988e9..c4a72f9 100644 --- a/Sources/Workspace/Views/ChatPanelOverlay.swift +++ b/Sources/Workspace/Views/ChatPanelOverlay.swift @@ -70,6 +70,10 @@ struct ChatPanelOverlay: View { .map { ($0, store.chatPanelFrame(of: $0).onScreenRect) } ) { store.raiseChatPanel($0) } ) + // The keys the panels answer for, caught the way their clicks are + // and in the same file. See ``park(_:in:)`` for why this is a + // monitor and not a menu item. + .onWindowKeyEvent { event, window in park(event, in: window) } // The window's size, which is what a *floating* panel is placed and // clamped against — one is dragged by hand, and a window-like thing // you may not drop where you like is not one. A folded panel is held @@ -82,6 +86,39 @@ struct ChatPanelOverlay: View { .onChange(of: geometry.size) { _, size in store.chatPanelsDidLayout(in: size) } } } + + /// ⇧⌘→ by default: the conversation being typed in goes down to the + /// bottom-right corner of the centre pane. Returns true when it is taken. + /// + /// **Only while a conversation holds the keyboard**, which is the whole of + /// what makes an arrow safe to bind at all. ⇧⌘→ is "select to end of line" + /// in every text view on the Mac, and a menu key equivalent is dispatched + /// before the thing with focus ever sees the event — so as a menu item this + /// would take the key away from the editor, from a comment box and from a + /// commit message. A monitor can ask *who is typing* first, and hands the + /// event straight back when the answer is not a panel. (⌘→ without ⇧ was + /// the first default and is not bindable by anyone: the system answers it + /// before the app is asked.) + /// + /// A folded conversation cannot be the one holding the keyboard — folding + /// gives it up (see ``WorkspaceStore/collapseChatPanel(_:)``) — so nothing + /// here has to say what parking one would mean. + private func park(_ event: NSEvent, in window: NSWindow) -> Bool { + // The repository switcher takes the arrows for itself while its row is + // up, and it is another window monitor: which of the two sees a key + // first is AppKit's business, so this says out loud that the row wins + // rather than leaving it to the order they were installed in. + guard window.attachedSheet == nil, !store.isSwitchingProjects, + let chord = KeyboardShortcuts.shared.chord(for: .parkChat), + chord.matches(event), + let surface = TerminalFocus.focused(in: window), + // The centre pane's shell is a terminal with focus too, and it is + // not a panel: nothing of this is its business. + let panel = store.visibleChats.first(where: { $0.session.view === surface }) + else { return false } + store.parkChatPanel(panel) + return true + } } /// The dock: every folded conversation as a bar, in one row along the bottom of diff --git a/Sources/Workspace/Views/ChatPanelView.swift b/Sources/Workspace/Views/ChatPanelView.swift index ccd371d..cbaa134 100644 --- a/Sources/Workspace/Views/ChatPanelView.swift +++ b/Sources/Workspace/Views/ChatPanelView.swift @@ -281,19 +281,23 @@ struct ChatPanelBarContent: View { ChatPanelMark(isWorking: panel.session.isWorking) // One line, repository first: `workspace | fix the scroll bug`. - // Stacked, the two competed for a 30pt bar and the conversation — - // the half that actually changes — got the smaller type. + // Stacked, the two competed for a 30pt bar. // - // **The repository holds its width and the conversation gives way**, - // which is the same argument `dockedMinimumWidth` is written from: - // the dock is shared by every repository, so the project is what a - // folded bar must still be saying when there is no room left. Both - // are capped at one line, so a long conversation title ends in an + // **The repository is the one that is read**, so it carries the + // weight and the conversation sits behind it. The name a + // conversation is given is whatever it happened to be about, and it + // changes under you; which repository a floating bar belongs to is + // the thing being looked for when several are docked side by side. + // + // The same argument decides the width, and it did before the type + // followed it: **the repository holds its width and the + // conversation gives way** — see `dockedMinimumWidth`. Both are + // capped at one line, so a long conversation title ends in an // ellipsis rather than pushing the buttons off the end of the bar. HStack(spacing: 5) { Text(panel.projectName) - .font(.system(size: 12, weight: .medium)) - .foregroundStyle(.secondary) + .font(.system(size: 12, weight: .semibold)) + .foregroundStyle(.primary) .lineLimit(1) .truncationMode(.tail) .layoutPriority(1) @@ -302,7 +306,8 @@ struct ChatPanelBarContent: View { .foregroundStyle(.tertiary) .layoutPriority(1) Text(panel.title) - .font(.system(size: 12, weight: .medium)) + .font(.system(size: 12)) + .foregroundStyle(.secondary) .lineLimit(1) .truncationMode(.tail) } diff --git a/Sources/Workspace/Views/PullRequestDetailView.swift b/Sources/Workspace/Views/PullRequestDetailView.swift index 0ecabcf..c79f082 100644 --- a/Sources/Workspace/Views/PullRequestDetailView.swift +++ b/Sources/Workspace/Views/PullRequestDetailView.swift @@ -163,9 +163,31 @@ struct PullRequestDetailView: View { await store.loadBuilds(item, project: project, pr: pr) } + // How many reads this loop has made of the runs it is watching now, and + // which those are. A new run appearing — a push, a re-run — starts the + // count again, so it is always watched closely at first. + var rounds = 0 + var watching: Set = [] + while !Task.isCancelled { + let unsettled = Set(item.builds.filter { !$0.state.isSettled }.map(\.id)) + if unsettled != watching { + watching = unsettled + rounds = 0 + } + do { - try await Task.sleep(for: .seconds(10)) + try await Task.sleep(for: Self.buildInterval( + round: rounds, + unsettled: unsettled.count, + // A read that failed is not a verdict either. A tick that + // stumbles leaves the last good list alone, but a first + // load that failed leaves no list at all — and no list + // reads as "nothing is running", which is the slow tick. + // So an error keeps this on the close watch until the host + // answers, for the same reason `unknown` is not settled. + afterFailedRead: item.buildsError != nil + )) } catch { return } @@ -174,6 +196,40 @@ struct PullRequestDetailView: View { // tick tries again. guard NSApplication.shared.isActive, !item.isLoadingBuilds else { continue } await store.refreshBuilds(item, project: project, pr: pr) + rounds += 1 + } + } + + /// How long to wait before reading the runs again. + /// + /// A run still going is the thing the person is sitting here for, so the + /// first minute is read every ten seconds and it eases off from there: a + /// build that takes half an hour does not need a hundred and eighty reads + /// of it, and each one is a `gh` or `bkt` process. + /// + /// Once every run has its verdict there is no news left but a push, and + /// this loop used to keep asking every ten seconds anyway, for as long as + /// the pull request stayed on screen — a pull request left open for a + /// working day was thousands of calls to the host for an answer that + /// stopped changing in the first three minutes. So a settled pull request + /// drops to the slow tick, which is there to notice a run that appears + /// rather than one that changes. The count restarts the moment one does. + /// + /// `afterFailedRead` is there because settling and not knowing look the + /// same from here — both are an empty list of runs still going — and only + /// one of them means there is nothing left to ask about. + private static func buildInterval( + round: Int, + unsettled: Int, + afterFailedRead: Bool + ) -> Duration { + // Nothing running: only a push can change this, and that is the kind of + // thing the sidebar's five-minute sweep is paced for. + guard unsettled > 0 || afterFailedRead else { return .seconds(round < 6 ? 30 : 300) } + switch round { + case ..<6: return .seconds(10) + case ..<12: return .seconds(30) + default: return .seconds(60) } }