From 405d9bdda04c5271bfe5509b77896e37451018b4 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 08:01:37 -0500 Subject: [PATCH 01/12] Feat(keymaps): a binding can be removed outright, not just remapped The only way to get rid of a shortcut you kept hitting by accident was to remap it onto some chord you would never press: the recorder needed a key, the config file needed a value, and an empty override was quietly thrown away in favor of the default. Every keymap row now carries an Unbind button next to Change and Reset, the recorder's Clear became Unbind (Backspace still clears a recording), and an unbound row reads Unbound with the Custom badge until Reset brings the default back. The stored form is an override equal to the empty string. It is a deliberate state, distinct from a missing override (the default applies) and from a recording that has not captured a key yet: normalizeKeymapOverrides keeps it, getKeymapBinding returns it instead of falling back, and the matchers, the conflict check and the user-override scan treat it as a key that does not exist. TOML has no null, so config.toml carries it as "action.id" = "" with a # unbound marker, the reference block explains the convention, and a hand edit applies live like every other config change. Everything that advertises keys follows suit. The which-key hints drop an unbound leader action, the command palette shows no chord when any step of one is unbound, tooltips lose their parenthetical, and the manual prints "Unbound" where the key used to be. CodeMirror files an empty key name without complaint and would run the command on a keydown whose key is empty, so editor keymap entries now go through keyBindingsFor, which returns nothing for an unbound action; VimNav's leader and pane prefixes fall back to the empty string, which never equals a token read off an event, rather than reviving Space and Ctrl+W. Vim users get :unbind action.id, the ex twin of the button: it removes the key and a toast names what it was. Bare :unbind, or an id the catalog does not know, opens Settings on the Keymap page, which lists every id, instead of guessing. Both docs surfaces describe the button, the ex command and the config spelling; the website half lives in its own repository. Verified with unit tests for the model, the config round trip, the half-page keymap and the Settings row, and by driving the built app over CDP through the row Unbind, :unbind with a known and an unknown id, the leader hints before and after, a hand-edited "" picked up live, Tab completion of :unb, and Reset restoring the default. --- apps/desktop/src/main/app-config.test.ts | 14 ++++ apps/desktop/src/main/app-config.ts | 6 +- packages/app-core/src/components/Editor.tsx | 39 +++++++++ .../app-core/src/components/EditorPane.tsx | 68 ++++++--------- packages/app-core/src/components/HelpView.tsx | 83 ++++++++++++++----- .../src/components/SettingsModal.test.ts | 57 ++++++++++++- .../app-core/src/components/SettingsModal.tsx | 72 +++++++++++++--- packages/app-core/src/components/VimNav.tsx | 15 +++- packages/app-core/src/lib/commands.ts | 14 ++-- packages/app-core/src/lib/help.ts | 10 ++- packages/app-core/src/lib/keymaps.test.ts | 81 ++++++++++++++++++ packages/app-core/src/lib/keymaps.ts | 48 ++++++++++- .../app-core/src/lib/settings-navigation.ts | 5 +- .../src/lib/vim-half-page-keymap.test.ts | 12 ++- .../app-core/src/lib/vim-half-page-keymap.ts | 31 ++++--- packages/app-core/src/store.ts | 6 +- 16 files changed, 453 insertions(+), 108 deletions(-) diff --git a/apps/desktop/src/main/app-config.test.ts b/apps/desktop/src/main/app-config.test.ts index f091f471..dc7bf6da 100644 --- a/apps/desktop/src/main/app-config.test.ts +++ b/apps/desktop/src/main/app-config.test.ts @@ -281,3 +281,17 @@ describe('file watching', () => { expect(changes.at(-1)?.editorFontSize).toBe(22) }, 10000) }) + +describe('unbound keymaps in config.toml', () => { + it('writes an unbind as an empty binding, marks it, and reads it back as ""', () => { + const text = serializeConfig({ keymapOverrides: { 'global.zoomIn': '' } }) + expect(text).toContain('"global.zoomIn" = "" # unbound') + // The reference list explains the convention and no longer repeats the + // overridden action as a commented default. + expect(text).toContain('# An empty binding ("") removes the key entirely') + expect(text).not.toContain('# "global.zoomIn" = "Mod+="') + + const { portable } = deserializeConfig(text) + expect(portable.keymapOverrides).toEqual({ 'global.zoomIn': '' }) + }) +}) diff --git a/apps/desktop/src/main/app-config.ts b/apps/desktop/src/main/app-config.ts index da3ebf41..58cd09c6 100644 --- a/apps/desktop/src/main/app-config.ts +++ b/apps/desktop/src/main/app-config.ts @@ -416,6 +416,7 @@ const MAP_TABLE_FIELDS: Partial> = { table: 'keymaps', comment: [ 'Keymap overrides — only list the bindings you want to change.', + 'Set a binding to "" to remove the key entirely.', 'Find the full list of action IDs in Settings → Keymaps.' ], example: '"global.searchNotes" = "Mod+P"' @@ -606,11 +607,14 @@ function keymapSectionLines(rawOverrides: unknown): string[] { '# Keymap overrides. Add or uncomment "" = "" lines.', '# Binding syntax: "Mod+P" = Cmd/Ctrl+P, "Shift+Mod+K", "Ctrl+W", "Space",', '# or a two-key sequence like "g g". Uncomment a reference line to remap it.', + '# An empty binding ("") removes the key entirely: nothing triggers that', + '# action until you give it a key again or delete the line.', '[keymaps]' ] for (const [key, value] of Object.entries(overrides)) { - lines.push(`${tomlKey(key)} = ${tomlValue(value)}`) + const line = `${tomlKey(key)} = ${tomlValue(value)}` + lines.push(value === '' ? `${line} # unbound` : line) } lines.push('', '# --- All actions (defaults shown; uncomment + edit to override) ---') diff --git a/packages/app-core/src/components/Editor.tsx b/packages/app-core/src/components/Editor.tsx index 3ebf60e4..b863dc9c 100644 --- a/packages/app-core/src/components/Editor.tsx +++ b/packages/app-core/src/components/Editor.tsx @@ -63,10 +63,15 @@ import { focusPaneInDirection, focusPaneOrEdgePanel } from "../lib/pane-nav"; import { requestPaneMode } from "../lib/pane-mode"; import { getKeymapBinding, + getKeymapDefinitions, + getKeymapDisplay, getSequenceTokens, + UNBOUND_BINDING, type KeymapId, type KeymapOverrides, } from "../lib/keymaps"; +import { requestSettingsTarget } from "../lib/settings-navigation"; +import { useToastStore } from "../lib/toast"; import { navigateActiveBuffer, selectActiveBuffer, @@ -559,6 +564,39 @@ function registerVimCommands(): void { else state.setHarperEnabled(!state.harperEnabled); }, ); + // `:unbind ` removes an action's key entirely, the ex twin of + // the Unbind button under Settings, Keymaps. Without an argument, or with + // an id the catalog does not know, it opens that page, where every id is + // listed, instead of guessing. + Vim.defineEx( + "unbind", + "unbind", + (_cm: unknown, params: { argString?: string } | undefined) => { + const arg = (params?.argString ?? "").trim(); + const state = useStore.getState(); + const definition = getKeymapDefinitions().find((d) => d.id === arg); + if (!definition) { + if (arg) { + useToastStore + .getState() + .addToast(`No keymap action is called "${arg}"`, "info"); + } + requestSettingsTarget("keymaps"); + state.setSettingsOpen(true); + return; + } + const before = getKeymapDisplay(state.keymapOverrides, definition.id); + state.setKeymapBinding(definition.id, UNBOUND_BINDING); + useToastStore + .getState() + .addToast( + before + ? `Unbound ${definition.title} (was ${before})` + : `${definition.title} is already unbound`, + "success", + ); + }, + ); Vim.defineEx("quit", "q", () => { const state = useStore.getState(); if (isTasksViewActive(state)) { @@ -1174,6 +1212,7 @@ const MANUAL_EX_NAMES = new Set([ "q", "wq", "format", + "unbind", "tasks", "tag", "template", diff --git a/packages/app-core/src/components/EditorPane.tsx b/packages/app-core/src/components/EditorPane.tsx index 5b3d6425..f9b137ad 100644 --- a/packages/app-core/src/components/EditorPane.tsx +++ b/packages/app-core/src/components/EditorPane.tsx @@ -65,7 +65,7 @@ import { toggleCheckbox } from '../lib/cm-toggle-checkbox' import { completionKeymapExtension, completionNavKeymap } from '../lib/cm-completion-nav' import { vimAwareDefaultKeymap, vimAwareMarkdownKeymap, vimAwareSearchKeymap } from '../lib/cm-vim-default-keymap' import { isVimAwaitingArgument } from '../lib/vim-nav' -import { toCodeMirrorKey, vimHalfPageKeymap } from '../lib/vim-half-page-keymap' +import { keyBindingsFor, vimHalfPageKeymap } from '../lib/vim-half-page-keymap' import { scrollOff } from '../lib/cm-scrolloff' import { followLinkTarget } from '../lib/follow-link' import { pointerOverRange } from '../lib/cm-pointer-range' @@ -289,6 +289,7 @@ import { formatKeyToken, getKeymapBinding, getKeymapDisplay, + labelWithShortcut, type KeymapId, type KeymapOverrides } from '../lib/keymaps' @@ -345,44 +346,20 @@ function buildEditorKeymap(vimMode: boolean, overrides: KeymapOverrides): Extens // Move the current line (or selection) up/down — reorders the markdown so // it persists in the file. Listed before defaultKeymap so the configured // binding wins; works in Vim normal/insert and non-Vim alike. - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.moveLineUp')), - run: moveLineUp - }, - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.moveLineDown')), - run: moveLineDown - }, + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.moveLineUp'), moveLineUp), + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.moveLineDown'), moveLineDown), // Obsidian-style checkbox toggle: line -> `- [ ]` -> `[x]` and back. // Mode-agnostic like the line moves. - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.toggleCheckbox')), - run: toggleCheckbox - }, + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.toggleCheckbox'), toggleCheckbox), // Join a hard-wrapped paragraph back into one line so the pane wraps it // (#676). Mode-agnostic like the line moves; Vim mode also has `gq`. - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.reflowParagraph')), - run: reflowParagraph - }, + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.reflowParagraph'), reflowParagraph), // Step across inline markers, so a formatted word can be finished without // reaching for the arrow keys. Mode-agnostic like the line moves. (#490) - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.hopMarkerForward')), - run: markerHop.forward - }, - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.hopMarkerBackward')), - run: markerHop.backward - }, - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.foldHeading')), - run: foldHeadingAtCursor - }, - { - key: toCodeMirrorKey(getKeymapBinding(overrides, 'editor.unfoldHeading')), - run: unfoldHeadingAtCursor - }, + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.hopMarkerForward'), markerHop.forward), + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.hopMarkerBackward'), markerHop.backward), + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.foldHeading'), foldHeadingAtCursor), + ...keyBindingsFor(getKeymapBinding(overrides, 'editor.unfoldHeading'), unfoldHeadingAtCursor), // Inline-format shortcuts (bold/italic/code/strike/highlight/math/link). In // Vim mode VimNav owns these (its window handler also resolves the Ctrl+I // jumplist collision on Linux); in non-Vim mode that handler is disabled, so @@ -3725,10 +3702,10 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { )} void jumpToPreviousNote()} disabled={!canGoBack} tooltipAlign="left" @@ -3736,10 +3713,10 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { void jumpToNextNote()} disabled={!canGoForward} tooltipAlign="left" @@ -4365,13 +4342,16 @@ function ToggleGroup({ return (
{MODE_OPTIONS.map((option) => { - const shortcut = getKeymapDisplay(keymapOverrides, option.keymapId) + const label = labelWithShortcut( + option.tooltipLabel, + getKeymapDisplay(keymapOverrides, option.keymapId) + ) return (
diff --git a/packages/app-core/src/components/SettingsModal.test.ts b/packages/app-core/src/components/SettingsModal.test.ts index 116b0844..82adc73a 100644 --- a/packages/app-core/src/components/SettingsModal.test.ts +++ b/packages/app-core/src/components/SettingsModal.test.ts @@ -28,7 +28,7 @@ const mocks = vi.hoisted(() => { hiddenWorkflowPresets: [], hideBuiltinTemplates: false, interfaceFont: null, - keymapOverrides: {}, + keymapOverrides: {} as Record, lineNumberMode: "off", monoFont: null, previewMaxWidth: 760, @@ -60,6 +60,7 @@ const mocks = vi.hoisted(() => { vimMode: false, vimWrappedLineMotions: "logical", setVimWrappedLineMotions: vi.fn(), + setKeymapBinding: vi.fn(), whichKeyHintMode: "timed", whichKeyHintTimeoutMs: 1200, whichKeyHints: true, @@ -132,6 +133,7 @@ describe("SettingsModal date note directories", () => { vi.clearAllMocks(); mocks.state.vimMode = false; mocks.state.vimWrappedLineMotions = "logical"; + mocks.state.keymapOverrides = {}; ( globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } ).IS_REACT_ACT_ENVIRONMENT = true; @@ -301,4 +303,57 @@ describe("SettingsModal date note directories", () => { expect(host.textContent).toContain("Keep your vault available everywhere"); expect(host.textContent).toContain("Connect ZenNotes Cloud"); }); + async function openKeymapRow(title: string): Promise { + await act(async () => { + root.render(createElement(SettingsModal)); + }); + const keymapButton = [ + ...host.querySelectorAll("button"), + ].find((button) => button.textContent?.trim() === "Keymap"); + expect(keymapButton).toBeTruthy(); + await act(async () => keymapButton!.click()); + const filter = host.querySelector( + 'input[placeholder="Filter keymaps…"]', + ); + expect(filter).toBeTruthy(); + await act(async () => changeInput(filter!, title)); + const label = [...host.querySelectorAll("span")].find( + (span) => span.textContent === title, + ); + expect(label).toBeTruthy(); + const row = label!.closest(".justify-between"); + expect(row).toBeTruthy(); + return row!; + } + + function rowButton(row: HTMLElement, text: string): HTMLButtonElement { + const button = [...row.querySelectorAll("button")].find( + (candidate) => candidate.textContent?.trim() === text, + ); + expect(button, `${text} button`).toBeTruthy(); + return button!; + } + + it("unbinds a keymap from its row with an empty-string override", async () => { + const row = await openKeymapRow("Zoom in"); + const unbind = rowButton(row, "Unbind"); + expect(unbind.disabled).toBe(false); + await act(async () => unbind.click()); + expect(mocks.state.setKeymapBinding).toHaveBeenCalledWith( + "global.zoomIn", + "", + ); + }); + + it("shows an unbound keymap as Unbound and only offers Reset or Change", async () => { + mocks.state.keymapOverrides = { "global.zoomIn": "" }; + const row = await openKeymapRow("Zoom in"); + expect(row.textContent).toContain("Unbound"); + expect(rowButton(row, "Unbind").disabled).toBe(true); + expect(rowButton(row, "Reset").disabled).toBe(false); + + await act(async () => rowButton(row, "Change…").click()); + const recorder = document.body.textContent ?? ""; + expect(recorder).toContain("Current: Unbound"); + }); }); diff --git a/packages/app-core/src/components/SettingsModal.tsx b/packages/app-core/src/components/SettingsModal.tsx index 2ce2ae24..925d5f32 100644 --- a/packages/app-core/src/components/SettingsModal.tsx +++ b/packages/app-core/src/components/SettingsModal.tsx @@ -65,8 +65,11 @@ import { getKeymapDefinitionsByGroup, getKeymapDisplay, isMacPlatform, + isUnboundBinding, sequenceTokenFromEvent, shortcutBindingFromEvent, + UNBOUND_BINDING, + UNBOUND_LABEL, } from "../lib/keymaps"; import { resolveAuto, @@ -5396,10 +5399,12 @@ function KeymapSettings({ // before turning Vim mode back on, but still let the filter work. } if (!q) return true; + const display = + getKeymapDisplay(overrides, definition.id) || UNBOUND_LABEL; return ( definition.title.toLowerCase().includes(q) || definition.description.toLowerCase().includes(q) || - getKeymapDisplay(overrides, definition.id).toLowerCase().includes(q) + display.toLowerCase().includes(q) ); }); return items.length > 0 ? { ...group, items } : null; @@ -5425,8 +5430,9 @@ function KeymapSettings({
Record a new key or sequence for the app’s keyboard-first - actions. Standard accessibility fallbacks like arrows, Enter, - and Escape still work. + actions, or unbind one so no key triggers it. Standard + accessibility fallbacks like arrows, Enter, and Escape still + work.
@@ -5462,7 +5468,8 @@ function KeymapSettings({
{group.items.map((definition) => { const current = getKeymapBinding(overrides, definition.id); - const custom = !!overrides[definition.id]; + const custom = overrides[definition.id] !== undefined; + const unbound = isUnboundBinding(current); const conflict = findKeymapConflict( overrides, definition.id, @@ -5505,8 +5512,17 @@ function KeymapSettings({
- - {formatKeymapBinding(current, definition.kind)} + + {unbound + ? UNBOUND_LABEL + : formatKeymapBinding(current, definition.kind)} +
@@ -5570,12 +5606,14 @@ function KeymapRecorderModal({ currentBinding, onClose, onSave, + onUnbind, }: { definition: KeymapDefinition; overrides: KeymapOverrides; currentBinding: string; onClose: () => void; onSave: (binding: string) => void; + onUnbind: () => void; }): JSX.Element { const [binding, setBinding] = useState(currentBinding); const mac = isMacPlatform(); @@ -5660,8 +5698,8 @@ function KeymapRecorderModal({
{definition.kind === "shortcut" - ? `Press the shortcut you want. ${mac ? "Command" : "Ctrl"}-style chords are saved in the app’s cross-platform format.` - : `Press the sequence you want. Backspace removes the last token, and multi-step sequences stop at ${definition.maxTokens ?? 2} key${(definition.maxTokens ?? 2) === 1 ? "" : "s"}.`} + ? `Press the shortcut you want; Backspace clears it. ${mac ? "Command" : "Ctrl"}-style chords are saved in the app’s cross-platform format. Unbind leaves the action with no key at all.` + : `Press the sequence you want. Backspace removes the last token, and multi-step sequences stop at ${definition.maxTokens ?? 2} key${(definition.maxTokens ?? 2) === 1 ? "" : "s"}. Unbind leaves the action with no key at all.`}
{conflict && ( @@ -5676,7 +5714,10 @@ function KeymapRecorderModal({ )}
- Current: {formatKeymapBinding(currentBinding, definition.kind)} + Current:{" "} + {isUnboundBinding(currentBinding) + ? UNBOUND_LABEL + : formatKeymapBinding(currentBinding, definition.kind)}
Default:{" "} @@ -5689,10 +5730,17 @@ function KeymapRecorderModal({
) : ( - Custom templates require a local vault. Built-in templates still - work here. + Custom templates need ZenNotes server 2.46 or later. Update the + server and{" "} + {workspaceMode === "remote" + ? "reconnect this workspace" + : "reload"} + ; built-in templates still work here. )}
diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index b441a2aa..fc7b70fd 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -1104,7 +1104,7 @@ export const HELP_SETTINGS: HelpSettingsSection[] = [ { label: 'Create a custom template', detail: 'Author a new template as markdown with optional frontmatter (`name`, `description`, `category`, `titleTemplate`, `targetFolder`, `targetSubpath`) and variables like `{{title}}`, `{{date}}`, `{{date:FORMAT}}`, `{{time}}`, `{{week}}`, and `{{cursor}}`. It is saved as a `.md` file in `.zennotes/templates/`.' }, { label: 'Edit or reset built-ins', detail: 'Press Edit on a built-in to fork an editable copy that shadows the original everywhere; Reset removes the copy and restores the built-in. Custom templates can be edited or deleted directly.' }, { label: 'Remove or restore built-ins', detail: 'Hide all the shipped templates with “Remove Built-in Templates” (a button here, or the command palette; it asks first), and bring them back with “Restore Built-in Templates”. Your custom templates, and anything already pointing at a built-in by id, keep working.' }, - { label: 'Where templates appear', detail: 'Use a template via the picker (`Space t` / `:template` / “New Note from Template…”), from a folder’s right-click “New from template”, or as the assigned daily/weekly note template. Custom templates require a local vault; built-ins work everywhere.' } + { label: 'Where templates appear', detail: 'Use a template via the picker (`Space t` / `:template` / “New Note from Template…”), from a folder’s right-click “New from template”, or as the assigned daily/weekly note template. Custom templates work on a local vault, in the self-hosted web client, and on a remote vault served by ZenNotes server 2.46 or later; they are `.md` files in the vault’s `.zennotes/templates/`, so one saved in any client shows up in the others. Built-ins work everywhere.' } ] }, { diff --git a/packages/app-core/src/store.test.ts b/packages/app-core/src/store.test.ts index 6235cd86..d55daeb0 100644 --- a/packages/app-core/src/store.test.ts +++ b/packages/app-core/src/store.test.ts @@ -2253,3 +2253,82 @@ describe('renaming the open note while the watcher reports the move (#713)', () expect(JSON.stringify(useStore.getState().paneLayout)).not.toContain(OLD) }) }) + +describe('custom templates on the change feed (#723)', () => { + it('re-lists templates on a templates-scope event without touching the note tree', async () => { + const listTemplates = vi + .fn() + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([ + { sourcePath: '.zennotes/templates/adr.md', raw: '---\nname: Decision Record\n---\n# {{title}}\n' } + ]) + const listNotes = vi.fn().mockResolvedValue([makeNote('- [ ] old task')]) + installZen({ listTemplates, listNotes }) + + const { useStore } = await loadStore() + await useStore.getState().loadCustomTemplates() + expect(useStore.getState().customTemplates).toEqual([]) + const notesListedBefore = listNotes.mock.calls.length + + await useStore.getState().applyChange({ + kind: 'change', + path: '.zennotes/templates/adr.md', + folder: 'inbox', + scope: 'templates' + }) + + expect(listTemplates).toHaveBeenCalledTimes(2) + expect(useStore.getState().customTemplates.map((t) => [t.id, t.name])).toEqual([ + ['custom:adr', 'Decision Record'] + ]) + expect(listNotes.mock.calls.length).toBe(notesListedBefore) + }) + + it('re-lists templates after a change-feed gap', async () => { + const listTemplates = vi.fn().mockResolvedValue([ + { sourcePath: '.zennotes/templates/weekly.md', raw: '---\nname: Weekly\n---\n' } + ]) + installZen({ listTemplates }) + + const { useStore } = await loadStore() + await useStore.getState().applyChange({ kind: 'change', path: '', folder: 'inbox', scope: 'resync' }) + + expect(listTemplates).toHaveBeenCalled() + expect(useStore.getState().customTemplates.map((t) => t.name)).toEqual(['Weekly']) + }) +}) + +describe('remote workspace capabilities after boot (#723)', () => { + it('re-reads the workspace info once getCurrentVault has connected the server', async () => { + const base = { + mode: 'remote', + baseUrl: 'http://127.0.0.1:7878', + authConfigured: false, + profileId: null, + bootError: null + } + // The first read happens before the main process connects; capabilities + // are unknown then. Only the second read, after the connection, has them. + const getRemoteWorkspaceInfo = vi + .fn() + .mockResolvedValueOnce({ ...base, capabilities: null }) + .mockResolvedValue({ ...base, capabilities: { supportsCustomTemplates: true, supportsWatch: true } }) + installZen({ + onVaultChange: vi.fn(() => vi.fn()), + getAppInfo: vi.fn().mockReturnValue({ runtime: 'desktop' }), + getServerCapabilities: vi.fn().mockResolvedValue({}), + getCurrentVault: vi.fn().mockResolvedValue({ root: '/srv/vault', name: 'vault' }), + getRemoteWorkspaceInfo + }) + + const { useStore } = await loadStore() + await useStore.getState().init() + + expect(getRemoteWorkspaceInfo).toHaveBeenCalledTimes(2) + expect(useStore.getState().workspaceMode).toBe('remote') + expect(useStore.getState().remoteWorkspaceInfo?.capabilities).toEqual({ + supportsCustomTemplates: true, + supportsWatch: true + }) + }) +}) diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index 346f3765..a8140695 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -6320,7 +6320,10 @@ export const useStore = create((set, get) => { .catch((err) => { console.error('resync vault settings failed', err) }), - tasksSurfaceVisible(get()) ? get().refreshTasks() : Promise.resolve() + tasksSurfaceVisible(get()) ? get().refreshTasks() : Promise.resolve(), + // Templates ride the feed too (scope 'templates'), so a gap may have + // swallowed a template saved on another device. + get().loadCustomTemplates() ]) const stateAfter = get() const openTabs = [...new Set(allLeaves(stateAfter.paneLayout).flatMap((leaf) => leaf.tabs))] @@ -6385,6 +6388,13 @@ export const useStore = create((set, get) => { await get().loadNoteComments(ev.path) return } + if (ev.scope === 'templates') { + // A custom template changed on disk: another client on this vault, a + // synced dotfile, or this app's own save. Re-list the templates, not + // the note tree; a template is not a note. + await get().loadCustomTemplates() + return + } if (ev.scope === 'database') { // On delete, forget the database instead of re-reading a file that's gone // (which throws "Database not found"); otherwise sync from disk. @@ -9128,7 +9138,7 @@ export const useStore = create((set, get) => { try { const remoteWorkspaceProfilesPromise = get().refreshRemoteWorkspaceProfiles() const localVaultsPromise = get().refreshLocalVaults() - const [remoteWorkspaceInfo, serverCapabilities] = await Promise.all([ + const [bootWorkspaceInfo, serverCapabilities] = await Promise.all([ get().refreshWorkspaceContext(), window.zen.getServerCapabilities().catch(() => null) ]) @@ -9136,8 +9146,8 @@ export const useStore = create((set, get) => { void remoteWorkspaceProfilesPromise void localVaultsPromise set({ - workspaceMode: workspaceModeFrom(remoteWorkspaceInfo), - remoteWorkspaceInfo, + workspaceMode: workspaceModeFrom(bootWorkspaceInfo), + remoteWorkspaceInfo: bootWorkspaceInfo, workspaceSetupError: null, workspaceRestored: true, vaultSettings: DEFAULT_VAULT_SETTINGS @@ -9148,6 +9158,14 @@ export const useStore = create((set, get) => { return } const vault = await window.zen.getCurrentVault() + // getCurrentVault is what connects a configured remote workspace, so + // the info fetched above predates the connection: its capabilities and + // bootError are still null, and keeping it would leave Settings + // believing the server advertises nothing (#723). Ask again now that + // the answer exists. + const remoteWorkspaceInfo = bootWorkspaceInfo + ? await get().refreshWorkspaceContext() + : bootWorkspaceInfo void remoteWorkspaceProfilesPromise void localVaultsPromise if (vault) { diff --git a/packages/bridge-contract/src/ipc.ts b/packages/bridge-contract/src/ipc.ts index 56bfe3e6..72401dfa 100644 --- a/packages/bridge-contract/src/ipc.ts +++ b/packages/bridge-contract/src/ipc.ts @@ -776,6 +776,11 @@ export interface ServerCapabilities { /** Server-side workflow file CRUD plus journalled apply/undo. Absent before * 2.29, where the web client must keep Workflows read-only. */ supportsWorkflows?: boolean + /** Custom-template CRUD under `.zennotes/templates/` (the `/templates` + * routes), the same files the desktop keeps for a local vault. Absent + * before 2.46, which keeps Settings, Templates read-only on the web client + * and on a desktop connected to that server (#723). */ + supportsCustomTemplates?: boolean } export interface ServerSessionStatus { @@ -850,12 +855,16 @@ export type VaultChangeKind = 'add' | 'change' | 'unlink' * watch socket that reconnected): anything may have happened while the feed * was down, so the renderer re-pulls every surface the feed keeps fresh. * Servers never emit it. */ +/** `templates` is a custom template under `.zennotes/templates/` changing on + * disk; the path is that file's vault-relative path, and the client re-lists + * templates rather than notes. */ export type VaultChangeScope = | 'content' | 'vault-settings' | 'comments' | 'database' | 'folder' + | 'templates' | 'resync' export interface VaultChangeEvent { From 5e09fab7401aa326ebb55e9fb85134bd7aef37d5 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 09:21:50 -0500 Subject: [PATCH 03/12] Fix(cli): zn create and the MCP tools follow primaryNotesLocation: root (#745) With Settings, Vault set to "Vault root", zn create and the MCP create_note tool still wrote new notes into inbox/, and vault_info reported inbox, while zn list and zn search found root-level notes fine. @diazkev314 hit it on 2.45.0 and could not find where the CLI read primaryNotesLocation at all. It did read it, and then let the layout outrank it. The CLI and the MCP server share readPrimaryNotesLocation, which decided the mode from the vault's directory tree first (loose content at the root meant root, notes inside inbox/ meant inbox) and consulted vault.json only when the layout was ambiguous. That rule dates from the day root mode shipped, written so a vault switched in Settings but not yet migrated kept filing new notes next to its old ones. The app does the opposite: an explicit setting is the answer, and the layout is inferred only when vault.json leaves the question unstated. With old notes still in inbox/, the two halves disagreed on the same vault: Settings said root, the app created notes at the root, and the CLI kept writing into inbox/. The file wins now, on every side. An explicit primaryNotesLocation decides, and only a vault without one, or one a sandboxed process cannot read, is inferred from its layout, the same way the app infers it. The count of notes inside inbox/ that used to outrank the file is gone. Reads were already right and are unchanged. Three tests pin the order: the file says root while old notes sit in inbox/, the file says inbox while loose notes sit at the root, and no file at all. Verified through the running app over CDP: the vault switched to Vault root in Settings, then zn create with no --vault (it follows the vault the app has open) and the MCP vault_info and create_note tools over stdio, with both new notes appearing in the app at the root while the old ones stayed in inbox/. --- apps/desktop/src/mcp/vault-ops.test.ts | 47 +++++++++++++++++ apps/desktop/src/mcp/vault-ops.ts | 70 +++++++------------------- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/apps/desktop/src/mcp/vault-ops.test.ts b/apps/desktop/src/mcp/vault-ops.test.ts index 271d4f1f..15169c2b 100644 --- a/apps/desktop/src/mcp/vault-ops.test.ts +++ b/apps/desktop/src/mcp/vault-ops.test.ts @@ -7,6 +7,7 @@ import { createNote, insertAtLineInBody, listNotes, + readPrimaryNotesLocation, renameNote, replaceInBody, scanAllTasks, @@ -242,3 +243,49 @@ describe('pure body edits shared with the remote backend (#688)', () => { expect(insertAtLineInBody('one', -5, 'top')).toBe('top\none') }) }) + +// The app treats an explicit primaryNotesLocation as the answer and infers +// from the layout only when vault.json leaves it unstated. The CLI and MCP +// used to let the layout outrank the file, so a vault switched to root mode +// whose old notes still sat in inbox/ kept getting new notes filed there +// (#745). The seeded inbox/GitHub note is exactly that leftover. +describe('primary notes location follows vault.json (#745)', () => { + it('files a new note at the root when vault.json says root, old inbox notes or not', async () => { + await mkdir(path.join(root, '.zennotes'), { recursive: true }) + await writeFile( + path.join(root, '.zennotes', 'vault.json'), + JSON.stringify({ primaryNotesLocation: 'root' }) + ) + expect(await readPrimaryNotesLocation(root)).toBe('root') + + const meta = await createNote(root, 'inbox', 'Test', '', 'test') + expect(meta.path).toBe('Test.md') + expect(meta.folder).toBe('inbox') + expect(await readFile(path.join(root, 'Test.md'), 'utf8')).toBe('test') + }) + + it('keeps filing into inbox/ when vault.json says inbox, whatever sits at the root', async () => { + await mkdir(path.join(root, '.zennotes'), { recursive: true }) + await writeFile( + path.join(root, '.zennotes', 'vault.json'), + JSON.stringify({ primaryNotesLocation: 'inbox' }) + ) + await writeFile(path.join(root, 'Loose.md'), '# Loose\n') + expect(await readPrimaryNotesLocation(root)).toBe('inbox') + + const meta = await createNote(root, 'inbox', 'Test') + expect(meta.path).toBe('inbox/Test.md') + }) + + it('infers from the layout only when vault.json leaves the question open', async () => { + // No vault.json and notes only in inbox/: a classic ZenNotes vault. + expect(await readPrimaryNotesLocation(root)).toBe('inbox') + // A loose root note flips the inference to a flat vault. + await writeFile(path.join(root, 'Loose.md'), '# Loose\n') + expect(await readPrimaryNotesLocation(root)).toBe('root') + // A vault.json that is silent about it changes nothing. + await mkdir(path.join(root, '.zennotes'), { recursive: true }) + await writeFile(path.join(root, '.zennotes', 'vault.json'), JSON.stringify({ systemFolderPaths: {} })) + expect(await readPrimaryNotesLocation(root)).toBe('root') + }) +}) diff --git a/apps/desktop/src/mcp/vault-ops.ts b/apps/desktop/src/mcp/vault-ops.ts index 2942d5e2..6da2802c 100644 --- a/apps/desktop/src/mcp/vault-ops.ts +++ b/apps/desktop/src/mcp/vault-ops.ts @@ -188,61 +188,29 @@ async function countLooseRootContent(root: string, paths: SystemFolderPathsMap): return count } -/** Recursively count .md files under a given directory. Used to see - * whether `/inbox/` actually has content. */ -async function countMdFilesRecursively(dir: string): Promise { - let entries: import('node:fs').Dirent[] - try { - entries = await fs.readdir(dir, { withFileTypes: true }) - } catch { - return 0 - } - let count = 0 - for (const entry of entries) { - if (entry.name.startsWith('.')) continue - const full = path.join(dir, entry.name) - if (entry.isDirectory()) count += await countMdFilesRecursively(full) - else if (entry.isFile() && entry.name.toLowerCase().endsWith('.md')) count += 1 - } - return count -} - -/** Decide whether this vault uses inbox-mode or root-mode for its - * primary notes area. The vault's on-disk layout is the strongest - * signal — the explicit `vault.json` setting is consulted only when - * the layout is genuinely ambiguous (a fresh, empty vault). +/** Decide whether this vault keeps its primary notes in `inbox/` or at the + * vault root. An explicit `primaryNotesLocation` in vault.json is the + * answer, exactly as it is for the app (`getVaultSettings` in + * main/vault.ts): the layout is consulted only when the file leaves the + * question open, because it is missing, unreadable (a TCC-restricted child + * process), or silent about it. * - * This deliberately ignores `vault.json` when it disagrees with the - * layout so that: - * - * - A user who switched modes in Settings but whose vault hasn't - * been migrated yet still gets notes filed where their existing - * notes live. - * - A user whose `vault.json` was never created (or was deleted / - * restored from a sync) still gets correct behavior. - * - Sandboxed / TCC-restricted child processes that can't read - * `vault.json` still pick the right answer from `readdir` calls - * that succeeded. + * This used to be the other way round, with the layout outranking the file + * so that a vault switched to root mode but not yet migrated kept filing new + * notes next to its old ones in inbox/. That put the CLI and MCP at odds + * with the app on the very same vault: Settings said root, the app created + * notes at the root, and `zn create` and `create_note` kept writing into + * inbox/ while `vault_info` reported inbox (#745). The file wins now, on + * every side of the bridge. */ export async function readPrimaryNotesLocation(root: string): Promise { + const explicit = await readExplicitPrimaryNotesLocation(root) + if (explicit) return explicit + // Loose .md files or user folders at the root mean a flat, Obsidian-style + // vault; anything else defaults to inbox, as a fresh ZenNotes vault does. + // Mirrors inferPrimaryNotesLocation in main/vault.ts. const paths = await readSystemFolderPaths(root) - const [rootContent, inboxNotes, explicit] = await Promise.all([ - countLooseRootContent(root, paths), - countMdFilesRecursively(path.join(root, resolvedFolderDirName('inbox', paths))), - readExplicitPrimaryNotesLocation(root) - ]) - - // Strong layout signal — root has user-organized content (loose - // .md files, custom subfolders). The vault is laid out flat. - if (rootContent >= 1) return 'root' - - // Strong layout signal — only inbox/ has notes, root is empty or - // just system folders. Classic ZenNotes lifecycle layout. - if (inboxNotes >= 1) return 'inbox' - - // Ambiguous (empty vault). Trust the explicit setting if present, - // otherwise default to inbox (matches a fresh ZenNotes install). - return explicit ?? 'inbox' + return (await countLooseRootContent(root, paths)) >= 1 ? 'root' : 'inbox' } /** The absolute directory that holds notes for a given top-level From ad3d781ab61c939dcd97a310005325e7ccb9d09e Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 10:44:25 -0500 Subject: [PATCH 04/12] Fix(math): display blocks inside callouts render in both views (#748) A $$ block inside an Obsidian-style callout rendered nowhere. In the editor it stayed raw, and in the reading view the callout lost its body while, with Typst selected, one failing formula swallowed the rest of the note. @OstrichDowneyJr reported it with Typst; KaTeX has the same two bugs, it just fails more quietly. The editor's live preview only renders a block whose fences own their lines, and it counted the "> " before a callout's fence as prose. It now accepts blockquote markers there, strips them from the formula (they are the quote's, not the math's), and gives the rendered block the callout card's own classes, resolved from the outermost Blockquote node, so the card cm-wysiwyg-blocks draws line by line stays whole around the widget instead of breaking in two. The reading view's fault was in normalizeBlockMathFences, the pre-pass that rewrites editor-legal fence shapes into the form remark-math parses. After copying a canonical block that came earlier in the note it advanced one line and re-scanned that block's closing fence as an opener, then paired it with the callout's "> $$", which it took for content hugging a fence, and wrote a bare $$ outside the quote. The pass now splits every line into its quote prefix and content, matches fences on the content, writes the prefix back on whatever it emits (an empty ">" line where a blank would end the quote), closes a block only at its own quote depth, and moves past a canonical block instead of re-reading its closing fence. The currency guard's raw source slice also drops the markers of continuation lines, so a span demoted to text inside a callout no longer comes back as "$ > x $". Bare "$" on its own line is still not a display block in ZenNotes, with either engine; that is "$$". Reproduced on the pre-fix build in both views and verified on the fixed one in the built app with Typst selected: the callout's block renders inside the card in the editor, the reading view renders both blocks with the right source, and inline math inside callouts is untouched. Tests cover the editor scan and the normalizer, including the fence that merely ends a prose line and the one-line "> $$x^2$$" form. --- .../app-core/src/lib/cm-math-render.test.ts | 26 ++++++ packages/app-core/src/lib/cm-math-render.ts | 85 ++++++++++++++++--- packages/app-core/src/lib/markdown.test.ts | 52 ++++++++++++ packages/app-core/src/lib/markdown.ts | 80 +++++++++++++---- 4 files changed, 217 insertions(+), 26 deletions(-) diff --git a/packages/app-core/src/lib/cm-math-render.test.ts b/packages/app-core/src/lib/cm-math-render.test.ts index bdec3748..04bc6dfe 100644 --- a/packages/app-core/src/lib/cm-math-render.test.ts +++ b/packages/app-core/src/lib/cm-math-render.test.ts @@ -38,6 +38,32 @@ describe('mathRenderExtension', () => { view.destroy() }) + it('renders a $$ block inside a callout, without the quote markers, as part of the card (#748)', () => { + const view = mount('start\n\n> [!note]\n> $$\n> a+b\n> $$\n\nend') + const blocks = view.dom.querySelectorAll('.cm-math-block') + expect(blocks.length).toBe(1) + expect(blocks[0].textContent).not.toContain('>') + expect(blocks[0].textContent).toContain('a') + expect(blocks[0].classList.contains('cm-callout')).toBe(true) + expect(blocks[0].classList.contains('cm-callout-note')).toBe(true) + view.destroy() + }) + + it('renders a $$ block inside a plain blockquote with the quote bar (#748)', () => { + const view = mount('start\n\n> $$\n> a+b\n> $$\n\nend') + const block = view.dom.querySelector('.cm-math-block') + expect(block).not.toBeNull() + expect(block?.classList.contains('cm-wq-quote')).toBe(true) + expect(block?.classList.contains('cm-callout')).toBe(false) + view.destroy() + }) + + it('still leaves a $$ with prose before it literal', () => { + const view = mount('start\n\nsee $$\na+b\n$$\n\nend') + expect(view.dom.querySelectorAll('.cm-math-block').length).toBe(0) + view.destroy() + }) + it('numbers equation environments in document order', () => { const view = mount( [ diff --git a/packages/app-core/src/lib/cm-math-render.ts b/packages/app-core/src/lib/cm-math-render.ts index 27cabc91..0a3d21e6 100644 --- a/packages/app-core/src/lib/cm-math-render.ts +++ b/packages/app-core/src/lib/cm-math-render.ts @@ -21,6 +21,7 @@ import { Decoration, type DecorationSet, EditorView, WidgetType } from '@codemir import katex from 'katex' import type { MathRenderer } from '@shared/app-config' import { peekTypstMathSvg, renderTypstMathToSvg } from './typst-math-render' +import { calloutGroupFor } from './callout-types' import { numberLatexEquationEnvironments } from './latex-equation-numbering' /** Which typesetter the live-preview widgets use. Supplied by @@ -50,6 +51,57 @@ const typstPreambleFacet = Facet.define({ const INLINE_MATH_RE = /(? `, `> > `, ` >`. */ +const QUOTE_MARKERS_RE = /^(?:[ \t]{0,3}>[ \t]?)+/ +const CALLOUT_HEADER_RE = /^(?:[ \t]{0,3}>[ \t]?)+\[!(\w+)\]/ + +/** How many blockquote markers `text` is made of: 0 for blank, null when it + * holds anything else. What precedes a fence on its line must be one of + * those two for the fence to own its line. */ +function quoteDepthOf(text: string): number | null { + if (text.trim() === '') return 0 + const markers = text.match(QUOTE_MARKERS_RE) + if (!markers || text.slice(markers[0].length).trim() !== '') return null + return (markers[0].match(/>/g) ?? []).length +} + +/** Drop up to `depth` blockquote markers from the start of every line. The + * markers belong to the quote, not to the formula. */ +function stripQuoteMarkers(text: string, depth: number): string { + if (depth === 0) return text + return text + .split('\n') + .map((line) => { + let rest = line + for (let d = 0; d < depth; d++) { + const marker = rest.match(/^[ \t]{0,3}>[ \t]?/) + if (!marker) break + rest = rest.slice(marker[0].length) + } + return rest + }) + .join('\n') +} + +/** The classes that make a block widget part of the quote it sits in: the + * callout card's body classes, or the plain quote's bar. cm-wysiwyg-blocks + * draws the card line by line, and a widget replacing some of those lines + * has to wear the same classes or the card breaks in two around the formula. */ +function quoteFrameClasses(state: EditorState, pos: number): string { + // The outermost quote is the one drawing the card, so keep climbing. + let node = syntaxTree(state).resolveInner(pos, 1) + let quoteFrom = -1 + for (;;) { + if (node.name === 'Blockquote') quoteFrom = node.from + const parent = node.parent + if (!parent) break + node = parent + } + if (quoteFrom < 0) return '' + const header = state.doc.lineAt(quoteFrom).text.match(CALLOUT_HEADER_RE) + if (header) return `cm-callout cm-callout-${calloutGroupFor(header[1])}` + return 'cm-wq-quote' +} function renderKatex(el: HTMLElement, latex: string, display: boolean): void { try { @@ -138,7 +190,9 @@ class BlockMathWidget extends WidgetType { constructor( readonly latex: string, readonly renderer: MathRenderer, - readonly preamble = '' + readonly preamble = '', + /** Quote or callout classes when the block sits inside one (#748). */ + readonly frame = '' ) { super() } @@ -146,12 +200,13 @@ class BlockMathWidget extends WidgetType { return ( other.latex === this.latex && other.renderer === this.renderer && - other.preamble === this.preamble + other.preamble === this.preamble && + other.frame === this.frame ) } toDOM(): HTMLElement { const el = document.createElement('div') - el.className = 'cm-math-block' + el.className = this.frame ? `cm-math-block ${this.frame}` : 'cm-math-block' renderMath(el, this.latex, true, this.renderer, this.preamble) return el } @@ -214,16 +269,21 @@ function buildMathRender(state: EditorState): MathRenderValue { if (isInsideCode(state, rawFrom)) continue const openLine = doc.lineAt(rawFrom) const closeLine = doc.lineAt(rawTo) - // Only render when the fences own their lines (nothing but whitespace before - // the opening `$$` and after the closing `$$`), so the whole-line block - // replace can never swallow surrounding prose. + // Only render when the fences own their lines (nothing but whitespace, or + // a blockquote's markers, before the opening `$$` and nothing after the + // closing one), so the whole-line block replace can never swallow + // surrounding prose. A fence inside a callout is a fence too: its `> ` + // used to read as prose and the block stayed raw there (#748). const before = openLine.text.slice(0, rawFrom - openLine.from) const after = closeLine.text.slice(rawTo - closeLine.from) - if (before.trim() !== '' || after.trim() !== '') continue + const depth = quoteDepthOf(before) + if (depth === null || after.trim() !== '') continue + const source = stripQuoteMarkers(inner, depth) + if (!source.trim()) continue const numbered = renderer === 'katex' - ? numberLatexEquationEnvironments(inner, equationNumber) - : { latex: inner, nextNumber: equationNumber } + ? numberLatexEquationEnvironments(source, equationNumber) + : { latex: source, nextNumber: equationNumber } equationNumber = numbered.nextNumber // Reserve the whole-line span so inline scanning skips inside it, whether the // block ends up rendered or revealed. @@ -235,7 +295,12 @@ function buildMathRender(state: EditorState): MathRenderValue { to: closeLine.to, deco: Decoration.replace({ block: true, - widget: new BlockMathWidget(numbered.latex, renderer, preamble) + widget: new BlockMathWidget( + numbered.latex, + renderer, + preamble, + depth > 0 ? quoteFrameClasses(state, openLine.from) : '' + ) }) }) } diff --git a/packages/app-core/src/lib/markdown.test.ts b/packages/app-core/src/lib/markdown.test.ts index 32a36eff..4a4eb12a 100644 --- a/packages/app-core/src/lib/markdown.test.ts +++ b/packages/app-core/src/lib/markdown.test.ts @@ -535,3 +535,55 @@ describe('callout titles keep their inline markup (#549)', () => { expect(renderMarkdown('> [!note]x is not a marker')).not.toContain('callout') }) }) + +// A display block inside an Obsidian-style callout used to lose the reading +// view entirely: the fence normalizer re-scanned the closing fence of the block +// BEFORE the callout as an opener, took the callout's `> $$` for content +// hugging a fence, and rewrote the note with a bare `$$` outside the quote +// that swallowed everything after it (#748). +describe('display math inside callouts (#748)', () => { + afterEach(() => setMarkdownMathRenderer('katex')) + + it('renders a $$ block inside a callout, after a block outside it', () => { + const html = renderMarkdown( + ['$$', 'a', '$$', '', '> [!note]', '> $$', '> x_1 = 2', '> $$', '', 'After.'].join('\n') + ) + expect(html.match(/katex-display/g)?.length).toBe(2) + expect(html).toMatch(/
]*>[\s\S]*katex-display[\s\S]*<\/div>/) + expect(html).not.toContain('>') + expect(html).toMatch(/]*>After\.<\/p>/) + }) + + it('hands Typst the formula without the quote markers', () => { + setMarkdownMathRenderer('typst') + const html = renderMarkdown(['> [!tip]', '> $$', '> x_1 = frac(det W_1, det A)', '> $$'].join('\n')) + expect(html).toContain('data-typst-source="x_1 = frac(det W_1, det A)"') + expect(html).not.toContain('zen-typst-error') + }) + + it('expands a one-line $$x^2$$ inside a callout like it does outside', () => { + const html = renderMarkdown('> [!note]\n> $$x^2$$\n\nAfter.') + expect(html.match(/katex-display/g)?.length).toBe(1) + expect(html).toMatch(/
]*>[\s\S]*katex-display[\s\S]*<\/div>/) + expect(html).toMatch(/]*>After\.<\/p>/) + }) + + it('never pairs a block with a later line that merely ends in $$', () => { + const html = renderMarkdown(['$$', 'a', '$$', '', 'The fence is $$', '', 'After.'].join('\n')) + expect(html.match(/katex-display/g)?.length).toBe(1) + expect(html).toContain('The fence is $$') + expect(html).toMatch(/]*>After\.<\/p>/) + }) + + it('keeps the quote markers out of a demoted $…$ span inside a callout', () => { + // Bare `$` lines are not a display block in ZenNotes; the currency guard + // demotes the span to text in both views. Inside a callout that text used + // to carry the `> ` of every continuation line. + const outside = renderMarkdown(['$', 'x_1 = 2', '$'].join('\n')) + const inside = renderMarkdown(['> [!note]', '> $', '> x_1 = 2', '> $'].join('\n')) + expect(outside).not.toContain('katex') + expect(inside).not.toContain('katex') + expect(inside).not.toContain('>') + expect(inside).toMatch(/
]*>[\s\S]*\$\nx_1 = 2\n\$[\s\S]*<\/div>/) + }) +}) diff --git a/packages/app-core/src/lib/markdown.ts b/packages/app-core/src/lib/markdown.ts index 6df3f2a9..432dd9cf 100644 --- a/packages/app-core/src/lib/markdown.ts +++ b/packages/app-core/src/lib/markdown.ts @@ -898,7 +898,11 @@ function remarkCurrencyGuard() { const start = node.position?.start?.offset const end = node.position?.end?.offset if (start == null || end == null) return - const token = source.slice(start, end) + // The raw source between the node's offsets. Inside a blockquote the + // continuation lines still carry their `> ` markers here, which belong + // to the quote, not the formula: a span demoted inside a callout used + // to come back as `$ > x $` (#748). + const token = dropQuoteMarkersAfterFirstLine(source.slice(start, end)) if (STRICT_INLINE_MATH_RE.test(token)) return // `$$…$$` in a table cell: genuine display math, not currency. The // editor's table widget renders it in display mode, so swap the node's @@ -1105,6 +1109,28 @@ function escapeTableMathPipes(src: string): string { return changed ? out.join('\n') : src } +/** The blockquote markers that open a line (`> `, `> > `) and what follows + * them. A fence inside a callout carries the markers on every line, so the + * normalizer looks past them and puts them back on whatever it emits. */ +const QUOTE_PREFIX_RE = /^((?:[ \t]{0,3}>[ \t]?)+)/ + +function splitQuotePrefix(line: string): { prefix: string; depth: number; content: string } { + const match = line.match(QUOTE_PREFIX_RE) + if (!match) return { prefix: '', depth: 0, content: line } + const prefix = match[1] + return { prefix, depth: (prefix.match(/>/g) ?? []).length, content: line.slice(prefix.length) } +} + +/** A raw source slice that starts mid-line: its first line has no marker + * (that sits before the slice), the following lines do when the slice lives + * in a blockquote. A paragraph line cannot start with `>` anywhere else, so + * stripping markers from the continuation lines never touches prose. */ +function dropQuoteMarkersAfterFirstLine(text: string): string { + if (!text.includes('\n')) return text + const [first, ...rest] = text.split('\n') + return [first, ...rest.map((line) => splitQuotePrefix(line).content)].join('\n') +} + /** * remark-math only closes a `$$` block on a line containing nothing but the * closing fence, while the editor's live preview (cm-math-render) also accepts @@ -1114,6 +1140,11 @@ function escapeTableMathPipes(src: string): string { * parses exactly what the editor renders. Fenced code is left untouched, and * anything the editor itself rejects (mid-line `$$`, empty or unclosed blocks) * passes through unchanged — canonical notes come back byte-identical. + * + * Fences inside a blockquote count as fences (#748): the quote markers are + * looked past when a line is read and put back on every line written, and a + * block only closes at its own quote depth, so a fence in a callout can never + * be paired with one outside it. */ function normalizeBlockMathFences(src: string, loose = false): string { if (!src.includes('$$')) return src @@ -1124,7 +1155,8 @@ function normalizeBlockMathFences(src: string, loose = false): string { let i = 0 while (i < lines.length) { const raw = lines[i] - const trimmed = raw.trim() + const { prefix, depth, content } = splitQuotePrefix(raw) + const trimmed = content.trim() if (codeFence) { out.push(raw) if (trimmed.startsWith(codeFence)) codeFence = null @@ -1143,12 +1175,12 @@ function normalizeBlockMathFences(src: string, loose = false): string { let indent: string | null = null let rest = '' let proseBefore = '' - const strictOpen = raw.match(/^( {0,3})\$\$(?!\$)(.*)$/) + const strictOpen = content.match(/^( {0,3})\$\$(?!\$)(.*)$/) if (strictOpen) { indent = strictOpen[1] rest = strictOpen[2] } else if (loose) { - const looseOpen = raw.match(/^( {0,3})(.+?)\s*\$\$(?!\$)\s*$/) + const looseOpen = content.match(/^( {0,3})(.+?)\s*\$\$(?!\$)\s*$/) if (looseOpen && !looseOpen[2].includes('$$')) { indent = looseOpen[1] proseBefore = looseOpen[2] @@ -1166,7 +1198,7 @@ function normalizeBlockMathFences(src: string, loose = false): string { if (restTrimmed.endsWith('$$') && restTrimmed.indexOf('$$') === restTrimmed.length - 2) { const inner = restTrimmed.slice(0, -2) if (inner.trim() !== '') { - out.push(`${indent}$$`, inner, `${indent}$$`) + out.push(`${prefix}${indent}$$`, `${prefix}${inner}`, `${prefix}${indent}$$`) changed = true i++ continue @@ -1183,8 +1215,11 @@ function normalizeBlockMathFences(src: string, loose = false): string { let closeHasContent = false let closeTrailing = '' for (let k = i + 1; k < lines.length; k++) { - const t = lines[k].trim() + const line = splitQuotePrefix(lines[k]) + const t = line.content.trim() if (!t.includes('$$')) continue + // A fence at another quote depth belongs to another block, or to none. + if (line.depth !== depth) break if (t === '$$') { close = k } else if (t.endsWith('$$') && t.indexOf('$$') === t.length - 2) { @@ -1201,22 +1236,35 @@ function normalizeBlockMathFences(src: string, loose = false): string { } break } - const alreadyCanonical = - restTrimmed === '' && !closeHasContent && proseBefore === '' && closeTrailing === '' - if (close === -1 || alreadyCanonical) { - // Unclosed, editor-rejected, or already canonical: leave untouched. + if (close === -1) { + // Unclosed or editor-rejected: leave the line untouched. out.push(raw) i++ continue } + const alreadyCanonical = + restTrimmed === '' && !closeHasContent && proseBefore === '' && closeTrailing === '' + if (alreadyCanonical) { + // Already the fence-on-its-own-line form: copy the block through and + // move past its closing fence. Re-scanning that fence as an opener + // paired it with the next `$$` in the note, and a callout's `> $$` + // then read as content hugging a fence: the rewrite left a bare `$$` + // outside the quote, and that block swallowed everything after it. + for (let k = i; k <= close; k++) out.push(lines[k]) + i = close + 1 + continue + } + // An empty quote line (`>`) keeps a blockquote open where a blank line + // would end it; outside a quote the prefix is empty and this is a blank. + const blank = prefix.trimEnd() if (proseBefore !== '') { // Prose leading the open fence becomes its own paragraph. - out.push(`${indent}${proseBefore}`, '') + out.push(`${prefix}${indent}${proseBefore}`, blank) changed = true } - out.push(`${indent}$$`) + out.push(`${prefix}${indent}$$`) if (restTrimmed !== '') { - out.push(rest) + out.push(`${prefix}${rest}`) changed = true } for (let k = i + 1; k < close; k++) out.push(lines[k]) @@ -1225,13 +1273,13 @@ function normalizeBlockMathFences(src: string, loose = false): string { const rawClose = lines[close] const idx = rawClose.lastIndexOf('$$') const beforeDollar = rawClose.slice(0, idx) - if (beforeDollar.trim() !== '') out.push(beforeDollar) - out.push(`${indent}$$`, '', `${indent}${closeTrailing}`) + if (splitQuotePrefix(beforeDollar).content.trim() !== '') out.push(beforeDollar) + out.push(`${prefix}${indent}$$`, blank, `${prefix}${indent}${closeTrailing}`) changed = true } else if (closeHasContent) { const rawClose = lines[close] const idx = rawClose.lastIndexOf('$$') - out.push(rawClose.slice(0, idx), `${indent}$$`) + out.push(rawClose.slice(0, idx), `${prefix}${indent}$$`) changed = true } else { out.push(lines[close]) From ac480df85d2fc086147bb6434e6986af6fc06157 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 10:59:27 -0500 Subject: [PATCH 05/12] Fix(math): Typst formulas are the size KaTeX formulas are, in the text color (#746) With Typst selected, inline math sat visibly smaller than the text around it, where the KaTeX rendering of the same formula fit. @cyperion saw it on 2.45.0 and reported it as a scaling bug, which it is. KaTeX draws Computer Modern at 1.21 times the surrounding text size; its own stylesheet says `.katex { font-size: 1.21em }`, because the family sits small on its em square and a plain 1em reads undersized next to prose. The Typst SVG was sized at that plain 1em: its dimensions come back in points at the 11pt the formula is compiled at, and the wrapper divided by 11 to get em. New Computer Modern shares the metrics, so the same formula came out a fifth smaller, 52 px wide against KaTeX's 63 for `E = h nu` at the default text size. The wrapper now applies KaTeX's factor to both dimensions, in the editor and the reading view, display blocks included, so switching engines no longer changes how big the math is. The Math size setting still scales on top. Found on the way: the recolor that makes Typst glyphs follow the theme only knew fills. Typst exports glyphs as black fills and the rules a formula draws as shapes, a square root's bar and a fraction line, as black strokes, so those stayed black and vanished on a dark theme. Every black fill or stroke is recolored now, in the spellings Typst's export uses. styleSvg is exported for the new test, which pins the factor and both recolors. Measured in the built app before and after on the same note, with the KaTeX rendering as the reference. --- .../src/lib/typst-math-render.test.ts | 46 +++++++++++++++++++ .../app-core/src/lib/typst-math-render.ts | 34 ++++++++++---- 2 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 packages/app-core/src/lib/typst-math-render.test.ts diff --git a/packages/app-core/src/lib/typst-math-render.test.ts b/packages/app-core/src/lib/typst-math-render.test.ts new file mode 100644 index 00000000..57001e4b --- /dev/null +++ b/packages/app-core/src/lib/typst-math-render.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { styleSvg } from './typst-math-render' + +// Typst hands back an SVG measured in points at the 11pt text size every +// formula is compiled at, with its paint spelled out as black. The wrapper +// sizes it in em at KaTeX's 1.21 factor, so the two engines agree on how big a +// formula is next to prose (a plain 1em made Typst a fifth smaller), and turns +// every black fill and stroke into currentColor so it follows the theme (#746). +describe('styleSvg', () => { + const svg = (w: number, h: number, body = ''): string => + `${body}` + + it('sizes the formula the way KaTeX sizes Computer Modern', () => { + // 11pt of Typst text is one em of the compiled document, drawn at 1.21em. + const inline = styleSvg(svg(11, 11), false) + expect(inline).toContain('width: 1.2100em; height: 1.2100em;') + expect(inline).toContain('display: inline-block; vertical-align: middle;') + const block = styleSvg(svg(22, 8), true) + expect(block).toContain('width: 2.4200em; height: 0.8800em;') + expect(block).toContain('display: block; margin: 0 auto;') + }) + + it('drops the intrinsic pt size', () => { + const out = styleSvg(svg(11, 11), false) + expect(out).not.toMatch(/width="11pt"|height="11pt"/) + }) + + it('recolors black glyph fills and black shape strokes to currentColor', () => { + const out = styleSvg( + svg( + 11, + 11, + '' + + '' + + '' + + '' + ), + true + ) + expect(out).toContain('') + expect(out).toContain('fill="none" stroke="currentColor"') + expect(out).toContain('fill="currentColor" stroke="currentColor"') + expect(out).toContain('fill="#ff0000"') + expect(out).not.toMatch(/(fill|stroke)="(#000000|#000|rgb\(0, 0, 0\))"/) + }) +}) diff --git a/packages/app-core/src/lib/typst-math-render.ts b/packages/app-core/src/lib/typst-math-render.ts index 6d63f64d..c326e877 100644 --- a/packages/app-core/src/lib/typst-math-render.ts +++ b/packages/app-core/src/lib/typst-math-render.ts @@ -42,9 +42,18 @@ const FONT_URLS = [newCMRegularUrl, newCMBoldUrl, newCMItalicUrl, newCMMathUrl] /** Text size we compile every formula at; SVG dimensions come back in points, * and are converted to `em` relative to this so the rendered math scales with - * the reader's font size (the pt→px factor cancels: `heightEm = ptHeight / 11`). */ + * the reader's font size (the pt→px factor cancels: `heightEm = ptHeight / 11`, + * times the KaTeX factor below). */ const BASE_TEXT_PT = 11 +/** KaTeX draws Computer Modern at 1.21em of the surrounding text (its own + * stylesheet: `.katex { font-size: 1.21em }`), because the family sits small + * on its em square and a plain 1em reads undersized next to prose. New + * Computer Modern shares those metrics, so a Typst formula sized at 1em came + * out a fifth smaller than KaTeX's rendering of the same source (#746). Size + * it the way KaTeX does, so switching engines does not change the size. */ +const KATEX_EM_SCALE = 1.21 + export type TypstRenderResult = | { ok: true; svg: string } | { ok: false; error: string } @@ -119,21 +128,26 @@ function buildDocument(source: string, display: boolean, preamble: string): stri ].join('\n') } +/** Every spelling of black Typst's SVG export uses, on a fill or a stroke. + * Glyphs arrive as `fill="#000000"`; the rules a formula draws as shapes (a + * square root's bar, a fraction line) arrive as `stroke="#000"`, and a + * recolor that only knew fills left those black on a dark theme (#746). */ +const BLACK_PAINT_RE = /\b(fill|stroke)="(?:#000000|#000|black|rgb\(0,\s*0,\s*0\))"/g + /** - * Post-process Typst's SVG so it drops into a note: recolor black glyph fills - * to `currentColor` (theme-aware, no re-render on theme switch) and swap the - * intrinsic pt width/height for `em` sizes that track the surrounding font. + * Post-process Typst's SVG so it drops into a note: recolor black paint, fills + * and strokes alike, to `currentColor` (theme-aware, no re-render on theme + * switch) and swap the intrinsic pt width/height for `em` sizes that track the + * surrounding font. */ -function styleSvg(rawSvg: string, display: boolean): string { - let svg = rawSvg - .replace(/fill="#000000"/g, 'fill="currentColor"') - .replace(/fill="#000"/g, 'fill="currentColor"') +export function styleSvg(rawSvg: string, display: boolean): string { + let svg = rawSvg.replace(BLACK_PAINT_RE, '$1="currentColor"') const viewBox = svg.match(/viewBox="0 0 ([\d.]+) ([\d.]+)"/) const widthPt = viewBox ? Number.parseFloat(viewBox[1]) : 0 const heightPt = viewBox ? Number.parseFloat(viewBox[2]) : 0 - const widthEm = (widthPt / BASE_TEXT_PT).toFixed(4) - const heightEm = (heightPt / BASE_TEXT_PT).toFixed(4) + const widthEm = ((widthPt * KATEX_EM_SCALE) / BASE_TEXT_PT).toFixed(4) + const heightEm = ((heightPt * KATEX_EM_SCALE) / BASE_TEXT_PT).toFixed(4) // The app's CSS reset makes every `svg` display:block; override that so inline // math flows in the text (centered on the line, since the SVG carries no From 808486188947bb11e66b1f74cbe00503f90c5b70 Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 12:09:53 -0500 Subject: [PATCH 06/12] Feat(editor): the @ menu picks any date from a calendar (#743) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing @ offered Today, Yesterday, Tomorrow and Now; any other day meant typing it out by hand, which is exactly the case a date picker exists for. The list now ends with Date… (@date, @cal and @pick narrow to it). Picking it opens a calendar on today with today's cell focused, so the keyboard flow is complete without a Tab: arrows move a day or a week, PageUp and PageDown change the month (Shift: the year), Home and End go to the ends of the week, and Enter inserts the ISO date where the @ stood and returns focus to the note. A digit pressed on the grid moves typing to the field, and the grid follows the typed date as it takes shape; a day the month does not have is refused rather than rolled forward. With Vim mode on, h j k l move and t jumps to today; with it off, letters stay inert, the rule the list views follow. Escape leaves the note as it was before the @. The quick options are unchanged, and the calendar honours the week start setting. The trigger is removed before the calendar opens, so a dismissed picker leaves clean text and a picked date lands exactly where the @ was; the position is re-clamped at insert time in case the document shrank under the modal. The picker is a prompt in the app's sense: it is requested through promptDate(), the calendar twin of promptApp(), hosted next to the prompt and confirm hosts, and carries data-prompt-modal so VimNav and the list views hand over the keyboard. A month change replaces every grid cell, which drops focus to the body before the effect that refocuses runs, so the decision to refocus is recorded by the move itself; the built app caught this where jsdom did not. Both docs surfaces describe the calendar. Verified with unit tests for the date math, the modal (keyboard, paging, typed dates, Vim gating) and the menu's hand-off, and in the built app over CDP with Vim on and off. Closes #743 --- packages/app-core/src/App.tsx | 4 + .../src/components/DatePickerHost.tsx | 32 +++ .../src/components/DatePickerModal.test.ts | 155 ++++++++++ .../src/components/DatePickerModal.tsx | 266 ++++++++++++++++++ .../src/lib/cm-date-shortcuts.test.ts | 91 +++++- .../app-core/src/lib/cm-date-shortcuts.ts | 46 ++- packages/app-core/src/lib/date-picker.test.ts | 130 +++++++++ packages/app-core/src/lib/date-picker.ts | 162 +++++++++++ .../app-core/src/lib/date-prompt-requests.ts | 50 ++++ packages/app-core/src/lib/help.ts | 4 +- 10 files changed, 931 insertions(+), 9 deletions(-) create mode 100644 packages/app-core/src/components/DatePickerHost.tsx create mode 100644 packages/app-core/src/components/DatePickerModal.test.ts create mode 100644 packages/app-core/src/components/DatePickerModal.tsx create mode 100644 packages/app-core/src/lib/date-picker.test.ts create mode 100644 packages/app-core/src/lib/date-picker.ts create mode 100644 packages/app-core/src/lib/date-prompt-requests.ts diff --git a/packages/app-core/src/App.tsx b/packages/app-core/src/App.tsx index 336e6f8d..0771066a 100644 --- a/packages/app-core/src/App.tsx +++ b/packages/app-core/src/App.tsx @@ -20,6 +20,7 @@ import { NoteList } from './components/NoteList' import { TitleBar } from './components/TitleBar' import { PromptHost } from './components/PromptHost' import { ConfirmHost } from './components/ConfirmHost' +import { DatePickerHost } from './components/DatePickerHost' import { PublishNoteHost } from './components/PublishNoteHost' import { CloudConflictReviewHost } from './components/CloudConflictReviewHost' import { ServerDirectoryPickerHost } from './components/ServerDirectoryPickerHost' @@ -1123,6 +1124,7 @@ function App(): JSX.Element { + @@ -1141,6 +1143,7 @@ function App(): JSX.Element { + @@ -1214,6 +1217,7 @@ function App(): JSX.Element { )} + diff --git a/packages/app-core/src/components/DatePickerHost.tsx b/packages/app-core/src/components/DatePickerHost.tsx new file mode 100644 index 00000000..18367c5a --- /dev/null +++ b/packages/app-core/src/components/DatePickerHost.tsx @@ -0,0 +1,32 @@ +import { lazy, Suspense, useEffect, useState } from 'react' +import { + getDatePromptRequest, + settleDatePromptRequest, + subscribeDatePromptRequests, + type DatePromptRequest +} from '../lib/date-prompt-requests' + +const DatePickerModal = lazy(async () => { + const module = await import('./DatePickerModal') + return { default: module.DatePickerModal } +}) + +export function DatePickerHost(): JSX.Element | null { + const [request, setRequest] = useState(getDatePromptRequest) + + useEffect(() => { + return subscribeDatePromptRequests(setRequest) + }, []) + + if (!request) return null + + return ( + + settleDatePromptRequest(request, iso)} + onCancel={() => settleDatePromptRequest(request, null)} + /> + + ) +} diff --git a/packages/app-core/src/components/DatePickerModal.test.ts b/packages/app-core/src/components/DatePickerModal.test.ts new file mode 100644 index 00000000..3ca45774 --- /dev/null +++ b/packages/app-core/src/components/DatePickerModal.test.ts @@ -0,0 +1,155 @@ +// @vitest-environment jsdom + +import { act, createElement } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { useStore } from '../store' +import { DatePickerModal, type DatePickerOptions } from './DatePickerModal' + +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +function key(target: Element, key: string, init: KeyboardEventInit = {}): void { + act(() => { + target.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init })) + }) +} + +function cell(iso: string): HTMLButtonElement { + const el = document.querySelector(`[data-date-cell="${iso}"]`) + if (!el) throw new Error(`no cell for ${iso}`) + return el +} + +function selectedIso(): string | null { + return document.querySelector('[role="gridcell"][aria-selected="true"]')?.getAttribute('data-date-cell') ?? null +} + +function monthShown(): string { + return document.querySelector('[data-date-picker-month]')?.textContent ?? '' +} + +describe('DatePickerModal', () => { + let root: Root | null = null + let container: HTMLDivElement | null = null + const onSubmit = vi.fn<(iso: string) => void>() + const onCancel = vi.fn<() => void>() + + function mount(options: DatePickerOptions): void { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => { + root!.render(createElement(DatePickerModal, { options, onSubmit, onCancel })) + }) + } + + beforeEach(() => { + onSubmit.mockReset() + onCancel.mockReset() + useStore.setState({ vimMode: false, calendarWeekStart: 'monday' }) + }) + + afterEach(() => { + act(() => root?.unmount()) + container?.remove() + root = null + container = null + }) + + it('opens on the given day with that cell focused, and Enter inserts it', () => { + mount({ initialDate: '2026-09-08' }) + expect(selectedIso()).toBe('2026-09-08') + expect(monthShown()).toBe('September 2026') + expect(document.activeElement).toBe(cell('2026-09-08')) + key(cell('2026-09-08'), 'Enter') + expect(onSubmit).toHaveBeenCalledWith('2026-09-08') + }) + + it('moves with arrows and paging, following the selection across months', () => { + mount({ initialDate: '2026-09-08' }) + key(cell('2026-09-08'), 'ArrowRight') + expect(selectedIso()).toBe('2026-09-09') + key(cell('2026-09-09'), 'ArrowDown') + expect(selectedIso()).toBe('2026-09-16') + expect(document.activeElement).toBe(cell('2026-09-16')) + key(cell('2026-09-16'), 'PageDown') + expect(selectedIso()).toBe('2026-10-16') + expect(monthShown()).toBe('October 2026') + // A month change replaces every cell; focus must land on the new one, + // not fall to the body with the unmounted button. + expect(document.activeElement).toBe(cell('2026-10-16')) + key(cell('2026-10-16'), 'PageUp', { shiftKey: true }) + expect(selectedIso()).toBe('2025-10-16') + expect(monthShown()).toBe('October 2025') + expect(document.activeElement).toBe(cell('2025-10-16')) + key(cell('2025-10-16'), 'Enter') + expect(onSubmit).toHaveBeenCalledWith('2025-10-16') + }) + + it('keeps letters inert with Vim mode off and makes them moves with it on', () => { + mount({ initialDate: '2026-09-08' }) + key(cell('2026-09-08'), 'l') + expect(selectedIso()).toBe('2026-09-08') + + act(() => useStore.setState({ vimMode: true })) + key(cell('2026-09-08'), 'l') + expect(selectedIso()).toBe('2026-09-09') + key(cell('2026-09-09'), 'j') + expect(selectedIso()).toBe('2026-09-16') + }) + + it('takes a typed date: the grid follows it and Enter inserts it', () => { + mount({ initialDate: '2026-09-08' }) + const input = document.querySelector('input[aria-label="Date"]')! + // A digit pressed on the grid moves typing to the field, starting fresh. + key(cell('2026-09-08'), '2') + expect(document.activeElement).toBe(input) + expect(input.value).toBe('2') + + const setValue = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + act(() => { + setValue.call(input, '2027-03-14') + input.dispatchEvent(new Event('input', { bubbles: true })) + }) + expect(selectedIso()).toBe('2027-03-14') + expect(monthShown()).toBe('March 2027') + + key(input, 'Enter') + expect(onSubmit).toHaveBeenCalledWith('2027-03-14') + }) + + it('refuses a typed day that does not exist instead of inserting a rolled-over one', () => { + mount({ initialDate: '2026-09-08' }) + const input = document.querySelector('input[aria-label="Date"]')! + const setValue = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + act(() => { + setValue.call(input, '2026-02-30') + input.dispatchEvent(new Event('input', { bubbles: true })) + }) + key(input, 'Enter') + expect(onSubmit).not.toHaveBeenCalled() + expect(document.body.textContent).toContain('Type a real date as YYYY-MM-DD.') + // The grid still stands on the last real day. + expect(selectedIso()).toBe('2026-09-08') + }) + + it('cancels from the footer and marks itself as a prompt for the key routers', () => { + mount({ initialDate: '2026-09-08' }) + expect(document.querySelector('[data-prompt-modal]')).not.toBeNull() + const cancel = Array.from(document.querySelectorAll('button')).find( + (b) => b.textContent === 'Cancel' + )! + act(() => cancel.click()) + expect(onCancel).toHaveBeenCalledTimes(1) + expect(onSubmit).not.toHaveBeenCalled() + }) + + it('falls back to today when the initial date is malformed', () => { + mount({ initialDate: 'yesterday' }) + const now = new Date() + const today = `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String( + now.getDate() + ).padStart(2, '0')}` + expect(selectedIso()).toBe(today) + }) +}) diff --git a/packages/app-core/src/components/DatePickerModal.tsx b/packages/app-core/src/components/DatePickerModal.tsx new file mode 100644 index 00000000..cc80c164 --- /dev/null +++ b/packages/app-core/src/components/DatePickerModal.tsx @@ -0,0 +1,266 @@ +import { useEffect, useMemo, useRef, useState } from 'react' +import { useStore } from '../store' +import { + addMonths, + buildMonthGrid, + datePickerMoveForKey, + firstOfMonth, + formatISODate, + monthTitle, + moveDate, + parseISODate, + startOfDay +} from '../lib/date-picker' +import { isImeComposing } from '../lib/ime' +import { resolveWeekStartDay } from '../lib/week-start' +import { Modal } from './ui/Modal' +import { Button } from './ui/Button' +import { ChevronLeftIcon, ChevronRightIcon } from './icons' + +export interface DatePickerOptions { + title?: string + description?: string + /** `YYYY-MM-DD` the picker opens on; today when absent or malformed. */ + initialDate?: string + okLabel?: string +} + +const DAY_LABELS = ['Su', 'Mo', 'Tu', 'We', 'Th', 'Fr', 'Sa'] + +/** + * The calendar behind `@date` (#743). Keyboard first: it opens with the + * selected day focused, so arrows move and Enter inserts without a Tab + * anywhere; typing a digit from the grid jumps to the text field for a date + * typed outright. The grid always shows the month of the selected day, so a + * move can never land on a day the user cannot see. Carries + * `data-prompt-modal` like the text prompt so VimNav and the list views hand + * the keyboard over while it is open. + */ +export function DatePickerModal({ + options, + onSubmit, + onCancel +}: { + options: DatePickerOptions + onSubmit: (iso: string) => void + onCancel: () => void +}): JSX.Element { + const vimMode = useStore((s) => s.vimMode) + const weekStart = useStore((s) => s.calendarWeekStart) + const firstDay = resolveWeekStartDay(weekStart) + const today = useMemo(() => startOfDay(new Date()), []) + const [selected, setSelected] = useState( + () => (options.initialDate ? parseISODate(options.initialDate) : null) ?? today + ) + const [anchor, setAnchor] = useState(() => firstOfMonth(selected)) + const [typed, setTyped] = useState(() => formatISODate(selected)) + const [error, setError] = useState(null) + const inputRef = useRef(null) + const gridRef = useRef(null) + const selectedCellRef = useRef(null) + // Set by a keyboard move in the grid; the effect below consumes it. + const refocusGridRef = useRef(false) + + const selectedIso = formatISODate(selected) + const todayIso = formatISODate(today) + const grid = useMemo(() => buildMonthGrid(anchor, firstDay), [anchor, firstDay]) + const dayLabels = useMemo( + () => Array.from({ length: 7 }, (_, i) => DAY_LABELS[(firstDay + i) % 7]), + [firstDay] + ) + // Roving tabindex: one cell is reachable by Tab, the selected day when the + // grid shows it, else the first of the month being browsed. + const tabbableIso = grid.some((day) => formatISODate(day) === selectedIso) + ? selectedIso + : formatISODate(anchor) + + function select(next: Date): void { + setSelected(next) + setAnchor(firstOfMonth(next)) + setTyped(formatISODate(next)) + setError(null) + } + + // A keyboard move re-renders the grid with a new selected cell; put focus + // on it so the next arrow continues from there. A move into another month + // replaces every cell, which drops focus to the body before this effect + // runs, so the decision to refocus is recorded by the move itself rather + // than read from where focus happens to be. + useEffect(() => { + if (!refocusGridRef.current) return + refocusGridRef.current = false + selectedCellRef.current?.focus() + }, [selectedIso]) + + function submitTyped(): void { + const parsed = parseISODate(typed.trim()) + if (!parsed) { + setError('Type a real date as YYYY-MM-DD.') + return + } + onSubmit(formatISODate(parsed)) + } + + const hint = vimMode + ? 'h j k l or arrows move, t is today. PageUp/PageDown change the month, with Shift the year. Enter inserts.' + : 'Arrows move. PageUp/PageDown change the month, with Shift the year. Enter inserts.' + + return ( + + +
+ { + const value = e.target.value + setTyped(value) + setError(null) + // The grid follows a complete date as it is typed; partial input + // leaves the last valid day selected. + const parsed = parseISODate(value.trim()) + if (parsed) { + setSelected(parsed) + setAnchor(firstOfMonth(parsed)) + } + }} + onKeyDown={(e) => { + if (isImeComposing(e)) return + if (e.key === 'Enter') { + e.preventDefault() + submitTyped() + } else if (e.key === 'ArrowDown') { + e.preventDefault() + gridRef.current + ?.querySelector(`[data-date-cell="${tabbableIso}"]`) + ?.focus() + } + }} + className="w-full rounded-md border border-paper-300 bg-paper-50 px-2.5 py-1.5 text-sm text-ink-900 outline-none focus:border-accent" + /> + {error &&
{error}
} +
+ + + {monthTitle(anchor)} + + +
+
{ + if (isImeComposing(e)) return + if (e.metaKey || e.ctrlKey || e.altKey) return + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault() + onSubmit(selectedIso) + return + } + if (/^\d$/.test(e.key)) { + // A digit means "let me type it": hand the keystroke to the + // text field as the first character of a fresh date. + e.preventDefault() + setTyped(e.key) + setError(null) + inputRef.current?.focus() + return + } + const move = datePickerMoveForKey(e.key, { shift: e.shiftKey, vimMode }) + if (!move) return + e.preventDefault() + refocusGridRef.current = true + select(moveDate(selected, move, firstDay, today)) + }} + > + {dayLabels.map((label, i) => ( +
+ {label} +
+ ))} + {grid.map((day) => { + const iso = formatISODate(day) + const inMonth = day.getMonth() === anchor.getMonth() + const isSelected = iso === selectedIso + const isToday = iso === todayIso + return ( + + ) + })} +
+
+ {hint} + +
+
+ + + + + + ) +} diff --git a/packages/app-core/src/lib/cm-date-shortcuts.test.ts b/packages/app-core/src/lib/cm-date-shortcuts.test.ts index 71fc745f..10a3b872 100644 --- a/packages/app-core/src/lib/cm-date-shortcuts.test.ts +++ b/packages/app-core/src/lib/cm-date-shortcuts.test.ts @@ -1,5 +1,18 @@ -import { describe, expect, it } from 'vitest' -import { formatClockTime } from './cm-date-shortcuts' +// @vitest-environment jsdom + +import { CompletionContext, type Completion } from '@codemirror/autocomplete' +import { EditorState } from '@codemirror/state' +import { EditorView } from '@codemirror/view' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { dateShortcutSource, formatClockTime } from './cm-date-shortcuts' + +const promptMocks = vi.hoisted(() => ({ + promptDate: vi.fn<(options?: unknown) => Promise>() +})) + +vi.mock('./date-prompt-requests', () => ({ + promptDate: promptMocks.promptDate +})) const at = (h: number, m: number) => new Date(2026, 0, 1, h, m) @@ -19,3 +32,77 @@ describe('formatClockTime', () => { expect(formatClockTime(at(23, 59), '12h')).toBe('11:59 PM') }) }) + +function optionsFor(doc: string): readonly Completion[] { + const state = EditorState.create({ doc, selection: { anchor: doc.length } }) + const result = dateShortcutSource(new CompletionContext(state, doc.length, false)) + return result?.options ?? [] +} + +function labels(doc: string): string[] { + return optionsFor(doc).map((o) => o.label) +} + +function todayIso(): string { + const now = new Date() + return `${now.getFullYear()}-${String(now.getMonth() + 1).padStart(2, '0')}-${String( + now.getDate() + ).padStart(2, '0')}` +} + +describe('the @ menu', () => { + it('keeps the quick options and adds Date… at the end', () => { + expect(labels('meet @')).toEqual(['Today', 'Yesterday', 'Tomorrow', 'Now', 'Date…']) + }) + + it('finds the calendar by date, pick or calendar, and quick options by their names', () => { + expect(labels('meet @date')).toEqual(['Date…']) + expect(labels('meet @cal')).toEqual(['Date…']) + expect(labels('meet @pick')).toEqual(['Date…']) + expect(labels('meet @tom')).toEqual(['Tomorrow']) + expect(labels('meet @now')).toEqual(['Now']) + }) +}) + +describe('picking a date from the @ menu', () => { + let view: EditorView + + beforeEach(() => { + promptMocks.promptDate.mockReset() + }) + + afterEach(() => { + view.destroy() + }) + + function applyDatePick(doc: string): void { + view = new EditorView({ + parent: document.body, + state: EditorState.create({ doc, selection: { anchor: doc.length } }) + }) + const context = new CompletionContext(view.state, doc.length, false) + const result = dateShortcutSource(context)! + const pick = result.options.find((o) => o.label === 'Date…')! + const apply = pick.apply as (view: EditorView, c: Completion, from: number, to: number) => void + apply(view, pick, result.from, doc.length) + } + + it('drops the trigger, opens the calendar on today, and inserts the picked day where @ stood', async () => { + promptMocks.promptDate.mockResolvedValue('2027-03-14') + applyDatePick('meet @date') + // The trigger is gone before the calendar shows, so the note never + // carries a half-typed `@date` under the modal. + expect(view.state.doc.toString()).toBe('meet ') + expect(promptMocks.promptDate).toHaveBeenCalledWith({ initialDate: todayIso() }) + await vi.waitFor(() => expect(view.state.doc.toString()).toBe('meet 2027-03-14')) + expect(view.state.selection.main.head).toBe('meet 2027-03-14'.length) + }) + + it('leaves clean text when the calendar is dismissed', async () => { + promptMocks.promptDate.mockResolvedValue(null) + applyDatePick('meet @da') + expect(view.state.doc.toString()).toBe('meet ') + await new Promise((r) => setTimeout(r, 0)) + expect(view.state.doc.toString()).toBe('meet ') + }) +}) diff --git a/packages/app-core/src/lib/cm-date-shortcuts.ts b/packages/app-core/src/lib/cm-date-shortcuts.ts index 004895be..8939b033 100644 --- a/packages/app-core/src/lib/cm-date-shortcuts.ts +++ b/packages/app-core/src/lib/cm-date-shortcuts.ts @@ -2,6 +2,8 @@ import type { Completion, CompletionContext, CompletionResult } from '@codemirro import type { EditorView } from '@codemirror/view' import type { TimeFormat } from '@shared/app-config' import { useStore } from '../store' +import { formatISODate } from './date-picker' +import { promptDate } from './date-prompt-requests' interface DateShortcut { label: string @@ -9,6 +11,8 @@ interface DateShortcut { insert: string /** When set, computed fresh at apply time (used for the current time). */ dynamicInsert?: () => string + /** Opens the calendar instead of inserting `insert` (#743). */ + pick?: boolean searchText: string icon: string } @@ -17,10 +21,6 @@ function pad2(n: number): string { return String(n).padStart(2, '0') } -function formatISODate(date: Date): string { - return `${date.getFullYear()}-${pad2(date.getMonth() + 1)}-${pad2(date.getDate())}` -} - /** Wall-clock time in the given format: `14:30` (24h) or `2:30 PM` (12h). */ export function formatClockTime(date: Date, format: TimeFormat): string { const minutes = pad2(date.getMinutes()) @@ -80,7 +80,39 @@ function buildShortcuts(now = new Date()): DateShortcut[] { icon: '🕘' } - return [...dates, time] + // #743: any other day. Its detail names the outcome, since there is no + // date to preview until the calendar returns one. + const pick: DateShortcut = { + label: 'Date…', + detail: 'Pick any date', + insert: '', + pick: true, + searchText: 'date… date pick picker calendar choose any other day', + icon: '📅' + } + + return [...dates, time, pick] +} + +/** + * Trades the `@…` trigger for the calendar. The trigger goes first, so a + * dismissed picker leaves the text as if nothing was typed, and the picked + * date lands exactly where the `@` stood. The position is re-clamped at + * insert time: the modal blocks editing, but a note switched underneath it + * (a sync, a remote change) can still shorten the document. + */ +function pickDate(view: EditorView, from: number, to: number): void { + view.dispatch({ changes: { from, to, insert: '' }, selection: { anchor: from } }) + void promptDate({ initialDate: formatISODate(new Date()) }).then((iso) => { + if (iso) { + const at = Math.min(from, view.state.doc.length) + view.dispatch({ + changes: { from: at, insert: iso }, + selection: { anchor: at + iso.length } + }) + } + view.focus() + }) } function dateShortcutMatch(context: CompletionContext): { @@ -117,6 +149,10 @@ export function dateShortcutSource(context: CompletionContext): CompletionResult _kind: 'date', _icon: item.icon, apply: (view: EditorView, _completion: Completion, _from: number, to: number) => { + if (item.pick) { + pickDate(view, match.replaceFrom, to) + return + } const insert = item.dynamicInsert ? item.dynamicInsert() : item.insert view.dispatch({ changes: { from: match.replaceFrom, to, insert }, diff --git a/packages/app-core/src/lib/date-picker.test.ts b/packages/app-core/src/lib/date-picker.test.ts new file mode 100644 index 00000000..8526e68e --- /dev/null +++ b/packages/app-core/src/lib/date-picker.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it } from 'vitest' +import { + addMonths, + buildMonthGrid, + datePickerMoveForKey, + formatISODate, + moveDate, + parseISODate, + startOfWeek +} from './date-picker' + +describe('parseISODate', () => { + it('round-trips a real local calendar day', () => { + const date = parseISODate('2026-09-08') + expect(date).not.toBeNull() + expect(date!.getFullYear()).toBe(2026) + expect(date!.getMonth()).toBe(8) + expect(date!.getDate()).toBe(8) + expect(date!.getHours()).toBe(0) + expect(formatISODate(date!)).toBe('2026-09-08') + }) + + it('accepts a leap day and rejects a day the month does not have', () => { + expect(formatISODate(parseISODate('2028-02-29')!)).toBe('2028-02-29') + expect(parseISODate('2026-02-29')).toBeNull() + expect(parseISODate('2026-02-30')).toBeNull() + expect(parseISODate('2026-13-01')).toBeNull() + expect(parseISODate('2026-00-10')).toBeNull() + }) + + it('wants the strict YYYY-MM-DD shape', () => { + expect(parseISODate('2026-9-8')).toBeNull() + expect(parseISODate('08/09/2026')).toBeNull() + expect(parseISODate('2026-09-08T10:00')).toBeNull() + expect(parseISODate('')).toBeNull() + }) +}) + +describe('addMonths', () => { + it('clamps the day to the target month instead of rolling over', () => { + expect(formatISODate(addMonths(parseISODate('2026-01-31')!, 1))).toBe('2026-02-28') + expect(formatISODate(addMonths(parseISODate('2028-01-31')!, 1))).toBe('2028-02-29') + expect(formatISODate(addMonths(parseISODate('2026-03-31')!, -1))).toBe('2026-02-28') + expect(formatISODate(addMonths(parseISODate('2026-12-15')!, 1))).toBe('2027-01-15') + expect(formatISODate(addMonths(parseISODate('2028-02-29')!, 12))).toBe('2029-02-28') + }) +}) + +describe('buildMonthGrid', () => { + it('lays out six weeks starting on the configured weekday', () => { + const monday = buildMonthGrid(parseISODate('2026-09-01')!, 1) + expect(monday).toHaveLength(42) + expect(monday[0].getDay()).toBe(1) + // September 2026 starts on a Tuesday: one leading day from August. + expect(formatISODate(monday[0])).toBe('2026-08-31') + expect(formatISODate(monday[1])).toBe('2026-09-01') + expect(formatISODate(monday[41])).toBe('2026-10-11') + + const sunday = buildMonthGrid(parseISODate('2026-09-20')!, 0) + expect(sunday[0].getDay()).toBe(0) + expect(formatISODate(sunday[0])).toBe('2026-08-30') + }) +}) + +describe('moveDate', () => { + const firstDay = 1 + const at = (iso: string): Date => parseISODate(iso)! + + it('moves by day, week, month and year', () => { + expect(formatISODate(moveDate(at('2026-09-08'), 'prev-day', firstDay))).toBe('2026-09-07') + expect(formatISODate(moveDate(at('2026-09-08'), 'next-day', firstDay))).toBe('2026-09-09') + expect(formatISODate(moveDate(at('2026-09-08'), 'prev-week', firstDay))).toBe('2026-09-01') + expect(formatISODate(moveDate(at('2026-09-08'), 'next-week', firstDay))).toBe('2026-09-15') + expect(formatISODate(moveDate(at('2026-09-30'), 'prev-month', firstDay))).toBe('2026-08-30') + expect(formatISODate(moveDate(at('2026-01-31'), 'next-month', firstDay))).toBe('2026-02-28') + expect(formatISODate(moveDate(at('2026-09-08'), 'prev-year', firstDay))).toBe('2025-09-08') + expect(formatISODate(moveDate(at('2026-09-08'), 'next-year', firstDay))).toBe('2027-09-08') + }) + + it('crosses month and year boundaries a day at a time', () => { + expect(formatISODate(moveDate(at('2026-12-31'), 'next-day', firstDay))).toBe('2027-01-01') + expect(formatISODate(moveDate(at('2026-03-01'), 'prev-day', firstDay))).toBe('2026-02-28') + }) + + it('finds the ends of the week for the configured first day', () => { + // 2026-09-10 is a Thursday. + expect(formatISODate(moveDate(at('2026-09-10'), 'week-start', 1))).toBe('2026-09-07') + expect(formatISODate(moveDate(at('2026-09-10'), 'week-end', 1))).toBe('2026-09-13') + expect(formatISODate(moveDate(at('2026-09-10'), 'week-start', 0))).toBe('2026-09-06') + expect(formatISODate(moveDate(at('2026-09-10'), 'week-end', 0))).toBe('2026-09-12') + expect(formatISODate(startOfWeek(at('2026-09-07'), 1))).toBe('2026-09-07') + }) + + it('jumps to today at local midnight', () => { + const today = new Date(2026, 8, 8, 17, 45) + const moved = moveDate(at('2020-01-01'), 'today', firstDay, today) + expect(formatISODate(moved)).toBe('2026-09-08') + expect(moved.getHours()).toBe(0) + }) +}) + +describe('datePickerMoveForKey', () => { + it('follows the ARIA date-grid keys with any editing mode', () => { + const off = { shift: false, vimMode: false } + expect(datePickerMoveForKey('ArrowLeft', off)).toBe('prev-day') + expect(datePickerMoveForKey('ArrowRight', off)).toBe('next-day') + expect(datePickerMoveForKey('ArrowUp', off)).toBe('prev-week') + expect(datePickerMoveForKey('ArrowDown', off)).toBe('next-week') + expect(datePickerMoveForKey('PageUp', off)).toBe('prev-month') + expect(datePickerMoveForKey('PageDown', off)).toBe('next-month') + expect(datePickerMoveForKey('PageUp', { ...off, shift: true })).toBe('prev-year') + expect(datePickerMoveForKey('PageDown', { ...off, shift: true })).toBe('next-year') + expect(datePickerMoveForKey('Home', off)).toBe('week-start') + expect(datePickerMoveForKey('End', off)).toBe('week-end') + }) + + it('adds h/j/k/l and t only while Vim mode is on', () => { + const on = { shift: false, vimMode: true } + expect(datePickerMoveForKey('h', on)).toBe('prev-day') + expect(datePickerMoveForKey('l', on)).toBe('next-day') + expect(datePickerMoveForKey('k', on)).toBe('prev-week') + expect(datePickerMoveForKey('j', on)).toBe('next-week') + expect(datePickerMoveForKey('t', on)).toBe('today') + const off = { shift: false, vimMode: false } + for (const key of ['h', 'j', 'k', 'l', 't']) { + expect(datePickerMoveForKey(key, off)).toBeNull() + } + expect(datePickerMoveForKey('x', on)).toBeNull() + }) +}) diff --git a/packages/app-core/src/lib/date-picker.ts b/packages/app-core/src/lib/date-picker.ts new file mode 100644 index 00000000..6110ec5f --- /dev/null +++ b/packages/app-core/src/lib/date-picker.ts @@ -0,0 +1,162 @@ +/** + * Date math behind the `@date` calendar (#743): ISO round-trips, the month + * grid, and the moves its keyboard answers with. Everything works on local + * calendar days (never UTC): a date picked at 23:30 in Sydney must insert + * that day, not the one the UTC clock is still on. Kept pure so the picker's + * behavior is testable without rendering it, and so the `@` menu and the + * modal agree on what a day looks like on disk (`YYYY-MM-DD`). + */ + +function pad2(n: number): string { + return String(n).padStart(2, '0') +} + +export function formatISODate(date: Date): string { + return `${date.getFullYear()}-${pad2(date.getMonth() + 1)}-${pad2(date.getDate())}` +} + +/** + * Strict `YYYY-MM-DD` to a local-midnight Date, or null. A day that does not + * exist (2026-02-30) is rejected rather than rolled into March: the picker + * would otherwise insert a date the user never typed. + */ +export function parseISODate(text: string): Date | null { + const match = /^(\d{4})-(\d{2})-(\d{2})$/.exec(text) + if (!match) return null + const year = Number(match[1]) + const month = Number(match[2]) + const day = Number(match[3]) + if (month < 1 || month > 12 || day < 1 || day > 31) return null + const date = new Date(year, month - 1, day) + if (date.getFullYear() !== year || date.getMonth() !== month - 1 || date.getDate() !== day) { + return null + } + return date +} + +export function startOfDay(date: Date): Date { + return new Date(date.getFullYear(), date.getMonth(), date.getDate()) +} + +export function firstOfMonth(date: Date): Date { + return new Date(date.getFullYear(), date.getMonth(), 1) +} + +export function addDays(date: Date, days: number): Date { + return new Date(date.getFullYear(), date.getMonth(), date.getDate() + days) +} + +/** + * Same day-of-month `months` away, clamped to the target month's length + * (Jan 31 + 1 month is Feb 28, not Mar 3). PageUp/PageDown in the picker + * would otherwise skip past short months. + */ +export function addMonths(date: Date, months: number): Date { + const first = new Date(date.getFullYear(), date.getMonth() + months, 1) + const lastDay = new Date(first.getFullYear(), first.getMonth() + 1, 0).getDate() + return new Date(first.getFullYear(), first.getMonth(), Math.min(date.getDate(), lastDay)) +} + +/** First day of the week containing `date`, with `firstDay` as 0 (Sunday) .. 6. */ +export function startOfWeek(date: Date, firstDay: number): Date { + return addDays(date, -((date.getDay() - firstDay + 7) % 7)) +} + +/** 6-row (42-cell) grid for the month containing `anchor`, starting on `firstDay`. */ +export function buildMonthGrid(anchor: Date, firstDay: number): Date[] { + const start = startOfWeek(firstOfMonth(anchor), firstDay) + return Array.from({ length: 42 }, (_, i) => addDays(start, i)) +} + +export function monthTitle(date: Date): string { + return date.toLocaleDateString(undefined, { month: 'long', year: 'numeric' }) +} + +export type DatePickerMove = + | 'prev-day' + | 'next-day' + | 'prev-week' + | 'next-week' + | 'prev-month' + | 'next-month' + | 'prev-year' + | 'next-year' + | 'week-start' + | 'week-end' + | 'today' + +export function moveDate( + date: Date, + move: DatePickerMove, + firstDay: number, + today: Date = new Date() +): Date { + switch (move) { + case 'prev-day': + return addDays(date, -1) + case 'next-day': + return addDays(date, 1) + case 'prev-week': + return addDays(date, -7) + case 'next-week': + return addDays(date, 7) + case 'prev-month': + return addMonths(date, -1) + case 'next-month': + return addMonths(date, 1) + case 'prev-year': + return addMonths(date, -12) + case 'next-year': + return addMonths(date, 12) + case 'week-start': + return startOfWeek(date, firstDay) + case 'week-end': + return addDays(startOfWeek(date, firstDay), 6) + case 'today': + return startOfDay(today) + } +} + +/** + * The picker's keyboard: the WAI-ARIA date-grid pattern (arrows move a day or + * a week, PageUp/PageDown a month, with Shift a year, Home/End the week's + * ends), plus h/j/k/l and `t` (today) only while Vim mode is on. With Vim off + * a letter must never be a shortcut, the same rule the list views follow. + */ +export function datePickerMoveForKey( + key: string, + modifiers: { shift: boolean; vimMode: boolean } +): DatePickerMove | null { + switch (key) { + case 'ArrowLeft': + return 'prev-day' + case 'ArrowRight': + return 'next-day' + case 'ArrowUp': + return 'prev-week' + case 'ArrowDown': + return 'next-week' + case 'PageUp': + return modifiers.shift ? 'prev-year' : 'prev-month' + case 'PageDown': + return modifiers.shift ? 'next-year' : 'next-month' + case 'Home': + return 'week-start' + case 'End': + return 'week-end' + } + if (!modifiers.vimMode) return null + switch (key) { + case 'h': + return 'prev-day' + case 'l': + return 'next-day' + case 'k': + return 'prev-week' + case 'j': + return 'next-week' + case 't': + return 'today' + } + return null +} diff --git a/packages/app-core/src/lib/date-prompt-requests.ts b/packages/app-core/src/lib/date-prompt-requests.ts new file mode 100644 index 00000000..f92c8e33 --- /dev/null +++ b/packages/app-core/src/lib/date-prompt-requests.ts @@ -0,0 +1,50 @@ +import type { DatePickerOptions } from '../components/DatePickerModal' + +/** + * The calendar counterpart of `promptApp`: any code with no React context + * (a CodeMirror completion's `apply`, an ex command) can ask for a date and + * await the ISO string, while `DatePickerHost` renders the modal. One request + * at a time, like the text prompt: a second `promptDate` while one is open + * replaces it on screen and the first promise stays pending until settled. + */ +export type DatePromptRequest = { + options: DatePickerOptions + resolve: (value: string | null) => void +} + +let currentRequest: DatePromptRequest | null = null +const listeners = new Set<(request: DatePromptRequest | null) => void>() + +function emit(): void { + for (const listener of listeners) listener(currentRequest) +} + +export function getDatePromptRequest(): DatePromptRequest | null { + return currentRequest +} + +export function subscribeDatePromptRequests( + listener: (request: DatePromptRequest | null) => void +): () => void { + listeners.add(listener) + return () => { + listeners.delete(listener) + } +} + +/** Resolves with the picked `YYYY-MM-DD`, or null when the picker is dismissed. */ +export function promptDate(options: DatePickerOptions = {}): Promise { + return new Promise((resolve) => { + currentRequest = { options, resolve } + emit() + }) +} + +export function settleDatePromptRequest(request: DatePromptRequest, value: string | null): void { + const resolve = request.resolve + if (currentRequest === request) { + currentRequest = null + emit() + } + queueMicrotask(() => resolve(value)) +} diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index fc7b70fd..bf7b3de6 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -51,7 +51,7 @@ export const HELP_QUICK_START: HelpCard[] = [ { title: 'Insert structure while you type', body: - 'Type `/` to insert headings, lists, callouts, code blocks, tables, links, images, and other markdown structures. Type `@` to insert date shortcuts like Today and Tomorrow as ISO dates, or `@time` / `@now` for the current time.' + 'Type `/` to insert headings, lists, callouts, code blocks, tables, links, images, and other markdown structures. Type `@` to insert date shortcuts like Today and Tomorrow as ISO dates, `@time` / `@now` for the current time, or `@date` to pick any other day from a calendar.' }, { title: 'Format a selection', @@ -610,7 +610,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ keys: '@', action: 'Open date/time shortcuts', detail: - 'Show inline suggestions for Today, Yesterday, Tomorrow, and the current time (`@time` / `@now`) while writing so you can insert dates and times without leaving the keyboard.' + 'Show inline suggestions for Today, Yesterday, Tomorrow, the current time (`@time` / `@now`), and Date…, a calendar for any other day (`@date`: arrows move, PageUp/PageDown change the month, Enter inserts), so you can insert dates and times without leaving the keyboard.' }, { keys: 'Select text, then m', From cb63bf6d41194570a19f1d093045ba1e3c23989a Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 12:47:38 -0500 Subject: [PATCH 07/12] Feat(tasks): save a Tasks filter under a name and recall it (#731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filter box narrows the Tasks views to one project, one area, one context, but it was transient: the same query had to be retyped every time, so in practice people stopped reaching for it and went looking for a grouping mode instead. Grouping has one axis; filters compose. A query is now saved under a name, as the [saved_filters] table in config.toml ("Project alpha" = "@project:alpha", one line each), so it is diffable, syncs with the rest of the preferences, and a hand edit applies live. Recall is the point, so it is cheap from everywhere: a chip row under the Tasks header shows the saved filters in file order, a click applies one and a second click clears it; `:filter ` applies the saved query when the text is a saved name (any other text filters literally, as before); F (Vim mode) opens a picker that narrows as you type; and the command palette lists every saved filter as "Tasks: name", which opens the view already filtered from any note. Saving is a chip too: an unsaved query shows Save filter…, which asks for a name and offers the existing ones for an overwrite; `:savefilter ` does it from the ex line and `:delfilter ` forgets one. A chip's context menu renames or deletes it. Names match case-insensitively wherever a user types one, and the stored spelling is what the chips show. Edits keep the chip order: an overwrite or a rename stays in place, a new filter goes last. The map is a portable pref like the keymap overrides, validated on the way in from the file and from localStorage, so the web client keeps its saved filters in the browser. The Tasks list keys stay Vim-gated: with Vim off, the chips and the palette are the way in. Both docs surfaces describe it. Verified with unit tests for the map helpers, the config.toml round trip, the palette entries and the store, and in the built app over CDP with Vim on and off: chips from the file, the picker, the ex commands, a hand edit picked up live, and the palette entry from a note. Closes #731 --- apps/desktop/src/main/app-config.test.ts | 10 +- apps/desktop/src/main/app-config.ts | 9 + .../app-core/src/components/TasksView.tsx | 215 +++++++++++++++++- packages/app-core/src/lib/commands.test.ts | 19 ++ packages/app-core/src/lib/commands.ts | 16 ++ packages/app-core/src/lib/help.ts | 18 +- packages/app-core/src/lib/keymaps.ts | 12 + .../src/lib/saved-task-filters.test.ts | 103 +++++++++ .../app-core/src/lib/saved-task-filters.ts | 124 ++++++++++ packages/app-core/src/store.ts | 49 ++++ packages/shared-domain/src/app-config.ts | 7 +- packages/shared-domain/src/keymaps-catalog.ts | 1 + 12 files changed, 573 insertions(+), 10 deletions(-) create mode 100644 packages/app-core/src/lib/saved-task-filters.test.ts create mode 100644 packages/app-core/src/lib/saved-task-filters.ts diff --git a/apps/desktop/src/main/app-config.test.ts b/apps/desktop/src/main/app-config.test.ts index dc7bf6da..723813ab 100644 --- a/apps/desktop/src/main/app-config.test.ts +++ b/apps/desktop/src/main/app-config.test.ts @@ -111,7 +111,8 @@ describe('TOML serialization', () => { quickNoteTitlePrefix: 'Quick Note', keymapOverrides: { 'global.searchNotes': 'Mod+P' }, kanbanColumnTitles: { 'status:todo': 'To Do' }, - systemFolderLabels: { inbox: 'In' } + systemFolderLabels: { inbox: 'In' }, + savedTaskFilters: { 'Project alpha': '@project:alpha', Blocked: '@status:blocked' } } const text = serializeConfig(portable) @@ -135,6 +136,13 @@ describe('TOML serialization', () => { expect(round.keymapOverrides).toEqual({ 'global.searchNotes': 'Mod+P' }) expect(round.kanbanColumnTitles).toEqual({ 'status:todo': 'To Do' }) expect(round.systemFolderLabels).toEqual({ inbox: 'In' }) + // The [saved_filters] table keeps the order the chips show (#731). + expect(text).toContain('[saved_filters]') + expect(text).toContain('"Project alpha" = "@project:alpha"') + expect(Object.entries(round.savedTaskFilters as Record)).toEqual([ + ['Project alpha', '@project:alpha'], + ['Blocked', '@status:blocked'] + ]) }) it('persists null as empty string and reads it back as null', () => { diff --git a/apps/desktop/src/main/app-config.ts b/apps/desktop/src/main/app-config.ts index 58cd09c6..52c6f7b1 100644 --- a/apps/desktop/src/main/app-config.ts +++ b/apps/desktop/src/main/app-config.ts @@ -445,6 +445,15 @@ const MAP_TABLE_FIELDS: Partial> = { table: 'text_replacements', comment: ['Text replacements expanded while typing, keyed by trigger.'], example: '"->" = "→"' + }, + savedTaskFilters: { + table: 'saved_filters', + comment: [ + 'Saved Tasks filters: a name you pick = the filter query it stands for.', + 'Recall one from the chips above the task list, the command palette,', + 'or `:filter ` in the Tasks view; `:savefilter ` adds one.' + ], + example: '"Project alpha" = "@project:alpha"' } } diff --git a/packages/app-core/src/components/TasksView.tsx b/packages/app-core/src/components/TasksView.tsx index 8911d4fa..c56fedce 100644 --- a/packages/app-core/src/components/TasksView.tsx +++ b/packages/app-core/src/components/TasksView.tsx @@ -13,6 +13,9 @@ import { ContextMenu, type ContextMenuItem } from './ContextMenu' import { buildTaskMenuItems } from '../lib/task-context-menu' import { isImeComposing } from '../lib/ime' import { isAppOverlayOpen } from '../lib/overlay-open' +import { promptApp } from '../lib/prompt-requests' +import { useToastStore } from '../lib/toast' +import { findSavedTaskFilterName, savedTaskFilterNameForQuery } from '../lib/saved-task-filters' type GroupKey = 'today' | 'upcoming' | 'waiting' | 'forwarded' | 'done' | 'cancelled' @@ -48,6 +51,11 @@ export function TasksView(): JSX.Element { const filter = useStore((s) => s.tasksFilter) const cursorIndex = useStore((s) => s.taskCursorIndex) const setFilter = useStore((s) => s.setTasksFilter) + const savedFilters = useStore((s) => s.savedTaskFilters) + const saveTaskFilter = useStore((s) => s.saveTaskFilter) + const renameSavedTaskFilter = useStore((s) => s.renameSavedTaskFilter) + const deleteSavedTaskFilter = useStore((s) => s.deleteSavedTaskFilter) + const applySavedTaskFilter = useStore((s) => s.applySavedTaskFilter) const setCursorIndex = useStore((s) => s.setTaskCursorIndex) const refreshTasks = useStore((s) => s.refreshTasks) const openTaskAt = useStore((s) => s.openTaskAt) @@ -189,6 +197,107 @@ export function TasksView(): JSX.Element { [today, vimMode] ) + // Saved filters (#731): named queries kept in the prefs and mirrored to + // config.toml as [saved_filters]. The chip row shows them in stored order; + // the chip whose query is the current filter reads as active. + const savedEntries = useMemo(() => Object.entries(savedFilters), [savedFilters]) + const activeSavedName = useMemo( + () => savedTaskFilterNameForQuery(savedFilters, filter), + [savedFilters, filter] + ) + const toast = useCallback((message: string, kind: 'info' | 'success' = 'info'): void => { + useToastStore.getState().addToast(message, kind) + }, []) + // The name prompt lists the existing names, so picking one overwrites it + // with the current query instead of creating a near-duplicate. + const saveCurrentFilter = useCallback( + async (name?: string): Promise => { + const query = filter.trim() + if (!query) { + toast('Type a filter first, then save it') + return + } + let chosen = (name ?? '').trim() + if (!chosen) { + const names = Object.keys(savedFilters) + const answer = await promptApp({ + title: 'Save filter', + description: `A name for "${query}". Saved filters live in config.toml under [saved_filters].`, + placeholder: 'Project alpha', + okLabel: 'Save', + suggestions: names.map((n) => ({ value: n, detail: savedFilters[n] })), + suggestionsHint: names.length > 0 ? 'Pick an existing name to overwrite it.' : undefined + }) + if (answer === null) return + chosen = answer.trim() + if (!chosen) return + } + saveTaskFilter(chosen, query) + toast(`Saved filter "${chosen}"`, 'success') + }, + [filter, savedFilters, saveTaskFilter, toast] + ) + const pickSavedFilter = useCallback(async (): Promise => { + const names = Object.keys(savedFilters) + if (names.length === 0) { + toast('No saved filters yet. Type a filter, then press Save filter (or :savefilter )') + return + } + const answer = await promptApp({ + title: 'Saved filters', + placeholder: 'Name', + okLabel: 'Apply', + suggestions: names.map((n) => ({ value: n, detail: savedFilters[n] })), + autoHighlightFirst: true, + suggestionsHint: 'Type to narrow · ↑↓ move · Enter applies' + }) + if (answer === null) return + if (!applySavedTaskFilter(answer)) toast(`No saved filter called "${answer.trim()}"`) + }, [savedFilters, applySavedTaskFilter, toast]) + const renameSavedFilter = useCallback( + async (name: string): Promise => { + const answer = await promptApp({ + title: 'Rename saved filter', + initialValue: name, + okLabel: 'Rename' + }) + if (answer === null) return + const next = answer.trim() + if (!next || next === name) return + renameSavedTaskFilter(name, next) + }, + [renameSavedTaskFilter] + ) + const openSavedFilterMenu = useCallback( + (e: React.MouseEvent, name: string): void => { + e.preventDefault() + e.stopPropagation() + setMenu({ + x: e.clientX, + y: e.clientY, + items: [ + { + label: 'Apply', + onSelect: () => { + applySavedTaskFilter(name) + } + }, + { label: 'Rename…', onSelect: () => void renameSavedFilter(name) }, + { kind: 'separator' }, + { + label: 'Delete', + danger: true, + onSelect: () => { + deleteSavedTaskFilter(name) + toast(`Deleted saved filter "${name}"`) + } + } + ] + }) + }, + [applySavedTaskFilter, renameSavedFilter, deleteSavedTaskFilter, toast] + ) + // On first mount, pull fresh if we have nothing yet. useEffect(() => { if (tasks.length === 0 && !loading) void refreshTasks() @@ -312,8 +421,32 @@ export function TasksView(): JSX.Element { // input so the query lands in the box as typed. const spaceIdx = input.indexOf(' ') const head = (spaceIdx === -1 ? input : input.slice(0, spaceIdx)).toLowerCase() + const arg = spaceIdx === -1 ? '' : input.slice(spaceIdx + 1).trim() if (head === 'filter' || head === 'f') { - setFilter(spaceIdx === -1 ? '' : input.slice(spaceIdx + 1).trim()) + // A saved filter's name applies its query (#731); any other text is + // the query itself, as before. + if (arg && applySavedTaskFilter(arg)) return + setFilter(arg) + return + } + // `:savefilter ` keeps the current query under that name (bare, it + // asks for one); `:delfilter ` forgets it. (#731) + if (head === 'savefilter' || head === 'sf') { + void saveCurrentFilter(arg) + return + } + if (head === 'delfilter' || head === 'df') { + if (!arg) { + toast('Usage: :delfilter ') + return + } + const stored = findSavedTaskFilterName(savedFilters, arg) + if (!stored) { + toast(`No saved filter called "${arg}"`) + return + } + deleteSavedTaskFilter(stored) + toast(`Deleted saved filter "${stored}"`) return } const cmd = input.toLowerCase() @@ -384,7 +517,17 @@ export function TasksView(): JSX.Element { return } }, - [closeTasksView, refreshTasks, setFilter, setViewMode] + [ + closeTasksView, + refreshTasks, + setFilter, + setViewMode, + applySavedTaskFilter, + saveCurrentFilter, + deleteSavedTaskFilter, + savedFilters, + toast + ] ) // Window-level handler with two responsibilities: @@ -477,6 +620,13 @@ export function TasksView(): JSX.Element { return } + // View-independent like the filter box it recalls into. (#731) + if (seq('tasks.savedFilters')) { + consume() + void pickSavedFilter() + return + } + if (seq('nav.localEx')) { consume() setExValue('') @@ -582,7 +732,8 @@ export function TasksView(): JSX.Element { setFilter, viewMode, setViewMode, - newTaskFile + newTaskFile, + pickSavedFilter ]) return ( @@ -681,6 +832,58 @@ export function TasksView(): JSX.Element {
+ {/* Saved filters (#731): recall with a click, `:filter `, or F. + The row appears once there is something to recall, or a query + worth keeping. */} + {(savedEntries.length > 0 || filter.trim()) && ( +
+ {savedEntries.length > 0 && ( + + Saved + + )} + {savedEntries.map(([name, query]) => { + const active = name === activeSavedName + return ( + + ) + })} + {filter.trim() && !activeSavedName && ( + + )} +
+ )} + {viewMode === 'list' && (
{render.rows.length === 0 && !loading && ( @@ -801,14 +1004,14 @@ export function TasksView(): JSX.Element {
{viewMode === 'list' ? vimMode - ? 'j/k move · J/K reorder · Enter/o open · x toggle · i start · c cancel · right-click actions · :q close' + ? 'j/k move · J/K reorder · Enter/o open · x toggle · i start · c cancel · F saved filters · right-click actions · :q close' : '↑/↓ move · Shift+J/K reorder · Enter open · right-click actions' : viewMode === 'calendar' ? vimMode - ? 'h/j/k/l day · [ ] month · Tab pick · x toggle · i start · c cancel · drag to move · right-click actions · :q' + ? 'h/j/k/l day · [ ] month · Tab pick · x toggle · i start · c cancel · F saved filters · drag to move · right-click actions · :q' : 'h/j/k/l day · [ ] month · Tab pick · x toggle · drag to move · right-click actions' : vimMode - ? 'h/l column · j/k card · x toggle · i start · c cancel · Enter open · right-click actions · :q close' + ? 'h/l column · j/k card · x toggle · i start · c cancel · Enter open · F saved filters · right-click actions · :q close' : 'h/l column · j/k card · x toggle · Enter open · right-click actions'}
)} diff --git a/packages/app-core/src/lib/commands.test.ts b/packages/app-core/src/lib/commands.test.ts index ef3d0028..37eba34a 100644 --- a/packages/app-core/src/lib/commands.test.ts +++ b/packages/app-core/src/lib/commands.test.ts @@ -350,3 +350,22 @@ describe('note commands for a trashed note (#712)', () => { }) }) + +describe('saved Tasks filters (#731)', () => { + it('lists one palette entry per saved filter, which opens Tasks and applies it', async () => { + const { buildCommands, useStore } = await loadCommands() + expect(buildCommands().some((c) => c.id.startsWith('tasks.savedFilter.'))).toBe(false) + + useStore.getState().saveTaskFilter('Blocked', '@status:blocked') + useStore.getState().saveTaskFilter('Project alpha', '@project:alpha') + const entries = buildCommands().filter((c) => c.id.startsWith('tasks.savedFilter.')) + expect(entries.map((c) => c.title)).toEqual(['Tasks: Blocked', 'Tasks: Project alpha']) + expect(entries[1].keywords).toContain('@project:alpha') + + const openTasksView = vi.fn().mockResolvedValue(undefined) + useStore.setState({ openTasksView }) + await entries[1].run() + expect(openTasksView).toHaveBeenCalledTimes(1) + expect(useStore.getState().tasksFilter).toBe('@project:alpha') + }) +}) diff --git a/packages/app-core/src/lib/commands.ts b/packages/app-core/src/lib/commands.ts index a4ff90d6..a9eeb6cd 100644 --- a/packages/app-core/src/lib/commands.ts +++ b/packages/app-core/src/lib/commands.ts @@ -1340,6 +1340,22 @@ export function buildCommands(options?: { includeUnavailable?: boolean }): Comma } ) + // Saved Tasks filters (#731): one palette entry per name, so a filter is a + // few keystrokes away from any note. Opening the view resets the filter, so + // the query is applied once the open has settled. + for (const [name, query] of Object.entries(getState().savedTaskFilters)) { + cmds.push({ + id: `tasks.savedFilter.${name}`, + title: `${labels().tasks}: ${name}`, + category: 'View', + keywords: `saved filter tasks ${query}`, + run: async () => { + if (!isTasksViewActive(getState())) await getState().openTasksView() + getState().applySavedTaskFilter(name) + } + }) + } + /* ---------------- Editor preferences ---------------- */ cmds.push( { diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index bf7b3de6..df5d6563 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -292,7 +292,7 @@ export const HELP_CORE_CONCEPTS: HelpCard[] = [ { title: 'Filter the Tasks views to one project', body: - 'The filter box in the Tasks header narrows all three sub-views: the list, the calendar, and the Kanban board (where cards filter out but the columns stay put, so the board keeps its shape while you type). Press `/` to focus it (Vim mode), type in it directly, or run `:filter ` from the ex line; `Esc` or a bare `:filter` clears it, and the query survives switching views, so a filtered list stays filtered when you jump to the board. Matching is a simple substring check across the task text, the note title, tags (with the `#`, so `#project-beta` narrows to that tag), `!high`-style priorities, and `@key:value` fields. The fields are the project trick: tag tasks with `@project:alpha`, then filter `@project:alpha` while the board is grouped by status, and you have a one-project board; the header shows how many tasks match. A filtered board is still fully live: drag or `Shift+H`/`L` still move cards, and hand-arranged card order is preserved for the cards the filter is hiding.' + 'The filter box in the Tasks header narrows all three sub-views: the list, the calendar, and the Kanban board (where cards filter out but the columns stay put, so the board keeps its shape while you type). Press `/` to focus it (Vim mode), type in it directly, or run `:filter ` from the ex line; `Esc` or a bare `:filter` clears it, and the query survives switching views, so a filtered list stays filtered when you jump to the board. Matching is a simple substring check across the task text, the note title, tags (with the `#`, so `#project-beta` narrows to that tag), `!high`-style priorities, and `@key:value` fields. The fields are the project trick: tag tasks with `@project:alpha`, then filter `@project:alpha` while the board is grouped by status, and you have a one-project board; the header shows how many tasks match. A filtered board is still fully live: drag or `Shift+H`/`L` still move cards, and hand-arranged card order is preserved for the cards the filter is hiding. A query worth typing twice is worth saving: press **Save filter…** in the row under the header (or run `:savefilter `) and it becomes a chip there; click a chip, run `:filter `, or press `F` (Vim mode) to pick one from a list, and the command palette lists every saved filter as "Tasks: name" so you can jump to it from any note. Right-click a chip to rename or delete it. Saved filters are kept in config.toml under `[saved_filters]`, one `"Name" = "query"` line each, so they travel with your dotfiles and a hand edit applies live.' }, { title: 'Forward a task to another note', @@ -656,6 +656,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ { keys: 'r', action: 'Restore trashed note', detail: 'Trash view only: restore the selected trashed note.' }, { keys: 'x / d', action: 'Delete forever', detail: 'Trash view only: permanently delete the selected trashed note after confirmation.' }, { keys: '/', action: 'Filter the view', detail: 'Focus the local filter box for tasks, tag matches, or trashed notes.' }, + { keys: 'F', action: 'Pick a saved Tasks filter', detail: 'Tasks view: open the list of saved filters and apply one. Save the current query with the Save filter… chip or `:savefilter `.' }, { keys: ':', action: 'Open local ex prompt', detail: 'Run the view-specific command line inside Tasks or Tags.' }, { keys: 'Esc', action: 'Clear the filter', detail: 'Clears an active filter. These views are tabs, so Esc no longer closes them — close with :q or the ✕ in the tab header.' } ] @@ -739,6 +740,21 @@ export const HELP_VIM_COMMANDS: HelpExCommand[] = [ summary: 'Remove an action’s key entirely', detail: 'Leave an action with no key at all, instead of parking it on some obscure chord: `:unbind global.zoomIn` takes Zoom in off its key until it is rebound or reset under Settings, Keymaps. Bare `:unbind` opens that page, which lists every action id. The unbind travels in `config.toml` as `\"global.zoomIn\" = \"\"`.' }, + { + command: ':filter ', + summary: 'Filter the Tasks views', + detail: 'In the Tasks view, narrows the list, the calendar, and the Kanban board to tasks matching the text; when the text is the name of a saved filter, that filter is applied instead. Bare `:filter` (or `:f`) clears it.' + }, + { + command: ':savefilter ', + summary: 'Save the current Tasks filter under a name', + detail: 'Keeps the query in the Tasks filter box as a saved filter called `` (bare `:savefilter`, or `:sf`, asks for the name). It shows up as a chip under the Tasks header, in the command palette as “Tasks: name”, and in config.toml under `[saved_filters]`.' + }, + { + command: ':delfilter ', + summary: 'Delete a saved Tasks filter', + detail: 'Forgets the saved filter called `` (`:df` for short). Deleting the line under `[saved_filters]` in config.toml does the same.' + }, { command: ':q', summary: 'Close the current tab or virtual view', diff --git a/packages/app-core/src/lib/keymaps.ts b/packages/app-core/src/lib/keymaps.ts index 4be423fd..d1e9ce4d 100644 --- a/packages/app-core/src/lib/keymaps.ts +++ b/packages/app-core/src/lib/keymaps.ts @@ -117,6 +117,7 @@ export type KeymapId = | "nav.unarchive" | "tasks.moveTaskUp" | "tasks.moveTaskDown" + | "tasks.savedFilters" | "editor.moveLineUp" | "editor.moveLineDown" | "editor.hopMarkerForward" @@ -1187,6 +1188,17 @@ const KEYMAP_DEFINITIONS: KeymapDefinition[] = [ defaultBinding: "J", maxTokens: 1, }, + { + id: "tasks.savedFilters", + kind: "sequence", + scope: "views", + group: "view-actions", + title: "Pick a saved Tasks filter", + description: + "Open the picker of saved Tasks filters and apply one (Tasks view, Vim mode). Save the current filter with :savefilter .", + defaultBinding: "F", + maxTokens: 1, + }, { id: "editor.hopMarkerForward", kind: "shortcut", diff --git a/packages/app-core/src/lib/saved-task-filters.test.ts b/packages/app-core/src/lib/saved-task-filters.test.ts new file mode 100644 index 00000000..422a9076 --- /dev/null +++ b/packages/app-core/src/lib/saved-task-filters.test.ts @@ -0,0 +1,103 @@ +import { describe, expect, it } from 'vitest' +import { + MAX_SAVED_TASK_FILTERS, + findSavedTaskFilterName, + normalizeSavedTaskFilters, + renameSavedTaskFilter, + savedTaskFilterNameForQuery, + savedTaskFilterQuery, + withSavedTaskFilter, + withoutSavedTaskFilter +} from './saved-task-filters' + +const filters = { 'Project alpha': '@project:alpha', Blocked: '@status:blocked', 'This week': 'due:' } + +describe('normalizeSavedTaskFilters', () => { + it('keeps string entries in their order and trims them', () => { + expect( + normalizeSavedTaskFilters({ ' Blocked ': ' @status:blocked ', Alpha: '@project:alpha' }) + ).toEqual({ Blocked: '@status:blocked', Alpha: '@project:alpha' }) + expect(Object.keys(normalizeSavedTaskFilters(filters))).toEqual([ + 'Project alpha', + 'Blocked', + 'This week' + ]) + }) + + it('drops blanks, non-strings, duplicates by case, and anything that is not a table', () => { + expect(normalizeSavedTaskFilters(null)).toEqual({}) + expect(normalizeSavedTaskFilters(['a'])).toEqual({}) + expect(normalizeSavedTaskFilters('x')).toEqual({}) + expect( + normalizeSavedTaskFilters({ '': 'q', a: '', b: 3, Blocked: 'one', blocked: 'two' }) + ).toEqual({ Blocked: 'one' }) + }) + + it('caps the number of entries', () => { + const many: Record = {} + for (let i = 0; i < MAX_SAVED_TASK_FILTERS + 5; i += 1) many[`f${i}`] = `q${i}` + expect(Object.keys(normalizeSavedTaskFilters(many))).toHaveLength(MAX_SAVED_TASK_FILTERS) + }) +}) + +describe('lookups', () => { + it('finds a name and its query regardless of case', () => { + expect(findSavedTaskFilterName(filters, 'blocked')).toBe('Blocked') + expect(findSavedTaskFilterName(filters, ' PROJECT ALPHA ')).toBe('Project alpha') + expect(findSavedTaskFilterName(filters, 'nope')).toBeNull() + expect(findSavedTaskFilterName(filters, '')).toBeNull() + expect(savedTaskFilterQuery(filters, 'BLOCKED')).toBe('@status:blocked') + expect(savedTaskFilterQuery(filters, 'nope')).toBeNull() + }) + + it('maps the current query back to the chip it came from', () => { + expect(savedTaskFilterNameForQuery(filters, '@status:blocked')).toBe('Blocked') + expect(savedTaskFilterNameForQuery(filters, ' @STATUS:blocked ')).toBe('Blocked') + expect(savedTaskFilterNameForQuery(filters, '@status:block')).toBeNull() + expect(savedTaskFilterNameForQuery(filters, '')).toBeNull() + }) +}) + +describe('edits keep the display order', () => { + it('adds a new filter at the end', () => { + const next = withSavedTaskFilter(filters, 'Urgent', '!high') + expect(Object.keys(next)).toEqual(['Project alpha', 'Blocked', 'This week', 'Urgent']) + expect(next.Urgent).toBe('!high') + expect(filters).not.toHaveProperty('Urgent') + }) + + it('overwrites an existing name in place, adopting the new spelling', () => { + const next = withSavedTaskFilter(filters, 'BLOCKED', '@status:waiting') + expect(Object.keys(next)).toEqual(['Project alpha', 'BLOCKED', 'This week']) + expect(next.BLOCKED).toBe('@status:waiting') + }) + + it('refuses blanks and a full list', () => { + expect(withSavedTaskFilter(filters, ' ', 'q')).toBe(filters) + expect(withSavedTaskFilter(filters, 'x', ' ')).toBe(filters) + const full: Record = {} + for (let i = 0; i < MAX_SAVED_TASK_FILTERS; i += 1) full[`f${i}`] = `q${i}` + expect(withSavedTaskFilter(full, 'one more', 'q')).toBe(full) + expect(Object.keys(withSavedTaskFilter(full, 'F3', 'changed'))).toHaveLength(MAX_SAVED_TASK_FILTERS) + }) + + it('removes by name in any case and ignores unknown names', () => { + expect(Object.keys(withoutSavedTaskFilter(filters, 'blocked'))).toEqual(['Project alpha', 'This week']) + expect(withoutSavedTaskFilter(filters, 'nope')).toBe(filters) + }) + + it('renames in place and refuses a taken or blank name', () => { + const next = renameSavedTaskFilter(filters, 'blocked', 'Waiting on others') + expect(Object.keys(next)).toEqual(['Project alpha', 'Waiting on others', 'This week']) + expect(next['Waiting on others']).toBe('@status:blocked') + expect(renameSavedTaskFilter(filters, 'Blocked', 'this WEEK')).toBe(filters) + expect(renameSavedTaskFilter(filters, 'Blocked', ' ')).toBe(filters) + expect(renameSavedTaskFilter(filters, 'nope', 'x')).toBe(filters) + // Re-casing a name is a rename onto itself. + expect(Object.keys(renameSavedTaskFilter(filters, 'Blocked', 'BLOCKED'))).toEqual([ + 'Project alpha', + 'BLOCKED', + 'This week' + ]) + }) +}) diff --git a/packages/app-core/src/lib/saved-task-filters.ts b/packages/app-core/src/lib/saved-task-filters.ts new file mode 100644 index 00000000..874ff145 --- /dev/null +++ b/packages/app-core/src/lib/saved-task-filters.ts @@ -0,0 +1,124 @@ +/** + * Saved Tasks filters (#731): a name the user picks, and the filter query it + * stands for. The map lives in the portable prefs and mirrors to config.toml + * as the `[saved_filters]` table, so it is hand-editable and travels with + * dotfiles. Names are matched case-insensitively wherever a user types one + * (`:filter `, the picker), while the stored spelling is what the + * chips show. Insertion order is the display order, and the helpers below + * keep it through a rename or an overwrite so a chip does not jump to the + * end when its query is updated. + */ + +export type SavedTaskFilters = Record + +export const MAX_SAVED_TASK_FILTERS = 50 +export const MAX_SAVED_TASK_FILTER_NAME_LENGTH = 60 +export const MAX_SAVED_TASK_FILTER_QUERY_LENGTH = 200 + +/** Validate an untrusted value (config.toml, localStorage) into a clean map. */ +export function normalizeSavedTaskFilters(value: unknown): SavedTaskFilters { + if (!value || typeof value !== 'object' || Array.isArray(value)) return {} + const normalized: SavedTaskFilters = {} + for (const [rawName, rawQuery] of Object.entries(value as Record)) { + if (typeof rawQuery !== 'string') continue + const name = rawName.trim() + const query = rawQuery.trim() + if (!name || !query) continue + if (name.length > MAX_SAVED_TASK_FILTER_NAME_LENGTH) continue + if (findSavedTaskFilterName(normalized, name)) continue + normalized[name] = query.slice(0, MAX_SAVED_TASK_FILTER_QUERY_LENGTH) + if (Object.keys(normalized).length >= MAX_SAVED_TASK_FILTERS) break + } + return normalized +} + +/** The stored spelling of `name`, matched case-insensitively, or null. */ +export function findSavedTaskFilterName(filters: SavedTaskFilters, name: string): string | null { + const wanted = name.trim().toLowerCase() + if (!wanted) return null + for (const stored of Object.keys(filters)) { + if (stored.toLowerCase() === wanted) return stored + } + return null +} + +/** The query saved under `name` (case-insensitive), or null. */ +export function savedTaskFilterQuery(filters: SavedTaskFilters, name: string): string | null { + const stored = findSavedTaskFilterName(filters, name) + return stored === null ? null : filters[stored] +} + +/** + * The saved filter whose query is `query` (trimmed, case-insensitive, since + * matching ignores case), or null. Drives the highlighted chip. + */ +export function savedTaskFilterNameForQuery( + filters: SavedTaskFilters, + query: string +): string | null { + const wanted = query.trim().toLowerCase() + if (!wanted) return null + for (const [name, saved] of Object.entries(filters)) { + if (saved.trim().toLowerCase() === wanted) return name + } + return null +} + +/** + * `filters` with `name` set to `query`. A name already present (in any + * casing) keeps its position and takes the new spelling and query; a new + * name goes last. + */ +export function withSavedTaskFilter( + filters: SavedTaskFilters, + name: string, + query: string +): SavedTaskFilters { + const cleanName = name.trim().slice(0, MAX_SAVED_TASK_FILTER_NAME_LENGTH) + const cleanQuery = query.trim().slice(0, MAX_SAVED_TASK_FILTER_QUERY_LENGTH) + if (!cleanName || !cleanQuery) return filters + const existing = findSavedTaskFilterName(filters, cleanName) + const next: SavedTaskFilters = {} + for (const [stored, saved] of Object.entries(filters)) { + if (stored === existing) next[cleanName] = cleanQuery + else next[stored] = saved + } + if (existing === null) { + if (Object.keys(next).length >= MAX_SAVED_TASK_FILTERS) return filters + next[cleanName] = cleanQuery + } + return next +} + +/** `filters` without `name` (case-insensitive); unchanged when absent. */ +export function withoutSavedTaskFilter(filters: SavedTaskFilters, name: string): SavedTaskFilters { + const stored = findSavedTaskFilterName(filters, name) + if (stored === null) return filters + const next: SavedTaskFilters = {} + for (const [key, saved] of Object.entries(filters)) { + if (key !== stored) next[key] = saved + } + return next +} + +/** + * `filters` with `from` renamed to `to`, keeping its position and query. + * Unchanged when `from` is unknown, `to` is blank, or `to` already names a + * different filter. + */ +export function renameSavedTaskFilter( + filters: SavedTaskFilters, + from: string, + to: string +): SavedTaskFilters { + const stored = findSavedTaskFilterName(filters, from) + const cleanTo = to.trim().slice(0, MAX_SAVED_TASK_FILTER_NAME_LENGTH) + if (stored === null || !cleanTo) return filters + const taken = findSavedTaskFilterName(filters, cleanTo) + if (taken !== null && taken !== stored) return filters + const next: SavedTaskFilters = {} + for (const [key, saved] of Object.entries(filters)) { + next[key === stored ? cleanTo : key] = saved + } + return next +} diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index a8140695..58a34a46 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -226,6 +226,14 @@ import { normalizeTextReplacements, type TextReplacements } from './lib/cm-text-replacements' +import { + normalizeSavedTaskFilters, + renameSavedTaskFilter as renameSavedTaskFilterEntry, + savedTaskFilterQuery, + withSavedTaskFilter, + withoutSavedTaskFilter, + type SavedTaskFilters +} from './lib/saved-task-filters' import { normalizeEditorTabSize } from './lib/editor-tab-size' import { recentNoteToggleTarget } from './lib/recent-note-toggle' @@ -569,6 +577,8 @@ interface Prefs { textReplacementsEnabled: boolean /** Trigger to replacement mappings, such as `->` to `→`. */ textReplacements: TextReplacements + /** Saved Tasks filters by name, the `[saved_filters]` table in config.toml (#731). */ + savedTaskFilters: SavedTaskFilters /** Auto-insert matching `[]`, `()`, and `{}` delimiters while typing. */ autoPairs: boolean /** Also auto-insert matching quotes outside Markdown code spans and blocks. */ @@ -1012,6 +1022,7 @@ export const DEFAULT_PREFS: Prefs = { markdownSnippets: true, textReplacementsEnabled: true, textReplacements: { '->': '→' }, + savedTaskFilters: {}, autoPairs: true, autoPairQuotesInProse: false, hideBuiltinTemplates: false, @@ -1204,6 +1215,9 @@ function normalizePrefs(p: Partial): Prefs { textReplacements: normalizeTextReplacements( p.textReplacements ?? DEFAULT_PREFS.textReplacements ), + savedTaskFilters: normalizeSavedTaskFilters( + p.savedTaskFilters ?? DEFAULT_PREFS.savedTaskFilters + ), autoPairs: typeof p.autoPairs === 'boolean' ? p.autoPairs : DEFAULT_PREFS.autoPairs, autoPairQuotesInProse: typeof p.autoPairQuotesInProse === 'boolean' @@ -2223,6 +2237,7 @@ function collectPrefs(s: { markdownSnippets: boolean textReplacementsEnabled: boolean textReplacements: TextReplacements + savedTaskFilters: SavedTaskFilters autoPairs: boolean autoPairQuotesInProse: boolean hideBuiltinTemplates: boolean @@ -2321,6 +2336,7 @@ function collectPrefs(s: { markdownSnippets: s.markdownSnippets, textReplacementsEnabled: s.textReplacementsEnabled, textReplacements: s.textReplacements, + savedTaskFilters: s.savedTaskFilters, autoPairs: s.autoPairs, autoPairQuotesInProse: s.autoPairQuotesInProse, hideBuiltinTemplates: s.hideBuiltinTemplates, @@ -2845,6 +2861,7 @@ interface Store { markdownSnippets: boolean textReplacementsEnabled: boolean textReplacements: TextReplacements + savedTaskFilters: SavedTaskFilters /** Auto-insert matching `[]`, `()`, and `{}` delimiters while typing. Persisted. */ autoPairs: boolean /** Also auto-insert matching quotes outside Markdown code spans and blocks. Persisted. */ @@ -3352,6 +3369,13 @@ interface Store { setMarkdownSnippets: (on: boolean) => void setTextReplacementsEnabled: (on: boolean) => void setTextReplacements: (replacements: TextReplacements) => void + /** Saved Tasks filters (#731). Names match case-insensitively; edits keep the + * chip order, and every change is mirrored to config.toml with the prefs. */ + saveTaskFilter: (name: string, query: string) => void + renameSavedTaskFilter: (from: string, to: string) => void + deleteSavedTaskFilter: (name: string) => void + /** Set the Tasks filter to the query saved under `name`; false when unknown. */ + applySavedTaskFilter: (name: string) => boolean setAutoPairs: (on: boolean) => void setAutoPairQuotesInProse: (on: boolean) => void setHideBuiltinTemplates: (hidden: boolean) => void @@ -4683,6 +4707,7 @@ export const useStore = create((set, get) => { markdownSnippets: loadPrefs().markdownSnippets, textReplacementsEnabled: loadPrefs().textReplacementsEnabled, textReplacements: loadPrefs().textReplacements, + savedTaskFilters: loadPrefs().savedTaskFilters, autoPairs: loadPrefs().autoPairs, autoPairQuotesInProse: loadPrefs().autoPairQuotesInProse, hideBuiltinTemplates: loadPrefs().hideBuiltinTemplates, @@ -7385,6 +7410,30 @@ export const useStore = create((set, get) => { set({ textReplacements: normalizeTextReplacements(replacements) }) savePrefs(collectPrefs(get())) }, + saveTaskFilter: (name, query) => { + const next = withSavedTaskFilter(get().savedTaskFilters, name, query) + if (next === get().savedTaskFilters) return + set({ savedTaskFilters: next }) + savePrefs(collectPrefs(get())) + }, + renameSavedTaskFilter: (from, to) => { + const next = renameSavedTaskFilterEntry(get().savedTaskFilters, from, to) + if (next === get().savedTaskFilters) return + set({ savedTaskFilters: next }) + savePrefs(collectPrefs(get())) + }, + deleteSavedTaskFilter: (name) => { + const next = withoutSavedTaskFilter(get().savedTaskFilters, name) + if (next === get().savedTaskFilters) return + set({ savedTaskFilters: next }) + savePrefs(collectPrefs(get())) + }, + applySavedTaskFilter: (name) => { + const query = savedTaskFilterQuery(get().savedTaskFilters, name) + if (query === null) return false + set({ tasksFilter: query, taskCursorIndex: 0 }) + return true + }, setAutoPairs: (on) => { set({ autoPairs: on }) savePrefs(collectPrefs(get())) diff --git a/packages/shared-domain/src/app-config.ts b/packages/shared-domain/src/app-config.ts index cd51bb55..a773a987 100644 --- a/packages/shared-domain/src/app-config.ts +++ b/packages/shared-domain/src/app-config.ts @@ -144,7 +144,9 @@ export const PORTABLE_PREF_KEYS = [ 'showArchivedTasks', 'kanbanGroupBy', 'kanbanColumnTitles', - 'kanbanStatuses' + 'kanbanStatuses', + // tasks + 'savedTaskFilters' ] as const export type PortablePrefKey = (typeof PORTABLE_PREF_KEYS)[number] @@ -260,5 +262,6 @@ export const PORTABLE_DEFAULTS: Record = { showArchivedTasks: false, kanbanGroupBy: 'status', kanbanColumnTitles: {}, - kanbanStatuses: [] + kanbanStatuses: [], + savedTaskFilters: {} } diff --git a/packages/shared-domain/src/keymaps-catalog.ts b/packages/shared-domain/src/keymaps-catalog.ts index 35c5210a..95a93eff 100644 --- a/packages/shared-domain/src/keymaps-catalog.ts +++ b/packages/shared-domain/src/keymaps-catalog.ts @@ -146,6 +146,7 @@ export const KEYMAP_CATALOG: KeymapCatalogEntry[] = [ { id: "nav.toggleTask", group: "view-actions", defaultBinding: "x", title: "Toggle task" }, { id: "tasks.moveTaskUp", group: "view-actions", defaultBinding: "K", title: "Move task up" }, { id: "tasks.moveTaskDown", group: "view-actions", defaultBinding: "J", title: "Move task down" }, + { id: "tasks.savedFilters", group: "view-actions", defaultBinding: "F", title: "Pick a saved Tasks filter" }, { id: "editor.hopMarkerForward", group: "view-actions", defaultBinding: "Alt+]", defaultBindingMac: "Ctrl+.", title: "Hop past next marker" }, { id: "editor.hopMarkerBackward", group: "view-actions", defaultBinding: "Alt+[", defaultBindingMac: "Ctrl+,", title: "Hop before previous marker" }, { id: "editor.foldHeading", group: "view-actions", defaultBinding: "Alt+Mod+F", title: "Fold heading" }, From 40410ce7b610b8a9d8b29b9f9fdcd0f00f3fa90b Mon Sep 17 00:00:00 2001 From: Adib Hanna Date: Tue, 8 Sep 2026 14:05:41 -0500 Subject: [PATCH 08/12] Feat(comments): discuss a note with an assistant through its comments (#738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A note's comments were a one-voice affair: flat, unsigned, and reachable only from the app. Reviewing a draft with an assistant meant pasting the note into a chat and carrying the answers back by hand, which is exactly the loop the comments panel exists to avoid. Comments now thread and carry a name. A record has an optional author (absent for the vault's owner, the assistant's name otherwise) and an optional parentId that files a reply under a top-level comment; the panel shows each thread as one card with its replies, the avatar and name on every entry, and `a` (or the Reply arrow) opens a reply box under the selected thread with ⌘↵ to send. A reply keeps its thread's anchor, so the editor draws one marker per conversation and re-anchoring moves a thread together. The MCP server gets four tools on the same sidecar: list_comments (each thread with the anchored passage, the line it sits on now, who wrote what, and the replies), add_comment (a new thread anchored to a passage copied from the note, or note-level), reply_to_comment and resolve_comment. Replies are signed with the connected client's name from the initialize handshake (claude-code reads as Claude Code, claude-ai as Claude, codex-cli as Codex), so its words and the user's stay apart in the panel. The server instructions tell a model to hold a review through the threads rather than by editing the body, and to resolve only when the user says so. `zn comment list|add|reply|resolve` exposes the same operations, and both work against a folder or a ZenNotes server. The record shape is one module now, shared-domain/note-comments.ts, used by the desktop main process, the MCP server and the CLI, with the Go server as its mirror; the anchor helpers moved there from app-core. A reply whose parent is missing stays as a comment of its own rather than vanishing, on every side. Both docs surfaces describe threads, the `a` key and the tools. Verified with unit tests (shared normalizer and threading, comment-ops on a temp vault, the MCP tool list and reply attribution, the CLI group, the Go round trip) and in the built app with a real MCP session: a comment from the keyboard, the assistant's reply and its own anchored thread arriving live in the panel, a reply from the panel, and a resolve moving the thread. Closes #738 --- apps/desktop/src/cli/backend.ts | 15 + .../desktop/src/cli/commands/comments.test.ts | 84 +++++ apps/desktop/src/cli/commands/comments.ts | 106 +++++++ apps/desktop/src/cli/help.ts | 11 + apps/desktop/src/cli/index.ts | 11 + apps/desktop/src/cli/remote/client.ts | 12 + apps/desktop/src/main/vault.ts | 54 +--- apps/desktop/src/mcp/comment-ops.test.ts | 116 +++++++ apps/desktop/src/mcp/comment-ops.ts | 161 ++++++++++ apps/desktop/src/mcp/instructions.ts | 22 ++ apps/desktop/src/mcp/server.test.ts | 60 +++- apps/desktop/src/mcp/server.ts | 142 +++++++++ apps/desktop/src/mcp/vault-ops.ts | 47 +++ apps/server/internal/vault/types.go | 5 + apps/server/internal/vault/vault.go | 26 ++ apps/server/internal/vault/vault_test.go | 36 +++ .../app-core/src/components/CommentsPanel.tsx | 286 ++++++++++++++++-- .../app-core/src/components/EditorPane.tsx | 4 +- packages/app-core/src/components/VimNav.tsx | 7 + packages/app-core/src/lib/comments.ts | 52 +--- packages/app-core/src/lib/help.ts | 5 +- packages/bridge-contract/src/ipc.ts | 7 + .../shared-domain/src/note-comments.test.ts | 108 +++++++ packages/shared-domain/src/note-comments.ts | 208 +++++++++++++ 24 files changed, 1456 insertions(+), 129 deletions(-) create mode 100644 apps/desktop/src/cli/commands/comments.test.ts create mode 100644 apps/desktop/src/cli/commands/comments.ts create mode 100644 apps/desktop/src/mcp/comment-ops.test.ts create mode 100644 apps/desktop/src/mcp/comment-ops.ts create mode 100644 packages/shared-domain/src/note-comments.test.ts create mode 100644 packages/shared-domain/src/note-comments.ts diff --git a/apps/desktop/src/cli/backend.ts b/apps/desktop/src/cli/backend.ts index bf45889c..6bc4ab23 100644 --- a/apps/desktop/src/cli/backend.ts +++ b/apps/desktop/src/cli/backend.ts @@ -40,6 +40,7 @@ import { prependToNote, readDatabaseVaultLayout, readNote, + readNoteComments, readPrimaryNotesLocation, readVaultFileTextOrNull, renameFolder, @@ -55,7 +56,10 @@ import { toggleTaskInBody, unarchiveNote, writeNote, + writeNoteComments, writeVaultFileText, + type NoteComment, + type NoteCommentInput, type NoteContent, type NoteFolder, type NoteMeta, @@ -148,6 +152,9 @@ export interface VaultBackend { backlinks(rel: string): Promise scanAllTasks(opts?: { includeExcluded?: boolean }): Promise toggleTask(taskId: string): Promise + /** A note's comments as stored (#738); `writeComments` replaces the list. */ + listComments(rel: string): Promise + writeComments(rel: string, comments: NoteCommentInput[]): Promise /** Database (`.base`) operations, composed from this backend's file IO via * @shared/database-ops — the same composition the web and desktop remote * clients use, so `zn base` writes the identical on-disk format. (#556) */ @@ -270,6 +277,9 @@ class LocalBackend implements VaultBackend { scanAllTasks = (opts?: { includeExcluded?: boolean }): Promise => scanAllTasks(this.root, opts) toggleTask = (taskId: string): Promise => toggleTask(this.root, taskId) + listComments = (rel: string): Promise => readNoteComments(this.root, rel) + writeComments = (rel: string, comments: NoteCommentInput[]): Promise => + writeNoteComments(this.root, rel, comments) private dbOps: DatabaseOps | null = null databaseOps = (): DatabaseOps => { @@ -417,6 +427,11 @@ class RemoteBackend implements VaultBackend { scanAllTasks = (opts?: { includeExcluded?: boolean }): Promise => this.client.scanTasks(opts) + listComments = (rel: string): Promise => + this.client.readComments(normalizeRelPath(rel)) + writeComments = (rel: string, comments: NoteCommentInput[]): Promise => + this.client.writeComments(normalizeRelPath(rel), comments) + /** No task-toggle endpoint exists, so the note is read, the same transform a * local toggle applies is applied here, and the server re-parses the result * — which keeps the Go and TypeScript task parsers honest with each other. */ diff --git a/apps/desktop/src/cli/commands/comments.test.ts b/apps/desktop/src/cli/commands/comments.test.ts new file mode 100644 index 00000000..908d27df --- /dev/null +++ b/apps/desktop/src/cli/commands/comments.test.ts @@ -0,0 +1,84 @@ +import { promises as fsp } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createBackend, type VaultBackend } from '../backend' +import type { ParsedArgs } from '../args' +import { cmdCommentAdd, cmdCommentList, cmdCommentReply, cmdCommentResolve } from './comments' + +// `zn comment` (#738) against a folder vault: the same sidecar the app and +// the MCP tools read, so a thread started here shows up in the panel. + +function args(positionals: string[], flags: Array<[string, string]> = []): ParsedArgs { + return { positionals, flags: new Map(flags.map(([k, v]) => [k, [v]])) } +} + +let root: string +let backend: VaultBackend +let out: string[] + +beforeEach(async () => { + root = await fsp.mkdtemp(path.join(os.tmpdir(), 'zen-comment-cli-')) + await fsp.mkdir(path.join(root, 'inbox'), { recursive: true }) + await fsp.writeFile(path.join(root, 'inbox', 'Plan.md'), '# Plan\n\nShip the beta in October.\n') + backend = createBackend({ kind: 'local', root }) + out = [] + vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { + out.push(String(chunk)) + return true + }) +}) + +afterEach(async () => { + vi.restoreAllMocks() + await fsp.rm(root, { recursive: true, force: true }) +}) + +describe('zn comment', () => { + it('adds, lists, answers and resolves a thread', async () => { + await cmdCommentAdd( + backend, + args(['inbox/Plan.md', 'Still realistic?'], [['anchor', 'Ship the beta in October.']]) + ) + expect(out.join('')).toMatch(/Commented on inbox\/Plan\.md \(.+, line 3\)/) + + out = [] + await cmdCommentList(backend, args(['inbox/Plan.md'], [['json', 'true']])) + const threads = JSON.parse(out.join('')) as Array<{ id: string; author: string | null; line: number }> + expect(threads).toHaveLength(1) + expect(threads[0].author).toBeNull() + expect(threads[0].line).toBe(3) + + out = [] + await cmdCommentReply( + backend, + args(['inbox/Plan.md', threads[0].id, 'Yes, the blocker runs at night.'], [['author', 'Claude']]) + ) + expect(out.join('')).toContain('Replied in') + + out = [] + await cmdCommentList(backend, args(['inbox/Plan.md'])) + const text = out.join('') + expect(text).toContain('You') + expect(text).toContain('> Ship the beta in October.') + expect(text).toContain('Claude') + expect(text).toContain('Yes, the blocker runs at night.') + + out = [] + await cmdCommentResolve(backend, args(['inbox/Plan.md', threads[0].id])) + expect(out.join('')).toContain('Resolved') + out = [] + await cmdCommentList(backend, args(['inbox/Plan.md'])) + expect(out.join('')).toContain('No open comments') + out = [] + await cmdCommentList(backend, args(['inbox/Plan.md'], [['all', 'true']])) + expect(out.join('')).toContain('(resolved)') + }) + + it('explains usage when the path or body is missing', async () => { + await expect(cmdCommentAdd(backend, args([]))).rejects.toThrow(/Usage: zn comment add/) + await expect(cmdCommentAdd(backend, args(['inbox/Plan.md']))).rejects.toThrow(/Usage: zn comment add/) + await expect(cmdCommentReply(backend, args(['inbox/Plan.md']))).rejects.toThrow(/Usage: zn comment reply/) + await expect(cmdCommentResolve(backend, args(['inbox/Plan.md']))).rejects.toThrow(/Usage: zn comment resolve/) + }) +}) diff --git a/apps/desktop/src/cli/commands/comments.ts b/apps/desktop/src/cli/commands/comments.ts new file mode 100644 index 00000000..5dfef00e --- /dev/null +++ b/apps/desktop/src/cli/commands/comments.ts @@ -0,0 +1,106 @@ +/** + * `zn comment ...` (#738): list, add, reply to and resolve the comments on a + * note, the same operations the MCP tools expose, so a script or an agent + * without MCP can join a review thread. + */ + +import type { VaultBackend } from '../backend.js' +import { getBool, getString, type ParsedArgs } from '../args.js' +import { emitJson, emitLine, emitOk } from '../format.js' +import { + addComment, + listCommentThreads, + replyToComment, + resolveComment, + type CommentThreadView +} from '../../mcp/comment-ops.js' + +function requirePath(args: ParsedArgs, usage: string): string { + const rel = getString(args, 'path') ?? args.positionals[0] + if (!rel) throw new Error(`Usage: ${usage}`) + return rel +} + +function requireBody(args: ParsedArgs, positionalIndex: number, usage: string): string { + const body = getString(args, 'body') ?? args.positionals[positionalIndex] + if (!body || !body.trim()) throw new Error(`Usage: ${usage}`) + return body +} + +function when(ms: number): string { + return new Date(ms).toISOString().replace('T', ' ').slice(0, 16) +} + +function printThread(thread: CommentThreadView): void { + const who = thread.author ?? 'You' + const state = thread.resolved ? ' (resolved)' : '' + emitLine(`${thread.id} ${who} ${when(thread.createdAt)} line ${thread.line}${state}`) + if (thread.anchorText) emitLine(` > ${thread.anchorText}`) + emitLine(` ${thread.body.replace(/\n/g, '\n ')}`) + for (const reply of thread.replies) { + emitLine(` ${reply.id} ${reply.author ?? 'You'} ${when(reply.createdAt)}`) + emitLine(` ${reply.body.replace(/\n/g, '\n ')}`) + } +} + +export async function cmdCommentList(vault: VaultBackend, args: ParsedArgs): Promise { + const rel = requirePath(args, 'zn comment list [--all] [--json]') + const threads = await listCommentThreads(vault, rel, { includeResolved: getBool(args, 'all') }) + if (getBool(args, 'json')) { + emitJson(threads) + return + } + if (threads.length === 0) { + emitLine(getBool(args, 'all') ? 'No comments.' : 'No open comments. Pass --all to include resolved ones.') + return + } + threads.forEach((thread, index) => { + if (index > 0) emitLine('') + printThread(thread) + }) +} + +export async function cmdCommentAdd(vault: VaultBackend, args: ParsedArgs): Promise { + const usage = 'zn comment add "" [--anchor ""] [--author ]' + const rel = requirePath(args, usage) + const body = requireBody(args, 1, usage) + const thread = await addComment(vault, { + path: rel, + body, + anchorText: getString(args, 'anchor'), + author: getString(args, 'author') + }) + if (getBool(args, 'json')) { + emitJson(thread) + return + } + emitOk(`Commented on ${rel} (${thread.id}${thread.anchorText ? `, line ${thread.line}` : ''})`) +} + +export async function cmdCommentReply(vault: VaultBackend, args: ParsedArgs): Promise { + const usage = 'zn comment reply "" [--author ]' + const rel = requirePath(args, usage) + const id = getString(args, 'id') ?? args.positionals[1] + if (!id) throw new Error(`Usage: ${usage}`) + const body = requireBody(args, 2, usage) + const thread = await replyToComment(vault, { path: rel, id, body, author: getString(args, 'author') }) + if (getBool(args, 'json')) { + emitJson(thread) + return + } + emitOk(`Replied in ${thread.id} on ${rel} (${thread.replies.length} ${thread.replies.length === 1 ? 'reply' : 'replies'})`) +} + +export async function cmdCommentResolve(vault: VaultBackend, args: ParsedArgs): Promise { + const usage = 'zn comment resolve [--reopen]' + const rel = requirePath(args, usage) + const id = getString(args, 'id') ?? args.positionals[1] + if (!id) throw new Error(`Usage: ${usage}`) + const reopen = getBool(args, 'reopen') + const thread = await resolveComment(vault, { path: rel, id, resolved: !reopen }) + if (getBool(args, 'json')) { + emitJson(thread) + return + } + emitOk(`${reopen ? 'Reopened' : 'Resolved'} ${thread.id} on ${rel}`) +} diff --git a/apps/desktop/src/cli/help.ts b/apps/desktop/src/cli/help.ts index f342354a..8b7c51e6 100644 --- a/apps/desktop/src/cli/help.ts +++ b/apps/desktop/src/cli/help.ts @@ -138,6 +138,15 @@ const SECTIONS: Array<{ heading: string; rows: CommandRow[] }> = [ { name: 'task toggle ', description: 'Flip a task checkbox by stable id' } ] }, + { + heading: 'COMMENTS', + rows: [ + { name: 'comment list ', description: 'Comment threads on a note, with anchors and replies', flags: '--all --json' }, + { name: 'comment add ""', description: 'Start a thread, optionally anchored to text from the note', flags: '--anchor --author --json' }, + { name: 'comment reply ""', description: 'Answer in a thread', flags: '--author --json' }, + { name: 'comment resolve ', description: 'Resolve a thread (or reopen it)', flags: '--reopen --json' } + ] + }, { heading: 'VAULT', rows: [ @@ -203,6 +212,8 @@ const EXAMPLES: string[] = [ 'zn list --server home # a self-hosted ZenNotes server', 'zn capture "from CI" --server https://notes.example.com', 'zn task list --unchecked --tag work', + 'zn comment list inbox/Plan.md', + 'zn comment reply inbox/Plan.md "Agreed, fixed in the second paragraph." --author Claude', 'zn open ~/Downloads/notes.md', 'zn open ~/code/project/docs # focus a folder as a session' ] diff --git a/apps/desktop/src/cli/index.ts b/apps/desktop/src/cli/index.ts index 79e37e26..7e07e8dc 100644 --- a/apps/desktop/src/cli/index.ts +++ b/apps/desktop/src/cli/index.ts @@ -45,6 +45,12 @@ import { cmdFolderRename } from './commands/folders.js' import { cmdTaskList, cmdTaskToggle } from './commands/tasks.js' +import { + cmdCommentAdd, + cmdCommentList, + cmdCommentReply, + cmdCommentResolve +} from './commands/comments.js' import { cmdTagFind, cmdTagList } from './commands/tags.js' import { cmdVaultInfo, cmdVaultList } from './commands/vault.js' import { cmdCapture } from './commands/capture.js' @@ -132,6 +138,10 @@ async function main(argv: string[]): Promise { 'tag find': cmdTagFind, 'task list': cmdTaskList, 'task toggle': cmdTaskToggle, + 'comment list': cmdCommentList, + 'comment add': cmdCommentAdd, + 'comment reply': cmdCommentReply, + 'comment resolve': cmdCommentResolve, 'vault info': cmdVaultInfo, 'base list': cmdBaseList, 'base create': cmdBaseCreate, @@ -163,6 +173,7 @@ function peelSubcommand( folder: ['list', 'create', 'rename', 'delete'], tag: ['list', 'find'], task: ['list', 'toggle'], + comment: ['list', 'add', 'reply', 'resolve'], vault: ['info', 'list'], base: ['list', 'create', 'rows', 'get', 'add', 'set'] } diff --git a/apps/desktop/src/cli/remote/client.ts b/apps/desktop/src/cli/remote/client.ts index 4a3a165f..5f7006b0 100644 --- a/apps/desktop/src/cli/remote/client.ts +++ b/apps/desktop/src/cli/remote/client.ts @@ -14,6 +14,8 @@ import { remoteJsonRequest } from '../../main/remote/connection.js' import type { + NoteComment, + NoteCommentInput, NoteContent, NoteFolder, NoteMeta, @@ -85,6 +87,16 @@ export class CliRemoteClient { return this.get(`/api/search/text?${params.toString()}`) } + /** A note's comment sidecar, through the same two routes the desktop + * remote client and the web client use (#738). */ + readComments(relPath: string): Promise { + return this.get(`/api/comments/read?path=${encodeURIComponent(relPath)}`) + } + + writeComments(relPath: string, comments: NoteCommentInput[]): Promise { + return this.post('/api/comments/write', { path: relPath, comments }) + } + scanTasks(opts?: { includeExcluded?: boolean }): Promise { return this.get( opts?.includeExcluded ? '/api/tasks?includeExcluded=1' : '/api/tasks' diff --git a/apps/desktop/src/main/vault.ts b/apps/desktop/src/main/vault.ts index de4e608e..47832097 100644 --- a/apps/desktop/src/main/vault.ts +++ b/apps/desktop/src/main/vault.ts @@ -53,6 +53,7 @@ import { VaultInfo } from '@shared/ipc' import { DEMO_TOUR_DIR } from '@shared/demo-tour' +import { normalizeNoteComments } from '@shared/note-comments' import { FRONTMATTER_BLOCK_RE, frontmatterTags } from '@shared/frontmatter' import { IMAGE_FILE_EXTENSIONS, pastedImageFilename } from '@shared/pasted-image' import { @@ -3135,59 +3136,6 @@ export async function writeNote(root: string, rel: string, body: string): Promis return await readMeta(root, abs, folder) } -function normalizeNoteComment(input: NoteCommentInput, notePath: string): NoteComment | null { - const body = typeof input.body === 'string' ? input.body.trim() : '' - if (!body) return null - const now = Date.now() - const rawStart = Number.isFinite(input.anchorStart) ? Math.max(0, Math.floor(input.anchorStart)) : 0 - const rawEnd = Number.isFinite(input.anchorEnd) ? Math.max(0, Math.floor(input.anchorEnd)) : rawStart - const anchorStart = Math.min(rawStart, rawEnd) - const anchorEnd = Math.max(rawStart, rawEnd) - const anchorText = - typeof input.anchorText === 'string' - ? input.anchorText.replace(/\s+/g, ' ').trim().slice(0, 500) - : '' - return { - id: typeof input.id === 'string' && input.id.trim() ? input.id.trim() : randomUUID(), - notePath, - anchorStart, - anchorEnd, - anchorText, - body, - createdAt: - typeof input.createdAt === 'number' && Number.isFinite(input.createdAt) - ? input.createdAt - : now, - updatedAt: - typeof input.updatedAt === 'number' && Number.isFinite(input.updatedAt) - ? input.updatedAt - : now, - resolvedAt: - typeof input.resolvedAt === 'number' && Number.isFinite(input.resolvedAt) - ? input.resolvedAt - : null - } -} - -function normalizeNoteComments(raw: unknown, notePath: string): NoteComment[] { - const values = Array.isArray(raw) - ? raw - : raw && typeof raw === 'object' && Array.isArray((raw as { comments?: unknown }).comments) - ? (raw as { comments: unknown[] }).comments - : [] - const seen = new Set() - const comments: NoteComment[] = [] - for (const value of values) { - if (!value || typeof value !== 'object') continue - const comment = normalizeNoteComment(value as NoteCommentInput, notePath) - if (!comment || seen.has(comment.id)) continue - seen.add(comment.id) - comments.push(comment) - } - comments.sort((a, b) => a.createdAt - b.createdAt || a.id.localeCompare(b.id)) - return comments -} - export async function readNoteComments(root: string, rel: string): Promise { const notePath = toPosix(rel) const abs = noteCommentsPath(root, notePath) diff --git a/apps/desktop/src/mcp/comment-ops.test.ts b/apps/desktop/src/mcp/comment-ops.test.ts new file mode 100644 index 00000000..471c1353 --- /dev/null +++ b/apps/desktop/src/mcp/comment-ops.test.ts @@ -0,0 +1,116 @@ +import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { createBackend, type VaultBackend } from '../cli/backend' +import { addComment, anchorForText, listCommentThreads, replyToComment, resolveComment } from './comment-ops' + +// The comment operations behind the MCP tools and `zn comment` (#738), run +// against a real folder through the local backend so the sidecar the app +// reads is exactly what these write. + +let root: string +let backend: VaultBackend +const NOTE = 'inbox/Plan.md' + +beforeEach(async () => { + root = await mkdtemp(path.join(os.tmpdir(), 'zennotes-comment-ops-')) + await mkdir(path.join(root, 'inbox'), { recursive: true }) + await writeFile( + path.join(root, 'inbox', 'Plan.md'), + '# Plan\n\nShip the beta in October.\n\nThe migration runs at night.\n' + ) + backend = createBackend({ kind: 'local', root }) +}) + +afterEach(async () => { + await rm(root, { recursive: true, force: true }) +}) + +describe('anchorForText', () => { + const doc = 'Alpha beta\nGamma Delta\n' + it('finds the passage exactly, then ignoring case, and refuses what is not there', () => { + expect(anchorForText(doc, 'Gamma')).toEqual({ anchorStart: 11, anchorEnd: 16, anchorText: 'Gamma' }) + expect(anchorForText(doc, 'gamma delta')).toEqual({ anchorStart: 11, anchorEnd: 22, anchorText: 'Gamma Delta' }) + expect(anchorForText(doc, undefined)).toEqual({ anchorStart: 0, anchorEnd: 0, anchorText: '' }) + expect(() => anchorForText(doc, 'omega')).toThrow(/anchor_text was not found/) + }) +}) + +describe('comment threads on a note', () => { + it('adds an anchored comment, answers it in a thread, and resolves it', async () => { + const thread = await addComment(backend, { + path: NOTE, + body: 'Is October still realistic?', + anchorText: 'Ship the beta in October.', + author: undefined + }) + expect(thread.author).toBeNull() + expect(thread.line).toBe(3) + expect(thread.anchorText).toBe('Ship the beta in October.') + expect(thread.replies).toEqual([]) + + const answered = await replyToComment(backend, { + path: NOTE, + id: thread.id, + body: 'Yes: the migration is the only blocker and it runs at night.', + author: 'Claude Code' + }) + expect(answered.id).toBe(thread.id) + expect(answered.replies).toHaveLength(1) + expect(answered.replies[0].author).toBe('Claude Code') + + // A reply to the reply lands in the same thread, one level deep. + const again = await replyToComment(backend, { + path: NOTE, + id: answered.replies[0].id, + body: 'Agreed, keep October.' + }) + expect(again.id).toBe(thread.id) + expect(again.replies.map((r) => r.author)).toEqual(['Claude Code', null]) + + const open = await listCommentThreads(backend, NOTE) + expect(open.map((t) => t.id)).toEqual([thread.id]) + + const done = await resolveComment(backend, { path: NOTE, id: again.replies[1].id }) + expect(done.id).toBe(thread.id) + expect(done.resolved).toBe(true) + expect(await listCommentThreads(backend, NOTE)).toEqual([]) + expect((await listCommentThreads(backend, NOTE, { includeResolved: true }))[0].resolved).toBe(true) + + const reopened = await resolveComment(backend, { path: NOTE, id: thread.id, resolved: false }) + expect(reopened.resolved).toBe(false) + + // The sidecar is where the app reads: same path, same envelope. + const sidecar = JSON.parse( + await readFile(path.join(root, '.zennotes', 'comments', 'inbox', 'Plan.md.comments.json'), 'utf8') + ) as { version: number; comments: Array> } + expect(sidecar.version).toBe(1) + expect(sidecar.comments).toHaveLength(3) + expect(sidecar.comments[1]).toMatchObject({ parentId: thread.id, author: 'Claude Code' }) + expect(sidecar.comments[0]).not.toHaveProperty('author') + }) + + it('reports the line an anchor sits on after the note moved', async () => { + const thread = await addComment(backend, { + path: NOTE, + body: 'Night runs need a rollback plan.', + anchorText: 'The migration runs at night.' + }) + expect(thread.line).toBe(5) + await writeFile( + path.join(root, 'inbox', 'Plan.md'), + '# Plan\n\nA new paragraph first.\n\nShip the beta in October.\n\nThe migration runs at night.\n' + ) + const [moved] = await listCommentThreads(backend, NOTE) + expect(moved.line).toBe(7) + }) + + it('refuses an empty body, an unknown id, and text that is not in the note', async () => { + await expect(addComment(backend, { path: NOTE, body: ' ' })).rejects.toThrow(/empty/) + await expect(addComment(backend, { path: NOTE, body: 'x', anchorText: 'not here' })).rejects.toThrow(/not found/) + await expect(replyToComment(backend, { path: NOTE, id: 'nope', body: 'x' })).rejects.toThrow(/No comment with id/) + await expect(resolveComment(backend, { path: NOTE, id: 'nope' })).rejects.toThrow(/No comment with id/) + expect(await listCommentThreads(backend, NOTE)).toEqual([]) + }) +}) diff --git a/apps/desktop/src/mcp/comment-ops.ts b/apps/desktop/src/mcp/comment-ops.ts new file mode 100644 index 00000000..d56cd81e --- /dev/null +++ b/apps/desktop/src/mcp/comment-ops.ts @@ -0,0 +1,161 @@ +/** + * Comment operations for the MCP tools and `zn comment` (#738), composed + * from a backend's note read and comment read/write so a local folder and a + * ZenNotes server behave the same. The shapes here are what a model sees: + * threads (a top-level comment with its replies), the anchored text and the + * line it sits on today, and who said what. + */ + +import { + lineOfOffset, + resolveCommentAnchor, + threadNoteComments, + threadRootOf +} from '@shared/note-comments' +import type { NoteComment, NoteCommentInput } from '@shared/ipc' +import type { VaultBackend } from '../cli/backend.js' + +export interface CommentView { + id: string + author: string | null + body: string + createdAt: number + updatedAt: number +} + +export interface CommentThreadView extends CommentView { + /** The text the comment was written on, as stored. Empty for a note-level comment. */ + anchorText: string + /** 1-based line the anchor sits on in the note as it is now. */ + line: number + resolved: boolean + resolvedAt: number | null + replies: CommentView[] +} + +function view(comment: NoteComment): CommentView { + return { + id: comment.id, + author: comment.author ?? null, + body: comment.body, + createdAt: comment.createdAt, + updatedAt: comment.updatedAt + } +} + +export async function listCommentThreads( + backend: VaultBackend, + rel: string, + opts: { includeResolved?: boolean } = {} +): Promise { + const [note, comments] = await Promise.all([backend.readNote(rel), backend.listComments(rel)]) + const doc = note.body + return threadNoteComments(comments) + .filter((thread) => opts.includeResolved || thread.comment.resolvedAt == null) + .map((thread) => ({ + ...view(thread.comment), + anchorText: thread.comment.anchorText, + line: lineOfOffset(doc, resolveCommentAnchor(thread.comment, doc).from), + resolved: thread.comment.resolvedAt != null, + resolvedAt: thread.comment.resolvedAt, + replies: thread.replies.map(view) + })) +} + +/** + * Where a new comment attaches. `anchorText` must appear in the note as + * written (an exact match first, then one ignoring case); without it the + * comment is note-level, anchored at the top. + */ +export function anchorForText( + doc: string, + anchorText: string | undefined +): Pick { + const wanted = (anchorText ?? '').trim() + if (!wanted) return { anchorStart: 0, anchorEnd: 0, anchorText: '' } + let at = doc.indexOf(wanted) + if (at < 0) at = doc.toLowerCase().indexOf(wanted.toLowerCase()) + if (at < 0) { + throw new Error( + `anchor_text was not found in the note. Pass the text exactly as it appears (read_note shows it), or omit it for a note-level comment.` + ) + } + return { + anchorStart: at, + anchorEnd: at + wanted.length, + anchorText: doc.slice(at, at + wanted.length).replace(/\s+/g, ' ').trim().slice(0, 500) + } +} + +export async function addComment( + backend: VaultBackend, + input: { path: string; body: string; anchorText?: string; author?: string } +): Promise { + const body = input.body.trim() + if (!body) throw new Error('body must not be empty') + const note = await backend.readNote(input.path) + const anchor = anchorForText(note.body, input.anchorText) + const now = Date.now() + const current = await backend.listComments(input.path) + const draft: NoteCommentInput = { + notePath: input.path, + ...anchor, + body, + author: input.author, + createdAt: now, + updatedAt: now, + resolvedAt: null + } + const written = await backend.writeComments(input.path, [...current, draft]) + const created = written.find((c) => c.createdAt === now && c.body === body) ?? written[written.length - 1] + const threads = await listCommentThreads(backend, input.path, { includeResolved: true }) + return threads.find((t) => t.id === created.id) ?? threads[threads.length - 1] +} + +export async function replyToComment( + backend: VaultBackend, + input: { path: string; id: string; body: string; author?: string } +): Promise { + const body = input.body.trim() + if (!body) throw new Error('body must not be empty') + const current = await backend.listComments(input.path) + const root = threadRootOf(current, input.id) + if (!root) throw new Error(`No comment with id ${input.id} on ${input.path}. Use list_comments to find ids.`) + const now = Date.now() + const draft: NoteCommentInput = { + notePath: input.path, + anchorStart: root.anchorStart, + anchorEnd: root.anchorEnd, + anchorText: root.anchorText, + body, + author: input.author, + parentId: root.id, + createdAt: now, + updatedAt: now, + resolvedAt: null + } + await backend.writeComments(input.path, [...current, draft]) + const threads = await listCommentThreads(backend, input.path, { includeResolved: true }) + const thread = threads.find((t) => t.id === root.id) + if (!thread) throw new Error(`Thread ${root.id} vanished while replying`) + return thread +} + +export async function resolveComment( + backend: VaultBackend, + input: { path: string; id: string; resolved?: boolean } +): Promise { + const current = await backend.listComments(input.path) + const root = threadRootOf(current, input.id) + if (!root) throw new Error(`No comment with id ${input.id} on ${input.path}. Use list_comments to find ids.`) + const resolved = input.resolved ?? true + const now = Date.now() + const next = current.map((c) => + c.id === root.id ? { ...c, resolvedAt: resolved ? now : null, updatedAt: now } : c + ) + await backend.writeComments(input.path, next) + const threads = await listCommentThreads(backend, input.path, { includeResolved: true }) + const thread = threads.find((t) => t.id === root.id) + if (!thread) throw new Error(`Thread ${root.id} vanished while resolving`) + return thread +} diff --git a/apps/desktop/src/mcp/instructions.ts b/apps/desktop/src/mcp/instructions.ts index aef9ea1b..aab786c5 100644 --- a/apps/desktop/src/mcp/instructions.ts +++ b/apps/desktop/src/mcp/instructions.ts @@ -213,6 +213,28 @@ when the folder is \`Linear Algebra/\`), synonyms, and feeling tags \`tasks: false\`/\`note\`, excluded folders). Pass includeExcluded: true only when the user asks for everything. +## Comments: reviewing a note together + +Notes carry comment threads, kept beside the note and shown in the app's +Comments panel. A user who asks you to review, answer, or discuss a note +usually means through those threads, like a pull request, not by editing +the body. + +- Start with \`list_comments\` on the note. Each thread shows the passage it + is anchored to, the line it sits on now, who wrote what (\`author\` is + null for the user), and the replies so far. +- Answer a thread with \`reply_to_comment\`; it lands under the user's + comment, signed with your name. Reply to every open thread you were asked + about, one reply per thread, and keep replies short and concrete. +- Raise something new with \`add_comment\`, passing \`anchor_text\` copied + verbatim from the note so the comment highlights that passage in the app. + Omit it only for a note-level remark. +- Change the note body only when the user asks for the change; when a thread + ends in "do it", make the edit with the editing tools, then reply in the + thread saying what changed. +- \`resolve_comment\` only when the user says the thread is settled or asks + you to close it. Resolved threads stay in the note's history. + ## Self-check before every write Scan the markdown before sending it. Fix, don\u2019t ship: diff --git a/apps/desktop/src/mcp/server.test.ts b/apps/desktop/src/mcp/server.test.ts index 9fcfdf6b..3b1708ae 100644 --- a/apps/desktop/src/mcp/server.test.ts +++ b/apps/desktop/src/mcp/server.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest' import type { VaultBackend } from '../cli/backend' import { RemoteRequestError } from '../main/remote/connection' -import { callTool, describeToolError, listToolNames } from './server' +import { callTool, commentAuthorForClient, describeToolError, listToolNames } from './server' // Only the members a given test reaches are implemented; the cast keeps the // stubs honest about being partial. @@ -116,7 +116,11 @@ describe('tools run through the backend', () => { 'append_to_note', 'prepend_to_note', 'insert_at_line', - 'replace_in_note' + 'replace_in_note', + 'list_comments', + 'add_comment', + 'reply_to_comment', + 'resolve_comment' ]) }) }) @@ -134,3 +138,55 @@ describe('describeToolError', () => { expect(describeToolError(new RemoteRequestError('nope', 500))).toBe('nope') }) }) + +describe('comment tools (#738)', () => { + it('lists the four comment tools', () => { + const names = listToolNames() + for (const name of ['list_comments', 'add_comment', 'reply_to_comment', 'resolve_comment']) { + expect(names).toContain(name) + } + }) + + it('signs a comment with the connected client, readably', () => { + expect(commentAuthorForClient('claude-code')).toBe('Claude Code') + expect(commentAuthorForClient('claude-ai')).toBe('Claude') + expect(commentAuthorForClient('codex-cli')).toBe('Codex') + expect(commentAuthorForClient('my_custom-agent')).toBe('My Custom Agent') + expect(commentAuthorForClient(null)).toBe('Assistant') + expect(commentAuthorForClient(' ')).toBe('Assistant') + }) + + it('reply_to_comment threads under the top-level comment with the author', async () => { + let stored: Array> = [ + { + id: 'c1', + notePath: 'inbox/Plan.md', + anchorStart: 8, + anchorEnd: 33, + anchorText: 'Ship the beta in October.', + body: 'Still realistic?', + createdAt: 1, + updatedAt: 1, + resolvedAt: null + } + ] + const result = (await callTool( + 'reply_to_comment', + { path: 'inbox/Plan.md', id: 'c1', body: 'Yes, the blocker runs at night.' }, + backend({ + readNote: async () => + ({ path: 'inbox/Plan.md', body: '# Plan\n\nShip the beta in October.\n' }) as never, + listComments: async () => stored as never, + writeComments: async (_rel, comments) => { + stored = comments.map((c, i) => ({ ...c, id: (c as { id?: string }).id ?? `c${i + 1}` })) + return stored as never + } + }) + )) as { id: string; replies: Array<{ author: string | null; body: string }> } + expect(result.id).toBe('c1') + expect(result.replies).toEqual([ + expect.objectContaining({ author: 'Assistant', body: 'Yes, the blocker runs at night.' }) + ]) + expect(stored[1]).toMatchObject({ parentId: 'c1', anchorText: 'Ship the beta in October.' }) + }) +}) diff --git a/apps/desktop/src/mcp/server.ts b/apps/desktop/src/mcp/server.ts index c7caef8e..cb67b9c9 100644 --- a/apps/desktop/src/mcp/server.ts +++ b/apps/desktop/src/mcp/server.ts @@ -22,12 +22,45 @@ import { createBackend, type VaultBackend } from '../cli/backend.js' import { resolveDefaultTarget } from '../cli/vault-target.js' import { RemoteRequestError } from '../main/remote/connection.js' import type { NoteFolder } from './vault-ops.js' +import { addComment, listCommentThreads, replyToComment, resolveComment } from './comment-ops.js' interface ToolDef { schema: Tool handler: (args: Record, backend: VaultBackend) => Promise } +/* ---------- Comment authorship ---------------------------------------- */ + +// The MCP client's name from the initialize handshake ("claude-code", +// "claude-ai", "codex-cli"), read as a display name so a comment left by an +// assistant says who left it. Set once the session is initialized; the +// fallback covers direct callTool use and clients that send nothing. +let connectedClientName: string | null = null + +const CLIENT_DISPLAY_NAMES: Record = { + 'claude-ai': 'Claude', + 'claude-code': 'Claude Code', + 'claude-desktop': 'Claude', + 'codex-cli': 'Codex', + codex: 'Codex' +} + +export function commentAuthorForClient(clientName: string | null | undefined): string { + const raw = (clientName ?? '').trim() + if (!raw) return 'Assistant' + const known = CLIENT_DISPLAY_NAMES[raw.toLowerCase()] + if (known) return known + return raw + .split(/[-_\s]+/) + .filter(Boolean) + .map((word) => word[0].toUpperCase() + word.slice(1)) + .join(' ') +} + +function defaultCommentAuthor(): string { + return commentAuthorForClient(connectedClientName) +} + /* ---------- Argument helpers ----------------------------------------- */ function requireString(args: Record, key: string): string { @@ -53,6 +86,13 @@ function requireFolder(args: Record, key: string): NoteFolder { return value } +function optionalBoolean(args: Record, key: string): boolean | undefined { + const value = args[key] + if (value == null) return undefined + if (typeof value !== 'boolean') throw new Error(`${key} must be a boolean`) + return value +} + function optionalNumber(args: Record, key: string): number | undefined { const value = args[key] if (value == null) return undefined @@ -789,6 +829,104 @@ const TOOLS: ToolDef[] = [ const occurrence = (optionalString(args, 'occurrence') as 'first' | 'all' | undefined) ?? 'first' return await backend.replaceInNote(rel, find, replace, occurrence) } + }, + { + schema: { + name: 'list_comments', + description: + 'The comment threads on a note: each top-level comment with the text it is anchored to, the line that text sits on now, who wrote it (author is null for the vault owner), and its replies in order. Read this before reviewing or answering a discussion; unresolved threads only unless include_resolved is true.', + inputSchema: { + type: 'object', + properties: { + path: { type: 'string', description: 'Vault-relative note path.' }, + include_resolved: { type: 'boolean', description: 'Also list resolved threads. Default false.' } + }, + required: ['path'] + } + }, + handler: async (args, backend) => + await listCommentThreads(backend, requireString(args, 'path'), { + includeResolved: optionalBoolean(args, 'include_resolved') ?? false + }) + }, + { + schema: { + name: 'add_comment', + description: + 'Start a new comment thread on a note, attributed to you. Pass anchor_text, a passage copied exactly from the note, to attach the comment to it (the app highlights it and jumps there); omit it for a note-level comment. Markdown is fine in the body. Returns the new thread.', + inputSchema: { + type: 'object', + properties: { + path: { type: 'string', description: 'Vault-relative note path.' }, + body: { type: 'string', description: 'The comment, in Markdown.' }, + anchor_text: { + type: 'string', + description: 'Text from the note the comment is about, verbatim. Omit for a note-level comment.' + }, + author: { + type: 'string', + description: 'Display name to sign with. Defaults to the connected client (e.g. "Claude Code").' + } + }, + required: ['path', 'body'] + } + }, + handler: async (args, backend) => + await addComment(backend, { + path: requireString(args, 'path'), + body: requireString(args, 'body'), + anchorText: optionalString(args, 'anchor_text'), + author: optionalString(args, 'author') ?? defaultCommentAuthor() + }) + }, + { + schema: { + name: 'reply_to_comment', + description: + 'Answer a comment in its thread, attributed to you. id is a thread id (or any reply id in it) from list_comments; the reply keeps the thread\u2019s anchor. Use this to respond to the user\u2019s comments the way you would on a pull request, instead of editing the note body. Returns the updated thread.', + inputSchema: { + type: 'object', + properties: { + path: { type: 'string', description: 'Vault-relative note path.' }, + id: { type: 'string', description: 'A comment id from list_comments.' }, + body: { type: 'string', description: 'The reply, in Markdown.' }, + author: { + type: 'string', + description: 'Display name to sign with. Defaults to the connected client.' + } + }, + required: ['path', 'id', 'body'] + } + }, + handler: async (args, backend) => + await replyToComment(backend, { + path: requireString(args, 'path'), + id: requireString(args, 'id'), + body: requireString(args, 'body'), + author: optionalString(args, 'author') ?? defaultCommentAuthor() + }) + }, + { + schema: { + name: 'resolve_comment', + description: + 'Mark a comment thread resolved (or reopen it with resolved: false). Resolve only when the discussion is settled or the user asks; the thread stays in the note\u2019s history and moves to the Resolved section of the app\u2019s Comments panel.', + inputSchema: { + type: 'object', + properties: { + path: { type: 'string', description: 'Vault-relative note path.' }, + id: { type: 'string', description: 'A comment id from list_comments.' }, + resolved: { type: 'boolean', description: 'false reopens the thread. Default true.' } + }, + required: ['path', 'id'] + } + }, + handler: async (args, backend) => + await resolveComment(backend, { + path: requireString(args, 'path'), + id: requireString(args, 'id'), + resolved: optionalBoolean(args, 'resolved') ?? true + }) } ] @@ -853,6 +991,10 @@ export async function runMcpServer(): Promise { } ) + server.oninitialized = () => { + connectedClientName = server.getClientVersion()?.name ?? null + } + server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: TOOLS.map((t) => t.schema) })) diff --git a/apps/desktop/src/mcp/vault-ops.ts b/apps/desktop/src/mcp/vault-ops.ts index 6da2802c..266137ef 100644 --- a/apps/desktop/src/mcp/vault-ops.ts +++ b/apps/desktop/src/mcp/vault-ops.ts @@ -13,6 +13,13 @@ import path from 'node:path' import os from 'node:os' import { parse as parseToml } from 'smol-toml' import { retitleLeadingHeading } from '@shared/note-heading-sync' +import { + NOTE_COMMENTS_DIR, + NOTE_COMMENTS_SUFFIX, + normalizeNoteComments +} from '@shared/note-comments' +import type { NoteComment, NoteCommentInput } from '@shared/ipc' +export type { NoteComment, NoteCommentInput } import { noteTasksMode, type NoteTasksMode } from '@shared/tasks' import { isPathExcludedFromTasks, @@ -1893,6 +1900,46 @@ export async function insertAtLine( /* ---------- Backlinks ------------------------------------------------- */ +/* ---------- Note comments (#738) --------------------------------------- */ + +/** The sidecar beside a note: `.zennotes/comments/.comments.json`, the + * same path the desktop and the Go server use, validated against escapes. */ +function noteCommentsPath(root: string, rel: string): string { + const commentsRoot = path.join(root, INTERNAL_VAULT_DIR, NOTE_COMMENTS_DIR) + return resolveSafe(commentsRoot, `${toPosix(rel)}${NOTE_COMMENTS_SUFFIX}`) +} + +export async function readNoteComments(root: string, rel: string): Promise { + const notePath = toPosix(rel) + try { + const raw = await fs.readFile(noteCommentsPath(root, notePath), 'utf8') + return normalizeNoteComments(JSON.parse(raw), notePath) + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return [] + if (err instanceof SyntaxError) return [] + throw err + } +} + +/** Replace a note's comments. An empty list removes the sidecar, as the app + * does, so a note with no comments leaves nothing behind. */ +export async function writeNoteComments( + root: string, + rel: string, + comments: NoteCommentInput[] +): Promise { + const notePath = toPosix(rel) + const normalized = normalizeNoteComments(comments, notePath) + const abs = noteCommentsPath(root, notePath) + if (normalized.length === 0) { + await fs.rm(abs, { force: true }) + return [] + } + await fs.mkdir(path.dirname(abs), { recursive: true }) + await fs.writeFile(abs, JSON.stringify({ version: 1, comments: normalized }, null, 2), 'utf8') + return normalized +} + export async function backlinks(root: string, rel: string): Promise { const abs = resolveSafe(root, rel) const all = await listNotes(root) diff --git a/apps/server/internal/vault/types.go b/apps/server/internal/vault/types.go index 0c1492f7..79bfde3c 100644 --- a/apps/server/internal/vault/types.go +++ b/apps/server/internal/vault/types.go @@ -352,6 +352,11 @@ type NoteComment struct { CreatedAt int64 `json:"createdAt"` UpdatedAt int64 `json:"updatedAt"` ResolvedAt *int64 `json:"resolvedAt"` + // Author is who wrote it: empty for the vault's owner, an assistant's + // name otherwise. ParentID threads a reply under a top-level comment. + // Both mirror shared-domain/note-comments.ts (#738). + Author string `json:"author,omitempty"` + ParentID string `json:"parentId,omitempty"` } // FolderEntry — mirrors shared/ipc.ts FolderEntry. diff --git a/apps/server/internal/vault/vault.go b/apps/server/internal/vault/vault.go index f13bf06e..35c98fa0 100644 --- a/apps/server/internal/vault/vault.go +++ b/apps/server/internal/vault/vault.go @@ -1628,9 +1628,21 @@ func normalizeComment(input NoteComment, notePath string) (NoteComment, bool) { CreatedAt: createdAt, UpdatedAt: updatedAt, ResolvedAt: input.ResolvedAt, + Author: normalizeCommentAuthor(input.Author), + ParentID: strings.TrimSpace(input.ParentID), }, true } +// normalizeCommentAuthor collapses whitespace and caps the name, mirroring +// normalizeCommentAuthor in shared-domain/note-comments.ts. +func normalizeCommentAuthor(raw string) string { + author := strings.Join(strings.Fields(raw), " ") + if len(author) > 80 { + author = author[:80] + } + return author +} + func normalizeComments(inputs []NoteComment, notePath string) []NoteComment { out := make([]NoteComment, 0, len(inputs)) seen := map[string]struct{}{} @@ -1651,6 +1663,20 @@ func normalizeComments(inputs []NoteComment, notePath string) []NoteComment { } return out[i].CreatedAt < out[j].CreatedAt }) + // A reply whose parent is gone (or is itself) stays as a comment of its + // own rather than vanishing from the thread view. + ids := make(map[string]struct{}, len(out)) + for _, comment := range out { + ids[comment.ID] = struct{}{} + } + for i := range out { + if out[i].ParentID == "" { + continue + } + if _, ok := ids[out[i].ParentID]; !ok || out[i].ParentID == out[i].ID { + out[i].ParentID = "" + } + } return out } diff --git a/apps/server/internal/vault/vault_test.go b/apps/server/internal/vault/vault_test.go index 12193675..0dc1e7cf 100644 --- a/apps/server/internal/vault/vault_test.go +++ b/apps/server/internal/vault/vault_test.go @@ -1289,3 +1289,39 @@ func TestHarperSettingsRoundTripAndNormalize(t *testing.T) { t.Errorf("empty harper block should be dropped, got %+v", cleared.Harper) } } + +func TestNoteCommentsKeepAuthorAndThreadReplies(t *testing.T) { + root := t.TempDir() + v, err := New(root, Options{}) + if err != nil { + t.Fatal(err) + } + meta, err := v.WriteNote("inbox/Reviewed.md", "line one\nline two\nline three") + if err != nil { + t.Fatalf("write note: %v", err) + } + written, err := v.WriteNoteComments(meta.Path, []NoteComment{ + {ID: "c1", Body: "Is this right?", CreatedAt: 1, UpdatedAt: 1}, + {ID: "c2", Body: "Yes, see line 3.", CreatedAt: 2, UpdatedAt: 2, Author: " Claude Code ", ParentID: " c1 "}, + {ID: "c3", Body: "orphan", CreatedAt: 3, UpdatedAt: 3, ParentID: "missing"}, + }) + if err != nil { + t.Fatalf("write comments: %v", err) + } + if len(written) != 3 { + t.Fatalf("expected 3 comments, got %d", len(written)) + } + read, err := v.ReadNoteComments(meta.Path) + if err != nil { + t.Fatalf("read comments: %v", err) + } + if read[1].Author != "Claude Code" || read[1].ParentID != "c1" { + t.Fatalf("reply lost its author or parent: %#v", read[1]) + } + if read[0].Author != "" || read[0].ParentID != "" { + t.Fatalf("top-level comment gained fields: %#v", read[0]) + } + if read[2].ParentID != "" { + t.Fatalf("orphan reply kept a missing parent: %#v", read[2]) + } +} diff --git a/packages/app-core/src/components/CommentsPanel.tsx b/packages/app-core/src/components/CommentsPanel.tsx index 73fe6b4b..0c51d180 100644 --- a/packages/app-core/src/components/CommentsPanel.tsx +++ b/packages/app-core/src/components/CommentsPanel.tsx @@ -10,6 +10,7 @@ import { import type { NoteComment, NoteContent } from '@shared/ipc' import { useStore } from '../store' import { commentQuote } from '../lib/comments' +import { threadNoteComments, type NoteCommentThread } from '@shared/note-comments' import { renderMarkdown } from '../lib/markdown' import { usePanelResize } from '../lib/use-panel-resize' import { PanelResizeHandle } from './PanelResizeHandle' @@ -98,6 +99,8 @@ export function CommentsPanel({ setBody('') setEditingId(null) setEditBody('') + setReplyingId(null) + setReplyBody('') onClearDraft() }, [note.path]) @@ -111,15 +114,23 @@ export function CommentsPanel({ return () => cancelAnimationFrame(raf) }, [activeCommentId, comments]) + // Threads (#738): a top-level comment with its replies. The panel's rows are + // the threads; a reply lives inside its card, so j/k walk conversations. + const threads = useMemo(() => threadNoteComments(comments), [comments]) const unresolved = useMemo( - () => comments.filter((comment) => comment.resolvedAt == null), - [comments] + () => threads.filter((thread) => thread.comment.resolvedAt == null), + [threads] ) const resolved = useMemo( - () => comments.filter((comment) => comment.resolvedAt != null), - [comments] + () => threads.filter((thread) => thread.comment.resolvedAt != null), + [threads] ) - const orderedComments = useMemo(() => [...unresolved, ...resolved], [resolved, unresolved]) + const orderedComments = useMemo( + () => [...unresolved, ...resolved].map((thread) => thread.comment), + [resolved, unresolved] + ) + const [replyingId, setReplyingId] = useState(null) + const [replyBody, setReplyBody] = useState('') useEffect(() => { if (!commentsFocused) return @@ -174,6 +185,31 @@ export function CommentsPanel({ setEditBody('') } + const startReply = (thread: NoteCommentThread): void => { + setReplyingId(thread.comment.id) + setReplyBody('') + setActiveCommentId(thread.comment.id) + } + + // A reply keeps the thread's anchor, so the editor keeps one marker per + // conversation and re-anchoring moves the whole thread together. + const submitReply = async (thread: NoteCommentThread): Promise => { + const trimmed = replyBody.trim() + if (!trimmed) return + const root = thread.comment + await addNoteComment({ + notePath: note.path, + anchorStart: root.anchorStart, + anchorEnd: root.anchorEnd, + anchorText: root.anchorText, + body: trimmed, + parentId: root.id + }) + setReplyingId(null) + setReplyBody('') + setActiveCommentId(root.id) + } + let rowIndex = 0 return ( @@ -223,6 +259,7 @@ export function CommentsPanel({ + @@ -291,30 +328,40 @@ export function CommentsPanel({
) : (
- {unresolved.map((comment) => ( + {unresolved.map((thread) => ( startReply(thread)} + onCancelReply={() => { + setReplyingId(null) + setReplyBody('') + }} + onSubmitReply={() => void submitReply(thread)} onJump={() => { - setActiveCommentId(comment.id) - onJump(comment) + setActiveCommentId(thread.comment.id) + onJump(thread.comment) }} - onEdit={() => startEdit(comment)} + onEdit={() => startEdit(thread.comment)} onCancelEdit={() => { setEditingId(null) setEditBody('') }} - onSave={() => void saveEdit(comment)} + onSave={() => void saveEdit(thread.comment)} onResolve={() => - void updateNoteComment(note.path, comment.id, { resolvedAt: Date.now() }) + void updateNoteComment(note.path, thread.comment.id, { resolvedAt: Date.now() }) } - onDelete={() => void deleteNoteComment(note.path, comment.id)} + onDelete={() => void deleteNoteComment(note.path, thread.comment.id)} /> ))} {resolved.length > 0 && unresolved.length > 0 && ( @@ -322,30 +369,40 @@ export function CommentsPanel({ Resolved
)} - {resolved.map((comment) => ( + {resolved.map((thread) => ( startReply(thread)} + onCancelReply={() => { + setReplyingId(null) + setReplyBody('') + }} + onSubmitReply={() => void submitReply(thread)} onJump={() => { - setActiveCommentId(comment.id) - onJump(comment) + setActiveCommentId(thread.comment.id) + onJump(thread.comment) }} - onEdit={() => startEdit(comment)} + onEdit={() => startEdit(thread.comment)} onCancelEdit={() => { setEditingId(null) setEditBody('') }} - onSave={() => void saveEdit(comment)} + onSave={() => void saveEdit(thread.comment)} onResolve={() => - void updateNoteComment(note.path, comment.id, { resolvedAt: null }) + void updateNoteComment(note.path, thread.comment.id, { resolvedAt: null }) } - onDelete={() => void deleteNoteComment(note.path, comment.id)} + onDelete={() => void deleteNoteComment(note.path, thread.comment.id)} /> ))}
@@ -355,14 +412,30 @@ export function CommentsPanel({ ) } +/** Display name and avatar letter: the vault's owner has no stored author. */ +function authorLabel(comment: Pick): string { + return comment.author?.trim() || 'You' +} + +function authorInitial(comment: Pick): string { + return authorLabel(comment).slice(0, 1).toUpperCase() +} + function CommentCard({ comment, + replies, rowIndex, active, commentsFocused, editing, editBody, onEditBody, + replying, + replyBody, + onReplyBody, + onReply, + onCancelReply, + onSubmitReply, onJump, onEdit, onCancelEdit, @@ -371,12 +444,19 @@ function CommentCard({ onDelete }: { comment: NoteComment + replies: NoteComment[] rowIndex: number active: boolean commentsFocused: boolean editing: boolean editBody: string onEditBody: (body: string) => void + replying: boolean + replyBody: string + onReplyBody: (body: string) => void + onReply: () => void + onCancelReply: () => void + onSubmitReply: () => void onJump: () => void onEdit: () => void onCancelEdit: () => void @@ -385,6 +465,7 @@ function CommentCard({ onDelete: () => void }): JSX.Element { const resolved = comment.resolvedAt != null + const assistant = !!comment.author // Render the comment body as Markdown (sanitized). Cached by renderMarkdown, // memoized per-body so card re-renders (hover/selection) don't re-parse. const bodyHtml = useMemo(() => renderMarkdown(comment.body), [comment.body]) @@ -412,12 +493,21 @@ function CommentCard({ ].join(' ')} >
-
- Y +
+ {authorInitial(comment)}
- You + + {authorLabel(comment)} + {dateFormatter.format(new Date(comment.updatedAt))} @@ -504,6 +594,78 @@ function CommentCard({ dangerouslySetInnerHTML={{ __html: bodyHtml }} /> )} + + {replies.length > 0 && ( +
+ {replies.map((reply) => ( + + ))} +
+ )} + + {replying && ( +
+