diff --git a/agterm/Commands/CustomCommandRunner.swift b/agterm/Commands/CustomCommandRunner.swift index ccd9eda8..1f6a663b 100644 --- a/agterm/Commands/CustomCommandRunner.swift +++ b/agterm/Commands/CustomCommandRunner.swift @@ -30,11 +30,15 @@ final class CustomCommandRunner { /// How long a half-typed leader sequence waits for its next chord before abandoning (kitty-style). private static let leaderTimeout: TimeInterval = 1.5 - init(library: WindowLibrary, settings: SettingsModel, actions: AppActions, + /// Run counts behind the title-bar popover's most-used section; every spawn path records into it. + let usage: CustomCommandUsageStore + + init(library: WindowLibrary, settings: SettingsModel, actions: AppActions, usage: CustomCommandUsageStore, socketProvider: @escaping () -> String) { self.library = library self.settings = settings self.actions = actions + self.usage = usage self.socketProvider = socketProvider } @@ -377,6 +381,7 @@ final class CustomCommandRunner { } do { try process.run() + usage.record(command) } catch { logger.error("custom command \"\(name, privacy: .public)\" failed to spawn: \(error.localizedDescription, privacy: .public)") NotificationManager.shared.notifyCommandFailure(name: name, detail: error.localizedDescription) diff --git a/agterm/Views/WindowContentView+CustomCommands.swift b/agterm/Views/WindowContentView+CustomCommands.swift index f2333300..29f7f0c7 100644 --- a/agterm/Views/WindowContentView+CustomCommands.swift +++ b/agterm/Views/WindowContentView+CustomCommands.swift @@ -3,9 +3,13 @@ import AppKit import SwiftUI /// Title-bar custom-commands button and its popover, the mouse form of the ⌃⇧O palette (#570): every -/// `keymap.conf` command as a clickable row with its chord, in file order so a row never moves under a -/// learned mouse position. Split out of `WindowContentView` like `+RecentSessions`, whose button it mirrors. +/// `keymap.conf` command as a clickable row with its chord, the most-run ones grouped on top. Both groups +/// keep file order; usage changes only which commands sit in the top group. Split out of +/// `WindowContentView` like `+RecentSessions`, whose button it mirrors. extension WindowContentView { + /// How many most-run commands lead the list, and the command count above which that section appears. + static let mostUsedCommandLimit = 5 + /// Title-bar button opening the custom-commands popover. Disabled/dimmed with no parsed command or no /// active session: the runner ignores a command fired without one, so every row would silently no-op. /// Opening a popover is interactive-only, so it is control-API keep-in-sync exempt like the clock and bell. @@ -37,14 +41,28 @@ extension WindowContentView { } } - /// The popover body: the commands in keymap order, in a list that scrolls past a cap since the keymap - /// has none, as wide as its longest row between a floor and the recent-sessions popover's width. Tinted - /// like that popover. + /// The popover body: the most-run commands (only once the keymap holds more than the limit) above a + /// separator, then the rest, in a list that scrolls past a cap since the keymap has none, as wide as its + /// longest row between a floor and the recent-sessions popover's width. Counts decide only which rows + /// lead; both groups keep keymap order, so usage changes which commands sit on top and nothing else. + /// Tinted like the recent-sessions popover. The counts are read on every open, so runs from a chord or + /// the palette count too. private func customCommandsPopover(_ commands: [CustomCommand]) -> some View { let metrics = GhosttyApp.shared.interfaceMetrics + let mostUsed = commands.count > Self.mostUsedCommandLimit + ? actions.customCommandRunner?.usage.load().mostUsed(of: commands, limit: Self.mostUsedCommandLimit) ?? [] + : [] + let leadingIDs = Set(mostUsed.map(\.id)) + let leading = commands.filter { leadingIDs.contains($0.id) } + let rest = commands.filter { !leadingIDs.contains($0.id) } return ScrollView { VStack(spacing: 2) { - ForEach(commands) { customCommandRow($0) } + ForEach(leading) { customCommandRow($0, accessibilityID: "custom-command-top-row") } + if !leading.isEmpty { + Rectangle().fill(chromeText.opacity(0.25)).frame(height: 1) + .padding(.horizontal, 8).padding(.vertical, 4) + } + ForEach(rest) { customCommandRow($0, accessibilityID: "custom-command-row") } } .padding(6) } @@ -54,9 +72,10 @@ extension WindowContentView { .presentationBackground(terminalColor) } - private func customCommandRow(_ command: CustomCommand) -> some View { + private func customCommandRow(_ command: CustomCommand, accessibilityID: String) -> some View { CustomCommandPopoverRow(title: command.name, shortcut: command.shortcut.isEmpty ? nil : command.shortcut, - foreground: chromeText, hoverColor: popoverHoverColor) { runFromPopover(command) } + foreground: chromeText, hoverColor: popoverHoverColor, + accessibilityID: accessibilityID) { runFromPopover(command) } } /// Commit a row click: note activity (so auto-follow can't pull the selection away), run the command @@ -78,6 +97,7 @@ private struct CustomCommandPopoverRow: View { let shortcut: String? let foreground: Color let hoverColor: Color + let accessibilityID: String let onSelect: () -> Void @State private var hovering = false private let metrics = GhosttyApp.shared.interfaceMetrics @@ -128,6 +148,6 @@ private struct CustomCommandPopoverRow: View { } .buttonStyle(.plain) .onHover { hovering = $0 } - .accessibilityIdentifier("custom-command-row") + .accessibilityIdentifier(accessibilityID) } } diff --git a/agterm/agtermApp.swift b/agterm/agtermApp.swift index 103565a4..b395687c 100644 --- a/agterm/agtermApp.swift +++ b/agterm/agtermApp.swift @@ -100,6 +100,7 @@ struct agtermApp: App { // server's bound socket path for `{AGT_SOCKET}`. _customCommandRunner = State(initialValue: CustomCommandRunner( library: library, settings: settingsModel, actions: actions, + usage: CustomCommandUsageStore(directory: stateDirectory), socketProvider: { controlServer.resolvedSocketPath })) // follows macOS light/dark via KVO on NSApp.effectiveAppearance; dependency-free, started in `.task`. _appearanceObserver = State(initialValue: SystemAppearanceObserver()) diff --git a/agtermCore/Sources/agtermCore/CustomCommandUsage.swift b/agtermCore/Sources/agtermCore/CustomCommandUsage.swift new file mode 100644 index 00000000..115570cc --- /dev/null +++ b/agtermCore/Sources/agtermCore/CustomCommandUsage.swift @@ -0,0 +1,73 @@ +import Foundation + +/// Run counts for `keymap.conf` commands, keyed by name, backing the most-used section of the title-bar +/// custom-commands popover. The name is the durable key: `CustomCommand.id` is minted on every parse, and +/// the parser rejects a second `command` line with a name already taken. +public struct CustomCommandUsage: Codable, Equatable, Sendable { + public static let currentVersion = 1 + + public var version: Int + public var counts: [String: Int] + + public init(version: Int = CustomCommandUsage.currentVersion, counts: [String: Int] = [:]) { + self.version = version + self.counts = counts + } + + /// Count one run of `command`. + public mutating func record(_ command: CustomCommand) { + counts[command.name, default: 0] += 1 + } + + /// The most-run of `commands`, at most `limit`, in descending count with ties kept in `commands` order. + /// A command never run is left out, and a count whose command is no longer in the keymap takes no slot. + public func mostUsed(of commands: [CustomCommand], limit: Int) -> [CustomCommand] { + commands.enumerated() + .compactMap { index, command -> (count: Int, index: Int, command: CustomCommand)? in + guard let count = counts[command.name], count > 0 else { return nil } + return (count, index, command) + } + .sorted { $0.count != $1.count ? $0.count > $1.count : $0.index < $1.index } + .prefix(max(0, limit)) + .map(\.command) + } +} + +/// On-disk home of `CustomCommandUsage`: `/custom-command-usage.json`, read tolerantly (a missing, +/// corrupt or foreign-version file counts as empty) and written atomically, like `RecentClosedStore`. +public struct CustomCommandUsageStore: Sendable { + private let directory: URL + private let fileName: String + + private var fileURL: URL { directory.appendingPathComponent(fileName) } + + public init(directory: URL, fileName: String = "custom-command-usage.json") { + self.directory = directory + self.fileName = fileName + } + + public func load() -> CustomCommandUsage { + guard let data = try? Data(contentsOf: fileURL), + let usage = try? JSONDecoder().decode(CustomCommandUsage.self, from: data), + usage.version == CustomCommandUsage.currentVersion else { return CustomCommandUsage() } + return usage + } + + /// Count one run of `command` and persist the result. + public func record(_ command: CustomCommand) { + var usage = load() + usage.record(command) + save(usage) + } + + private func save(_ usage: CustomCommandUsage) { + do { + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let encoder = JSONEncoder() + encoder.outputFormatting = [.prettyPrinted, .sortedKeys] + try encoder.encode(usage).write(to: fileURL, options: .atomic) + } catch { + NSLog("agterm: save custom command usage failed: %@", String(describing: error)) + } + } +} diff --git a/agtermCore/Sources/agtermCore/Keymap.swift b/agtermCore/Sources/agtermCore/Keymap.swift index af522b51..38fecba9 100644 --- a/agtermCore/Sources/agtermCore/Keymap.swift +++ b/agtermCore/Sources/agtermCore/Keymap.swift @@ -710,9 +710,10 @@ private func splitMapAlternatives(_ parsed: Alternatives, line: Int, return (menuChord, alternatives) } -/// Parse the remainder of a `command` line (after the verb): `"" [chord] `. On any failure -/// it appends a diagnostic and leaves `commandLines` untouched. The kept alternatives ride alongside the -/// command; `applySurvivingShortcuts` is what turns them back into `CustomCommand.shortcut`. +/// Parse the remainder of a `command` line (after the verb): `"" [chord] `. On any failure, +/// a name already taken included, it appends a diagnostic and leaves `commandLines` untouched. The kept +/// alternatives ride alongside the command; `applySurvivingShortcuts` is what turns them back into +/// `CustomCommand.shortcut`. private func parseCommandLine(_ rest: String, line: Int, commandLines: inout [ParsedCommandLine], diagnostics: inout [KeymapDiagnostic]) { guard rest.first == "\"", let closeQuote = rest.dropFirst().firstIndex(of: "\"") else { @@ -755,6 +756,13 @@ private func parseCommandLine(_ rest: String, line: Int, commandLines: inout [Pa return } + // the name is the identity a run count is stored under, so a second definition cannot share it. + guard !commandLines.contains(where: { $0.command.name == name }) else { + diagnostics.append(KeymapDiagnostic(line: line, + message: "command '\(name)' is already defined; command skipped")) + return + } + commandLines.append(ParsedCommandLine(command: CustomCommand(name: name, command: shellLine, shortcut: ""), alternatives: kept)) } diff --git a/agtermCore/Tests/agtermCoreTests/CustomCommandUsageTests.swift b/agtermCore/Tests/agtermCoreTests/CustomCommandUsageTests.swift new file mode 100644 index 00000000..c7440937 --- /dev/null +++ b/agtermCore/Tests/agtermCoreTests/CustomCommandUsageTests.swift @@ -0,0 +1,58 @@ +import Foundation +import Testing +@testable import agtermCore + +struct CustomCommandUsageTests { + private let finder = CustomCommand(name: "Finder", command: "open .", shortcut: "cmd+shift+f") + private let lazygit = CustomCommand(name: "Lazygit", command: "lazygit", shortcut: "") + private let cheatsheet = CustomCommand(name: "Cheatsheet", command: "less keys.md", shortcut: "ctrl+a>k") + + @Test func recordCountsRunsByName() { + var usage = CustomCommandUsage() + usage.record(finder) + usage.record(finder) + usage.record(lazygit) + #expect(usage.counts == ["Finder": 2, "Lazygit": 1]) + } + + @Test func mostUsedOrdersByCountThenKeymapOrderAndSkipsNeverRun() { + var usage = CustomCommandUsage() + usage.record(cheatsheet) + usage.record(lazygit) + usage.record(finder) + usage.record(cheatsheet) + let commands = [finder, lazygit, cheatsheet] + #expect(usage.mostUsed(of: commands, limit: 5).map(\.name) == ["Cheatsheet", "Finder", "Lazygit"]) + #expect(usage.mostUsed(of: commands, limit: 2).map(\.name) == ["Cheatsheet", "Finder"]) + #expect(usage.mostUsed(of: [lazygit, finder], limit: 5).map(\.name) == ["Lazygit", "Finder"]) + #expect(usage.mostUsed(of: commands, limit: 0).isEmpty) + #expect(CustomCommandUsage().mostUsed(of: commands, limit: 5).isEmpty) + } + + @Test func mostUsedIgnoresCountsForCommandsNoLongerInTheKeymap() { + var usage = CustomCommandUsage(counts: ["Gone": 9, "Zero": 0]) + usage.record(finder) + let zero = CustomCommand(name: "Zero", command: "true", shortcut: "") + #expect(usage.mostUsed(of: [zero, finder], limit: 5).map(\.name) == ["Finder"]) + } + + @Test func storeRoundTripsAndReadsMissingOrForeignFilesAsEmpty() throws { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("custom-command-usage-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: directory) } + let file = directory.appendingPathComponent("custom-command-usage.json") + let store = CustomCommandUsageStore(directory: directory) + #expect(store.load() == CustomCommandUsage()) + + store.record(finder) + store.record(finder) + #expect(store.load() == CustomCommandUsage(counts: ["Finder": 2])) + + let foreign = CustomCommandUsage(version: CustomCommandUsage.currentVersion + 1, counts: ["x": 3]) + try JSONEncoder().encode(foreign).write(to: file) + #expect(store.load() == CustomCommandUsage()) + + try Data("not json".utf8).write(to: file) + #expect(store.load() == CustomCommandUsage()) + } +} diff --git a/agtermCore/Tests/agtermCoreTests/KeymapTests.swift b/agtermCore/Tests/agtermCoreTests/KeymapTests.swift index 43a109c2..3dfad39a 100644 --- a/agtermCore/Tests/agtermCoreTests/KeymapTests.swift +++ b/agtermCore/Tests/agtermCoreTests/KeymapTests.swift @@ -237,6 +237,18 @@ struct KeymapTests { #expect(command.command == "./deploy.sh") } + @Test func parseCommandDuplicateNameKeepsTheFirstDefinition() { + let (keymap, diagnostics) = parseKeymap(""" + command "Deploy" cmd+shift+d ./deploy.sh + command "Deploy" ./deploy.sh --prod + command "Other" ./other.sh + """) + #expect(keymap.commands.map(\.name) == ["Deploy", "Other"]) + #expect(keymap.commands[0].command == "./deploy.sh") + #expect(keymap.commands[0].shortcut == "cmd+shift+d") + #expect(diagnostics == [KeymapDiagnostic(line: 2, message: "command 'Deploy' is already defined; command skipped")]) + } + @Test func parseCommandBareKeyRejectedAsShortcut() { // a bare key would shadow that key in the terminal, so it is never consumed as a shortcut. let (keymap, diagnostics) = parseKeymap("command \"X\" a echo hi") diff --git a/agtermTests/CustomCommandRunnerTests.swift b/agtermTests/CustomCommandRunnerTests.swift index a2354eb5..4745bbed 100644 --- a/agtermTests/CustomCommandRunnerTests.swift +++ b/agtermTests/CustomCommandRunnerTests.swift @@ -77,7 +77,7 @@ final class CustomCommandRunnerTests: XCTestCase { let actions = AppActions(library: library) actions.settingsModel = settings let runner = CustomCommandRunner(library: library, settings: settings, actions: actions, - socketProvider: { "" }) + usage: CustomCommandUsageStore(directory: stateDir), socketProvider: { "" }) runner.start() started.append(runner) let store = try XCTUnwrap(library.activeStore) diff --git a/agtermTests/FullScreenChordTests.swift b/agtermTests/FullScreenChordTests.swift index e6ab3b3b..1f44127b 100644 --- a/agtermTests/FullScreenChordTests.swift +++ b/agtermTests/FullScreenChordTests.swift @@ -35,6 +35,7 @@ final class FullScreenChordTests: XCTestCase { settings: SettingsModel(library: library, settingsStore: SettingsStore(directory: stateDir)), actions: AppActions(library: library), + usage: CustomCommandUsageStore(directory: stateDir), socketProvider: { "" }) // `NSWindow` defaults isReleasedWhenClosed to true; see the hosted-test rule in ui-tests.md. window = RecordingWindow(contentRect: NSRect(x: 0, y: 0, width: 400, height: 300), @@ -84,7 +85,8 @@ final class FullScreenChordTests: XCTestCase { settings.setConfigDirectory(configDir.path) let actions = AppActions(library: library) actions.settingsModel = settings - return CustomCommandRunner(library: library, settings: settings, actions: actions, socketProvider: { "" }) + return CustomCommandRunner(library: library, settings: settings, actions: actions, + usage: CustomCommandUsageStore(directory: stateDir), socketProvider: { "" }) } func testShippedChordTogglesFullScreenAndIsConsumed() throws { diff --git a/agtermUITests/CustomCommandsButtonUITests.swift b/agtermUITests/CustomCommandsButtonUITests.swift index 2681699a..fccbc77c 100644 --- a/agtermUITests/CustomCommandsButtonUITests.swift +++ b/agtermUITests/CustomCommandsButtonUITests.swift @@ -2,10 +2,11 @@ import XCTest /// End-to-end tests for the title-bar custom-commands button, the mouse form of ⌃⇧O (#570). It is off by /// default and opts in through `shownInterfaceElements`; it enables only with parsed commands and an active -/// session; its popover lists every command with its chord in keymap order. +/// session; its popover lists every command with its chord, the most-run ones first once there are more +/// than five. /// /// SCOPE NOTE: as for the recent-sessions button, a synthesized click on a row inside the `NSPopover` fires -/// nothing, so the row → run glue is verified by hand. +/// nothing, so the row → run glue is verified by hand; the run counts are driven here through a chord. @MainActor final class CustomCommandsButtonUITests: ControlAPITestCase { override var seededSettings: [String: Any]? { @@ -36,7 +37,50 @@ final class CustomCommandsButtonUITests: ControlAPITestCase { let first = rows.element(boundBy: 0).label XCTAssertTrue(first.contains("Touch One"), "rows keep keymap order, got: \(first)") XCTAssertTrue(first.contains("cmd+shift+e"), "a bound command shows its chord, got: \(first)") - XCTAssertTrue(rows.element(boundBy: 1).label.contains("Touch Two"), "an unbound command lists too") + XCTAssertEqual(app.buttons.matching(identifier: "custom-command-top-row").count, 0, + "the most-used section needs more than five commands") + } + + func testMostRunCommandsLeadOncePastFive() throws { + let marker = markerDir.appendingPathComponent("six") + var keymap = "" + for index in 1...5 { keymap += "command \"Cmd \(index)\" echo \(index)\n" } + keymap += "command \"Touch Six\" cmd+shift+e touch '\(marker.path)'\n" + try relaunch(withKeymap: keymap) + focusTerminal() + XCTAssertTrue(chordFiresMarker(marker) { app.typeKey("e", modifierFlags: [.command, .shift]) }, + "⌘⇧E should run the sixth command and touch the marker file") + + let button = app.buttons["custom-commands-button"] + XCTAssertTrue(pollEnabled(button, true, timeout: 10), "six parsed commands should enable the button") + let top = app.buttons.matching(identifier: "custom-command-top-row") + openPopover(button, until: top.firstMatch, timeout: 10) + XCTAssertEqual(top.count, 1, "only the run command has a count, so the most-used section holds one row") + XCTAssertTrue(top.firstMatch.label.contains("Touch Six"), "the run command should lead, got: \(top.firstMatch.label)") + XCTAssertEqual(app.buttons.matching(identifier: "custom-command-row").count, 5, "the rest follow below the separator") + } + + // counts seeded straight into the usage file: seven commands with counts rising in file order, so the + // top group is the last five by count while still rendering in file order. + func testMostRunGroupKeepsFileOrder() throws { + var keymap = "" + for name in ["A", "B", "C", "D", "E", "F", "G"] { keymap += "command \"Cmd \(name)\" true\n" } + let counts = Dictionary(uniqueKeysWithValues: ["A", "B", "C", "D", "E", "F", "G"].enumerated() + .map { ("Cmd \($0.element)", $0.offset + 1) }) + let usage = try JSONSerialization.data(withJSONObject: ["version": 1, "counts": counts]) + try usage.write(to: stateDir.appendingPathComponent("custom-command-usage.json")) + try relaunch(withKeymap: keymap) + + let button = app.buttons["custom-commands-button"] + XCTAssertTrue(pollEnabled(button, true, timeout: 10), "seven parsed commands should enable the button") + let top = app.buttons.matching(identifier: "custom-command-top-row") + openPopover(button, until: top.firstMatch, timeout: 10) + let leading = top.allElementsBoundByIndex.map(\.label) + XCTAssertEqual(leading.count, 5, "the five most-run commands lead, got: \(leading)") + XCTAssertEqual(leading.map { String($0.prefix(5)) }, ["Cmd C", "Cmd D", "Cmd E", "Cmd F", "Cmd G"], + "the top group keeps file order rather than count order, got: \(leading)") + let rest = app.buttons.matching(identifier: "custom-command-row").allElementsBoundByIndex.map(\.label) + XCTAssertEqual(rest.map { String($0.prefix(5)) }, ["Cmd A", "Cmd B"], "the rest follow in file order, got: \(rest)") } /// (Re)opens the popover until `row` appears. The transient popover can dismiss before the first @@ -61,4 +105,27 @@ final class CustomCommandsButtonUITests: ControlAPITestCase { } return element.exists && element.isEnabled == expected } + + /// Click the seeded session row so the terminal surface takes first responder, then let the responder + /// bounce settle, so the chord resolves from that surface. + private func focusTerminal() { + let row = app.staticTexts["session-row"].firstMatch + XCTAssertTrue(row.waitForHittable(timeout: 20), "seeded session should be hittable") + row.click() + let deadline = Date().addingTimeInterval(2) + while Date() < deadline, row.isSelected == false { + RunLoop.current.run(until: Date().addingTimeInterval(0.05)) + } + } + + /// Run `press` and poll for `marker`, retrying the press a few times: the first burst after + /// `focusTerminal` can land before the surface is genuinely first responder and be dropped. + private func chordFiresMarker(_ marker: URL, attempts: Int = 6, perAttempt: TimeInterval = 2.5, + press: () -> Void) -> Bool { + for _ in 0..Navigate ▸ Custom Commands palette, ctrl+a>o is the chord that fires it — Ctrl-A, then O — and everything after that is an ordinary shell line. The chord is - optional: leave it out and the action exists in the palette only. For a mouse path, turn on the + optional: leave it out and the action exists in the palette only. Names are unique: a second + command line with a + name already taken is skipped with a diagnostic. For a mouse path, turn on the Custom commands title-bar button in Settings ▸ Interface: it - lists the same commands with their chords in a popover, in the order of the file, so put the ones - you reach for most first. + lists the same commands with their chords in a popover, in the order of the file. Once there are more than + five, the five you run most are grouped on top, still in file order; use changes only which commands + sit in that group.

The building block is session context. Every custom command is handed the