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 549810a68..fff578b6f 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -1835,8 +1835,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 9c5cdd1d1..dffa0fd06 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -82,6 +82,7 @@ import { buildAppCommands, builtinCommandKeyDefaults, builtinCommandMatchProbes, + findAppCommandById, observeAppCommandDispatch, } from "./lib/appCommands"; import { buildAppMenus } from "./lib/appMenus"; @@ -1046,6 +1047,7 @@ export function App({ stepDiffLine, selectCursorLine: setCursorLine, selectLayoutMode, + saveDraftNote, startUserNote: () => startUserNote(), toggleAgentNotes, toggleCopyDecorations, @@ -1064,6 +1066,7 @@ export function App({ ], publishCommandExecuted, ); + const draftSaveKeyLabel = findAppCommandById(appCommands, "hunk.review.saveNote")?.keyLabels[0]; useExtensionRuntimeBindings({ commands: appCommands, navigation: extensionNavigationBindings, @@ -1151,7 +1154,6 @@ export function App({ discardViewPreferencesAndQuit, neverAskToSaveViewPreferencesAndQuit, closeSaveConfigPrompt, - saveDraftNote, showAgentSkill, showHelp, switchMenu, @@ -1367,6 +1369,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 fe8b6f44c..5b9a8f0bc 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 23327df90..a5319db03 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, @@ -470,8 +469,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") { @@ -503,9 +503,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 a3b6b5036..70c1be79f 100644 --- a/test/pty/notes.test.ts +++ b/test/pty/notes.test.ts @@ -239,7 +239,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); @@ -250,7 +250,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"); @@ -259,7 +259,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 @@ -828,7 +828,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 c376bbc25..0648590df 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -384,7 +384,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. - `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.