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 890221878..b84c5ffe7 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -2014,8 +2014,9 @@ identity for existing handlers and `canonicalCommandId` names the replacement. E 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), `"extension"` (an diff --git a/docs/keybindings.md b/docs/keybindings.md index 31f1cfa09..94160d961 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -136,6 +136,7 @@ Review and shared commands: | `hunk.review.previousHunk` | Previous hunk | `[` | | `hunk.review.previousNote` | Previous review note | `N` | | `hunk.review.replyToActiveNote` | Reply to 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` | @@ -188,10 +189,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/packages/hunk/src/core/run/commandCatalog.test.ts b/packages/hunk/src/core/run/commandCatalog.test.ts index d576e5173..fd7411e62 100644 --- a/packages/hunk/src/core/run/commandCatalog.test.ts +++ b/packages/hunk/src/core/run/commandCatalog.test.ts @@ -127,6 +127,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/packages/hunk/src/core/run/commandCatalog.ts b/packages/hunk/src/core/run/commandCatalog.ts index 21323d326..0deefe11c 100644 --- a/packages/hunk/src/core/run/commandCatalog.ts +++ b/packages/hunk/src/core/run/commandCatalog.ts @@ -215,6 +215,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.deleteActiveNote", title: "Delete active review note", diff --git a/packages/hunk/src/ui/App.tsx b/packages/hunk/src/ui/App.tsx index 69c284042..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, @@ -1190,6 +1191,7 @@ export function App({ stepDiffLine, selectCursorLine, selectLayoutMode, + saveDraftNote, hasVisualSelection: () => selectionActionsRef.current?.hasSelection() ?? false, startVisualSelection: () => selectionActionsRef.current?.beginKeyboardSelection(), copySelection: () => selectionActionsRef.current?.copy(), @@ -1218,6 +1220,7 @@ export function App({ ], publishCommandExecuted, ); + const draftSaveKeyLabel = findAppCommandById(appCommands, "hunk.review.saveNote")?.keyLabels[0]; const selectionCommentKeyLabel = findAppCommandById(appCommands, "hunk.review.startNote") ?.keyLabels[0]; const selectionCopyKeyLabel = findAppCommandById(appCommands, "hunk.review.copySelection") @@ -1564,6 +1567,7 @@ export function App({ onRemoveLiveNote={review.removeLiveComment} onRemoveUserNote={review.removeUserNote} onSaveDraftNote={saveDraftNote} + draftSaveKeyLabel={draftSaveKeyLabel} onStartUserNoteAtHunk={startUserNote} onUpdateDraftNote={updateDraftNote} onBlurDraftNote={blurDraftNote} diff --git a/packages/hunk/src/ui/AppHost.keybindings.test.tsx b/packages/hunk/src/ui/AppHost.keybindings.test.tsx index 878d37fd5..6ed2ab6a6 100644 --- a/packages/hunk/src/ui/AppHost.keybindings.test.tsx +++ b/packages/hunk/src/ui/AppHost.keybindings.test.tsx @@ -101,6 +101,7 @@ async function withAppHost( externalQuitSignal?: AbortSignal, extensionOwnership: "owned" | "borrowed" = "owned", extensionSession?: ExtensionSession, + renderOptions?: { kittyKeyboard?: boolean }, ) { let quitCount = 0; const setup = await testRender( @@ -111,7 +112,7 @@ async function withAppHost( extensionOwnership={extensionOwnership} {...(extensionSession ? { extensionSession } : {})} />, - { width: 120, height: 24 }, + { width: 120, height: 24, ...renderOptions }, ); try { @@ -267,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([]); }); }); @@ -453,4 +461,132 @@ 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, + "owned", + 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/packages/hunk/src/ui/components/panes/AgentInlineNote.test.tsx b/packages/hunk/src/ui/components/panes/AgentInlineNote.test.tsx index bb0866fd9..4ed4eb517 100644 --- a/packages/hunk/src/ui/components/panes/AgentInlineNote.test.tsx +++ b/packages/hunk/src/ui/components/panes/AgentInlineNote.test.tsx @@ -540,4 +540,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/packages/hunk/src/ui/components/panes/AgentInlineNote.tsx b/packages/hunk/src/ui/components/panes/AgentInlineNote.tsx index 7d62a66f5..fe5e94e4f 100644 --- a/packages/hunk/src/ui/components/panes/AgentInlineNote.tsx +++ b/packages/hunk/src/ui/components/panes/AgentInlineNote.tsx @@ -212,15 +212,7 @@ export function AgentInlineNote({ noteIndex?: number; /** Join the card's top-right corner to the external range rail. */ rangeGuideConnection?: "terminate" | "continue"; - draft?: { - body: string; - focused: boolean; - onBlur?: () => void; - onCancel: () => void; - onFocus?: () => void; - onInput: (value: string) => void; - onSave: (editorBody?: string) => void; - }; + draft?: VisibleAgentNote["draft"]; actions?: AgentInlineNoteActions; /** Make this saved note the keyboard action target when its card is clicked. */ onActivate?: () => void; @@ -523,7 +515,7 @@ export function AgentInlineNote({ style={{ width: renderedItemWidth, height: 1, backgroundColor }} > - {item.keyLabel} + {item.keyLabel ? {item.keyLabel} : null} {item.displayLabel ? ( {`${item.keyLabel ? " " : ""}${item.displayLabel}`} @@ -548,7 +540,7 @@ export function AgentInlineNote({ const draftActionItems: BorderActionItem[] = [ { id: "save", - keyLabel: "^S", + keyLabel: draft.saveKeyLabel ?? "", label: "save", onMouseUp: () => draft.onSave(textareaRef.current?.plainText), }, diff --git a/packages/hunk/src/ui/components/panes/DiffPane.tsx b/packages/hunk/src/ui/components/panes/DiffPane.tsx index 9aac4db32..a9e097124 100644 --- a/packages/hunk/src/ui/components/panes/DiffPane.tsx +++ b/packages/hunk/src/ui/components/panes/DiffPane.tsx @@ -368,6 +368,7 @@ export function DiffPane({ onRemoveLiveNote, onRemoveUserNote, onSaveDraftNote, + draftSaveKeyLabel, onStartUserNoteAtHunk, onUpdateDraftNote, onBlurDraftNote, @@ -456,6 +457,8 @@ export function DiffPane({ onRemoveLiveNote?: (noteId: string) => void; onRemoveUserNote?: (noteId: string) => void; onSaveDraftNote?: (editorBody?: string) => void; + /** Live chord for the draft save action; omitted when `hunk.review.saveNote` is unbound. */ + draftSaveKeyLabel?: string; onStartUserNoteAtHunk?: StartUserNoteAtHunk; onUpdateDraftNote?: (body: string) => void; onBlurDraftNote?: () => void; @@ -744,6 +747,7 @@ export function DiffPane({ onFocus: onFocusDraftNote, onInput: onUpdateDraftNote ?? (() => {}), onSave: onSaveDraftNote ?? (() => {}), + ...(draftSaveKeyLabel ? { saveKeyLabel: draftSaveKeyLabel } : {}), }, }); if (draftNote.kind === "edit" && draftNote.targetNoteId) { @@ -814,6 +818,7 @@ export function DiffPane({ onRemoveLiveNote, onRemoveUserNote, onSaveDraftNote, + draftSaveKeyLabel, onUpdateDraftNote, noteActionKeyLabels, showAgentNotes, diff --git a/packages/hunk/src/ui/components/ui-components.test.tsx b/packages/hunk/src/ui/components/ui-components.test.tsx index e3caa135d..9e39feb2b 100644 --- a/packages/hunk/src/ui/components/ui-components.test.tsx +++ b/packages/hunk/src/ui/components/ui-components.test.tsx @@ -3014,6 +3014,7 @@ describe("UI components", () => { onCancel, onInput: () => {}, onSave, + saveKeyLabel: "Ctrl+S", }} />, { width: 64, height: measured + 1 }, @@ -3022,7 +3023,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; @@ -3138,6 +3139,7 @@ describe("UI components", () => { onCancel: () => {}, onInput: () => {}, onSave: () => {}, + saveKeyLabel: "Ctrl+S", }} file={file} anchorSide="new" @@ -3153,8 +3155,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); @@ -3182,6 +3186,7 @@ describe("UI components", () => { onCancel: () => {}, onInput: () => {}, onSave: () => {}, + saveKeyLabel: "Ctrl+S", }} file={file} anchorSide="new" @@ -3195,7 +3200,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); @@ -3862,13 +3867,13 @@ describe("UI components", () => { const frame = await captureFrame( {}} />, 76, - 39, + 41, ); const expectedRows = [ @@ -3898,6 +3903,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", @@ -3983,13 +3989,13 @@ describe("UI components", () => { const frame = await captureFrame( {}} />, 76, - 39, + 41, ); expect(frame).toContain("Ctrl+X"); diff --git a/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts b/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts index a6aed327a..f0313fde3 100644 --- a/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/packages/hunk/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"; import { handleViewPreferenceQuitPromptKey } from "../lib/viewPreferenceQuitKeys"; @@ -162,6 +163,7 @@ export function useAppKeyboardShortcuts({ const acceptExtensionDialogRef = useRef(acceptExtensionDialog); const cancelExtensionDialogRef = useRef(cancelExtensionDialog); const moveExtensionDialogSelectionRef = useRef(moveExtensionDialogSelection); + const saveDraftNoteRef = useRef(saveDraftNote); activeMenuIdRef.current = activeMenuId; commandsRef.current = commands; @@ -182,6 +184,7 @@ export function useAppKeyboardShortcuts({ acceptExtensionDialogRef.current = acceptExtensionDialog; cancelExtensionDialogRef.current = cancelExtensionDialog; moveExtensionDialogSelectionRef.current = moveExtensionDialogSelection; + saveDraftNoteRef.current = saveDraftNote; /** * Stop a key dead: the focused renderable never sees it, and neither do @@ -460,8 +463,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") { @@ -493,9 +497,15 @@ export function useAppKeyboardShortcuts({ return "mine"; } - if (isSaveDraftNoteKey(key)) { - saveDraftNote(renderer.currentFocusedEditor?.plainText); - return "mine"; + const save = findAppCommandById(commandsRef.current, "hunk.review.saveNote"); + const saveOwner = noteComposerSaveOwner(save?.keys ?? [], key, () => { + // The live textarea can be ahead of React state; persist that buffer first. + // Dispatch still runs so `command_executed` fires; a repeat save finds no draft. + saveDraftNoteRef.current(renderer.currentFocusedEditor?.plainText); + return 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/packages/hunk/src/ui/lib/agentAnnotations.ts b/packages/hunk/src/ui/lib/agentAnnotations.ts index d9fc1faff..93423326b 100644 --- a/packages/hunk/src/ui/lib/agentAnnotations.ts +++ b/packages/hunk/src/ui/lib/agentAnnotations.ts @@ -47,7 +47,9 @@ export interface VisibleAgentNote { onCancel: () => void; onFocus?: () => void; onInput: (value: string) => void; - onSave: () => void; + onSave: (editorBody?: string) => void; + /** Live chord for save, from `hunk.review.saveNote`; omitted when unbound. */ + saveKeyLabel?: string; }; } diff --git a/packages/hunk/src/ui/lib/appCommands.test.ts b/packages/hunk/src/ui/lib/appCommands.test.ts index 99ad9e0b6..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) => @@ -61,6 +64,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"), @@ -74,6 +78,7 @@ function createTestCommands(resolvedKeys?: ResolvedCommandKeys) { toggleFilesPane: record("toggleFilesPane"), triggerEditSelectedFile: record("triggerEditSelectedFile"), triggerRefreshCurrentInput: record("triggerRefreshCurrentInput"), + ...overrides, }; return { commands: buildAppCommands(options), ran }; @@ -276,6 +281,9 @@ describe("builtinCommandKeyDefaults", () => { "u", "ctrl+u", ]); + expect(defaults.find((entry) => entry.id === "hunk.review.saveNote")?.defaultKeys).toEqual([ + "ctrl+s", + ]); // Commands with contextual or menu routing ship unbound and remain user-bindable. expect( defaults @@ -393,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 cc8f487e9..3897c4d78 100644 --- a/packages/hunk/src/ui/lib/appCommands.ts +++ b/packages/hunk/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; @@ -117,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; @@ -138,6 +142,7 @@ export interface BuildAppCommandsOptions { stepDiffLine: (delta: number) => void; selectCursorLine: (style: CursorLine) => void; selectLayoutMode: (mode: LayoutMode) => void; + saveDraftNote: (editorBody?: string) => void; hasVisualSelection?: () => boolean; startVisualSelection?: () => void; copySelection?: () => void; @@ -212,6 +217,10 @@ function builtinCommandHandlers( isEnabled: () => Boolean(options.canReplyToActiveNote), run: () => options.replyToActiveNote?.(), }, + "hunk.review.saveNote": { + isEnabled: () => Boolean(options.canSaveDraftNote), + run: () => options.saveDraftNote(), + }, "hunk.review.deleteActiveNote": { isEnabled: () => Boolean(options.canDeleteActiveNote), run: () => options.deleteActiveNote?.(), @@ -354,6 +363,7 @@ const NOOP_COMMAND_OPTIONS: BuildAppCommandsOptions = (() => { canAlignCurrentLine: false, canApplyFilePresentationToAllMatching: false, canRefreshCurrentInput: true, + canSaveDraftNote: true, alignCurrentLine: noop, applyFilePresentationToAllMatching: noop, focusFilter: noop, @@ -367,6 +377,7 @@ const NOOP_COMMAND_OPTIONS: BuildAppCommandsOptions = (() => { stepDiffLine: noop, selectCursorLine: noop, selectLayoutMode: noop, + saveDraftNote: noop, hasVisualSelection: () => false, startVisualSelection: noop, copySelection: noop, diff --git a/packages/hunk/src/ui/lib/appMenus.test.ts b/packages/hunk/src/ui/lib/appMenus.test.ts index 8fd4fa020..5204c16e5 100644 --- a/packages/hunk/src/ui/lib/appMenus.test.ts +++ b/packages/hunk/src/ui/lib/appMenus.test.ts @@ -53,6 +53,7 @@ function createTestCommands(overrides: Partial = {}) { selectCursorLine: noop, stepDiffLine: noop, selectLayoutMode: noop, + saveDraftNote: noop, startUserNote: noop, toggleAgentNotes: noop, toggleCopyDecorations: record("toggleCopyDecorations"), diff --git a/packages/hunk/src/ui/lib/helpContent.test.ts b/packages/hunk/src/ui/lib/helpContent.test.ts index 64329e5e9..392ec77df 100644 --- a/packages/hunk/src/ui/lib/helpContent.test.ts +++ b/packages/hunk/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/packages/hunk/src/ui/lib/helpContent.ts b/packages/hunk/src/ui/lib/helpContent.ts index 3eaac6852..710067af0 100644 --- a/packages/hunk/src/ui/lib/helpContent.ts +++ b/packages/hunk/src/ui/lib/helpContent.ts @@ -112,6 +112,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", diff --git a/packages/hunk/src/ui/lib/keyboard.ts b/packages/hunk/src/ui/lib/keyboard.ts index 73d66afab..b47ef9e68 100644 --- a/packages/hunk/src/ui/lib/keyboard.ts +++ b/packages/hunk/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/packages/hunk/src/ui/lib/ui-lib.test.ts b/packages/hunk/src/ui/lib/ui-lib.test.ts index 657183955..0dd57c890 100644 --- a/packages/hunk/src/ui/lib/ui-lib.test.ts +++ b/packages/hunk/src/ui/lib/ui-lib.test.ts @@ -14,7 +14,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, @@ -226,6 +231,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 c4eaa1eee..007b7794f 100644 --- a/test/pty/notes.test.ts +++ b/test/pty/notes.test.ts @@ -240,7 +240,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); @@ -251,7 +251,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"); @@ -260,7 +260,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 @@ -1065,7 +1065,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 cf956ee91..15d4978d2 100644 --- a/website/src/content/docs/docs/configure/keybindings.md +++ b/website/src/content/docs/docs/configure/keybindings.md @@ -32,6 +32,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 3a2e900cb..ba760a97a 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -420,7 +420,7 @@ Subscribe to a lifecycle or UI event. Handlers may be async; Hunk never blocks t - Starting with extension API v23, `layout_changed` adds `canonicalMode` and `canonicalLayout`, which use `"unified"`. The original `mode` and `layout` fields remain available for compatibility and continue to report `"stack"` where their canonical counterparts report `"unified"`. To preserve exhaustive existing source, `ExtensionLayoutMode` and `ExtensionResolvedLayout` retain their pre-v23 shapes; new extensions use `ExtensionCanonicalLayoutMode` and `ExtensionCanonicalResolvedLayout`. Hunk will keep `"stack"` and the legacy fields until a separately announced major API revision. - `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. - `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`. For a renamed command, `commandId` preserves the deprecated identity and `canonicalCommandId` names its replacement. 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`. For a renamed command, `commandId` preserves the deprecated identity and `canonicalCommandId` names its replacement. 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), `"extension"` (an in-process extension request), 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 the snapshot shape command handlers read through `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` to invalidate extension-owned state and read the complete current record from a later command snapshot.