From 2237c4d44102610a001bf7a1d8fb8e160fdc18df Mon Sep 17 00:00:00 2001 From: Ajay-Satish-01 Date: Mon, 31 Aug 2026 16:17:00 -0500 Subject: [PATCH 1/3] feat(ui): let note composer save be remapped (#846) Zellij and similar multiplexers swallow Ctrl-S before Hunk sees it, so the composer save shortcut is now hunk.review.saveNote (default ctrl+s) instead of a hardcoded widget key. --- .changeset/remappable-save-note.md | 5 + docs/extensions.md | 5 +- docs/keybindings.md | 16 ++- src/core/run/commandCatalog.test.ts | 3 + src/core/run/commandCatalog.ts | 10 ++ src/ui/App.tsx | 5 +- src/ui/AppHost.keybindings.test.tsx | 129 +++++++++++++++++- .../components/panes/AgentInlineNote.test.tsx | 59 ++++++++ src/ui/components/panes/AgentInlineNote.tsx | 37 +++-- src/ui/components/panes/DiffPane.tsx | 5 + src/ui/components/ui-components.test.tsx | 22 +-- src/ui/hooks/useAppKeyboardShortcuts.ts | 19 +-- src/ui/lib/agentAnnotations.ts | 2 + src/ui/lib/appCommands.test.ts | 4 + src/ui/lib/appCommands.ts | 7 +- src/ui/lib/appMenus.test.ts | 1 + src/ui/lib/helpContent.test.ts | 7 + src/ui/lib/helpContent.ts | 1 + src/ui/lib/keyboard.ts | 81 +++++++++-- src/ui/lib/ui-lib.test.ts | 59 +++++++- test/pty/notes.test.ts | 8 +- .../docs/docs/configure/keybindings.md | 2 +- .../content/docs/docs/extend/extension-api.md | 2 +- 23 files changed, 428 insertions(+), 61 deletions(-) create mode 100644 .changeset/remappable-save-note.md diff --git a/.changeset/remappable-save-note.md b/.changeset/remappable-save-note.md new file mode 100644 index 000000000..1b24dafcc --- /dev/null +++ b/.changeset/remappable-save-note.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Make the note composer save shortcut remappable via `hunk.review.saveNote` (default `ctrl+s`). diff --git a/docs/extensions.md b/docs/extensions.md index 03e00e332..f4bdc8de2 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -1822,8 +1822,9 @@ it, whether the user reached it through a key, a menu, an old command alias, or may still have detached async work in flight; this event observes the accepted user action, not promise settlement. Listen for ids rather than key chords so behavior follows the user's live `[keybindings]` table. Browser/session actions lower to shared review intents rather than terminal -commands and do not emit this event. Modal widget keys such as Escape, Enter, note-editor Ctrl-S, -and F10 menu navigation are also not commands. +commands and do not emit this event. Modal widget keys such as Escape, Enter, +and F10 menu navigation are also not commands. The note composer's save shortcut +is `hunk.review.saveNote` and does emit this event. `session_reload`'s `reason` is `"watch"` (the watcher saw the source change), `"daemon"` (an agent command through the session broker), or `"manual"` (the diff --git a/docs/keybindings.md b/docs/keybindings.md index 356b8f1e3..71b605ff4 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -73,6 +73,7 @@ The built-in commands and the keys they ship with: | `hunk.review.previousFile` | Previous file | `,` | | `hunk.review.previousHunk` | Previous hunk | `[` | | `hunk.review.replyToActiveNote` | Reply to the active review note | `R` | +| `hunk.review.saveNote` | Save a draft review note | `ctrl+s` | | `hunk.review.scrollCodeLeft` | Scroll code left (shifted scrolls fast) | `left`, `shift+left` | | `hunk.review.scrollCodeRight` | Scroll code right (shifted scrolls fast) | `right`, `shift+right` | | `hunk.review.startNote` | Add a review note | `c` | @@ -122,10 +123,17 @@ invoke these same public `hunk.*` commands. Routing precedence is host prompts and dialogs, menus/overlays, focused text inputs, an interactive file-view mode, a session extension keyboard mode, then the command table and focused review widget. Keys that belong to a dialog, -menu, or focused text input — `Esc`, `Enter`, `Ctrl-S` while writing a note — -are part of those widgets rather than commands, and are not remappable. Escape -is also the reserved exit from each active extension mode, so an extension -cannot trap the keyboard. +menu, or focused text input — `Esc`, `Enter` — are part of those widgets rather +than commands, and are not remappable. The note composer's save shortcut is the +command `hunk.review.saveNote` (default `ctrl+s`) and is remappable; while the +composer is focused it still wins over the command table, using the resolved +chord. Escape is also the reserved exit from each active extension mode, so an +extension cannot trap the keyboard. + +```toml +[keybindings] +"hunk.review.saveNote" = "ctrl+enter" # Zellij-friendly; default is ctrl+s +``` `[keybindings]` is read from your user config only — never from a repository's `.hunk/config.toml`. Which keys do what is a property of your keyboard and your diff --git a/src/core/run/commandCatalog.test.ts b/src/core/run/commandCatalog.test.ts index 2b04000d4..0f50656d4 100644 --- a/src/core/run/commandCatalog.test.ts +++ b/src/core/run/commandCatalog.test.ts @@ -113,6 +113,9 @@ describe("app command catalog", () => { expect( lowerAppCommandToReviewIntent(entry("hunk.app.quit"), { count: 1, state }), ).toBeUndefined(); + expect( + lowerAppCommandToReviewIntent(entry("hunk.review.saveNote"), { count: 1, state }), + ).toBeUndefined(); }); test("lowers a new note at the current selection, with an optional measured line", () => { diff --git a/src/core/run/commandCatalog.ts b/src/core/run/commandCatalog.ts index 473151191..1d69c08e7 100644 --- a/src/core/run/commandCatalog.ts +++ b/src/core/run/commandCatalog.ts @@ -188,6 +188,16 @@ const BUILTIN_COMMANDS = [ publicToExtensions: true, closesMenu: true, }, + { + id: "hunk.review.saveNote", + title: "Save review note", + category: "review", + defaultKeys: ["ctrl+s"], + // The TUI draft buffer is this client's; persist already goes through + // `notes/create-user` / `notes/update-user` inside the save handler. + locus: "client-local", + publicToExtensions: true, + }, { id: "hunk.review.pageDown", title: "Scroll down one page", diff --git a/src/ui/App.tsx b/src/ui/App.tsx index ac62e8205..93ebb527a 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -81,6 +81,7 @@ import { buildAppCommands, builtinCommandKeyDefaults, builtinCommandMatchProbes, + findAppCommandById, observeAppCommandDispatch, } from "./lib/appCommands"; import { buildAppMenus } from "./lib/appMenus"; @@ -1039,6 +1040,7 @@ export function App({ stepDiffLine, selectCursorLine: setCursorLine, selectLayoutMode, + saveDraftNote, startUserNote: () => startUserNote(), toggleAgentNotes, toggleCopyDecorations, @@ -1057,6 +1059,7 @@ export function App({ ], publishCommandExecuted, ); + const draftSaveKeyLabel = findAppCommandById(appCommands, "hunk.review.saveNote")?.keyLabels[0]; useExtensionRuntimeBindings({ commands: appCommands, navigation: extensionNavigationBindings, @@ -1144,7 +1147,6 @@ export function App({ discardViewPreferencesAndQuit, neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, - saveDraftNote, showAgentSkill, showHelp, switchMenu, @@ -1360,6 +1362,7 @@ export function App({ onRemoveLiveNote={review.removeLiveComment} onRemoveUserNote={review.removeUserNote} onSaveDraftNote={saveDraftNote} + draftSaveKeyLabel={draftSaveKeyLabel} onStartUserNoteAtHunk={startUserNote} onUpdateDraftNote={updateDraftNote} onBlurDraftNote={blurDraftNote} diff --git a/src/ui/AppHost.keybindings.test.tsx b/src/ui/AppHost.keybindings.test.tsx index 1416daf55..23156b84a 100644 --- a/src/ui/AppHost.keybindings.test.tsx +++ b/src/ui/AppHost.keybindings.test.tsx @@ -97,6 +97,7 @@ async function withAppHost( bootstrap: AppBootstrap, body: (setup: Awaited>, quits: () => number) => Promise, externalQuitSignal?: AbortSignal, + renderOptions?: { kittyKeyboard?: boolean }, ) { let quitCount = 0; const setup = await testRender( @@ -105,7 +106,7 @@ async function withAppHost( externalQuitSignal={externalQuitSignal} onQuit={() => (quitCount += 1)} />, - { width: 120, height: 24 }, + { width: 120, height: 24, ...renderOptions }, ); try { @@ -375,4 +376,130 @@ describe("user keybindings", () => { expect(seen).toEqual(["hunk.app.toggleFocusArea"]); }); }); + + test("a remapped save-note chord saves a draft and emits command_executed", async () => { + const repo = createTestRepo("hunk-keybindings-save-note-remap-"); + const bootstrap = await launchWithConfig( + repo, + '[keybindings]\n"hunk.review.saveNote" = "ctrl+enter"\n', + ); + const extensions = createEmptyExtensionLoadResult(repo); + const seen: string[] = []; + extensions.registry.eventHandlers.command_executed.push({ + extensionId: "coach", + handler: ({ commandId }) => { + seen.push(commandId); + }, + }); + bootstrap.extensions = extensions; + + // Kitty encodes Ctrl+Enter as CSI-u; legacy mock input would emit a bare + // return and drop the ctrl flag. + await withAppHost( + bootstrap, + async (setup) => { + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("Remapped save."); + }); + await flush(setup); + expect(setup.captureCharFrame()).toContain("Ctrl+Enter save"); + + await act(async () => { + setup.mockInput.pressKey("s", { ctrl: true }); + }); + await flush(setup); + expect(setup.captureCharFrame()).toContain("Draft note"); + expect(setup.captureCharFrame()).not.toContain("Your note"); + + await act(async () => { + await setup.mockInput.pressKeys(["\u001b[115;5u"]); + }); + await flush(setup); + expect(setup.captureCharFrame()).toContain("Draft note"); + expect(setup.captureCharFrame()).not.toContain("Your note"); + + seen.length = 0; + await act(async () => { + setup.mockInput.pressEnter({ ctrl: true }); + }); + await flush(setup); + expect(seen).toEqual(["hunk.review.saveNote"]); + const saved = setup.captureCharFrame(); + expect(saved).toContain("Your note"); + expect(saved).toContain("Remapped save."); + expect(saved).not.toContain("Draft note"); + }, + undefined, + { kittyKeyboard: true }, + ); + }); + + test("unbinding save-note leaves Ctrl-S doing nothing in the composer", async () => { + const repo = createTestRepo("hunk-keybindings-save-note-unbind-"); + const bootstrap = await launchWithConfig( + repo, + '[keybindings]\n"hunk.review.saveNote" = false\n', + ); + + await withAppHost(bootstrap, async (setup) => { + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("Still a draft."); + }); + await flush(setup); + + await act(async () => { + setup.mockInput.pressKey("s", { ctrl: true }); + }); + await flush(setup); + let frame = setup.captureCharFrame(); + expect(frame).toContain("Draft note"); + expect(frame).toContain("Still a draft."); + expect(frame).not.toContain("Your note"); + + await act(async () => { + await setup.mockInput.pressKeys(["\u001b[115;5u"]); + }); + await flush(setup); + frame = setup.captureCharFrame(); + expect(frame).toContain("Draft note"); + expect(frame).toContain("Still a draft."); + expect(frame).not.toContain("Your note"); + }); + }); + + test("CSI-u Ctrl-S does not save after save-note is remapped away", async () => { + const repo = createTestRepo("hunk-keybindings-save-note-csiu-remap-"); + const bootstrap = await launchWithConfig( + repo, + '[keybindings]\n"hunk.review.saveNote" = "ctrl+enter"\n', + ); + + await withAppHost(bootstrap, async (setup) => { + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("Encoding net off."); + }); + await flush(setup); + + await act(async () => { + await setup.mockInput.pressKeys(["\u001b[115;5u"]); + }); + await flush(setup); + const frame = setup.captureCharFrame(); + expect(frame).toContain("Draft note"); + expect(frame).toContain("Encoding net off."); + expect(frame).not.toContain("Your note"); + }); + }); }); diff --git a/src/ui/components/panes/AgentInlineNote.test.tsx b/src/ui/components/panes/AgentInlineNote.test.tsx index b62a54b30..e69bf0c54 100644 --- a/src/ui/components/panes/AgentInlineNote.test.tsx +++ b/src/ui/components/panes/AgentInlineNote.test.tsx @@ -362,4 +362,63 @@ describe("AgentInlineNote draft composer", () => { } } }); + + test("draft footer shows the resolved save chord and omits it when unbound", async () => { + const labeled = await testRender( + {}, + onCancel: () => {}, + onSave: () => {}, + saveKeyLabel: "Ctrl+Enter", + }} + />, + { width: 120, height: 12 }, + ); + + try { + await flush(labeled); + expect(labeled.captureCharFrame()).toContain("Ctrl+Enter save Esc cancel"); + } finally { + await act(async () => { + labeled.renderer.destroy(); + }); + } + + const unbound = await testRender( + {}, + onCancel: () => {}, + onSave: () => {}, + }} + />, + { width: 120, height: 12 }, + ); + + try { + await flush(unbound); + const frame = unbound.captureCharFrame(); + expect(frame).toContain("save Esc cancel"); + expect(frame).not.toContain("Ctrl+S save"); + } finally { + await act(async () => { + unbound.renderer.destroy(); + }); + } + }); }); diff --git a/src/ui/components/panes/AgentInlineNote.tsx b/src/ui/components/panes/AgentInlineNote.tsx index c22798d0e..b3e18986d 100644 --- a/src/ui/components/panes/AgentInlineNote.tsx +++ b/src/ui/components/panes/AgentInlineNote.tsx @@ -36,6 +36,14 @@ interface BorderActionItem { onMouseUp: () => void; } +/** Cells one action occupies: key, optional space, then label. */ +function actionItemTextWidth(keyLabel: string, label: string) { + if (!keyLabel) { + return label.length; + } + return label ? keyLabel.length + 1 + label.length : keyLabel.length; +} + interface AgentInlineNoteLine { kind: "summary" | "rationale"; text: string; @@ -204,15 +212,7 @@ export function AgentInlineNote({ layout: Exclude; noteCount?: number; noteIndex?: number; - draft?: { - body: string; - focused: boolean; - onBlur?: () => void; - onCancel: () => void; - onFocus?: () => void; - onInput: (value: string) => void; - onSave: () => void; - }; + draft?: VisibleAgentNote["draft"]; actions?: AgentInlineNoteActions; /** Legacy compact delete affordance; semantic cards use explicit `actions`. */ onClose?: () => void; @@ -388,7 +388,7 @@ export function AgentInlineNote({ const availableItemsWidth = Math.max(0, boxWidth - 4); const fullItemsWidth = items.reduce( (total, item, index) => - total + item.keyLabel.length + 1 + item.label.length + (index > 0 ? 1 : 0), + total + actionItemTextWidth(item.keyLabel, item.label) + (index > 0 ? 1 : 0), 0, ); const renderedItems = items.map((item) => ({ @@ -397,10 +397,7 @@ export function AgentInlineNote({ })); const itemsWidth = renderedItems.reduce( (total, item, index) => - total + - item.keyLabel.length + - (item.displayLabel ? 1 + item.displayLabel.length : 0) + - (index > 0 ? 1 : 0), + total + actionItemTextWidth(item.keyLabel, item.displayLabel) + (index > 0 ? 1 : 0), 0, ); const innerWidth = Math.max(0, boxWidth - 2); @@ -425,8 +422,8 @@ export function AgentInlineNote({ {renderedItems.map((item, index) => { const hovered = hoveredActionId === item.id; const backgroundColor = hovered ? theme.accentMuted : theme.panel; - const itemWidth = - item.keyLabel.length + (item.displayLabel ? 1 + item.displayLabel.length : 0); + const itemWidth = actionItemTextWidth(item.keyLabel, item.displayLabel); + const labelPrefix = item.keyLabel && item.displayLabel ? " " : ""; return ( - {item.keyLabel} + {item.keyLabel ? {item.keyLabel} : null} {item.displayLabel ? ( - {` ${item.displayLabel}`} + {`${labelPrefix}${item.displayLabel}`} ) : null} @@ -468,7 +467,7 @@ export function AgentInlineNote({ const draftTitleText = fitText(` ${titleText} `, Math.max(0, boxWidth - 4)); const draftTopBorderSuffix = `${"─".repeat(Math.max(0, boxWidth - 3 - draftTitleText.length))}╮`; const draftActionItems: BorderActionItem[] = [ - { id: "save", keyLabel: "^S", label: "save", onMouseUp: draft.onSave }, + { id: "save", keyLabel: draft.saveKeyLabel ?? "", label: "save", onMouseUp: draft.onSave }, { id: "cancel", keyLabel: "Esc", label: "cancel", onMouseUp: draft.onCancel }, ]; const draftTextareaRows = draftVisibleLineCount; diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 1f1fcdb92..a69e4da0a 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -345,6 +345,7 @@ export function DiffPane({ onRemoveLiveNote, onRemoveUserNote, onSaveDraftNote, + draftSaveKeyLabel, onStartUserNoteAtHunk, onUpdateDraftNote, onBlurDraftNote, @@ -416,6 +417,8 @@ export function DiffPane({ onRemoveLiveNote?: (noteId: string) => void; onRemoveUserNote?: (noteId: string) => void; onSaveDraftNote?: () => void; + /** Live chord for the draft save action; omitted when `hunk.review.saveNote` is unbound. */ + draftSaveKeyLabel?: string; onStartUserNoteAtHunk?: (fileId: string, hunkIndex: number, target?: UserNoteLineTarget) => void; onUpdateDraftNote?: (body: string) => void; onBlurDraftNote?: () => void; @@ -694,6 +697,7 @@ export function DiffPane({ onFocus: onFocusDraftNote, onInput: onUpdateDraftNote ?? (() => {}), onSave: onSaveDraftNote ?? (() => {}), + ...(draftSaveKeyLabel ? { saveKeyLabel: draftSaveKeyLabel } : {}), }, }); if (draftNote.kind === "edit" && draftNote.targetNoteId) { @@ -762,6 +766,7 @@ export function DiffPane({ onRemoveLiveNote, onRemoveUserNote, onSaveDraftNote, + draftSaveKeyLabel, onUpdateDraftNote, showAgentNotes, ]); diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 74d9acd5c..92600bb78 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -2937,6 +2937,7 @@ describe("UI components", () => { onCancel, onInput: () => {}, onSave, + saveKeyLabel: "Ctrl+S", }} />, { width: 64, height: measured + 1 }, @@ -2945,7 +2946,7 @@ describe("UI components", () => { try { await act(async () => setup.renderOnce()); const restingLines = setup.captureCharFrame().split("\n"); - expect(restingLines[measured - 1]).toContain("^S save Esc cancel"); + expect(restingLines[measured - 1]).toContain("Ctrl+S save Esc cancel"); expect(restingLines[measured - 1]?.trimStart().startsWith("╰")).toBe(true); const saveColumn = restingLines[measured - 1]!.indexOf("save") + 1; @@ -3061,6 +3062,7 @@ describe("UI components", () => { onCancel: () => {}, onInput: () => {}, onSave: () => {}, + saveKeyLabel: "Ctrl+S", }} file={file} anchorSide="new" @@ -3076,8 +3078,10 @@ describe("UI components", () => { expect(lines[0]).toContain("╭─ Draft note - src/core/cli.ts R611 "); expect(lines[1]).toContain("│ │"); expect(lines[2]).toContain("│ Here's my comment. I think we should think"); - expect(lines[3]).toContain("^S save Esc cancel"); - const saveLine = lines.find((line) => line.includes("^S save") && line.includes("Esc cancel")); + expect(lines[3]).toContain("Ctrl+S save Esc cancel"); + const saveLine = lines.find( + (line) => line.includes("Ctrl+S save") && line.includes("Esc cancel"), + ); expect(saveLine).toBeDefined(); expect(saveLine!.indexOf("save")).toBeGreaterThan(lines[2]!.indexOf("Here's")); expect(saveLine?.trimStart().startsWith("╰")).toBe(true); @@ -3105,6 +3109,7 @@ describe("UI components", () => { onCancel: () => {}, onInput: () => {}, onSave: () => {}, + saveKeyLabel: "Ctrl+S", }} file={file} anchorSide="new" @@ -3118,7 +3123,7 @@ describe("UI components", () => { const lines = frame.split("\n"); const saveLineIndex = lines.findIndex( - (line) => line.includes("^S save") && line.includes("Esc cancel"), + (line) => line.includes("Ctrl+S save") && line.includes("Esc cancel"), ); expect(lines.some((line) => line.includes(body.slice(0, 10)))).toBe(true); expect(lines.some((line) => line.includes(body.slice(-10)))).toBe(true); @@ -3693,13 +3698,13 @@ describe("UI components", () => { const frame = await captureFrame( {}} />, 76, - 39, + 41, ); const expectedRows = [ @@ -3729,6 +3734,7 @@ describe("UI components", () => { "Review", "/ focus file filter", "c create review note", + "Ctrl+S save draft note", "Tab toggle files/filter focus", "F10 open menus", "r reload the review", @@ -3760,13 +3766,13 @@ describe("UI components", () => { const frame = await captureFrame( {}} />, 76, - 39, + 41, ); expect(frame).toContain("Ctrl+X"); diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index ac288555c..7b6060fd9 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -10,12 +10,13 @@ import type { MenuId } from "../components/chrome/menu"; import { dispatchAppCommand, executeAppCommand, + findAppCommandById, type AppCommand, verticalCommandDirection, } from "../lib/appCommands"; import type { ExtensionDialogRequest } from "../lib/extensionDialogs"; import { toExtensionKeyEvent } from "../lib/extensionKeyEvent"; -import { isEscapeKey, isSaveDraftNoteKey } from "../lib/keyboard"; +import { isEscapeKey, noteComposerSaveOwner } from "../lib/keyboard"; import { routeKeyOwnership, type KeyOwner } from "../lib/keyRouting"; type FocusArea = "files" | "filter" | "note"; @@ -68,7 +69,6 @@ export interface UseAppKeyboardShortcutsOptions { discardViewPreferencesAndQuit: () => void; neverAskToSaveViewPreferencesAndQuit: () => void; closeSaveConfigPrompt: () => void; - saveDraftNote: () => void; showAgentSkill: boolean; showHelp: boolean; switchMenu: (delta: number) => void; @@ -128,7 +128,6 @@ export function useAppKeyboardShortcuts({ discardViewPreferencesAndQuit, neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, - saveDraftNote, showAgentSkill, showHelp, switchMenu, @@ -469,8 +468,9 @@ export function useAppKeyboardShortcuts({ * * Both inputs receive their characters through OpenTUI's renderable path, * which consuming would cut off — so plain typing is `"focused"`, and only - * the inputs' explicit escape hatches (Tab out of the filter, Escape/Ctrl-S - * on a draft) are acted on here and owned as `"mine"`. + * the inputs' explicit escape hatches (Tab out of the filter, Escape on a + * draft, and the resolved save-note chord) are acted on here and owned as + * `"mine"`. */ const handleFocusedInputShortcut = (key: KeyEvent): KeyOwner => { if (focusAreaRef.current === "filter") { @@ -499,9 +499,12 @@ export function useAppKeyboardShortcuts({ return "mine"; } - if (isSaveDraftNoteKey(key)) { - saveDraftNote(); - return "mine"; + const save = findAppCommandById(commandsRef.current, "hunk.review.saveNote"); + const saveOwner = noteComposerSaveOwner(save?.keys ?? [], key, () => + executeAppCommand(commandsRef.current, "hunk.review.saveNote"), + ); + if (saveOwner) { + return saveOwner; } // Everything else is the note draft's text, including keys that double as diff --git a/src/ui/lib/agentAnnotations.ts b/src/ui/lib/agentAnnotations.ts index a57030040..379d57fff 100644 --- a/src/ui/lib/agentAnnotations.ts +++ b/src/ui/lib/agentAnnotations.ts @@ -42,6 +42,8 @@ export interface VisibleAgentNote { onFocus?: () => void; onInput: (value: string) => void; onSave: () => void; + /** Live chord for save, from `hunk.review.saveNote`; omitted when unbound. */ + saveKeyLabel?: string; }; } diff --git a/src/ui/lib/appCommands.test.ts b/src/ui/lib/appCommands.test.ts index 7e0ddd04b..6ad9d30e7 100644 --- a/src/ui/lib/appCommands.test.ts +++ b/src/ui/lib/appCommands.test.ts @@ -59,6 +59,7 @@ function createTestCommands(resolvedKeys?: ResolvedCommandKeys) { selectCursorLine: record("selectCursorLine"), stepDiffLine: record("stepDiffLine"), selectLayoutMode: record("selectLayoutMode"), + saveDraftNote: record("saveDraftNote"), startUserNote: record("startUserNote"), toggleAgentNotes: record("toggleAgentNotes"), toggleCopyDecorations: record("toggleCopyDecorations"), @@ -268,6 +269,9 @@ describe("builtinCommandKeyDefaults", () => { "u", "ctrl+u", ]); + expect(defaults.find((entry) => entry.id === "hunk.review.saveNote")?.defaultKeys).toEqual([ + "ctrl+s", + ]); // The menu-only commands ship unbound, and are reported so users can bind them. expect( defaults diff --git a/src/ui/lib/appCommands.ts b/src/ui/lib/appCommands.ts index 09160f14d..d211e3452 100644 --- a/src/ui/lib/appCommands.ts +++ b/src/ui/lib/appCommands.ts @@ -27,7 +27,9 @@ const FAST_CODE_HORIZONTAL_SCROLL_COLUMNS = 8; * `useAppKeyboardShortcuts`. Modal navigation (arrow keys inside a dialog, * escape closing a prompt) is deliberately not a command: those keys are the * structure of the widget that owns them, not shortcuts a user rebinds or an - * extension extends. + * extension extends. The note composer's save shortcut is a command + * (`hunk.review.saveNote`); focused-input routing still claims it first so + * typing is not stolen, but the chord comes from the resolved keymap. */ export const MAX_APP_COMMAND_COUNT = 10_000; @@ -130,6 +132,7 @@ export interface BuildAppCommandsOptions { stepDiffLine: (delta: number) => void; selectCursorLine: (style: CursorLine) => void; selectLayoutMode: (mode: LayoutMode) => void; + saveDraftNote: () => void; startUserNote: () => void; toggleAgentNotes: () => void; toggleCopyDecorations: () => void; @@ -191,6 +194,7 @@ function builtinCommandHandlers( isEnabled: () => Boolean(options.canReplyToActiveNote), run: () => options.replyToActiveNote?.(), }, + "hunk.review.saveNote": { run: () => options.saveDraftNote() }, "hunk.review.pageDown": { run: (_key, count) => options.scrollDiff(count, "viewport") }, "hunk.review.pageUp": { run: (_key, count) => options.scrollDiff(-count, "viewport") }, "hunk.review.halfPageDown": { run: (_key, count) => options.scrollDiff(count, "half") }, @@ -335,6 +339,7 @@ const NOOP_COMMAND_OPTIONS: BuildAppCommandsOptions = (() => { stepDiffLine: noop, selectCursorLine: noop, selectLayoutMode: noop, + saveDraftNote: noop, startUserNote: noop, toggleAgentNotes: noop, toggleCopyDecorations: noop, diff --git a/src/ui/lib/appMenus.test.ts b/src/ui/lib/appMenus.test.ts index a738816da..49d6492c1 100644 --- a/src/ui/lib/appMenus.test.ts +++ b/src/ui/lib/appMenus.test.ts @@ -52,6 +52,7 @@ function createTestCommands(overrides: Partial = {}) { selectCursorLine: noop, stepDiffLine: noop, selectLayoutMode: noop, + saveDraftNote: noop, startUserNote: noop, toggleAgentNotes: noop, toggleCopyDecorations: record("toggleCopyDecorations"), diff --git a/src/ui/lib/helpContent.test.ts b/src/ui/lib/helpContent.test.ts index 1666a41cc..956b6c8ce 100644 --- a/src/ui/lib/helpContent.test.ts +++ b/src/ui/lib/helpContent.test.ts @@ -42,6 +42,7 @@ describe("buildHelpSections", () => { expect(keysFor(sections, "page down")).toBe("PageDown / Space / f"); expect(keysFor(sections, "page up")).toBe("PageUp / b / Shift+Space"); expect(keysFor(sections, "jump to start")).toBe("g / Home"); + expect(keysFor(sections, "save draft note")).toBe("Ctrl+S"); }); test("keeps the rows that are not commands at all", () => { @@ -59,6 +60,9 @@ describe("buildHelpSections", () => { expect(keysFor(sections, "previous / next hunk")).toBe("[ / Ctrl+N"); expect(keysFor(sections, "quit")).toBe("Ctrl+X"); + expect(keysFor(helpSections({ "hunk.review.saveNote": "ctrl+enter" }), "save draft note")).toBe( + "Ctrl+Enter", + ); }); test("an unbound command drops out of its row, and an empty row drops out entirely", () => { @@ -71,6 +75,9 @@ describe("buildHelpSections", () => { expect(keysFor(sections, "previous / next hunk")).toBe("]"); // Nothing left to document, so the row is gone rather than blank. expect(keysFor(sections, "create review note")).toBeUndefined(); + expect( + keysFor(helpSections({ "hunk.review.saveNote": false }), "save draft note"), + ).toBeUndefined(); }); test("a disabled command is documented only while it can run", () => { diff --git a/src/ui/lib/helpContent.ts b/src/ui/lib/helpContent.ts index 0bb8ae799..62f67d271 100644 --- a/src/ui/lib/helpContent.ts +++ b/src/ui/lib/helpContent.ts @@ -107,6 +107,7 @@ const HELP_SECTIONS: readonly HelpSectionSpec[] = [ entries: [ { commandIds: ["hunk.review.focusFilter"], description: "focus file filter" }, { commandIds: ["hunk.review.startNote"], description: "create review note" }, + { commandIds: ["hunk.review.saveNote"], description: "save draft note" }, { commandIds: ["hunk.review.editActiveNote", "hunk.review.replyToActiveNote"], description: "edit / reply to active note", diff --git a/src/ui/lib/keyboard.ts b/src/ui/lib/keyboard.ts index 73d66afab..b47ef9e68 100644 --- a/src/ui/lib/keyboard.ts +++ b/src/ui/lib/keyboard.ts @@ -1,13 +1,16 @@ import type { KeyEvent } from "@opentui/core"; +import { matchesAnyKeyChord, parseKeyChord } from "../../lib/commandKeys"; +import type { KeyOwner } from "./keyRouting"; /** - * Key predicates for the surfaces that own their keys outright. + * Key predicates for the surfaces that own their keys outright, plus the + * encoding net that `ctrl+s` still needs after it became a remappable command. * * Shortcuts are declared as key chords in the command table * (`appCommands.ts`), which is what makes them remappable and reportable. What - * stays here is what modal widgets own — keys nobody rebinds — and where - * terminals disagree about the encoding enough that a chord could not describe - * the key faithfully. + * stays here is what modal widgets own — keys nobody rebinds, such as Escape — + * and where terminals disagree about the encoding enough that a chord could not + * describe the key faithfully. */ const CTRL_S = "\u0013"; @@ -27,16 +30,21 @@ export function isEscapeKey(key: KeyEvent) { /** * Match Ctrl-S across raw, Kitty/CSI-u, and tmux control-mode encodings. * + * Extra modifiers disqualify the event: the command table treats `ctrl+shift+s` + * as a different chord, and this net must not claim it. CSI-u for plain Ctrl-S + * is `\u001b[115;5u` (modifier 5); a shifted form is a different sequence. + * * Deliberately not delegated to the published `matchesKey("ctrl+s", key)`, * which now understands the bare C0 byte: this predicate is wider than a chord - * can be. It reads `raw`, a channel `ExtensionKeyEvent` does not carry; it - * accepts the CSI-u form the chord grammar has no spelling for; and it treats - * a bare `\u0013` byte as Ctrl-S whatever else the event reports, where chord - * matching must stay strict about modifiers so `ctrl+shift+s` remains a - * different binding. Delegating would narrow saving a draft note, so the - * overlap stays duplicated on purpose. + * can be. It reads `raw`, a channel `ExtensionKeyEvent` does not carry, and it + * accepts the CSI-u form the chord grammar has no spelling for. Delegating would + * drop those encodings, so the overlap stays duplicated on purpose. */ export function isSaveDraftNoteKey(key: KeyEvent) { + if (key.shift || key.meta || key.option) { + return false; + } + const name = key.name?.toLowerCase(); const sequence = key.sequence; const raw = key.raw; @@ -49,3 +57,56 @@ export function isSaveDraftNoteKey(key: KeyEvent) { raw === CTRL_S_CSI_U ); } + +/** Report whether one resolved chord is plain `ctrl+s`, regardless of spelling. */ +function isPlainCtrlSChord(chord: string) { + const parsed = parseKeyChord(chord); + return ( + !("error" in parsed) && + parsed.ctrl && + parsed.base === "s" && + !parsed.meta && + !parsed.option && + !parsed.shift + ); +} + +/** + * Match the note-composer save command against its resolved chords. + * + * Remapped chords go through the command table matcher. While the resolved set + * still includes plain `ctrl+s`, the wider encoding net from + * {@link isSaveDraftNoteKey} stays in force so CSI-u and `raw` keep saving. + * Unbound (empty keys) matches nothing. + */ +export function matchesSaveDraftNoteCommand(keys: readonly string[], key: KeyEvent) { + if (keys.length === 0) { + return false; + } + + if (matchesAnyKeyChord(keys)(key)) { + return true; + } + + return keys.some(isPlainCtrlSChord) && isSaveDraftNoteKey(key); +} + +/** + * Own a focused-composer save after matching the resolved chords. + * + * `execute` is the caller's `executeAppCommand` for `hunk.review.saveNote`. + * Returns `"mine"` only when that ran. A matched key whose execute fails is + * `"focused"` so the chord is not swallowed and is not saved through a widget + * fallback. Unmatched keys return undefined. + */ +export function noteComposerSaveOwner( + keys: readonly string[], + key: KeyEvent, + execute: () => boolean, +): KeyOwner | undefined { + if (!matchesSaveDraftNoteCommand(keys, key)) { + return undefined; + } + + return execute() ? "mine" : "focused"; +} diff --git a/src/ui/lib/ui-lib.test.ts b/src/ui/lib/ui-lib.test.ts index 1292f255d..8538e34e7 100644 --- a/src/ui/lib/ui-lib.test.ts +++ b/src/ui/lib/ui-lib.test.ts @@ -13,7 +13,12 @@ import { } from "../components/chrome/menu"; import { createVisibleAgentNote } from "./agentAnnotations"; import { buildAgentPopoverContent, resolveAgentPopoverPlacement } from "./agentPopover"; -import { isEscapeKey, isSaveDraftNoteKey } from "./keyboard"; +import { + isEscapeKey, + isSaveDraftNoteKey, + matchesSaveDraftNoteCommand, + noteComposerSaveOwner, +} from "./keyboard"; import { BoundedClusterWidthCache, CLUSTER_WIDTH_CACHE_MAX_ENTRIES, @@ -210,6 +215,58 @@ describe("ui helpers", () => { // Unmodified s and other ctrl chords must not save. expect(isSaveDraftNoteKey(createKeyEvent({ name: "s" }))).toBe(false); expect(isSaveDraftNoteKey(createKeyEvent({ ctrl: true, name: "x" }))).toBe(false); + // Extra modifiers are a different chord, including on CSI-u / raw. + expect(isSaveDraftNoteKey(createKeyEvent({ ctrl: true, shift: true, name: "s" }))).toBe(false); + expect(isSaveDraftNoteKey(createKeyEvent({ sequence: CTRL_S, shift: true }))).toBe(false); + expect(isSaveDraftNoteKey(createKeyEvent({ sequence: CTRL_S_CSI_U, shift: true }))).toBe(false); + }); + + test("save-draft-note command matching uses resolved chords and the Ctrl-S encoding net", () => { + const CTRL_S = "\u0013"; + const CTRL_S_CSI_U = "\u001b[115;5u"; + const ctrlS = createKeyEvent({ ctrl: true, name: "s" }); + const csiU = createKeyEvent({ sequence: CTRL_S_CSI_U }); + const remapped = createKeyEvent({ ctrl: true, name: "return" }); + + expect(matchesSaveDraftNoteCommand([], ctrlS)).toBe(false); + expect(matchesSaveDraftNoteCommand(["ctrl+s"], ctrlS)).toBe(true); + expect(matchesSaveDraftNoteCommand(["ctrl+s"], createKeyEvent({ sequence: CTRL_S }))).toBe( + true, + ); + expect(matchesSaveDraftNoteCommand(["ctrl+s"], csiU)).toBe(true); + expect(matchesSaveDraftNoteCommand(["Ctrl+s"], csiU)).toBe(true); + expect(matchesSaveDraftNoteCommand(["ctrl+enter"], remapped)).toBe(true); + expect(matchesSaveDraftNoteCommand(["ctrl+enter"], ctrlS)).toBe(false); + expect(matchesSaveDraftNoteCommand(["ctrl+enter"], csiU)).toBe(false); + expect(matchesSaveDraftNoteCommand(["alt+s"], csiU)).toBe(false); + expect( + matchesSaveDraftNoteCommand( + ["ctrl+s"], + createKeyEvent({ ctrl: true, shift: true, name: "s" }), + ), + ).toBe(false); + }); + + test("focused composer save owns the key only when execute-by-id runs", () => { + const ctrlS = createKeyEvent({ ctrl: true, name: "s" }); + const ran: string[] = []; + + expect( + noteComposerSaveOwner(["ctrl+s"], ctrlS, () => { + ran.push("save"); + return true; + }), + ).toBe("mine"); + expect(ran).toEqual(["save"]); + + expect(noteComposerSaveOwner(["ctrl+s"], ctrlS, () => false)).toBe("focused"); + expect( + noteComposerSaveOwner(["ctrl+enter"], ctrlS, () => { + ran.push("should-not-run"); + return true; + }), + ).toBeUndefined(); + expect(ran).toEqual(["save"]); }); test("fitText and padText clamp using the terminal fallback marker", () => { diff --git a/test/pty/notes.test.ts b/test/pty/notes.test.ts index bd2dab034..15c40f918 100644 --- a/test/pty/notes.test.ts +++ b/test/pty/notes.test.ts @@ -227,7 +227,7 @@ describe("PTY notes", () => { expect(freshDraft).toContain("Write a note"); const composerBorder = freshDraft .split("\n") - .find((line) => line.includes("^S save") && line.includes("Esc cancel")); + .find((line) => line.includes("Ctrl+S save") && line.includes("Esc cancel")); expect(composerBorder?.trimStart().startsWith("╰")).toBe(true); expect(composerBorder?.trimEnd().endsWith("╯")).toBe(true); @@ -238,7 +238,7 @@ describe("PTY notes", () => { }); const saveRowBeforeNewline = draftBeforeNewline .split("\n") - .findIndex((line) => line.includes("^S save") && line.includes("Esc cancel")); + .findIndex((line) => line.includes("Ctrl+S save") && line.includes("Esc cancel")); expect(saveRowBeforeNewline).toBeGreaterThanOrEqual(0); await session.type("\x0a"); @@ -247,7 +247,7 @@ describe("PTY notes", () => { (text) => { const saveRowAfterNewline = text .split("\n") - .findIndex((line) => line.includes("^S save") && line.includes("Esc cancel")); + .findIndex((line) => line.includes("Ctrl+S save") && line.includes("Esc cancel")); return ( text.includes("Please cover this edge case.") && saveRowAfterNewline > saveRowBeforeNewline @@ -816,7 +816,7 @@ describe("PTY notes", () => { await session.click(/\[\+\]/); await session.waitForText(/Draft note/, { timeout: 5_000 }); await session.type("Save this clicked draft."); - await session.click(/\^S save/); + await session.click(/Ctrl\+S save/); const saved = await session.waitForText(/Your note/, { timeout: 5_000 }); expect(saved).toContain("Save this clicked draft."); diff --git a/website/src/content/docs/docs/configure/keybindings.md b/website/src/content/docs/docs/configure/keybindings.md index 46a4c006f..a6726fd1c 100644 --- a/website/src/content/docs/docs/configure/keybindings.md +++ b/website/src/content/docs/docs/configure/keybindings.md @@ -31,6 +31,6 @@ Chords join `ctrl`, `alt`/`option`, `cmd`/`meta`, and `shift` with `+` around a The menus and the in-app help (`?`) show the keys for the commands they present, so a remap changes what they advertise. The full table of built-in command ids and their default keys lives in [`docs/keybindings.md`](https://github.com/modem-dev/hunk/blob/main/docs/keybindings.md) in the repository. Commands listed without a default key remain callable by id and can be assigned a shortcut; some also appear in menus. -Keys owned by a dialog, menu, or focused text input — `Esc`, `Enter`, `Ctrl-S` while writing a note — belong to those widgets and are not remappable. +Keys owned by a dialog, menu, or focused text input — `Esc`, `Enter` — belong to those widgets and are not remappable. The note composer's save shortcut is the command `hunk.review.saveNote` (default `ctrl+s`) and is remappable; while the composer is focused it still wins over the command table, using the resolved chord. `[keybindings]` is read from your user config only, never from a repository's `.hunk/config.toml`: which keys do what is a property of your keyboard and habits, so a checkout you review cannot rearrange them. diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index b78bb4d4f..46b0b4795 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -378,7 +378,7 @@ Subscribe to a lifecycle or UI event. Handlers may be async; Hunk never blocks t - A newly mounted instance receives `startup` before its first `changeset_loaded`; reloads deliver `changeset_loaded` before `session_reload` after the matching review commits. - `selection_changed` is trailing-debounced: holding `[`/`]` retargets many times a second, and handlers only care where the user landed. `fileId` and `hunkIndex` are `null` when nothing is selected. -- `command_executed` reports stable command ids after terminal dispatch from a key, menu, or `ctx.commands.execute`. Detached async extension work may still be running; the event observes the accepted action rather than promise settlement. It follows remapped keys; browser/session review intents and widget-owned Escape, Enter, note-editor Ctrl-S, and F10 menu navigation are not terminal commands. +- `command_executed` reports stable command ids after terminal dispatch from a key, menu, or `ctx.commands.execute`. Detached async extension work may still be running; the event observes the accepted action rather than promise settlement. It follows remapped keys; browser/session review intents and widget-owned Escape, Enter, and F10 menu navigation are not terminal commands. The note composer's save shortcut is `hunk.review.saveNote` and does emit this event. - `session_reload`'s `reason` is `"watch"`, `"daemon"` (an agent command through the session broker), or `"manual"`. - `note_created` and `note_edited` cover notes authored in Hunk's own UI this session. Agent session comments do not emit them, and a reload may remap or drop notes. Use them for incremental reactions; use `ctx.review.snapshot()` when a command needs the complete current saved-note record. - `shutdown` handlers get 250ms before Hunk exits anyway; treat it as best-effort flushing. UI authority has already been revoked, so shutdown is for releasing extension-owned resources rather than navigation or dialogs. From eea5ddc36351def6c447e485e4145442f1f328b1 Mon Sep 17 00:00:00 2001 From: Ajay-Satish-01 Date: Mon, 31 Aug 2026 16:29:12 -0500 Subject: [PATCH 2/3] docs: drop leftover note-editor Ctrl-S from the extension API The merge left a duplicate command_executed bullet and quote-style churn that failed oxfmt on CI. --- .../content/docs/docs/extend/extension-api.md | 144 ++++++++---------- 1 file changed, 63 insertions(+), 81 deletions(-) diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index 5aff2312e..0648590df 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -29,14 +29,14 @@ below its lowercase-kebab top-level name: ```ts hunk.registerCliCommand( - { name: 'greptile', summary: 'Work with Greptile', usage: '' }, + { name: "greptile", summary: "Work with Greptile", usage: "" }, async (args, ctx) => { - if (args[0] === 'sync') { - await ctx.stdout.write('Synced.\n'); - return { kind: 'exit' }; + if (args[0] === "sync") { + await ctx.stdout.write("Synced.\n"); + return { kind: "exit" }; } - await ctx.stderr.write('Preparing review…\n'); - return { kind: 'delegate', argv: ['diff'] }; + await ctx.stderr.write("Preparing review…\n"); + return { kind: "delegate", argv: ["diff"] }; }, ); ``` @@ -83,7 +83,7 @@ Request host behavior for the review session loading the extension. Training, demo, and presentation extensions can make their view-setting changes temporary: ```ts -hunk.configureSession({ viewPreferences: 'transient' }); +hunk.configureSession({ viewPreferences: "transient" }); ``` If any loaded extension requests this, Hunk skips the save-view-preferences @@ -96,11 +96,11 @@ Contribute one selectable theme. The object is the same shape as a `[themes. ```ts hunk.registerTheme({ - id: 'midnight-review', - label: 'Midnight Review', - base: 'catppuccin-mocha', - accent: '#7fd1ff', - syntaxScopes: { 'keyword.operator': '#7fd1ff' }, + id: "midnight-review", + label: "Midnight Review", + base: "catppuccin-mocha", + accent: "#7fd1ff", + syntaxScopes: { "keyword.operator": "#7fd1ff" }, }); ``` @@ -111,11 +111,11 @@ Theme ids are lowercase words separated by `-` or `_` and cannot reuse a built-i Map an extension, exact filename, or glob to an existing syntax-highlighting language. A string remains shorthand for a case-insensitive extension; object-form extension values receive the same trimming, leading-dot removal, and lowercasing: ```ts -hunk.registerFileLanguage('.zig', 'zig'); -hunk.registerFileLanguage({ kind: 'filename', value: 'BUILD' }, 'python'); +hunk.registerFileLanguage(".zig", "zig"); +hunk.registerFileLanguage({ kind: "filename", value: "BUILD" }, "python"); hunk.registerFileLanguage( - { kind: 'glob', value: 'generated/**/*.proto', target: 'path' }, - 'protobuf', + { kind: "glob", value: "generated/**/*.proto", target: "path" }, + "protobuf", ); ``` @@ -149,10 +149,10 @@ Mark character ranges inside Hunk's own diff rendering — search hits, diagnost ```ts hunk.registerLineHighlighter({ - id: 'todos', + id: "todos", highlight({ file }) { // Scan file.patch (or a readDocument result) and return marks. - return [{ side: 'new', line: 12, range: [4, 8], tone: 'warning' }]; + return [{ side: "new", line: 12, range: [4, 8], tone: "warning" }]; }, }); ``` @@ -178,7 +178,7 @@ Rewrite the loaded changeset before it reaches the review UI. Transforms run in ```ts hunk.transformChangeset((changeset) => ({ ...changeset, - files: changeset.files.filter((file) => !file.path.endsWith('.lock')), + files: changeset.files.filter((file) => !file.path.endsWith(".lock")), })); ``` @@ -194,22 +194,19 @@ Register a session-wide, deliberately activated keyboard interpretation. Modes r ```ts hunk.registerKeyboardMode({ - id: 'normal', - title: 'Vim navigation', + id: "normal", + title: "Vim navigation", onKey(key, ctx) { - if (key.sequence !== 'j') return 'pass'; - ctx.commands.execute('hunk.review.stepDown'); - return 'handled'; + if (key.sequence !== "j") return "pass"; + ctx.commands.execute("hunk.review.stepDown"); + return "handled"; }, }); -hunk.registerCommand( - { id: 'vim', title: 'Toggle Vim navigation', key: 'ctrl+v' }, - (ctx) => { - if (ctx.keyboardModes.isActive('normal')) ctx.keyboardModes.exitMode(); - else ctx.keyboardModes.enterMode('normal'); - }, -); +hunk.registerCommand({ id: "vim", title: "Toggle Vim navigation", key: "ctrl+v" }, (ctx) => { + if (ctx.keyboardModes.isActive("normal")) ctx.keyboardModes.exitMode(); + else ctx.keyboardModes.enterMode("normal"); +}); ``` `onKey` returns `"handled"`, `"pass"`, or `"exit"` synchronously. Optional `onEnter`/`onExit` callbacks reset extension-owned state such as counts and pending sequences; while either lifecycle callback runs, `enterMode()` and `exitMode()` return `false`. The context exposes only `cwd`, `notify`, public `commands`, activation-scoped `keyboardModes` controls, and `highlights` refresh controls. Those controls become inert on exit, so retained callbacks cannot replace a later mode. When the session mode is the highest-priority input owner, host-owned Escape exits it; the persistent status badge and Extensions-menu exit are clickable too. @@ -221,15 +218,12 @@ One session mode runs at a time. Entering another runs the outgoing `onExit` fir Register a named command, optionally bound to a key. Commands are the same mechanism Hunk's own shortcuts dispatch through — one table, one loop, built-ins first. ```ts -import type { HunkExtensionAPI } from 'hunkdiff/extension'; +import type { HunkExtensionAPI } from "hunkdiff/extension"; export default function (hunk: HunkExtensionAPI) { - hunk.registerCommand( - { id: 'hello', title: 'Say hello', key: 'ctrl+g' }, - (ctx) => { - ctx.notify('hello from a command'); - }, - ); + hunk.registerCommand({ id: "hello", title: "Say hello", key: "ctrl+g" }, (ctx) => { + ctx.notify("hello from a command"); + }); } ``` @@ -255,17 +249,15 @@ The handler fires when the key is pressed outside modal UI (dialogs, menus, and ```ts hunk.registerCommand( - { id: 'show-selection', title: 'Show the selected file', key: 'ctrl+y' }, + { id: "show-selection", title: "Show the selected file", key: "ctrl+y" }, (ctx) => { const { file, hunkIndex } = ctx.selection; if (!file) { - ctx.notify('No file selected'); + ctx.notify("No file selected"); return; } - ctx.notify( - hunkIndex === null ? file.path : `${file.path} — hunk ${hunkIndex + 1}`, - ); + ctx.notify(hunkIndex === null ? file.path : `${file.path} — hunk ${hunkIndex + 1}`); }, ); ``` @@ -298,7 +290,7 @@ A handler may be async; a failure becomes a warning naming your extension. ```ts hunk.registerCommand( - { id: 'reformat', title: 'Reformat the selected file', key: 'ctrl+r' }, + { id: "reformat", title: "Reformat the selected file", key: "ctrl+r" }, async (ctx) => { const file = ctx.selection.file; if (!file) { @@ -307,11 +299,11 @@ hunk.registerCommand( const proceed = await ctx.dialogs.confirm({ title: `Reformat ${file.path}?`, - body: 'The file is rewritten in place.', - confirmLabel: 'reformat', + body: "The file is rewritten in place.", + confirmLabel: "reformat", }); - ctx.notify(proceed ? `Reformatting ${file.path}` : 'Left it alone'); + ctx.notify(proceed ? `Reformatting ${file.path}` : "Left it alone"); }, ); ``` @@ -319,28 +311,22 @@ hunk.registerCommand( `select` fits acting on part of the selection — asking which hunk to jump to, then navigating there: ```ts -hunk.registerCommand( - { id: 'pick-hunk', title: 'Pick a hunk', key: 'ctrl+k' }, - async (ctx) => { - const file = ctx.selection.file; - const hunks = file?.hunks ?? []; - if (!file || hunks.length === 0) { - ctx.notify('Nothing to pick from', 'warning'); - return; - } +hunk.registerCommand({ id: "pick-hunk", title: "Pick a hunk", key: "ctrl+k" }, async (ctx) => { + const file = ctx.selection.file; + const hunks = file?.hunks ?? []; + if (!file || hunks.length === 0) { + ctx.notify("Nothing to pick from", "warning"); + return; + } - const labels = hunks.map((hunk) => hunk.header || `hunk ${hunk.index + 1}`); - const picked = await ctx.dialogs.select({ - title: 'Which hunk?', - options: labels, - }); + const labels = hunks.map((hunk) => hunk.header || `hunk ${hunk.index + 1}`); + const picked = await ctx.dialogs.select({ title: "Which hunk?", options: labels }); - // `navigation` is live, so the jump is valid even after awaiting the dialog. - if (picked !== null) { - ctx.navigation.selectHunk(file.id, labels.indexOf(picked)); - } - }, -); + // `navigation` is live, so the jump is valid even after awaiting the dialog. + if (picked !== null) { + ctx.navigation.selectHunk(file.id, labels.indexOf(picked)); + } +}); ``` Hunk draws the dialog; your text fills the title, body, and choices. Dialogs from installed extensions carry an `ext ` attribution line — the same marker `notify` toasts use — so a third-party prompt cannot present itself as Hunk asking. Hunk's own bundled extensions omit that redundant marker. @@ -360,12 +346,9 @@ One dialog shows at a time; concurrent requests queue in call order, across exte ```ts const file = ctx.selection.file; if (file && ctx.workspace.canWriteDocument(file.id)) { - const text = await ctx.workspace.readDocument(file.id, 'new'); + const text = await ctx.workspace.readDocument(file.id, "new"); if (text !== null) { - await ctx.workspace.writeDocument({ - fileId: file.id, - text: transform(text), - }); + await ctx.workspace.writeDocument({ fileId: file.id, text: transform(text) }); } } ``` @@ -400,9 +383,8 @@ Subscribe to a lifecycle or UI event. Handlers may be async; Hunk never blocks t - A newly mounted instance receives `startup` before its first `changeset_loaded`; reloads deliver `changeset_loaded` before `session_reload` after the matching review commits. - `selection_changed` is trailing-debounced: holding `[`/`]` retargets many times a second, and handlers only care where the user landed. `fileId` and `hunkIndex` are `null` when nothing is selected. -- `command_executed` reports stable command ids after terminal dispatch from a key, menu, or `ctx.commands.execute`. Detached async extension work may still be running; the event observes the accepted action rather than promise settlement. It follows remapped keys; browser/session review intents and widget-owned Escape, Enter, and F10 menu navigation are not terminal commands. The note composer's save shortcut is `hunk.review.saveNote` and does emit this event. - `hunk_viewed` fires when the settled `(file, hunk)` pair changes, including `[`/`]` inside one file. Current-line movement within a hunk does not emit it. `file_viewed` still fires only when the selected file object changes. -- `command_executed` reports stable command ids after terminal dispatch from a key, menu, or `ctx.commands.execute`. Detached async extension work may still be running; the event observes the accepted action rather than promise settlement. It follows remapped keys; browser/session review intents and widget-owned Escape, Enter, note-editor Ctrl-S, and F10 menu navigation are not terminal commands. +- `command_executed` reports stable command ids after terminal dispatch from a key, menu, or `ctx.commands.execute`. Detached async extension work may still be running; the event observes the accepted action rather than promise settlement. It follows remapped keys; browser/session review intents and widget-owned Escape, Enter, and F10 menu navigation are not terminal commands. The note composer's save shortcut is `hunk.review.saveNote` and does emit this event. - `session_reload`'s `reason` is `"watch"`, `"daemon"` (an agent command through the session broker), or `"manual"`. - `note_created` and `note_edited` cover notes authored in Hunk's own UI this session. Agent session comments do not emit them, and a reload may remap or drop notes. Use them for incremental reactions. - `note_changed` is store-backed: `kind` is `"created"`, `"updated"`, or `"removed"`, and `note` matches `ctx.review.snapshot()`. It includes agent session comments and user deletes; drafts never appear. Reloads that remap notes do not emit it — use `session_reload` plus `ctx.review.snapshot()` for the complete current record. @@ -413,16 +395,16 @@ Subscribe to a lifecycle or UI event. Handlers may be async; Hunk never blocks t A small bus shared by every loaded extension, for coordinating without coupling through global state. Namespace event names with your extension id. Delivery is fire-and-forget; events emitted while factories are still loading are queued until every extension has subscribed. ```ts -import type { HunkExtensionAPI } from 'hunkdiff/extension'; +import type { HunkExtensionAPI } from "hunkdiff/extension"; export default function (hunk: HunkExtensionAPI) { - hunk.events.on<{ fileCount: number }>('summary:ready', (payload, ctx) => { - if (payload.fileCount > 100) ctx.panes.open('summary'); + hunk.events.on<{ fileCount: number }>("summary:ready", (payload, ctx) => { + if (payload.fileCount > 100) ctx.panes.open("summary"); }); - hunk.on('changeset_loaded', ({ changeset }, ctx) => { - hunk.events.emit('summary:ready', { fileCount: changeset.files.length }); - ctx.panes.open('summary'); + hunk.on("changeset_loaded", ({ changeset }, ctx) => { + hunk.events.emit("summary:ready", { fileCount: changeset.files.length }); + ctx.panes.open("summary"); }); } ``` @@ -442,7 +424,7 @@ patterns = ["*.lock", "dist/**"] ``` ```ts -const patterns = (hunk.config.patterns as string[] | undefined) ?? ['*.lock']; +const patterns = (hunk.config.patterns as string[] | undefined) ?? ["*.lock"]; ``` ## `ctx.notify(message, type?)` From b3f5cd0dfdd568500082c2a44c957829764d95e8 Mon Sep 17 00:00:00 2001 From: Ajay-Satish-01 Date: Tue, 8 Sep 2026 12:36:41 -0500 Subject: [PATCH 3/3] fix(ui): enable save-note only while a draft exists Format the moved command catalog so oxfmt passes on CI, and keep the save chord from dispatching or emitting command_executed when there is no draft. --- packages/hunk/src/core/run/commandCatalog.ts | 628 +++++++++--------- packages/hunk/src/ui/App.tsx | 1 + .../hunk/src/ui/AppHost.keybindings.test.tsx | 7 + packages/hunk/src/ui/lib/appCommands.test.ts | 20 +- packages/hunk/src/ui/lib/appCommands.ts | 8 +- 5 files changed, 344 insertions(+), 320 deletions(-) diff --git a/packages/hunk/src/core/run/commandCatalog.ts b/packages/hunk/src/core/run/commandCatalog.ts index 5576a9788..0deefe11c 100644 --- a/packages/hunk/src/core/run/commandCatalog.ts +++ b/packages/hunk/src/core/run/commandCatalog.ts @@ -25,23 +25,23 @@ * describes UI vocabulary rather than review semantics, and that module stays purely about * what a review *is*. */ -import type { ReviewIntent } from '../review/intents'; -import type { ReviewSelectionScope } from '../review/navigation'; +import type { ReviewIntent } from "../review/intents"; +import type { ReviewSelectionScope } from "../review/navigation"; import { selectActiveEditableReviewNoteId, selectActiveRemovableReviewNote, selectActiveReplyableReviewNoteId, selectNormalizedSelection, selectReviewGapForSelection, -} from '../review/selectors'; -import type { ReviewState } from '../review/state'; -import type { ReviewNoteTargetV1 } from '../review/types'; +} from "../review/selectors"; +import type { ReviewState } from "../review/state"; +import type { ReviewNoteTargetV1 } from "../review/types"; /** Where one command's effect resolves, and therefore who may invoke it. */ -export type AppCommandLocus = 'semantic' | 'client-local' | 'host-only'; +export type AppCommandLocus = "semantic" | "client-local" | "host-only"; /** The id group a command lives under, and the menu-level grouping users read. */ -export type AppCommandCategory = 'app' | 'history' | 'review' | 'view'; +export type AppCommandCategory = "app" | "history" | "review" | "view"; /** The direction a command moves a vertically ordered surface. */ export type VerticalCommandDirection = -1 | 1; @@ -54,18 +54,18 @@ export type VerticalCommandDirection = -1 | 1; * answer instead of one per surface. */ export type AppCommandReviewEffect = - | { kind: 'selection/move'; scope: ReviewSelectionScope; direction: 1 | -1 } - | { kind: 'notes/toggle-visibility' } + | { kind: "selection/move"; scope: ReviewSelectionScope; direction: 1 | -1 } + | { kind: "notes/toggle-visibility" } /** Open a draft at the current selection; the client may supply a measured line. */ - | { kind: 'notes/start-draft' } + | { kind: "notes/start-draft" } /** Edit the active note when that same card is an editable reviewer note. */ - | { kind: 'notes/start-edit-active' } + | { kind: "notes/start-edit-active" } /** Reply to the active stored note. */ - | { kind: 'notes/start-reply-active' } + | { kind: "notes/start-reply-active" } /** Delete or dismiss the active stored leaf note. */ - | { kind: 'notes/remove-active' } + | { kind: "notes/remove-active" } /** Flip the gap the shared policy says this selection reaches. */ - | { kind: 'expansion/toggle-selected-gap' }; + | { kind: "expansion/toggle-selected-gap" }; export interface AppCommandCatalogEntry { /** Stable canonical identifier, `hunk..` for every built-in. */ @@ -102,506 +102,506 @@ export interface AppCommandCatalogEntry { */ const BUILTIN_COMMANDS = [ { - id: 'hunk.review.jumpToBottom', - title: 'Jump to end', - category: 'review', - defaultKeys: ['G', 'end'], - locus: 'client-local', + id: "hunk.review.jumpToBottom", + title: "Jump to end", + category: "review", + defaultKeys: ["G", "end"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.jumpToTop', - title: 'Jump to start', - category: 'review', - defaultKeys: ['g', 'home'], - locus: 'client-local', + id: "hunk.review.jumpToTop", + title: "Jump to start", + category: "review", + defaultKeys: ["g", "home"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.app.quit', - title: 'Quit', - category: 'app', - defaultKeys: ['q'], - locus: 'host-only', + id: "hunk.app.quit", + title: "Quit", + category: "app", + defaultKeys: ["q"], + locus: "host-only", publicToExtensions: true, }, { - id: 'hunk.app.toggleHelp', - title: 'Toggle help', - category: 'app', - defaultKeys: ['?'], - locus: 'client-local', + id: "hunk.app.toggleHelp", + title: "Toggle help", + category: "app", + defaultKeys: ["?"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.app.openAgentSkill', - title: 'Show agent skill', - category: 'app', + id: "hunk.app.openAgentSkill", + title: "Show agent skill", + category: "app", defaultKeys: [], - locus: 'host-only', + locus: "host-only", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.app.toggleFocusArea', - title: 'Switch focus between files and filter', - category: 'app', - defaultKeys: ['tab'], - locus: 'client-local', + id: "hunk.app.toggleFocusArea", + title: "Switch focus between files and filter", + category: "app", + defaultKeys: ["tab"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.focusFilter', - title: 'Focus the file filter', - category: 'review', - defaultKeys: ['/'], + id: "hunk.review.focusFilter", + title: "Focus the file filter", + category: "review", + defaultKeys: ["/"], // Moving keyboard focus is this client's business; the filter value it edits is // shared review state, changed through `filter/set` rather than by this command. - locus: 'client-local', + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.startVisualSelection', - title: 'Start visual selection', - category: 'review', - defaultKeys: ['v'], - locus: 'client-local', + id: "hunk.review.startVisualSelection", + title: "Start visual selection", + category: "review", + defaultKeys: ["v"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.copySelection', - title: 'Copy selection', - category: 'review', - defaultKeys: ['y'], - locus: 'client-local', + id: "hunk.review.copySelection", + title: "Copy selection", + category: "review", + defaultKeys: ["y"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.clearSelection', - title: 'Clear selection', - category: 'review', + id: "hunk.review.clearSelection", + title: "Clear selection", + category: "review", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.startNote', - title: 'Add a review note', - category: 'review', - defaultKeys: ['c'], - locus: 'semantic', - review: { kind: 'notes/start-draft' }, + id: "hunk.review.startNote", + title: "Add a review note", + category: "review", + defaultKeys: ["c"], + locus: "semantic", + review: { kind: "notes/start-draft" }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.editActiveNote', - title: 'Edit active review note', - category: 'review', - defaultKeys: ['E'], - locus: 'semantic', - review: { kind: 'notes/start-edit-active' }, + id: "hunk.review.editActiveNote", + title: "Edit active review note", + category: "review", + defaultKeys: ["E"], + locus: "semantic", + review: { kind: "notes/start-edit-active" }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.replyToActiveNote', - title: 'Reply to active review note', - category: 'review', - defaultKeys: ['R'], - locus: 'semantic', - review: { kind: 'notes/start-reply-active' }, + id: "hunk.review.replyToActiveNote", + title: "Reply to active review note", + category: "review", + defaultKeys: ["R"], + locus: "semantic", + review: { kind: "notes/start-reply-active" }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.saveNote', - title: 'Save review note', - category: 'review', - defaultKeys: ['ctrl+s'], + id: "hunk.review.saveNote", + title: "Save review note", + category: "review", + defaultKeys: ["ctrl+s"], // The TUI draft buffer is this client's; persist already goes through // `notes/create-user` / `notes/update-user` inside the save handler. - locus: 'client-local', + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.deleteActiveNote', - title: 'Delete active review note', - category: 'review', - defaultKeys: ['D'], - locus: 'semantic', - review: { kind: 'notes/remove-active' }, + id: "hunk.review.deleteActiveNote", + title: "Delete active review note", + category: "review", + defaultKeys: ["D"], + locus: "semantic", + review: { kind: "notes/remove-active" }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.previousNote', - title: 'Previous review note', - category: 'review', - defaultKeys: ['N'], - locus: 'client-local', + id: "hunk.review.previousNote", + title: "Previous review note", + category: "review", + defaultKeys: ["N"], + locus: "client-local", verticalDirection: -1, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.nextNote', - title: 'Next review note', - category: 'review', - defaultKeys: ['n'], - locus: 'client-local', + id: "hunk.review.nextNote", + title: "Next review note", + category: "review", + defaultKeys: ["n"], + locus: "client-local", verticalDirection: 1, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.pageDown', - title: 'Scroll down one page', - category: 'review', - defaultKeys: ['pagedown', 'space', 'f'], - locus: 'client-local', + id: "hunk.review.pageDown", + title: "Scroll down one page", + category: "review", + defaultKeys: ["pagedown", "space", "f"], + locus: "client-local", verticalDirection: 1, publicToExtensions: true, }, { - id: 'hunk.review.pageUp', - title: 'Scroll up one page', - category: 'review', - defaultKeys: ['pageup', 'b', 'shift+space'], - locus: 'client-local', + id: "hunk.review.pageUp", + title: "Scroll up one page", + category: "review", + defaultKeys: ["pageup", "b", "shift+space"], + locus: "client-local", verticalDirection: -1, publicToExtensions: true, }, { - id: 'hunk.review.halfPageDown', - title: 'Scroll down half a page', - category: 'review', - defaultKeys: ['d', 'ctrl+d'], - locus: 'client-local', + id: "hunk.review.halfPageDown", + title: "Scroll down half a page", + category: "review", + defaultKeys: ["d", "ctrl+d"], + locus: "client-local", verticalDirection: 1, publicToExtensions: true, }, { - id: 'hunk.review.halfPageUp', - title: 'Scroll up half a page', - category: 'review', - defaultKeys: ['u', 'ctrl+u'], - locus: 'client-local', + id: "hunk.review.halfPageUp", + title: "Scroll up half a page", + category: "review", + defaultKeys: ["u", "ctrl+u"], + locus: "client-local", verticalDirection: -1, publicToExtensions: true, }, { - id: 'hunk.review.stepDown', - title: 'Move down one line or note', - category: 'review', - defaultKeys: ['down', 'j'], - locus: 'client-local', + id: "hunk.review.stepDown", + title: "Move down one line or note", + category: "review", + defaultKeys: ["down", "j"], + locus: "client-local", verticalDirection: 1, publicToExtensions: true, }, { - id: 'hunk.review.stepUp', - title: 'Move up one line or note', - category: 'review', - defaultKeys: ['up', 'k'], - locus: 'client-local', + id: "hunk.review.stepUp", + title: "Move up one line or note", + category: "review", + defaultKeys: ["up", "k"], + locus: "client-local", verticalDirection: -1, publicToExtensions: true, }, { - id: 'hunk.review.scrollCodeLeft', - title: 'Scroll code left', - category: 'review', + id: "hunk.review.scrollCodeLeft", + title: "Scroll code left", + category: "review", // Both chords run the same command; the shifted one scrolls further, so the handler // reads the event rather than splitting this into two commands. - defaultKeys: ['left', 'shift+left'], - locus: 'client-local', + defaultKeys: ["left", "shift+left"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.scrollCodeRight', - title: 'Scroll code right', - category: 'review', - defaultKeys: ['right', 'shift+right'], - locus: 'client-local', + id: "hunk.review.scrollCodeRight", + title: "Scroll code right", + category: "review", + defaultKeys: ["right", "shift+right"], + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.alignCurrentLineTop', - title: 'Align current line to top', - category: 'review', + id: "hunk.review.alignCurrentLineTop", + title: "Align current line to top", + category: "review", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.alignCurrentLineCenter', - title: 'Align current line to center', - category: 'review', + id: "hunk.review.alignCurrentLineCenter", + title: "Align current line to center", + category: "review", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.review.alignCurrentLineBottom', - title: 'Align current line to bottom', - category: 'review', + id: "hunk.review.alignCurrentLineBottom", + title: "Align current line to bottom", + category: "review", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, }, { - id: 'hunk.view.cursorLineRow', - title: 'Highlight the current row', - category: 'view', + id: "hunk.view.cursorLineRow", + title: "Highlight the current row", + category: "view", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.cursorLineNumber', - title: 'Mark the current line number', - category: 'view', + id: "hunk.view.cursorLineNumber", + title: "Mark the current line number", + category: "view", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.cursorLineOff', - title: 'Hide the current-line marker', - category: 'view', + id: "hunk.view.cursorLineOff", + title: "Hide the current-line marker", + category: "view", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.layoutSplit', - title: 'Split layout', - category: 'view', - defaultKeys: ['1'], - locus: 'client-local', + id: "hunk.view.layoutSplit", + title: "Split layout", + category: "view", + defaultKeys: ["1"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.layoutUnified', - aliases: ['hunk.view.layoutStack'], - title: 'Unified layout', - category: 'view', - defaultKeys: ['2'], - locus: 'client-local', + id: "hunk.view.layoutUnified", + aliases: ["hunk.view.layoutStack"], + title: "Unified layout", + category: "view", + defaultKeys: ["2"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.layoutAuto', - title: 'Auto layout', - category: 'view', - defaultKeys: ['0'], - locus: 'client-local', + id: "hunk.view.layoutAuto", + title: "Auto layout", + category: "view", + defaultKeys: ["0"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.applyFilePresentationToAllMatching', - title: 'Apply the current file presentation to all matching files', - category: 'view', + id: "hunk.view.applyFilePresentationToAllMatching", + title: "Apply the current file presentation to all matching files", + category: "view", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleFilesPane', - aliases: ['hunk.view.toggleSidebar'], - title: 'Toggle files pane', - category: 'view', - defaultKeys: ['s'], - locus: 'client-local', + id: "hunk.view.toggleFilesPane", + aliases: ["hunk.view.toggleSidebar"], + title: "Toggle files pane", + category: "view", + defaultKeys: ["s"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.app.refresh', - title: 'Refresh the review', - category: 'app', - defaultKeys: ['r'], - locus: 'host-only', + id: "hunk.app.refresh", + title: "Refresh the review", + category: "app", + defaultKeys: ["r"], + locus: "host-only", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.openThemeSelector', - title: 'Choose theme', - category: 'view', - defaultKeys: ['t'], - locus: 'client-local', + id: "hunk.view.openThemeSelector", + title: "Choose theme", + category: "view", + defaultKeys: ["t"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleAgentNotes', - title: 'Toggle agent notes', - category: 'view', - defaultKeys: ['a'], - locus: 'semantic', - review: { kind: 'notes/toggle-visibility' }, + id: "hunk.view.toggleAgentNotes", + title: "Toggle agent notes", + category: "view", + defaultKeys: ["a"], + locus: "semantic", + review: { kind: "notes/toggle-visibility" }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleLineNumbers', - title: 'Toggle line numbers', - category: 'view', - defaultKeys: ['l'], - locus: 'client-local', + id: "hunk.view.toggleLineNumbers", + title: "Toggle line numbers", + category: "view", + defaultKeys: ["l"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleLineWrap', - title: 'Toggle line wrapping', - category: 'view', - defaultKeys: ['w'], - locus: 'client-local', + id: "hunk.view.toggleLineWrap", + title: "Toggle line wrapping", + category: "view", + defaultKeys: ["w"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleMenuBar', - title: 'Toggle menu bar', - category: 'view', - defaultKeys: ['M'], - locus: 'client-local', + id: "hunk.view.toggleMenuBar", + title: "Toggle menu bar", + category: "view", + defaultKeys: ["M"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleHunkHeaders', - title: 'Toggle hunk headers', - category: 'view', - defaultKeys: ['m'], - locus: 'client-local', + id: "hunk.view.toggleHunkHeaders", + title: "Toggle hunk headers", + category: "view", + defaultKeys: ["m"], + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.view.toggleCopyDecorations', - title: 'Toggle copy decorations', - category: 'view', + id: "hunk.view.toggleCopyDecorations", + title: "Toggle copy decorations", + category: "view", defaultKeys: [], - locus: 'client-local', + locus: "client-local", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.toggleHunkGap', - title: 'Expand or collapse context for the selected hunk', - category: 'review', - defaultKeys: ['z'], - locus: 'semantic', - review: { kind: 'expansion/toggle-selected-gap' }, + id: "hunk.review.toggleHunkGap", + title: "Expand or collapse context for the selected hunk", + category: "review", + defaultKeys: ["z"], + locus: "semantic", + review: { kind: "expansion/toggle-selected-gap" }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.editSelectedFile', - title: 'Open the selected file in your editor', - category: 'review', - defaultKeys: ['e'], - locus: 'host-only', + id: "hunk.review.editSelectedFile", + title: "Open the selected file in your editor", + category: "review", + defaultKeys: ["e"], + locus: "host-only", publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.previousHunk', - title: 'Previous hunk', - category: 'review', - defaultKeys: ['['], - locus: 'semantic', + id: "hunk.review.previousHunk", + title: "Previous hunk", + category: "review", + defaultKeys: ["["], + locus: "semantic", verticalDirection: -1, - review: { kind: 'selection/move', scope: 'hunk', direction: -1 }, + review: { kind: "selection/move", scope: "hunk", direction: -1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.nextHunk', - title: 'Next hunk', - category: 'review', - defaultKeys: [']'], - locus: 'semantic', + id: "hunk.review.nextHunk", + title: "Next hunk", + category: "review", + defaultKeys: ["]"], + locus: "semantic", verticalDirection: 1, - review: { kind: 'selection/move', scope: 'hunk', direction: 1 }, + review: { kind: "selection/move", scope: "hunk", direction: 1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.previousFile', - title: 'Previous file', - category: 'review', - defaultKeys: [','], - locus: 'semantic', + id: "hunk.review.previousFile", + title: "Previous file", + category: "review", + defaultKeys: [","], + locus: "semantic", verticalDirection: -1, - review: { kind: 'selection/move', scope: 'file', direction: -1 }, + review: { kind: "selection/move", scope: "file", direction: -1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.nextFile', - title: 'Next file', - category: 'review', - defaultKeys: ['.'], - locus: 'semantic', + id: "hunk.review.nextFile", + title: "Next file", + category: "review", + defaultKeys: ["."], + locus: "semantic", verticalDirection: 1, - review: { kind: 'selection/move', scope: 'file', direction: 1 }, + review: { kind: "selection/move", scope: "file", direction: 1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.previousAnnotatedHunk', - title: 'Previous annotated hunk', - category: 'review', - defaultKeys: ['{'], - locus: 'semantic', + id: "hunk.review.previousAnnotatedHunk", + title: "Previous annotated hunk", + category: "review", + defaultKeys: ["{"], + locus: "semantic", verticalDirection: -1, - review: { kind: 'selection/move', scope: 'annotated-hunk', direction: -1 }, + review: { kind: "selection/move", scope: "annotated-hunk", direction: -1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.nextAnnotatedHunk', - title: 'Next annotated hunk', - category: 'review', - defaultKeys: ['}'], - locus: 'semantic', + id: "hunk.review.nextAnnotatedHunk", + title: "Next annotated hunk", + category: "review", + defaultKeys: ["}"], + locus: "semantic", verticalDirection: 1, - review: { kind: 'selection/move', scope: 'annotated-hunk', direction: 1 }, + review: { kind: "selection/move", scope: "annotated-hunk", direction: 1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.previousAnnotatedFile', - title: 'Previous annotated file', - category: 'review', + id: "hunk.review.previousAnnotatedFile", + title: "Previous annotated file", + category: "review", defaultKeys: [], - locus: 'semantic', + locus: "semantic", verticalDirection: -1, - review: { kind: 'selection/move', scope: 'annotated-file', direction: -1 }, + review: { kind: "selection/move", scope: "annotated-file", direction: -1 }, publicToExtensions: true, closesMenu: true, }, { - id: 'hunk.review.nextAnnotatedFile', - title: 'Next annotated file', - category: 'review', + id: "hunk.review.nextAnnotatedFile", + title: "Next annotated file", + category: "review", defaultKeys: [], - locus: 'semantic', + locus: "semantic", verticalDirection: 1, - review: { kind: 'selection/move', scope: 'annotated-file', direction: 1 }, + review: { kind: "selection/move", scope: "annotated-file", direction: 1 }, publicToExtensions: true, closesMenu: true, }, @@ -613,10 +613,9 @@ const BUILTIN_COMMANDS = [ * Derived from the literal declaration above, which is why that one stays `as const` while * the exported catalog is widened to the entry shape consumers read. */ -export type AppCommandId = (typeof BUILTIN_COMMANDS)[number]['id']; +export type AppCommandId = (typeof BUILTIN_COMMANDS)[number]["id"]; -export const APP_COMMAND_CATALOG: readonly AppCommandCatalogEntry[] = - BUILTIN_COMMANDS; +export const APP_COMMAND_CATALOG: readonly AppCommandCatalogEntry[] = BUILTIN_COMMANDS; /** Canonical and compatibility names accepted for review-surface built-ins. */ export const APP_COMMAND_NAMES: ReadonlySet = new Set( @@ -624,12 +623,8 @@ export const APP_COMMAND_NAMES: ReadonlySet = new Set( ); /** Look one command up by its canonical id or a compatibility alias. */ -export function appCommandCatalogEntry( - id: string, -): AppCommandCatalogEntry | undefined { - return APP_COMMAND_CATALOG.find( - (entry) => entry.id === id || entry.aliases?.includes(id), - ); +export function appCommandCatalogEntry(id: string): AppCommandCatalogEntry | undefined { + return APP_COMMAND_CATALOG.find((entry) => entry.id === id || entry.aliases?.includes(id)); } /** @@ -640,9 +635,7 @@ export function appCommandCatalogEntry( * possibility that the command does not exist. */ export function builtinAppCommand(id: AppCommandId): AppCommandCatalogEntry { - return APP_COMMAND_CATALOG.find( - (entry) => entry.id === id, - ) as AppCommandCatalogEntry; + return APP_COMMAND_CATALOG.find((entry) => entry.id === id) as AppCommandCatalogEntry; } /** The facts a command needs to become one concrete review intent. */ @@ -688,44 +681,43 @@ export function lowerAppCommandToReviewIntent( { count, state, noteTarget, noteLocation }: AppCommandLoweringContext, ): ReviewIntent | undefined { switch (entry.review?.kind) { - case 'selection/move': + case "selection/move": return { - type: 'selection/move', + type: "selection/move", scope: entry.review.scope, delta: entry.review.direction * count, }; - case 'notes/toggle-visibility': - return { type: 'notes/set-visibility', visible: !state.showAgentNotes }; - case 'notes/start-draft': { - const { fileKey, hunkIndex } = - noteLocation ?? selectNormalizedSelection(state); + case "notes/toggle-visibility": + return { type: "notes/set-visibility", visible: !state.showAgentNotes }; + case "notes/start-draft": { + const { fileKey, hunkIndex } = noteLocation ?? selectNormalizedSelection(state); return fileKey === null ? undefined : { - type: 'notes/start-draft', + type: "notes/start-draft", fileKey, hunkIndex, ...(noteTarget ? { target: noteTarget } : {}), }; } - case 'notes/start-edit-active': { + case "notes/start-edit-active": { const noteId = selectActiveEditableReviewNoteId(state); - return noteId ? { type: 'notes/start-edit', noteId } : undefined; + return noteId ? { type: "notes/start-edit", noteId } : undefined; } - case 'notes/start-reply-active': { + case "notes/start-reply-active": { const noteId = selectActiveReplyableReviewNoteId(state); - return noteId ? { type: 'notes/start-reply', noteId } : undefined; + return noteId ? { type: "notes/start-reply", noteId } : undefined; } - case 'notes/remove-active': { + case "notes/remove-active": { const target = selectActiveRemovableReviewNote(state); if (!target) return undefined; - return target.source === 'user' - ? { type: 'notes/remove-user', noteId: target.noteId } - : { type: 'notes/remove-live', noteId: target.noteId }; + return target.source === "user" + ? { type: "notes/remove-user", noteId: target.noteId } + : { type: "notes/remove-live", noteId: target.noteId }; } - case 'expansion/toggle-selected-gap': { + case "expansion/toggle-selected-gap": { const target = selectReviewGapForSelection(state); - return target ? { type: 'expansion/toggle', ...target } : undefined; + return target ? { type: "expansion/toggle", ...target } : undefined; } case undefined: return undefined; diff --git a/packages/hunk/src/ui/App.tsx b/packages/hunk/src/ui/App.tsx index 9109368f3..acd9f2d0a 100644 --- a/packages/hunk/src/ui/App.tsx +++ b/packages/hunk/src/ui/App.tsx @@ -1161,6 +1161,7 @@ export function App({ canDeleteActiveNote: activeRemovableNote !== undefined && review.draftNote === null, canEditActiveNote: activeEditableNoteId !== undefined && review.draftNote === null, canReplyToActiveNote: activeReplyableNoteId !== undefined && review.draftNote === null, + canSaveDraftNote: review.draftNote !== null, canRefreshCurrentInput, alignCurrentLine, applyFilePresentationToAllMatching, diff --git a/packages/hunk/src/ui/AppHost.keybindings.test.tsx b/packages/hunk/src/ui/AppHost.keybindings.test.tsx index b7c9e95c4..6ed2ab6a6 100644 --- a/packages/hunk/src/ui/AppHost.keybindings.test.tsx +++ b/packages/hunk/src/ui/AppHost.keybindings.test.tsx @@ -268,6 +268,13 @@ describe("user keybindings", () => { }); await flush(setup); expect(seen).toContain("hunk.review.stepDown"); + + seen.length = 0; + await act(async () => { + setup.mockInput.pressKey("s", { ctrl: true }); + }); + await flush(setup); + expect(seen).toEqual([]); }); }); diff --git a/packages/hunk/src/ui/lib/appCommands.test.ts b/packages/hunk/src/ui/lib/appCommands.test.ts index ea49373c6..5a822659f 100644 --- a/packages/hunk/src/ui/lib/appCommands.test.ts +++ b/packages/hunk/src/ui/lib/appCommands.test.ts @@ -36,7 +36,10 @@ function keyEvent(fields: Partial): KeyEvent { } /** Build the built-in table over recording callbacks, plus the log it writes. */ -function createTestCommands(resolvedKeys?: ResolvedCommandKeys) { +function createTestCommands( + resolvedKeys?: ResolvedCommandKeys, + overrides: Partial = {}, +) { const ran: string[] = []; const record = (name: string) => @@ -75,6 +78,7 @@ function createTestCommands(resolvedKeys?: ResolvedCommandKeys) { toggleFilesPane: record("toggleFilesPane"), triggerEditSelectedFile: record("triggerEditSelectedFile"), triggerRefreshCurrentInput: record("triggerRefreshCurrentInput"), + ...overrides, }; return { commands: buildAppCommands(options), ran }; @@ -397,6 +401,20 @@ describe("executeAppCommand", () => { expect(executeAppCommand(commands, "nobody.registered.this")).toBe(false); expect(ran).toEqual([]); }); + + test("save-note stays idle until a draft exists", () => { + const idle = createTestCommands(); + expect(dispatchAppCommand(idle.commands, keyEvent({ name: "s", ctrl: true }))).toBeUndefined(); + expect(executeAppCommand(idle.commands, "hunk.review.saveNote")).toBe(false); + expect(idle.ran).toEqual([]); + + const drafting = createTestCommands(undefined, { canSaveDraftNote: true }); + expect(dispatchAppCommand(drafting.commands, keyEvent({ name: "s", ctrl: true }))?.id).toBe( + "hunk.review.saveNote", + ); + expect(executeAppCommand(drafting.commands, "hunk.review.saveNote")).toBe(true); + expect(drafting.ran).toEqual(["saveDraftNote", "saveDraftNote"]); + }); }); describe("observeAppCommandDispatch", () => { diff --git a/packages/hunk/src/ui/lib/appCommands.ts b/packages/hunk/src/ui/lib/appCommands.ts index 7530721f6..3897c4d78 100644 --- a/packages/hunk/src/ui/lib/appCommands.ts +++ b/packages/hunk/src/ui/lib/appCommands.ts @@ -119,6 +119,8 @@ export interface BuildAppCommandsOptions { canDeleteActiveNote?: boolean; canEditActiveNote?: boolean; canReplyToActiveNote?: boolean; + /** True while the composer has a draft the save command can persist. */ + canSaveDraftNote?: boolean; canRefreshCurrentInput: boolean; alignCurrentLine: (alignment: "top" | "center" | "bottom") => void; applyFilePresentationToAllMatching: () => void; @@ -215,7 +217,10 @@ function builtinCommandHandlers( isEnabled: () => Boolean(options.canReplyToActiveNote), run: () => options.replyToActiveNote?.(), }, - "hunk.review.saveNote": { run: () => options.saveDraftNote() }, + "hunk.review.saveNote": { + isEnabled: () => Boolean(options.canSaveDraftNote), + run: () => options.saveDraftNote(), + }, "hunk.review.deleteActiveNote": { isEnabled: () => Boolean(options.canDeleteActiveNote), run: () => options.deleteActiveNote?.(), @@ -358,6 +363,7 @@ const NOOP_COMMAND_OPTIONS: BuildAppCommandsOptions = (() => { canAlignCurrentLine: false, canApplyFilePresentationToAllMatching: false, canRefreshCurrentInput: true, + canSaveDraftNote: true, alignCurrentLine: noop, applyFilePresentationToAllMatching: noop, focusFilter: noop,