diff --git a/apps/desktop/package.json b/apps/desktop/package.json index b264f2b5..af6efe3e 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/desktop", "productName": "ZenNotes", - "version": "2.53.0", + "version": "2.54.0", "description": "ZenNotes desktop shell", "private": true, "main": "./out/main/index.js", diff --git a/apps/desktop/src/cli/commands/mcp.ts b/apps/desktop/src/cli/commands/mcp.ts index d0ea9632..8789436b 100644 --- a/apps/desktop/src/cli/commands/mcp.ts +++ b/apps/desktop/src/cli/commands/mcp.ts @@ -1,18 +1,28 @@ /** - * `zn mcp` — start the MCP server in stdio mode. The CLI process + * `zn mcp`: start the MCP server in stdio mode. The CLI process * effectively becomes the MCP server for as long as the calling * client (Claude Code, Claude Desktop, Codex) keeps the stdin pipe * open. We delegate to the same runMcpServer() the legacy - * out/main/mcp.js entry uses, so behavior is identical. + * out/main/mcp.js entry uses; the one difference is which vault the + * tools run against. `zn mcp` honours `--vault`, `--server` and + * `--token` like every other command (#831), so an agent can be + * pointed at a vault other than the one the desktop app has open, + * or at a server, from its MCP client config alone. Without flags it + * follows the environment and then the app, as before. A flag that + * names nothing is reported on stderr at startup and the server still + * starts; the CLI's usual "resolve, then fail the command" would leave + * the MCP client with a dead server and no message. */ import { runMcpServer } from '../../mcp/server.js' +import type { ParsedArgs } from '../args.js' +import { resolveTarget } from '../vault-target.js' -export async function cmdMcp(): Promise { - await runMcpServer() +export async function cmdMcp(args: ParsedArgs): Promise { + await runMcpServer({ resolveTarget: () => resolveTarget(args) }) // The MCP SDK's connect() returns once stdin/stdout listeners are // wired up. Node's event loop keeps the process alive while those - // listeners exist — but the CLI dispatcher would otherwise see this + // listeners exist, but the CLI dispatcher would otherwise see this // promise resolve and call process.exit(0), tearing down stdin // before the client can send any requests. Awaiting indefinitely // here pins the process to whatever lifetime the parent client diff --git a/apps/desktop/src/cli/index.ts b/apps/desktop/src/cli/index.ts index 7e07e8dc..186c6735 100644 --- a/apps/desktop/src/cli/index.ts +++ b/apps/desktop/src/cli/index.ts @@ -82,8 +82,11 @@ async function main(argv: string[]): Promise { // the subcommand before parsing flags so positionals don't include it. const { subcommand, parsed } = peelSubcommand(command, rest) + // The MCP server gets the parsed flags rather than a backend: it must boot + // even when `--vault` names nothing yet, warning on stderr and retrying on + // each tool call, so the client sees a server rather than an exit (#831). if (command === 'mcp') { - await cmdMcp() + await cmdMcp(parsed) return 0 } diff --git a/apps/desktop/src/cli/vault-target.ts b/apps/desktop/src/cli/vault-target.ts index 189e3c7c..6a3cc79c 100644 --- a/apps/desktop/src/cli/vault-target.ts +++ b/apps/desktop/src/cli/vault-target.ts @@ -145,9 +145,10 @@ export async function resolveVaultTarget( * The target when nothing named one: `ZENNOTES_SERVER` points at a server for * a whole shell session, `ZENNOTES_VAULT` at a folder, and otherwise the vault * the desktop app has open, a connected server included (#688). This is what - * `zn mcp` uses, so an agent works on the vault the user is looking at; the - * app's own token stays in the OS secret store, so a server that needs one - * gets it from `ZENNOTES_REMOTE_TOKEN` (or `--token`). + * `zn mcp` falls back to without `--vault` / `--server` (#831), so an agent + * works on the vault the user is looking at; the app's own token stays in the + * OS secret store, so a server that needs one gets it from + * `ZENNOTES_REMOTE_TOKEN` (or `--token`). */ export async function resolveDefaultTarget( env: NodeJS.ProcessEnv = process.env, diff --git a/apps/desktop/src/mcp/server.test.ts b/apps/desktop/src/mcp/server.test.ts index 3b1708ae..c1d08ee7 100644 --- a/apps/desktop/src/mcp/server.test.ts +++ b/apps/desktop/src/mcp/server.test.ts @@ -1,7 +1,22 @@ -import { describe, expect, it } from 'vitest' +import { createServer, type Server as HttpServer } from 'node:http' +import { promises as fsp } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { parse } from '../cli/args' import type { VaultBackend } from '../cli/backend' +import { resolveTarget, type VaultTarget } from '../cli/vault-target' import { RemoteRequestError } from '../main/remote/connection' -import { callTool, commentAuthorForClient, describeToolError, listToolNames } from './server' +import { + callTool, + commentAuthorForClient, + describeToolError, + listToolNames, + runMcpServer, + type McpServerOptions +} from './server' // Only the members a given test reaches are implemented; the cast keeps the // stubs honest about being partial. @@ -190,3 +205,171 @@ describe('comment tools (#738)', () => { expect(stored[1]).toMatchObject({ parentId: 'c1', anchorText: 'Ship the beta in October.' }) }) }) + +/** + * `zn mcp --vault beta` used to serve the vault the desktop app had open + * (#831): the CLI parsed the flags and then started the server without them. + * These sessions run the real server over an in-memory transport with the + * real flag parser and target resolution, against a scratch config whose + * active vault is "alpha". + */ +describe('runMcpServer follows the target it is given (#831)', () => { + let tmpDir: string + let configDir: string + let alpha: string + let beta: string + + async function writeConfig(config: Record): Promise { + await fsp.writeFile(path.join(configDir, 'zennotes.config.json'), JSON.stringify(config)) + } + + beforeAll(async () => { + tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'zen-mcp-831-')) + configDir = path.join(tmpDir, 'config') + alpha = path.join(tmpDir, 'alpha') + beta = path.join(tmpDir, 'beta') + await Promise.all( + [configDir, path.join(alpha, 'inbox'), path.join(beta, 'inbox')].map((dir) => + fsp.mkdir(dir, { recursive: true }) + ) + ) + }) + + afterAll(async () => { + await fsp.rm(tmpDir, { recursive: true, force: true }) + }) + + beforeEach(async () => { + vi.stubEnv('ZENNOTES_CONFIG_DIR', configDir) + vi.stubEnv('ZENNOTES_VAULT', '') + vi.stubEnv('ZENNOTES_SERVER', '') + vi.stubEnv('ZENNOTES_REMOTE_TOKEN', '') + await writeConfig({ + vaultRoot: alpha, + localVaults: [ + { root: alpha, name: 'alpha', lastOpenedAt: 2_000 }, + { root: beta, name: 'beta', lastOpenedAt: 1_000 } + ] + }) + }) + + afterEach(() => { + vi.unstubAllEnvs() + vi.restoreAllMocks() + }) + + /** A connected client for one server session; `vaultInfo` is what an agent + * sees when it calls the tool, `stderr` what the user sees at startup. */ + async function session(options: Omit = {}) { + const stderr: string[] = [] + vi.spyOn(process.stderr, 'write').mockImplementation((chunk) => { + stderr.push(String(chunk)) + return true + }) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + await runMcpServer({ ...options, transport: serverTransport }) + const client = new Client({ name: 'server-test', version: '0' }) + await client.connect(clientTransport) + return { + stderr, + vaultInfo: async () => { + const result = await client.callTool({ name: 'vault_info', arguments: {} }) + const text = (result.content as Array<{ text: string }>)[0].text + return result.isError ? { error: text } : { info: JSON.parse(text) as Record } + }, + close: () => client.close() + } + } + + /** What `zn mcp ` hands the server. */ + const flags = (...argv: string[]) => ({ resolveTarget: () => resolveTarget(parse(argv)) }) + + it('without a target follows the vault the app has open, quietly', async () => { + const s = await session() + expect((await s.vaultInfo()).info).toMatchObject({ kind: 'local', vaultRoot: alpha }) + expect(s.stderr).toEqual([]) + await s.close() + }) + + it('serves the vault --vault names, by path or by known name', async () => { + const byPath = await session(flags('--vault', beta)) + expect((await byPath.vaultInfo()).info).toMatchObject({ kind: 'local', vaultRoot: beta }) + await byPath.close() + + const byName = await session(flags('--vault', 'beta')) + expect((await byName.vaultInfo()).info).toMatchObject({ kind: 'local', vaultRoot: beta }) + await byName.close() + }) + + it('tells the user at startup when --vault names nothing, and still serves', async () => { + const s = await session(flags('--vault', path.join(tmpDir, 'no-such-vault'))) + // Said once, on stderr, before any tool call: that is where a terminal + // user and the MCP client's log see it. + expect(s.stderr).toHaveLength(1) + expect(s.stderr[0]).toContain('[zennotes-mcp] No vault named') + expect(s.stderr[0]).toContain('Known vaults: alpha, beta') + expect(s.stderr[0]).toContain('running anyway') + // The agent gets the same error instead of a silent fall-back to alpha. + const { error } = await s.vaultInfo() + expect(error).toContain('No vault named') + expect(error).toContain('alpha') + expect(s.stderr).toHaveLength(1) + await s.close() + }) + + it('reaches the server --server names and sends --token as its bearer token', async () => { + const seen: Array<{ url: string; auth: string | null }> = [] + const fake: HttpServer = createServer((req, res) => { + const url = new URL(req.url ?? '/', 'http://localhost') + seen.push({ url: url.pathname, auth: req.headers.authorization ?? null }) + const bodies: Record = { + '/api/vault': { root: '/srv/notes', name: 'notes' }, + '/api/vault/settings': { primaryNotesLocation: 'inbox', systemFolderPaths: null }, + '/api/folders': [] + } + res.writeHead(url.pathname in bodies ? 200 : 404, { 'Content-Type': 'application/json' }) + res.end(JSON.stringify(bodies[url.pathname] ?? null)) + }) + await new Promise((resolve) => fake.listen(0, '127.0.0.1', resolve)) + const address = fake.address() + if (address == null || typeof address === 'string') throw new Error('no port') + const baseUrl = `http://127.0.0.1:${address.port}` + + try { + const s = await session(flags('--server', `127.0.0.1:${address.port}`, '--token', 'secret-831')) + const { info } = await s.vaultInfo() + expect(info).toMatchObject({ kind: 'remote', server: baseUrl, authConfigured: true }) + expect(seen.length).toBeGreaterThan(0) + expect(seen.map((r) => r.auth)).toEqual(seen.map(() => 'Bearer secret-831')) + await s.close() + } finally { + await new Promise((resolve) => fake.close(() => resolve())) + } + }) + + it('pins the first vault that resolves and retries only after a failure', async () => { + // One attempt at startup (warned), one per failing tool call, then the + // session keeps the first vault that resolved. + const outcomes: Array = [ + new Error('not at startup'), + new Error('not yet'), + { kind: 'local', root: beta }, + { kind: 'local', root: alpha } + ] + let calls = 0 + const s = await session({ + resolveTarget: async () => { + const next = outcomes[calls++] + if (next instanceof Error) throw next + return next + } + }) + expect(s.stderr.join('')).toContain('not at startup') + expect((await s.vaultInfo()).error).toBe('Error: not yet') + expect((await s.vaultInfo()).info).toMatchObject({ vaultRoot: beta }) + // A further call must not move the session to alpha: the target is pinned. + expect((await s.vaultInfo()).info).toMatchObject({ vaultRoot: beta }) + expect(calls).toBe(3) + await s.close() + }) +}) diff --git a/apps/desktop/src/mcp/server.ts b/apps/desktop/src/mcp/server.ts index cb67b9c9..8f227dce 100644 --- a/apps/desktop/src/mcp/server.ts +++ b/apps/desktop/src/mcp/server.ts @@ -11,6 +11,7 @@ import { Server } from '@modelcontextprotocol/sdk/server/index.js' import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' +import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js' import { CallToolRequestSchema, ListToolsRequestSchema, @@ -19,7 +20,7 @@ import { import { resolveInstructions } from './instructions-store.js' import { createBackend, type VaultBackend } from '../cli/backend.js' -import { resolveDefaultTarget } from '../cli/vault-target.js' +import { resolveDefaultTarget, type VaultTarget } 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' @@ -965,16 +966,30 @@ export function describeToolError(err: unknown): string { return message } -export async function runMcpServer(): Promise { - // Resolve the vault lazily and once: the workspace the desktop app has - // open, a folder or a server (#688). When nothing is configured yet we - // still boot so the client surface stays consistent; every tool call then - // reports the missing-vault error, and the next call tries again rather - // than repeating a stale failure. +export interface McpServerOptions { + /** + * Which vault the tools run against. `zn mcp` passes the target its + * `--vault` / `--server` / `--token` flags name (#831); the legacy stdio + * entry has no flags and follows the environment, then the workspace the + * desktop app has open (#688), which is also the default here. + */ + resolveTarget?: () => Promise + /** Defaults to stdio, the only transport the clients speak. Tests bind an + * in-memory pair instead. */ + transport?: Transport +} + +export async function runMcpServer(options: McpServerOptions = {}): Promise { + const resolveTarget = options.resolveTarget ?? (() => resolveDefaultTarget()) + + // Resolve the vault once and keep it for the session. When nothing resolves + // yet we still boot so the client surface stays consistent; every tool call + // then reports the error, and the next call tries again rather than + // repeating a stale failure. let backendPromise: Promise | null = null const getBackend = (): Promise => { if (!backendPromise) { - backendPromise = resolveDefaultTarget().then(createBackend) + backendPromise = resolveTarget().then(createBackend) backendPromise.catch(() => { backendPromise = null }) @@ -982,6 +997,18 @@ export async function runMcpServer(): Promise { return backendPromise } + // Try at startup and say so on stderr when it fails: a terminal shows it at + // once and MCP clients keep it in their server logs, whereas waiting for the + // first tool call hid a `--vault` typo in a client config until the + // assistant tripped over it (#831). stdout is the protocol channel and + // stays clean. + await getBackend().catch((err: unknown) => { + const message = err instanceof Error ? err.message : String(err) + process.stderr.write( + `[zennotes-mcp] ${message} The MCP server is running anyway; every tool call returns this error until a vault resolves.\n` + ) + }) + const instructions = await resolveInstructions() const server = new Server( { name: 'zennotes', version: '0.1.0' }, @@ -1022,6 +1049,5 @@ export async function runMcpServer(): Promise { } }) - const transport = new StdioServerTransport() - await server.connect(transport) + await server.connect(options.transport ?? new StdioServerTransport()) } diff --git a/apps/share-viewer/package.json b/apps/share-viewer/package.json index f631afa7..47205ecc 100644 --- a/apps/share-viewer/package.json +++ b/apps/share-viewer/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/share-viewer", "private": true, - "version": "2.53.0", + "version": "2.54.0", "type": "module", "description": "Read-only renderer for publicly shared ZenNotes, embedded by the zennotes.org website", "homepage": "https://zennotes.org", diff --git a/apps/web/package.json b/apps/web/package.json index c4cc2252..40c3e4ec 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/web", "private": true, - "version": "2.53.0", + "version": "2.54.0", "type": "module", "description": "ZenNotes web client for self-hosted and hosted deployments", "homepage": "https://zennotes.org", diff --git a/package-lock.json b/package-lock.json index 99b5b6ef..344a27dc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "zennotes-monorepo", - "version": "2.53.0", + "version": "2.54.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "zennotes-monorepo", - "version": "2.53.0", + "version": "2.54.0", "hasInstallScript": true, "workspaces": [ "apps/*", @@ -23,7 +23,7 @@ }, "apps/desktop": { "name": "@zennotes/desktop", - "version": "2.53.0", + "version": "2.54.0", "license": "MIT", "dependencies": { "@codemirror/autocomplete": "^6.18.3", @@ -874,7 +874,7 @@ }, "apps/share-viewer": { "name": "@zennotes/share-viewer", - "version": "2.53.0", + "version": "2.54.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -945,7 +945,7 @@ }, "apps/web": { "name": "@zennotes/web", - "version": "2.53.0", + "version": "2.54.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -16382,7 +16382,7 @@ }, "packages/app-core": { "name": "@zennotes/app-core", - "version": "2.53.0", + "version": "2.54.0", "dependencies": { "@codemirror/autocomplete": "^6.18.3", "@codemirror/commands": "^6.7.1", @@ -16469,14 +16469,14 @@ }, "packages/bridge-contract": { "name": "@zennotes/bridge-contract", - "version": "2.53.0", + "version": "2.54.0", "devDependencies": { "typescript": "^5.7.2" } }, "packages/shared-domain": { "name": "@zennotes/shared-domain", - "version": "2.53.0", + "version": "2.54.0", "dependencies": { "@zennotes/bridge-contract": "*", "lz-string": "^1.5.0" @@ -16488,7 +16488,7 @@ }, "packages/shared-ui": { "name": "@zennotes/shared-ui", - "version": "2.53.0" + "version": "2.54.0" } } } diff --git a/package.json b/package.json index 1d1d4455..99add910 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "zennotes-monorepo", "private": true, - "version": "2.53.0", + "version": "2.54.0", "description": "ZenNotes monorepo for desktop, web, and self-hosted server builds", "packageManager": "npm@10.9.2", "engines": { diff --git a/packages/app-core/package.json b/packages/app-core/package.json index b222eb0b..22ae6422 100644 --- a/packages/app-core/package.json +++ b/packages/app-core/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/app-core", "private": true, - "version": "2.53.0", + "version": "2.54.0", "type": "module", "exports": { "./main": "./src/main.tsx", diff --git a/packages/app-core/src/components/Editor.tsx b/packages/app-core/src/components/Editor.tsx index 107ff572..22fd128b 100644 --- a/packages/app-core/src/components/Editor.tsx +++ b/packages/app-core/src/components/Editor.tsx @@ -11,6 +11,11 @@ import { useCallback, useEffect, useMemo, useRef } from "react"; import type { EditorView } from "@codemirror/view"; import { Vim, getCM } from "@replit/codemirror-vim"; import { registerDisplayLineMotion } from "../lib/cm-vim-display-line"; +import { + HALF_PAGE_MOTION, + halfPageMotionArgs, + registerHalfPageMotion, +} from "../lib/cm-vim-half-page-motion"; import { registerHeadingMotion } from "../lib/cm-vim-heading-motion"; import { registerReflowOperator } from "../lib/cm-vim-reflow"; import { @@ -142,48 +147,22 @@ function paneMapBindings( return [...new Set(bindings)]; } -/** - * Clamped half-page scroll for the editor, bound to Ctrl+D / Ctrl+U. - * - * Replaces CodeMirror-Vim's built-in ``/`` (`moveByScroll`), which - * derives its scroll target from the cursor's pixel coordinates. With live- - * preview decorations and folded headings shifting block heights, that math - * can resolve to the top of the document, snapping the cursor and viewport - * back to line 1 at the end of a note. Moving by display lines and scrolling - * by a fixed half-viewport — both clamped to the document bounds — can never - * wrap. Mirrors the clamped preview scroll (`scrollPreviewBy`) in VimNav. - */ -function editorHalfPage(view: EditorView | undefined, forward: boolean): void { - if (!view) return; - const scroller = view.scrollDOM; - const half = Math.max(1, Math.round(scroller.clientHeight / 2)); - const lineHeight = view.defaultLineHeight || 18; - const steps = Math.max(1, Math.round(half / lineHeight)); - let range = view.state.selection.main; - for (let i = 0; i < steps; i++) { - const next = view.moveVertically(range, forward); - if (next.head === range.head) break; // reached the first/last line — stop, never wrap - range = next; - } - const maxTop = Math.max(0, scroller.scrollHeight - scroller.clientHeight); - const nextTop = Math.max( - 0, - Math.min(maxTop, scroller.scrollTop + (forward ? half : -half)), - ); - view.dispatch({ selection: { anchor: range.head } }); - scroller.scrollTop = nextTop; -} +type VimKeymapMapping = { + id: KeymapId; + bindings: string[]; + // VimNav's global fallback stands down while the editor has focus (#578), + // so anything that used to reach it from a standing selection has to be + // mapped in visual context here as well. + contexts?: Array<"normal" | "visual">; +} & ( + | { action: string } + // A motion moves Vim's own selection head, so it also extends a visual + // selection; an action cannot (#825). + | { motion: string; motionArgs: Record } +); function syncVimKeymaps(overrides: KeymapOverrides): void { - const mappings: Array<{ - id: KeymapId; - action: string; - bindings: string[]; - // VimNav's global fallback stands down while the editor has focus (#578), - // so anything that used to reach it from a standing selection has to be - // mapped in visual context here as well. - contexts?: Array<"normal" | "visual">; - }> = [ + const mappings: VimKeymapMapping[] = [ { id: "vim.harperNext", action: "zenHarperNext", @@ -313,16 +292,25 @@ function syncVimKeymaps(overrides: KeymapOverrides): void { toVimSequence(getKeymapBinding(overrides, "vim.unfoldAll")), ].filter((binding): binding is string => !!binding), }, + // Half-page keys are a motion in normal AND visual context, so `v` + + // Ctrl+D grows the selection as far as Ctrl+D moves the cursor (#825). + // Operator-pending (`d`) is left to Vim's default motion, like + // j/k. The floating, Quick Note and external-file windows map the + // same motion to the default chords (`mapDefaultHalfPageKeys`). { id: "nav.halfPageDown", - action: "zenHalfPageDown", + contexts: ["normal", "visual"], + motion: HALF_PAGE_MOTION, + motionArgs: halfPageMotionArgs(true), bindings: [ toVimSequence(getKeymapBinding(overrides, "nav.halfPageDown")), ].filter((binding): binding is string => !!binding), }, { id: "nav.halfPageUp", - action: "zenHalfPageUp", + contexts: ["normal", "visual"], + motion: HALF_PAGE_MOTION, + motionArgs: halfPageMotionArgs(false), bindings: [ toVimSequence(getKeymapBinding(overrides, "nav.halfPageUp")), ].filter((binding): binding is string => !!binding), @@ -342,7 +330,13 @@ function syncVimKeymaps(overrides: KeymapOverrides): void { } for (const binding of mapping.bindings) { for (const context of contexts) { - Vim.mapCommand(binding, "action", mapping.action, {}, { context }); + if ("motion" in mapping) { + Vim.mapCommand(binding, "motion", mapping.motion, mapping.motionArgs, { + context, + }); + } else { + Vim.mapCommand(binding, "action", mapping.action, {}, { context }); + } } } syncedVimBindings[mapping.id] = mapping.bindings; @@ -576,6 +570,7 @@ function registerVimCommands(): void { () => useStore.getState().vimWrappedLineMotions, ); registerHeadingMotion(); + registerHalfPageMotion(); registerReflowOperator(); Vim.defineEx("write", "w", () => { @@ -1299,12 +1294,6 @@ function registerVimNoteCommands(): void { Vim.defineAction("unfoldHeadingAtCursor", () => runFold(unfoldCode as never)); Vim.defineAction("foldAllHeadings", () => runFold(foldAll as never)); Vim.defineAction("unfoldAllHeadings", () => runFold(unfoldAll as never)); - Vim.defineAction("zenHalfPageDown", (cm: ReturnType) => - editorHalfPage((cm as unknown as { cm6?: EditorView }).cm6, true), - ); - Vim.defineAction("zenHalfPageUp", (cm: ReturnType) => - editorHalfPage((cm as unknown as { cm6?: EditorView }).cm6, false), - ); Vim.defineEx("fold", "fold", () => runFold(foldCode as never)); Vim.defineEx("unfold", "unfold", () => runFold(unfoldCode as never)); Vim.defineEx("foldall", "foldall", () => runFold(foldAll as never)); diff --git a/packages/app-core/src/components/EditorPane.tsx b/packages/app-core/src/components/EditorPane.tsx index 1b2fdd5e..1d94e402 100644 --- a/packages/app-core/src/components/EditorPane.tsx +++ b/packages/app-core/src/components/EditorPane.tsx @@ -53,10 +53,9 @@ import { undo, undoDepth } from '@codemirror/commands' -import { markdown, markdownLanguage } from '@codemirror/lang-markdown' import { isImeComposing } from '../lib/ime' import { displayRowBoundaryKeymap } from '../lib/cm-display-row' -import { resolveCodeLanguage } from '../lib/cm-code-languages' +import { noteMarkdown } from '../lib/cm-markdown-language' import { customCodeFenceHighlightExtension } from '../lib/cm-custom-code-languages' import { markdownLinkExtension } from '../lib/cm-markdown-links' import { @@ -213,15 +212,19 @@ import { } from '../lib/tab-scroll-memory' import { activeOutlineLineForCursor, parseOutline } from '../lib/outline' import { + editorLandingTopMargin, findRenderedHeadingForOutlineLine, nextOutlinePreviewSyncLockUntil, outlineHeadingTextOffset, planPreviewJump, previewScrollTopForHeading, previewShowsNote, + previewShowsSourceLine, + previewVisibleSourceLines, scrollTopForElementRelativeTop, scrollTopForScrollRatio, - shouldSyncPreviewFromEditorViewport + shouldSyncPreviewFromEditorViewport, + type PreviewEditRequest } from '../lib/preview-outline-jump' import { ArchiveIcon, @@ -419,7 +422,7 @@ function buildEditorKeymap(vimMode: boolean, overrides: KeymapOverrides): Extens function markdownEditingExtensions(showHeadingLevelLabels = false): Extension[] { return [ - markdown({ base: markdownLanguage, codeLanguages: resolveCodeLanguage, addKeymap: false }), + noteMarkdown(), customCodeFenceHighlightExtension, markdownLinkExtension, vimAwareMarkdownKeymap, @@ -580,6 +583,46 @@ const OUTLINE_JUMP_TOP_MARGIN = 24 const OUTLINE_JUMP_SCROLL_SYNC_LOCK_MS = 450 const OUTLINE_JUMP_SCROLL_SYNC_SETTLE_MS = 120 const TASK_JUMP_HIGHLIGHT_MS = 1400 + +/** + * Where the editor lands when a pane leaves Preview. (#822) + * + * - `reading-position`: the default. The caret stays put while its line is + * still on screen in the reading view; once the reader has scrolled away + * from it, the editor opens on the block at the top of what they were + * reading instead of snapping back to a caret they left screens ago. + * - `caller`: the caller places the caret itself (a comment jump, a task + * jump), so the reading position must not override it. + * - a line: a block the reader pointed at, with the viewport offset that keeps + * it at the same height on screen. + */ +type EditorLanding = + | 'reading-position' + | 'caller' + | { line: number; topMargin: number } + +interface PendingEditorLanding { + path: string + line: number + topMargin: number +} + +function landEditorOnLine(view: EditorView, line: number, topMargin: number): void { + const safeLine = Math.min(Math.max(1, line), view.state.doc.lines) + const targetLine = view.state.doc.line(safeLine) + // Focus before moving the selection. CodeMirror mirrors a new selection + // into the DOM only while it owns focus; dispatched into an unfocused + // editor, the DOM selection stays parked where the last click left it + // (inside the editor that Preview had hidden), and the observer's next + // flush reads that stale caret back as a user selection, snapping the + // cursor to the old line a frame before the deferred focus arrives. + if (!view.hasFocus) view.focus() + view.dispatch({ + selection: { anchor: targetLine.from + outlineHeadingTextOffset(targetLine.text) }, + effects: EditorView.scrollIntoView(targetLine.from, { y: 'start', yMargin: topMargin }) + }) +} + const EMPTY_COMMENTS: NoteComment[] = [] const taskJumpHighlightEffect = StateEffect.define() const taskJumpHighlightDecoration = Decoration.line({ class: 'cm-task-jump-highlight' }) @@ -1034,6 +1077,9 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { // Preview (#543), or the target of a heading/block link followed while // reading (android#74). Applied and cleared from `onRendered`. const pendingPreviewLineRef = useRef<{ path: string; line: number } | null>(null) + // The reverse trip: the line the editor opens on when the pane leaves + // Preview, committed once the editor is back on screen. (#822) + const pendingEditorLandingRef = useRef(null) const lastProgrammaticPreviewTopRef = useRef(null) const lastRestoredPathRef = useRef(null) const vimCompartmentRef = useRef(null) @@ -1194,7 +1240,33 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { }, [revealSidePanel, setCalendarPanel]) - const applyPaneMode = useCallback((nextMode: PaneMode) => { + const lockOutlinePreviewSync = useCallback((durationMs = OUTLINE_JUMP_SCROLL_SYNC_LOCK_MS): void => { + // Outline jumps target a rendered heading; ratio sync can otherwise override them. + outlinePreviewSyncLockUntilRef.current = nextOutlinePreviewSyncLockUntil( + performance.now(), + durationMs, + outlinePreviewSyncLockUntilRef.current + ) + }, []) + + // The block at the top of what the reader has on screen, or null when the + // caret's own line is still in view (a peek at the rendering and back keeps + // the cursor exactly where it was) or the reading view is not this note's + // render yet. + const readingPositionLanding = useCallback((view: EditorView, path: string) => { + const previewEl = previewScrollRef.current + if (!previewShowsNote(previewEl, path)) return null + const visible = previewVisibleSourceLines(previewEl) + if (!visible) return null + const caretLine = view.state.doc.lineAt(view.state.selection.main.head).number + if (previewShowsSourceLine(visible, caretLine)) return null + return { line: visible.top, topMargin: OUTLINE_JUMP_TOP_MARGIN } + }, []) + + const applyPaneMode = useCallback(( + nextMode: PaneMode, + options: { landing?: EditorLanding } = {} + ) => { // Capture the cursor's line NOW, while the editor is still mounted: // preview-only mode tears the editor down, and "continue reading where I // was editing" needs this anchor to land the preview there. (#543) @@ -1211,6 +1283,26 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { line: view.state.doc.lineAt(view.state.selection.main.head).number } } + // And the way back: read the reading view's viewport NOW, while it is + // still in the DOM, so the editor can open where the reader is. (#822) + if (nextMode !== 'preview' && modeRef.current === 'preview' && activeTab) { + const landing = options.landing ?? 'reading-position' + let target: { line: number; topMargin: number } | null = null + if (landing === 'reading-position') { + if (view && viewPathRef.current === activeTab) { + target = readingPositionLanding(view, activeTab) + } + } else if (landing !== 'caller') { + target = landing + } + if (target) { + pendingEditorLandingRef.current = { path: activeTab, ...target } + // Preview → Split: hold the split sync until the editor has landed, + // or its first pass would drag the reading view to the editor's stale + // scroll position. The landing then re-aligns the reading view itself. + if (nextMode === 'split') lockOutlinePreviewSync() + } + } setPaneModeForPath(paneId, activeTab, nextMode) setActivePane(paneId) setFocusedPanel('editor') @@ -1221,7 +1313,15 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { } focusEditorNormalMode() }) - }, [activeTab, paneId, setPaneModeForPath, setActivePane, setFocusedPanel]) + }, [ + activeTab, + lockOutlinePreviewSync, + paneId, + readingPositionLanding, + setPaneModeForPath, + setActivePane, + setFocusedPanel + ]) // `zen:toggle-outline` — routed only to the active pane, same pattern // as the connections toggle. @@ -1315,15 +1415,6 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { return () => window.removeEventListener(ZEN_SET_PANE_MODE_EVENT, handler) }, [applyPaneMode, isActive]) - const lockOutlinePreviewSync = useCallback((durationMs = OUTLINE_JUMP_SCROLL_SYNC_LOCK_MS): void => { - // Outline jumps target a rendered heading; ratio sync can otherwise override them. - outlinePreviewSyncLockUntilRef.current = nextOutlinePreviewSyncLockUntil( - performance.now(), - durationMs, - outlinePreviewSyncLockUntilRef.current - ) - }, []) - const scrollPreviewToOutlineLine = useCallback((line: number): boolean => { // Works wherever the preview is mounted (split or preview), not in edit. if (mode === 'edit' || !content) return false @@ -1431,7 +1522,10 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { // Scroll the preview so the rendered block for `line` sits near the top: // the nearest data-source-line block at or above the line, like the split // sync's anchor walk, but from a bare line number (no live editor needed). - const scrollPreviewToSourceLine = useCallback((line: number): boolean => { + const scrollPreviewToSourceLine = useCallback(( + line: number, + topMargin = OUTLINE_JUMP_TOP_MARGIN + ): boolean => { const previewEl = previewScrollRef.current if (!previewEl) return false const blocks = previewEl.querySelectorAll('[data-source-line]') @@ -1446,7 +1540,7 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { } } const nextTop = anchor - ? scrollTopForElementRelativeTop(previewEl, anchor, OUTLINE_JUMP_TOP_MARGIN) + ? scrollTopForElementRelativeTop(previewEl, anchor, topMargin) : 0 previewEl.scrollTop = nextTop lastProgrammaticPreviewTopRef.current = previewEl.scrollTop @@ -1506,6 +1600,7 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { // visit. Declared before the pending-jump effect, so a jump that opens a // note in reading mode still sets its line after this reset. pendingPreviewLineRef.current = null + pendingEditorLandingRef.current = null outlinePreviewSyncLockUntilRef.current = 0 }, [content?.path]) @@ -1611,7 +1706,7 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { setActiveCommentId(comment.id) if (!view) return if (mode === 'preview') { - applyPaneMode('edit') + applyPaneMode('edit', { landing: 'caller' }) } const anchor = resolveCommentAnchor(comment, view.state.doc.toString()) const selection = @@ -1959,6 +2054,9 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { pointerOverRange(view, link.from, link.to, event.clientX, event.clientY) && followLinkTarget(link.target, { createWithoutAsking: true }) ) { + // Following the link ends its status-bar hover; a tap + // never sends the mouseleave that would (#820). + setHoveredLink(null) event.preventDefault() return true } @@ -1972,6 +2070,7 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { const sel = view.state.selection.main const rendered = sel.to < link.from || sel.from > link.to if (rendered && followLinkTarget(link.href)) { + setHoveredLink(null) event.preventDefault() return true } @@ -2618,7 +2717,7 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { // for the highlight it paints on the line. (android#74) const plan = planPreviewJump(pendingJumpLocation, content.body) if (plan.kind === 'edit') { - applyPaneMode('edit') + applyPaneMode('edit', { landing: 'caller' }) return } const previewEl = previewScrollRef.current @@ -3800,13 +3899,62 @@ export function EditorPane({ pane }: { pane: PaneLeaf }): JSX.Element { hasContentRef.current = content != null previewIsStaleRef.current = previewIsStale - const handlePreviewRequestEdit = useCallback(() => { + // Commit the line a reader carried out of Preview once the editor is on + // screen again: a frame after the mode switch, like an outline jump, so + // CodeMirror measures the freshly shown scroller before it scrolls. (#822) + useEffect(() => { + const target = pendingEditorLandingRef.current + if (!target || mode === 'preview' || !editorReady) return + if (target.path !== content?.path) return + const raf = requestAnimationFrame(() => { + const view = viewRef.current + if (!view || viewPathRef.current !== target.path) return + pendingEditorLandingRef.current = null + landEditorOnLine(view, target.line, target.topMargin) + if (mode === 'split') { + // Entering split reflowed the reading view to half its width, which + // moved the block the reader had at the top. Put it back there beside + // the editor's copy, and hold the split sync while both settle: its + // block-plus-pixel-offset mapping would otherwise pull the reading + // view a few lines off the line the editor just landed on. + lockOutlinePreviewSync() + scrollPreviewToSourceLine(target.line, target.topMargin) + } + }) + return () => cancelAnimationFrame(raf) + }, [content?.path, editorReady, lockOutlinePreviewSync, mode, scrollPreviewToSourceLine]) + + // A double-click on a rendered block (or the image embed's "Edit this + // block" button) opens that block in the editor, at the height it had on + // screen so the eye does not have to travel. Without a block to point at, + // leaving Preview still lands where the reader is. (#822) + const handlePreviewRequestEdit = useCallback((request?: PreviewEditRequest | null) => { + const previewEl = previewScrollRef.current + const landing: EditorLanding = + request?.sourceLine != null && previewEl + ? { + line: request.sourceLine, + topMargin: editorLandingTopMargin( + request.blockClientTop, + previewEl.getBoundingClientRect().top, + previewEl.clientHeight, + OUTLINE_JUMP_TOP_MARGIN + ) + } + : 'reading-position' if (mode === 'preview') { - applyPaneMode('edit') + applyPaneMode('edit', { landing }) return } + const view = viewRef.current + if (typeof landing === 'object' && view && viewPathRef.current === content?.path) { + // Split: the editor is already on screen. Hold the scroll sync so the + // reading view stays put while the editor comes to the block. + lockOutlinePreviewSync() + landEditorOnLine(view, landing.line, landing.topMargin) + } focusEditorNormalMode() - }, [applyPaneMode, mode]) + }, [applyPaneMode, content?.path, lockOutlinePreviewSync, mode]) // Editing follows the cursor so keyboard motion updates the Outline even // when the viewport barely moves. Preview mode remains scroll-driven. diff --git a/packages/app-core/src/components/ExternalFileApp.tsx b/packages/app-core/src/components/ExternalFileApp.tsx index 7d19779d..73f2871d 100644 --- a/packages/app-core/src/components/ExternalFileApp.tsx +++ b/packages/app-core/src/components/ExternalFileApp.tsx @@ -16,10 +16,11 @@ import { EditorView, drawSelection, highlightActiveLine, keymap } from '@codemir import { Vim, vim } from '@replit/codemirror-vim' import { history, historyKeymap, indentWithTab } from '@codemirror/commands' import { vimAwareDefaultKeymap, vimAwareMarkdownKeymap, vimAwareSearchKeymap } from '../lib/cm-vim-default-keymap' +import { mapDefaultHalfPageKeys, registerHalfPageMotion } from '../lib/cm-vim-half-page-motion' import { vimVisualHighlightExtension } from '../lib/cm-vim-visual-highlight' -import { markdown, markdownLanguage } from '@codemirror/lang-markdown' -import { resolveCodeLanguage } from '../lib/cm-code-languages' +import { noteMarkdown } from '../lib/cm-markdown-language' import { customCodeFenceHighlightExtension } from '../lib/cm-custom-code-languages' +import { vimHalfPageKeymap } from '../lib/vim-half-page-keymap' import { applyVimInsertEscape } from '../lib/vim-insert-escape' import { markdownListIndentPlugin } from '../lib/cm-markdown-list-indent' import { appMarkdownSnippetExtension } from '../lib/markdown-snippets-config' @@ -168,7 +169,7 @@ export function ExternalFileApp(): JSX.Element { editorTabSize(prefs.editorTabSize), highlightActiveLine(), prefs.wordWrap ? EditorView.lineWrapping : [], - markdown({ base: markdownLanguage, codeLanguages: resolveCodeLanguage, addKeymap: false }), + noteMarkdown(), customCodeFenceHighlightExtension, vimAwareMarkdownKeymap, markdownListIndentPlugin, @@ -178,6 +179,9 @@ export function ExternalFileApp(): JSX.Element { prefs.livePreview ? livePreviewPlugin : [], lineNumberExtension(prefs.lineNumberMode), keymap.of([ + // No keymap overrides in this window, so the default Ctrl+D / + // Ctrl+U reach Vim ahead of the search and history keymaps (#825). + ...vimHalfPageKeymap(prefs.vimMode, {}), indentWithTab, ...vimAwareDefaultKeymap(prefs.vimMode), ...historyKeymap, @@ -418,6 +422,9 @@ function registerExternalFileVimCommands(): void { if (externalFileVimRegistered) return externalFileVimRegistered = true + registerHalfPageMotion() + mapDefaultHalfPageKeys() + Vim.defineEx('write', 'w', () => { void externalFileHandlers.persist?.() }) diff --git a/packages/app-core/src/components/FloatingNoteApp.tsx b/packages/app-core/src/components/FloatingNoteApp.tsx index 788d0967..c5b532f5 100644 --- a/packages/app-core/src/components/FloatingNoteApp.tsx +++ b/packages/app-core/src/components/FloatingNoteApp.tsx @@ -29,14 +29,15 @@ import { Vim, vim } from '@replit/codemirror-vim' import { history, historyKeymap, indentWithTab } from '@codemirror/commands' import { vimAwareDefaultKeymap, vimAwareMarkdownKeymap, vimAwareSearchKeymap } from '../lib/cm-vim-default-keymap' import { vimVisualHighlightExtension } from '../lib/cm-vim-visual-highlight' -import { markdown, markdownLanguage } from '@codemirror/lang-markdown' -import { resolveCodeLanguage } from '../lib/cm-code-languages' +import { noteMarkdown } from '../lib/cm-markdown-language' import { customCodeFenceHighlightExtension } from '../lib/cm-custom-code-languages' import { markdownLinkExtension } from '../lib/cm-markdown-links' import { applyVimInsertEscape } from '../lib/vim-insert-escape' import { registerDisplayLineMotion } from '../lib/cm-vim-display-line' +import { mapDefaultHalfPageKeys, registerHalfPageMotion } from '../lib/cm-vim-half-page-motion' import { registerHeadingMotion } from '../lib/cm-vim-heading-motion' import { registerReflowOperator } from '../lib/cm-vim-reflow' +import { vimHalfPageKeymap } from '../lib/vim-half-page-keymap' import { isTouchPrimaryDevice, vimImeGuard } from '../lib/cm-vim-ime-guard' import { markdownListIndentPlugin } from '../lib/cm-markdown-list-indent' import { appMarkdownSnippetExtension } from '../lib/markdown-snippets-config' @@ -341,7 +342,7 @@ export function FloatingNoteApp({ notePath }: { notePath: string }): JSX.Element editorTabSize(prefs.editorTabSize), highlightActiveLine(), prefs.wordWrap ? EditorView.lineWrapping : [], - markdown({ base: markdownLanguage, codeLanguages: resolveCodeLanguage, addKeymap: false }), + noteMarkdown(), customCodeFenceHighlightExtension, markdownLinkExtension, vimAwareMarkdownKeymap, @@ -352,6 +353,9 @@ export function FloatingNoteApp({ notePath }: { notePath: string }): JSX.Element prefs.livePreview ? livePreviewPlugin : [], lineNumberExtension(prefs.lineNumberMode), keymap.of([ + // No keymap overrides in this window, so the default Ctrl+D / + // Ctrl+U reach Vim ahead of the search and history keymaps (#825). + ...vimHalfPageKeymap(prefs.vimMode, {}), indentWithTab, ...vimAwareDefaultKeymap(prefs.vimMode), ...historyKeymap, @@ -565,6 +569,8 @@ function registerFloatingVimCommands( floatingVimRegistered = true registerHeadingMotion() + registerHalfPageMotion() + mapDefaultHalfPageKeys() registerReflowOperator() Vim.defineEx('write', 'w', () => { diff --git a/packages/app-core/src/components/LazyPreview.tsx b/packages/app-core/src/components/LazyPreview.tsx index d7547770..0949d25a 100644 --- a/packages/app-core/src/components/LazyPreview.tsx +++ b/packages/app-core/src/components/LazyPreview.tsx @@ -1,5 +1,6 @@ import { lazy, Suspense } from 'react' import type { DiagramTabPayload } from '../lib/diagram-tabs' +import type { PreviewEditRequest } from '../lib/preview-outline-jump' const PreviewImpl = lazy(() => import('./Preview').then((mod) => ({ default: mod.Preview })) @@ -17,7 +18,7 @@ export function LazyPreview({ }: { markdown: string notePath: string - onRequestEdit?: (() => void) | null + onRequestEdit?: ((request?: PreviewEditRequest | null) => void) | null onRendered?: (() => void) | null }): JSX.Element { return ( diff --git a/packages/app-core/src/components/PinnedReferencePane.tsx b/packages/app-core/src/components/PinnedReferencePane.tsx index 75f04cf1..ca62d8f9 100644 --- a/packages/app-core/src/components/PinnedReferencePane.tsx +++ b/packages/app-core/src/components/PinnedReferencePane.tsx @@ -30,8 +30,7 @@ import { vim } from '@replit/codemirror-vim' import { history, historyKeymap, indentWithTab } from '@codemirror/commands' import { vimAwareDefaultKeymap, vimAwareMarkdownKeymap, vimAwareSearchKeymap } from '../lib/cm-vim-default-keymap' import { vimVisualHighlightExtension } from '../lib/cm-vim-visual-highlight' -import { markdown, markdownLanguage } from '@codemirror/lang-markdown' -import { resolveCodeLanguage } from '../lib/cm-code-languages' +import { noteMarkdown } from '../lib/cm-markdown-language' import { customCodeFenceHighlightExtension } from '../lib/cm-custom-code-languages' import { markdownLinkExtension } from '../lib/cm-markdown-links' import { @@ -226,7 +225,7 @@ export function PinnedReferencePane(): JSX.Element | null { ]), highlightActiveLine(), EditorView.lineWrapping, - markdown({ base: markdownLanguage, codeLanguages: resolveCodeLanguage, addKeymap: false }), + noteMarkdown(), customCodeFenceHighlightExtension, markdownLinkExtension, vimAwareMarkdownKeymap, diff --git a/packages/app-core/src/components/Preview-hovered-link.test.ts b/packages/app-core/src/components/Preview-hovered-link.test.ts new file mode 100644 index 00000000..e2ba209b --- /dev/null +++ b/packages/app-core/src/components/Preview-hovered-link.test.ts @@ -0,0 +1,116 @@ +// @vitest-environment jsdom + +import { act, createElement } from "react"; +import { createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { NoteMeta } from "@shared/ipc"; +import { setHoveredLink, useHoveredLinkStore } from "../lib/hovered-link"; +import { useStore } from "../store"; +import { Preview } from "./Preview"; + +const navMocks = vi.hoisted(() => ({ + openWikilinkTarget: vi.fn(async () => true), +})); + +vi.mock("../lib/wikilink-navigation", async (importOriginal) => ({ + ...(await importOriginal()), + openWikilinkTarget: navMocks.openWikilinkTarget, +})); + +( + globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean } +).IS_REACT_ACT_ENVIRONMENT = true; + +const alphaPlan = { + path: "Alpha plan.md", + title: "Alpha plan", + folder: "inbox", + siblingOrder: 0, + createdAt: 0, + updatedAt: 0, + size: 0, + tags: [], + wikilinks: [], + assetEmbeds: [], + hasAttachments: false, + excerpt: "", +} as NoteMeta; + +/** The preview attaches its DOM after an async render pass; wait for the link. */ +async function renderedWikilink(host: HTMLElement): Promise { + for (let i = 0; i < 50; i++) { + const anchor = host.querySelector("a.wikilink"); + if (anchor) return anchor; + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 5)); + }); + } + throw new Error("preview never rendered the wikilink"); +} + +describe("Preview status-bar link hover", () => { + beforeEach(() => { + navMocks.openWikilinkTarget.mockClear(); + Object.defineProperty(window, "matchMedia", { + configurable: true, + value: vi.fn().mockReturnValue({ + matches: false, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + }), + }); + Object.defineProperty(window, "zen", { + configurable: true, + value: { + getAppInfo: () => ({ runtime: "web" }), + // The hover card the mousemove opens reads the target note. + readNote: async () => ({ ...alphaPlan, body: "# Milestones" }), + }, + }); + useStore.setState({ notes: [alphaPlan] }); + setHoveredLink(null); + }); + + afterEach(() => { + delete (window as unknown as { zen?: unknown }).zen; + }); + + it("clears the hovered target when the wikilink is followed (#820)", async () => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + try { + act(() => + root.render( + createElement(Preview, { + markdown: "Go to [[Alpha plan#Milestones]] first.", + notePath: "Beta.md", + }), + ), + ); + const anchor = await renderedWikilink(host); + expect(anchor.dataset.resolvedPath).toBe("Alpha plan.md"); + + // A tap on a touch screen: the browser synthesizes mousemove and click + // on the link, and no mouseleave ever follows. + act(() => { + anchor.dispatchEvent(new MouseEvent("mousemove", { bubbles: true })); + }); + expect(useHoveredLinkStore.getState().href).toBe("Alpha plan#Milestones"); + + act(() => { + anchor.dispatchEvent( + new MouseEvent("click", { bubbles: true, cancelable: true }), + ); + }); + expect(navMocks.openWikilinkTarget).toHaveBeenCalledWith( + "Alpha plan.md", + "Alpha plan#Milestones", + ); + expect(useHoveredLinkStore.getState().href).toBeNull(); + } finally { + act(() => root.unmount()); + host.remove(); + } + }); +}); diff --git a/packages/app-core/src/components/Preview.tsx b/packages/app-core/src/components/Preview.tsx index ebaa663d..c084d311 100644 --- a/packages/app-core/src/components/Preview.tsx +++ b/packages/app-core/src/components/Preview.tsx @@ -38,6 +38,10 @@ import { isExcalidrawPath, isObsidianExcalidrawPath } from "@shared/excalidraw"; import { resolveExcalidrawEmbedPath } from "../lib/excalidraw-preview"; import { LazyExcalidrawPreview } from "./LazyExcalidrawPreview"; import { enhancePreviewHeadingFolds } from "../lib/preview-heading-fold"; +import { + previewEditRequestForTarget, + type PreviewEditRequest, +} from "../lib/preview-outline-jump"; import { renderDiagrams } from "../lib/diagram-renderers"; import { renderEmbeds, renderBookmarks } from "../lib/embed-renderers"; import { renderTypstMath } from "../lib/typst-math-render"; @@ -186,7 +190,7 @@ export const Preview = memo(function Preview({ }: { markdown: string; notePath: string; - onRequestEdit?: (() => void) | null; + onRequestEdit?: ((request?: PreviewEditRequest | null) => void) | null; onRendered?: (() => void) | null; }): JSX.Element { const ref = useRef(null); @@ -478,6 +482,12 @@ export const Preview = memo(function Preview({ } const anchor = target.closest("a") as HTMLAnchorElement | null; if (!anchor) return; + // Following a link ends its hover. A tap on a touch screen arrives as + // synthetic mouseover, mousemove and click with no mouseleave ever, so + // the target the mousemove put in the status bar would otherwise sit + // there until the next tap. A real pointer that is still over a link + // puts it back on its next move. (#820) + setHoveredLink(null); if (anchor.classList.contains("wikilink")) { e.preventDefault(); const path = anchor.dataset.resolvedPath; @@ -692,7 +702,22 @@ export const Preview = memo(function Preview({ const onMouseLeave = (): void => setHoveredLink(null); + // Double-click on a rendered block edits it right there, the way the VS + // Code markdown preview does; the block's source line and screen position + // travel with the request so the editor opens on it at the same height. + // Links, controls, embeds and diagrams keep their own double-click. (#822) + const onDoubleClick = (e: MouseEvent): void => { + if (e.button !== 0) return; + const requestEdit = onRequestEditRef.current; + if (!requestEdit) return; + const request = previewEditRequestForTarget(e.target); + if (!request) return; + e.preventDefault(); + requestEdit(request); + }; + root.addEventListener("click", onClick); + root.addEventListener("dblclick", onDoubleClick); root.addEventListener("mouseover", onMouseOver); root.addEventListener("mousemove", onMouseMove); root.addEventListener("mouseout", onMouseOut); @@ -702,6 +727,7 @@ export const Preview = memo(function Preview({ return () => { root.removeEventListener("click", onClick); + root.removeEventListener("dblclick", onDoubleClick); root.removeEventListener("mouseover", onMouseOver); root.removeEventListener("mousemove", onMouseMove); root.removeEventListener("mouseout", onMouseOut); diff --git a/packages/app-core/src/components/QuickCaptureApp.tsx b/packages/app-core/src/components/QuickCaptureApp.tsx index cc8cdd0f..00214ef6 100644 --- a/packages/app-core/src/components/QuickCaptureApp.tsx +++ b/packages/app-core/src/components/QuickCaptureApp.tsx @@ -44,12 +44,13 @@ import { history, historyKeymap, indentWithTab } from '@codemirror/commands' import { vimAwareDefaultKeymap, vimAwareMarkdownKeymap, vimAwareSearchKeymap } from '../lib/cm-vim-default-keymap' import { vimVisualHighlightExtension } from '../lib/cm-vim-visual-highlight' import { registerDisplayLineMotion } from '../lib/cm-vim-display-line' +import { mapDefaultHalfPageKeys, registerHalfPageMotion } from '../lib/cm-vim-half-page-motion' import { registerHeadingMotion } from '../lib/cm-vim-heading-motion' import { registerReflowOperator } from '../lib/cm-vim-reflow' +import { vimHalfPageKeymap } from '../lib/vim-half-page-keymap' import { isTouchPrimaryDevice, vimImeGuard } from '../lib/cm-vim-ime-guard' import { toggleWrap, wrapLink } from '../lib/cm-format' -import { markdown, markdownLanguage } from '@codemirror/lang-markdown' -import { resolveCodeLanguage } from '../lib/cm-code-languages' +import { noteMarkdown } from '../lib/cm-markdown-language' import { customCodeFenceHighlightExtension } from '../lib/cm-custom-code-languages' import { markdownLinkExtension } from '../lib/cm-markdown-links' import { markdownListIndentPlugin } from '../lib/cm-markdown-list-indent' @@ -222,6 +223,8 @@ function registerCaptureVimCommands( // #312: this window is a separate Electron renderer with its own Vim, so it // needs its own registration to get the main editor's j/k display-line motion. registerHeadingMotion() + registerHalfPageMotion() + mapDefaultHalfPageKeys() registerReflowOperator() Vim.defineEx('write', 'w', () => { @@ -476,7 +479,7 @@ export function QuickCaptureApp(): JSX.Element { editorTabSize(prefs.editorTabSize), highlightActiveLine(), EditorView.lineWrapping, - markdown({ base: markdownLanguage, codeLanguages: resolveCodeLanguage, addKeymap: false }), + noteMarkdown(), customCodeFenceHighlightExtension, markdownLinkExtension, vimAwareMarkdownKeymap, @@ -517,6 +520,9 @@ export function QuickCaptureApp(): JSX.Element { }) ), keymap.of([ + // No keymap overrides in this window, so the default Ctrl+D / + // Ctrl+U reach Vim ahead of the search and history keymaps (#825). + ...vimHalfPageKeymap(prefs.vimMode, {}), indentWithTab, ...vimAwareDefaultKeymap(prefs.vimMode), ...historyKeymap, diff --git a/packages/app-core/src/components/SearchCreateForm.tsx b/packages/app-core/src/components/SearchCreateForm.tsx new file mode 100644 index 00000000..a8be7e1e --- /dev/null +++ b/packages/app-core/src/components/SearchCreateForm.tsx @@ -0,0 +1,498 @@ +import { useEffect, useMemo, useRef, useState } from 'react' +import type { KeyboardEvent as ReactKeyboardEvent } from 'react' +import type { NoteMeta } from '@shared/ipc' +import { useStore } from '../store' +import { isImeComposing } from '../lib/ime' +import { isPaletteNextKey, isPalettePreviousKey } from '../lib/palette-nav' +import { resolveSystemFolderLabels } from '../lib/system-folder-labels' +import { isPrimaryNotesAtRoot, noteFolderSubpath } from '../lib/vault-layout' +import { resolveTypstPreambleFolder } from '../lib/typst-preamble' +import { countVaultTags } from '../lib/tags' +import { + addTag, + buildDestinationChoices, + checkNoteName, + destinationLabel, + filterDestinationChoices, + findNameCollision, + normalizeTag, + parseDestinationText, + rankTagChoices, + type AreaLabels, + type NoteDestination, + type SearchCreateDraft +} from '../lib/search-create' +import { Button } from './ui/Button' + +export interface SearchCreateTarget { + destination: NoteDestination + title: string + tags: string[] +} + +type Field = 'name' | 'folder' | 'tags' + +type TagRow = { kind: 'existing'; tag: string; count: number } | { kind: 'new'; tag: string } + +const INPUT_CLASS = + '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' +const ROW_CLASS = 'flex w-full min-w-0 items-center gap-3 px-4 py-2 text-left' + +/** Plain Enter, with none of the modifiers the form reads as another command. */ +function isPlainEnter(e: ReactKeyboardEvent): boolean { + return e.key === 'Enter' && !e.shiftKey && !e.metaKey && !e.ctrlKey && !e.altKey +} + +/** + * The second step of creating a note from search (#826): the name, the + * folder and the tags, each open to change before anything is written. The + * name arrives prefilled and selected, so the fast path is still Enter. Below + * the fields, the focused field's choices show as palette rows: folders while + * Folder has focus, tags while Tags has focus. + */ +export function SearchCreateForm({ + draft, + onBack, + onCreate, + onOpenExisting +}: { + draft: SearchCreateDraft + onBack: () => void + onCreate: (target: SearchCreateTarget) => void + onOpenExisting: (note: NoteMeta) => void +}): JSX.Element { + const notes = useStore((s) => s.notes) + const folders = useStore((s) => s.folders) + const vault = useStore((s) => s.vault) + const vaultSettings = useStore((s) => s.vaultSettings) + const systemFolderLabels = useStore((s) => s.systemFolderLabels) + const activeNote = useStore((s) => s.activeNote) + + const [name, setName] = useState(draft.name) + const [folderText, setFolderText] = useState(draft.folderText) + const [tags, setTags] = useState(draft.tags) + const [tagText, setTagText] = useState('') + const [focused, setFocused] = useState(null) + // -1 keeps the typed value; 0..n highlights a row, which Enter then picks. + const [folderActive, setFolderActive] = useState(-1) + const [tagActive, setTagActive] = useState(-1) + // The Folder field can arrive prefilled from a typed path. Landing in it + // lists every folder (the user came to look, or to change it); the list + // narrows only once they type. + const [folderTyped, setFolderTyped] = useState(false) + const nameRef = useRef(null) + const tagsRef = useRef(null) + const listRef = useRef(null) + + useEffect(() => { + nameRef.current?.focus() + nameRef.current?.select() + }, []) + + const labels: AreaLabels = useMemo(() => { + const resolved = resolveSystemFolderLabels(systemFolderLabels) + return { + inbox: isPrimaryNotesAtRoot(vaultSettings) ? (vault?.name ?? 'Vault') : resolved.inbox, + quick: resolved.quick, + archive: resolved.archive, + trash: resolved.trash + } + }, [systemFolderLabels, vault?.name, vaultSettings]) + + const choices = useMemo(() => buildDestinationChoices(folders, labels), [folders, labels]) + const tagCounts = useMemo( + () => + countVaultTags( + notes, + activeNote ? { path: activeNote.path, body: activeNote.body } : null, + resolveTypstPreambleFolder(vaultSettings?.typstPreambles?.folder) + ), + [notes, activeNote, vaultSettings?.typstPreambles?.folder] + ) + + const nameCheck = checkNoteName(name) + const destination = parseDestinationText(folderText) + const typedTag = tagText.trim() + const typedTagError = + typedTag && !normalizeTag(typedTag) + ? 'Tags start with a letter and use letters, digits, _ - or /.' + : null + const collision = + nameCheck.title && destination.destination + ? findNameCollision(nameCheck.title, destination.destination, notes, vaultSettings) + : null + const error = nameCheck.error ?? destination.error ?? typedTagError + const canCreate = !error && !collision?.sameFolder + + const folderRows = useMemo( + () => + focused === 'folder' ? filterDestinationChoices(choices, folderTyped ? folderText : '') : [], + [choices, focused, folderText, folderTyped] + ) + const tagRows = useMemo(() => { + if (focused !== 'tags') return [] + const rows: TagRow[] = rankTagChoices(tagText, tagCounts, tags).map((entry) => ({ + kind: 'existing', + ...entry + })) + const fresh = normalizeTag(tagText) + const known = (tag: string): boolean => + [...tagCounts.keys(), ...tags].some((t) => t.toLowerCase() === tag.toLowerCase()) + if (fresh && !known(fresh)) rows.push({ kind: 'new', tag: fresh }) + return rows + }, [focused, tagCounts, tagText, tags]) + + const activeRow = focused === 'folder' ? folderActive : focused === 'tags' ? tagActive : -1 + useEffect(() => { + if (activeRow < 0) return + listRef.current + ?.querySelector(`[data-search-form-idx="${activeRow}"]`) + ?.scrollIntoView({ block: 'nearest' }) + }, [activeRow]) + + const create = (): void => { + if (!canCreate || !nameCheck.title || !destination.destination) return + // Text still sitting in the Tags field counts: the user typed it and then + // reached for Create, not for Enter. + onCreate({ + destination: destination.destination, + title: nameCheck.title, + tags: addTag(tags, tagText, tagCounts) + }) + } + + const pickFolder = (value: string): void => { + setFolderText(value) + setFolderActive(-1) + tagsRef.current?.focus() + } + + const commitTag = (raw: string): void => { + setTags((prev) => addTag(prev, raw, tagCounts)) + setTagText('') + setTagActive(-1) + } + + const pickTagRow = (row: TagRow): void => commitTag(row.tag) + + const move = ( + rows: number, + setActive: (update: (prev: number) => number) => void, + delta: number + ): void => { + if (rows === 0) return + // Cycle through [typed value (-1), row 0 … n-1] and wrap around. + setActive((prev) => { + const next = prev + delta + if (next < -1) return rows - 1 + if (next >= rows) return -1 + return next + }) + } + + // Escape and the modified Enters belong to the whole form, whichever field + // has focus. Stopping propagation keeps the window-level Escape handler + // from closing the palette: here Escape means "back to the results". + const onFormKeyDown = (e: ReactKeyboardEvent): void => { + if (isImeComposing(e)) return + if (e.key === 'Escape') { + e.preventDefault() + e.stopPropagation() + onBack() + } else if (e.key === 'Enter' && e.shiftKey) { + e.preventDefault() + e.stopPropagation() + if (collision) onOpenExisting(collision.note) + } else if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) { + e.preventDefault() + e.stopPropagation() + create() + } + } + + const onFolderKeyDown = (e: ReactKeyboardEvent): void => { + if (isImeComposing(e)) return + if (isPaletteNextKey(e)) { + e.preventDefault() + move(folderRows.length, setFolderActive, 1) + } else if (isPalettePreviousKey(e)) { + e.preventDefault() + move(folderRows.length, setFolderActive, -1) + } else if (isPlainEnter(e)) { + e.preventDefault() + const row = folderActive >= 0 ? folderRows[folderActive] : undefined + if (row) pickFolder(row.value) + else create() + } + } + + const onTagsKeyDown = (e: ReactKeyboardEvent): void => { + if (isImeComposing(e)) return + if (isPaletteNextKey(e)) { + e.preventDefault() + move(tagRows.length, setTagActive, 1) + } else if (isPalettePreviousKey(e)) { + e.preventDefault() + move(tagRows.length, setTagActive, -1) + } else if (isPlainEnter(e)) { + e.preventDefault() + const row = tagActive >= 0 ? tagRows[tagActive] : undefined + if (row) pickTagRow(row) + else if (typedTag) { + if (normalizeTag(typedTag)) commitTag(typedTag) + } else create() + } else if (e.key === ' ' || e.key === ',') { + // A tag never contains either, so both finish the one being typed. + e.preventDefault() + if (normalizeTag(typedTag)) commitTag(typedTag) + } else if (e.key === 'Backspace' && tagText === '' && tags.length > 0) { + e.preventDefault() + setTags((prev) => prev.slice(0, -1)) + } + } + + const describe = (dest: NoteDestination): string => destinationLabel(dest, labels) + const collisionWhere = collision + ? describe({ + folder: collision.note.folder, + subpath: noteFolderSubpath(collision.note, vaultSettings) + }) + : '' + + return ( +
+
+
+ New note + + {destination.destination ? `in ${describe(destination.destination)}` : ''} + +
+
+ + setName(e.target.value)} + onFocus={() => setFocused('name')} + onBlur={() => setFocused((f) => (f === 'name' ? null : f))} + onKeyDown={(e) => { + if (isImeComposing(e)) return + if (isPlainEnter(e)) { + e.preventDefault() + create() + } + }} + placeholder="Note name" + className={INPUT_CLASS} + /> + + { + setFolderText(e.target.value) + setFolderTyped(true) + // Preselect the first match while typing, so Enter picks it + // without an arrow key first; the typed value stays one + // ArrowUp away, for a folder that does not exist yet. (#467) + setFolderActive(e.target.value.trim() ? 0 : -1) + }} + onFocus={() => { + setFocused('folder') + setFolderActive(-1) + setFolderTyped(false) + }} + onBlur={() => setFocused((f) => (f === 'folder' ? null : f))} + onKeyDown={onFolderKeyDown} + placeholder={`${labels.inbox} · type a folder like projects/ideas, or pick one below`} + autoCapitalize="none" + autoCorrect="off" + spellCheck={false} + className={INPUT_CLASS} + /> + +
{ + // The box is the field: clicking its padding or a chip lands + // in the input rather than dropping focus on the wrapper. + if (e.target !== tagsRef.current) { + e.preventDefault() + tagsRef.current?.focus() + } + }} + > + {tags.map((tag) => ( + + #{tag} + + + ))} + { + setTagText(e.target.value) + setTagActive(e.target.value.trim() ? 0 : -1) + }} + onFocus={() => { + setFocused('tags') + setTagActive(-1) + }} + onBlur={() => { + setFocused((f) => (f === 'tags' ? null : f)) + if (normalizeTag(typedTag)) commitTag(typedTag) + }} + onKeyDown={onTagsKeyDown} + placeholder={tags.length === 0 ? 'Add tags · space or , after each' : ''} + autoCapitalize="none" + autoCorrect="off" + spellCheck={false} + className="min-w-24 flex-1 bg-transparent py-0.5 text-sm text-ink-900 outline-none placeholder:text-ink-400" + /> +
+
+
+ {error ? ( + {error} + ) : collision ? ( + <> + + {collision.sameFolder + ? `"${collision.note.title}" already exists in ${collisionWhere}. Change the name, or open it.` + : `A note named "${collision.note.title}" already exists in ${collisionWhere}.`} + + + + ) : ( + + {destination.destination && nameCheck.title + ? `Creates "${nameCheck.title}" in ${describe(destination.destination)}${ + tags.length > 0 ? ` with ${tags.map((tag) => `#${tag}`).join(' ')}` : '' + }` + : ''} + + )} +
+
+ {(folderRows.length > 0 || tagRows.length > 0) && ( +
e.preventDefault()} + > + {focused === 'folder' && + folderRows.map((row, i) => ( + + ))} + {focused === 'tags' && + tagRows.map((row, i) => ( + + ))} +
+ )} +
+
+ + ↑↓ pick + + + ↵ create + + + esc back + +
+
+ + +
+
+
+ ) +} diff --git a/packages/app-core/src/components/SearchPalette.test.ts b/packages/app-core/src/components/SearchPalette.test.ts index 730cafb5..3d1667a6 100644 --- a/packages/app-core/src/components/SearchPalette.test.ts +++ b/packages/app-core/src/components/SearchPalette.test.ts @@ -12,6 +12,10 @@ import { SearchPalette } from './SearchPalette' const confirmApp = vi.hoisted(() => vi.fn(async () => true)) vi.mock('../lib/confirm-requests', () => ({ confirmApp })) +// Closing the palette hands focus back to the editor with a few timed retries; +// there is no editor here and the retries would outlive the jsdom window. +const focusEditorNormalMode = vi.hoisted(() => vi.fn()) +vi.mock('../lib/editor-focus', () => ({ focusEditorNormalMode })) function note(title: string): NoteMeta { return { @@ -150,3 +154,351 @@ describe('SearchPalette: Ctrl+D moves the highlighted note to Trash', () => { expect(document.querySelector('[data-search-idx="1"]')?.className).toContain('bg-paper-200') }) }) + +// #826: a search that finds no note with the typed name offers to create it: +// a create row after the results, and Shift+Enter from anywhere, both leading +// to a New note form where the name, folder and tags can change first. +describe('SearchPalette: the create row opens a New note form', () => { + let host: HTMLDivElement + let root: Root + let originalState: ReturnType + let originalScrollIntoView: PropertyDescriptor | undefined + let createAndOpen: ReturnType + let selectNote: ReturnType + + beforeEach(() => { + originalState = useStore.getState() + host = document.createElement('div') + document.body.append(host) + root = createRoot(host) + originalScrollIntoView = Object.getOwnPropertyDescriptor(Element.prototype, 'scrollIntoView') + Object.defineProperty(Element.prototype, 'scrollIntoView', { configurable: true, value: vi.fn() }) + + createAndOpen = vi.fn(async () => {}) + selectNote = vi.fn(async () => {}) + focusEditorNormalMode.mockClear() + useStore.setState({ + notes: [ + { ...note('Alpha'), tags: ['ops', 'prod'] }, + { ...note('Beta'), tags: ['ops'] }, + { ...note('Roadmap'), path: 'inbox/projects/Roadmap.md' } + ], + folders: [ + { folder: 'inbox', subpath: 'projects', siblingOrder: 0 }, + { folder: 'inbox', subpath: 'projects/ideas', siblingOrder: 0 }, + { folder: 'archive', subpath: 'old', siblingOrder: 0 } + ], + systemFolderLabels: {}, + selectedPath: null, + activeNote: null, + noteContents: {}, + noteDirty: {}, + searchOpen: true, + createAndOpen: createAndOpen as unknown as ReturnType['createAndOpen'], + selectNote: selectNote as unknown as ReturnType['selectNote'] + }) + act(() => root.render(createElement(SearchPalette))) + }) + + afterEach(() => { + act(() => root.unmount()) + host.remove() + document.body.innerHTML = '' + if (originalScrollIntoView) { + Object.defineProperty(Element.prototype, 'scrollIntoView', originalScrollIntoView) + } else { + delete (Element.prototype as { scrollIntoView?: unknown }).scrollIntoView + } + vi.restoreAllMocks() + useStore.setState(originalState, true) + }) + + const search = (): HTMLInputElement => { + const el = document.querySelector('input[placeholder^="Search notes"]') + if (!el) throw new Error('search input not rendered') + return el + } + const field = (id: 'name' | 'folder' | 'tags'): HTMLInputElement => { + const el = document.querySelector(`#search-create-${id}`) + if (!el) throw new Error(`${id} field not rendered`) + return el + } + const form = (): HTMLElement | null => document.querySelector('[data-search-create-form]') + const setValue = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + const type = (el: HTMLInputElement, text: string): void => { + act(() => { + setValue.call(el, text) + el.dispatchEvent(new Event('input', { bubbles: true })) + }) + } + const key = async (el: HTMLElement, key: string, init: KeyboardEventInit = {}): Promise => { + await act(async () => { + el.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...init })) + await Promise.resolve() + await Promise.resolve() + }) + } + const focus = (el: HTMLElement): void => { + act(() => el.focus()) + } + const createRow = (): HTMLButtonElement | null => + document.querySelector('[data-search-create]') + const highlighted = (): string | null => + document.querySelector('[data-search-idx].bg-paper-200')?.dataset.searchIdx ?? null + const formRows = (attr: 'folder' | 'tag'): string[] => + [...document.querySelectorAll(`[data-search-form-${attr}]`)].map( + (row) => row.dataset[attr === 'folder' ? 'searchFormFolder' : 'searchFormTag'] ?? '' + ) + const formHighlighted = (): string | null => + document.querySelector('[data-search-form-idx].bg-paper-200')?.dataset.searchFormIdx ?? + null + const chipTags = (): string[] => + [...document.querySelectorAll('[data-search-create-tag]')].map( + (chip) => chip.dataset.searchCreateTag ?? '' + ) + const status = (): { kind: string | undefined; text: string } => { + const el = document.querySelector('[data-search-create-status]') + return { kind: el?.dataset.searchCreateStatus, text: el?.textContent ?? '' } + } + const createButton = (): HTMLButtonElement => { + const el = [...document.querySelectorAll('button')].find( + (b) => b.textContent === 'Create' + ) + if (!el) throw new Error('Create button not rendered') + return el + } + + it('Shift+Enter opens the form with the name filled in and selected; Enter creates in Inbox', async () => { + type(search(), 'Meeting notes') + expect(createRow()?.textContent).toBe('Create "Meeting notes"…Inbox') + expect(document.body.textContent).not.toContain('No matches.') + + await key(search(), 'Enter', { shiftKey: true }) + expect(form()).not.toBeNull() + expect(createAndOpen).not.toHaveBeenCalled() + expect(field('name').value).toBe('Meeting notes') + expect(document.activeElement).toBe(field('name')) + expect(field('folder').value).toBe('') + expect(status()).toEqual({ kind: 'ok', text: 'Creates "Meeting notes" in Inbox' }) + + await key(field('name'), 'Enter') + expect(createAndOpen).toHaveBeenCalledWith('inbox', '', { title: 'Meeting notes', tags: [] }) + expect(selectNote).not.toHaveBeenCalled() + expect(useStore.getState().searchOpen).toBe(false) + // The name is settled, so the new note opens with the editor focused + // (like `:e name`), not with the title field waiting for one. + expect(focusEditorNormalMode).toHaveBeenCalledTimes(1) + }) + + it('the row follows the fuzzy matches; Enter on it opens the form instead of creating', async () => { + type(search(), 'Alph') + const rows = [...document.querySelectorAll('[data-search-idx]')] + expect(rows.map((r) => r.dataset.searchIdx)).toEqual(['0', '1']) + expect(rows[0].textContent).toBe('Alphainbox') + expect(rows[1].hasAttribute('data-search-create')).toBe(true) + expect(highlighted()).toBe('0') + + await key(search(), 'ArrowDown') + expect(highlighted()).toBe('1') + await key(search(), 'ArrowDown') + expect(highlighted()).toBe('1') + + await key(search(), 'Enter') + expect(form()).not.toBeNull() + expect(field('name').value).toBe('Alph') + expect(createAndOpen).not.toHaveBeenCalled() + }) + + it('plain Enter on a match still opens it, never the form', async () => { + type(search(), 'Alph') + await key(search(), 'Enter') + expect(selectNote).toHaveBeenCalledWith('inbox/Alpha.md') + expect(form()).toBeNull() + }) + + it('a typed path fills the Folder field, and the Folder picker filters, picks and moves on to Tags', async () => { + type(search(), 'projects/ideas/Q4 plan') + expect(createRow()?.textContent).toBe('Create "Q4 plan"…Inbox › projects/ideas') + await key(search(), 'Enter', { shiftKey: true }) + expect(field('folder').value).toBe('projects/ideas') + + // Focusing the field lists every folder (the three roots and their + // subfolders), nothing highlighted, so Enter would still create. Typing + // narrows the list and preselects the first hit. + focus(field('folder')) + expect(formRows('folder')).toEqual([ + '', + 'projects', + 'projects/ideas', + 'quick', + 'archive', + 'archive/old' + ]) + expect(formHighlighted()).toBeNull() + type(field('folder'), 'arch') + expect(formRows('folder')).toEqual(['archive', 'archive/old']) + expect(formHighlighted()).toBe('0') + await key(field('folder'), 'ArrowDown') + expect(formHighlighted()).toBe('1') + + await key(field('folder'), 'Enter') + expect(field('folder').value).toBe('archive/old') + expect(document.activeElement).toBe(field('tags')) + expect(status().text).toBe('Creates "Q4 plan" in Archive › old') + expect(createAndOpen).not.toHaveBeenCalled() + + await key(field('tags'), 'Enter') + expect(createAndOpen).toHaveBeenCalledWith('archive', 'old', { title: 'Q4 plan', tags: [] }) + }) + + it('a folder that does not exist yet is typed, not picked: ArrowUp keeps the typed value', async () => { + type(search(), 'Q4 plan') + await key(search(), 'Enter', { shiftKey: true }) + focus(field('folder')) + type(field('folder'), 'proj') + expect(formHighlighted()).toBe('0') + await key(field('folder'), 'ArrowUp') + expect(formHighlighted()).toBeNull() + await key(field('folder'), 'Enter') + expect(createAndOpen).toHaveBeenCalledWith('inbox', 'proj', { title: 'Q4 plan', tags: [] }) + }) + + it('a name already used in that folder blocks Create until it changes, and Shift+Enter opens the note', async () => { + type(search(), 'alpha') + await key(search(), 'Enter', { shiftKey: true }) + expect(status().kind).toBe('collision') + expect(status().text).toContain('"Alpha" already exists in Inbox. Change the name, or open it.') + expect(createButton().disabled).toBe(true) + + await key(field('name'), 'Enter') + expect(createAndOpen).not.toHaveBeenCalled() + expect(useStore.getState().searchOpen).toBe(true) + + type(field('name'), 'Alpha 2') + expect(status()).toEqual({ kind: 'ok', text: 'Creates "Alpha 2" in Inbox' }) + expect(createButton().disabled).toBe(false) + + type(field('name'), 'alpha') + await key(field('name'), 'Enter', { shiftKey: true }) + expect(selectNote).toHaveBeenCalledWith('inbox/Alpha.md') + expect(createAndOpen).not.toHaveBeenCalled() + expect(useStore.getState().searchOpen).toBe(false) + }) + + it('a same-named note elsewhere only warns, and the chosen folder decides', async () => { + type(search(), 'roadmap') + await key(search(), 'Enter', { shiftKey: true }) + expect(status().kind).toBe('collision') + expect(status().text).toContain('A note named "Roadmap" already exists in Inbox › projects.') + expect(createButton().disabled).toBe(false) + + type(field('folder'), 'projects') + expect(status().text).toContain('already exists in Inbox › projects. Change the name') + expect(createButton().disabled).toBe(true) + + type(field('folder'), '') + await key(field('name'), 'Enter') + expect(createAndOpen).toHaveBeenCalledWith('inbox', '', { title: 'roadmap', tags: [] }) + }) + + it('tags: query #words become chips, the picker offers vault tags, typing adds new ones', async () => { + type(search(), '#ops Runbook') + expect(createRow()?.textContent).toBe('Create "Runbook"…Inbox') + await key(search(), 'Enter', { shiftKey: true }) + expect(chipTags()).toEqual(['ops']) + + // No text: the vault's other tags, most used first, minus the chosen one. + focus(field('tags')) + expect(formRows('tag')).toEqual(['prod']) + expect(formHighlighted()).toBeNull() + + // A prefix preselects the vault tag; Enter takes that spelling. + type(field('tags'), 'pr') + expect(formRows('tag')).toEqual(['prod', 'pr']) + expect(formHighlighted()).toBe('0') + await key(field('tags'), 'Enter') + expect(chipTags()).toEqual(['ops', 'prod']) + expect(field('tags').value).toBe('') + + // Text no tag starts with is offered as new; a comma commits it as typed. + type(field('tags'), 'k8s') + expect(formRows('tag')).toEqual(['k8s']) + await key(field('tags'), ',') + expect(chipTags()).toEqual(['ops', 'prod', 'k8s']) + + // Backspace on an empty field takes the last chip back. + await key(field('tags'), 'Backspace') + expect(chipTags()).toEqual(['ops', 'prod']) + expect(status().text).toBe('Creates "Runbook" in Inbox with #ops #prod') + + await key(field('tags'), 'Enter') + expect(createAndOpen).toHaveBeenCalledWith('inbox', '', { + title: 'Runbook', + tags: ['ops', 'prod'] + }) + }) + + it('text left in the Tags field still counts when Create is clicked; text that is no tag blocks', async () => { + type(search(), 'Runbook') + await key(search(), 'Enter', { shiftKey: true }) + focus(field('tags')) + type(field('tags'), '9lives') + expect(status().kind).toBe('error') + expect(status().text).toMatch(/Tags start with a letter/) + expect(createButton().disabled).toBe(true) + + type(field('tags'), 'oncall') + expect(createButton().disabled).toBe(false) + await act(async () => { + createButton().click() + await Promise.resolve() + }) + expect(createAndOpen).toHaveBeenCalledWith('inbox', '', { title: 'Runbook', tags: ['oncall'] }) + }) + + it('Ctrl+Enter or Cmd+Enter creates from any field', async () => { + type(search(), 'Runbook') + await key(search(), 'Enter', { shiftKey: true }) + focus(field('folder')) + await key(field('folder'), 'Enter', { ctrlKey: true }) + expect(createAndOpen).toHaveBeenCalledWith('inbox', '', { title: 'Runbook', tags: [] }) + }) + + it('a name that cannot be a file opens the form with the reason; Escape goes back with the query kept', async () => { + type(search(), 'why?') + expect(createRow()?.textContent).toBe('Create "why?"…Inbox') + await key(search(), 'Enter', { shiftKey: true }) + expect(status().kind).toBe('error') + expect(status().text).toMatch(/cannot contain/) + expect(createButton().disabled).toBe(true) + await key(field('name'), 'Enter') + expect(createAndOpen).not.toHaveBeenCalled() + + await key(field('name'), 'Escape') + expect(form()).toBeNull() + expect(useStore.getState().searchOpen).toBe(true) + expect(search().value).toBe('why?') + expect(document.activeElement).toBe(search()) + }) + + it('a path into the Trash is refused in the Folder field', async () => { + type(search(), 'trash/Brand new') + await key(search(), 'Enter', { shiftKey: true }) + expect(field('name').value).toBe('Brand new') + expect(field('folder').value).toBe('trash') + expect(status()).toEqual({ kind: 'error', text: 'Notes cannot be created in the Trash.' }) + expect(createButton().disabled).toBe(true) + }) + + it('offers nothing for an empty or tag-only query', async () => { + expect(createRow()).toBeNull() + await key(search(), 'Enter', { shiftKey: true }) + expect(form()).toBeNull() + expect(useStore.getState().searchOpen).toBe(true) + + type(search(), '#ops') + expect(createRow()).toBeNull() + await key(search(), 'Enter', { shiftKey: true }) + expect(form()).toBeNull() + }) +}) diff --git a/packages/app-core/src/components/SearchPalette.tsx b/packages/app-core/src/components/SearchPalette.tsx index 8b85635e..8394930f 100644 --- a/packages/app-core/src/components/SearchPalette.tsx +++ b/packages/app-core/src/components/SearchPalette.tsx @@ -9,16 +9,32 @@ import { searchNoteIndex } from '../lib/note-search' import { focusEditorNormalMode } from '../lib/editor-focus' +import { + destinationLabel, + parseDestinationText, + searchCreateDraft, + type AreaLabels, + type SearchCreateDraft +} from '../lib/search-create' +import { resolveSystemFolderLabels } from '../lib/system-folder-labels' +import { isPrimaryNotesAtRoot } from '../lib/vault-layout' import { useToastStore } from '../lib/toast' import { Modal } from './ui/Modal' +import { SearchCreateForm, type SearchCreateTarget } from './SearchCreateForm' export function SearchPalette(): JSX.Element { const notes = useStore((s) => s.notes) const setSearchOpen = useStore((s) => s.setSearchOpen) const selectNote = useStore((s) => s.selectNote) + const createAndOpen = useStore((s) => s.createAndOpen) const trashNote = useStore((s) => s.trashNote) + const vault = useStore((s) => s.vault) + const vaultSettings = useStore((s) => s.vaultSettings) + const systemFolderLabels = useStore((s) => s.systemFolderLabels) const [query, setQuery] = useState('') const [active, setActive] = useState(0) + // While set, the palette shows the New note form instead of the results. + const [draft, setDraft] = useState(null) const inputRef = useRef(null) const listRef = useRef(null) @@ -28,15 +44,37 @@ export function SearchPalette(): JSX.Element { // more tags inline: `#ops #prod migration` means "notes tagged with // #ops AND #prod, fuzzy-matching 'migration'". Pure-tag queries (no // free text) still work — in that case we just list matching notes. - const { tagTokens } = useMemo(() => parseNoteSearchQuery(query), [query]) + const { freeText, tagTokens } = useMemo(() => parseNoteSearchQuery(query), [query]) const results = useMemo(() => { return searchNoteIndex(searchIndex, query, { limit: 20 }) }, [query, searchIndex]) + // A search that finds nothing is often the moment the note should start + // existing (#826). The free text drafts the note to create (its `#tags` + // become the note's tags); the create row sits after the results, at index + // results.length, and leads to a form where the name, folder and tags can + // still change, so it shows even when the name is taken or not yet valid. + const createRow = useMemo(() => searchCreateDraft(freeText, tagTokens), [freeText, tagTokens]) + const rowCount = results.length + (createRow ? 1 : 0) + const labels: AreaLabels = useMemo(() => { + const resolved = resolveSystemFolderLabels(systemFolderLabels) + return { + ...resolved, + inbox: isPrimaryNotesAtRoot(vaultSettings) ? (vault?.name ?? 'Vault') : resolved.inbox + } + }, [systemFolderLabels, vault?.name, vaultSettings]) + const createRowWhere = useMemo(() => { + if (!createRow) return '' + const parsed = parseDestinationText(createRow.folderText) + return parsed.destination ? destinationLabel(parsed.destination, labels) : '' + }, [createRow, labels]) + + // The input unmounts while the form shows, so focus it again when the form + // hands back (and on open, when there is no form yet). useEffect(() => { - inputRef.current?.focus() - }, []) + if (!draft) inputRef.current?.focus() + }, [draft]) useEffect(() => setActive(0), [query]) @@ -51,6 +89,17 @@ export function SearchPalette(): JSX.Element { focusEditorNormalMode() } + // Same landing as `:e name` and the dead-wikilink flow: the new note opens + // with the editor focused, because its name was settled in the form. + const createFromForm = async (target: SearchCreateTarget): Promise => { + setSearchOpen(false) + await createAndOpen(target.destination.folder, target.destination.subpath, { + title: target.title, + tags: target.tags + }) + focusEditorNormalMode() + } + const close = (): void => { setSearchOpen(false) focusEditorNormalMode() @@ -70,6 +119,19 @@ export function SearchPalette(): JSX.Element { setActive((a) => Math.max(0, Math.min(a, results.length - 2))) } + if (draft) { + return ( + + setDraft(null)} + onCreate={(target) => void createFromForm(target)} + onOpenExisting={(note) => void open(note)} + /> + + ) + } + return (
@@ -84,15 +146,20 @@ export function SearchPalette(): JSX.Element { if (isPaletteNextKey(e)) { e.preventDefault() e.stopPropagation() - setActive((a) => Math.min(results.length - 1, a + 1)) + setActive((a) => Math.max(0, Math.min(rowCount - 1, a + 1))) } else if (isPalettePreviousKey(e)) { e.preventDefault() e.stopPropagation() setActive((a) => Math.max(0, a - 1)) + } else if (e.key === 'Enter' && e.shiftKey) { + e.preventDefault() + e.stopPropagation() + if (createRow) setDraft(createRow) } else if (e.key === 'Enter') { e.preventDefault() const note = results[active] - if (note) open(note) + if (note) void open(note) + else if (createRow && active === results.length) setDraft(createRow) } else if ( e.ctrlKey && !e.metaKey && @@ -128,7 +195,7 @@ export function SearchPalette(): JSX.Element { )}
- {results.length === 0 ? ( + {rowCount === 0 ? (
No matches.
) : ( results.map((n, i) => ( @@ -151,18 +218,42 @@ export function SearchPalette(): JSX.Element { )) )} + {createRow && results.length > 0 && ( + -
+
↑↓{' '} - Ctrl+N/P{' '} - Ctrl+J/K move + Ctrl+N/P move ↵ open - Ctrl+D move to trash + Shift+↵ new note + + + Ctrl+D trash esc close diff --git a/packages/app-core/src/components/StatusBar.test.ts b/packages/app-core/src/components/StatusBar.test.ts index 0a2b6b67..ebc62542 100644 --- a/packages/app-core/src/components/StatusBar.test.ts +++ b/packages/app-core/src/components/StatusBar.test.ts @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { formatRelativeSyncTime } from "../lib/cloud-auto-sync"; import { useCloudSyncStatusStore } from "../lib/cloud-auto-sync"; +import { setHoveredLink, useHoveredLinkStore } from "../lib/hovered-link"; import { StatusBar } from "./StatusBar"; import { CloudConflictReviewHost } from "./CloudConflictReviewHost"; import { useStore } from "../store"; @@ -322,6 +323,77 @@ describe("cloud sync status time", () => { }); }); +describe("hovered link slot", () => { + beforeEach(() => { + useCloudSyncStatusStore.setState({ phase: "hidden" }); + useStore.setState({ notes: [], editorCursorPosition: null }); + setHoveredLink(null); + }); + + it("keeps the target while the same note is being typed in", () => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const slot = (): HTMLElement | null => + host.querySelector("[data-hovered-link]"); + + act(() => root.render(createElement(StatusBar, { note: noteAt("Alpha plan.md", "See [[Beta]]") }))); + act(() => setHoveredLink("Beta")); + expect(slot()?.textContent).toBe("Beta"); + + // A keystroke replaces the note object but not its path; the pointer is + // still on the link. + act(() => + root.render(createElement(StatusBar, { note: noteAt("Alpha plan.md", "See [[Beta]] now") })), + ); + expect(slot()?.textContent).toBe("Beta"); + expect(useHoveredLinkStore.getState().href).toBe("Beta"); + + act(() => root.unmount()); + host.remove(); + }); + + it("drops the target when the active note changes (#820)", () => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const slot = (): HTMLElement | null => + host.querySelector("[data-hovered-link]"); + + act(() => root.render(createElement(StatusBar, { note: noteAt("Alpha plan.md", "See [[Beta]]") }))); + // What a tap leaves behind: the synthetic mousemove set the target, and + // no mouseleave ever follows. + act(() => setHoveredLink("Alpha plan#Milestones")); + expect(slot()?.textContent).toBe("Alpha plan#Milestones"); + + act(() => root.render(createElement(StatusBar, { note: noteAt("Beta.md", "# Milestones") }))); + expect(slot()?.textContent).toBe(""); + expect(slot()?.title).toBe(""); + expect(useHoveredLinkStore.getState().href).toBeNull(); + + act(() => root.unmount()); + host.remove(); + }); +}); + +function noteAt(path: string, body: string): NoteContent { + return { + path, + title: path.replace(/\.md$/, ""), + folder: "inbox", + siblingOrder: 0, + createdAt: 0, + updatedAt: 0, + size: body.length, + tags: [], + wikilinks: [], + assetEmbeds: [], + hasAttachments: false, + excerpt: body, + body, + } as NoteContent; +} + function textVersion(path: string, text: string) { return { path, diff --git a/packages/app-core/src/components/StatusBar.tsx b/packages/app-core/src/components/StatusBar.tsx index 7f94dcc8..94cf43c5 100644 --- a/packages/app-core/src/components/StatusBar.tsx +++ b/packages/app-core/src/components/StatusBar.tsx @@ -3,7 +3,7 @@ import { useStore } from "../store"; import type { NoteContent, NoteMeta } from "@shared/ipc"; import { backlinksForNote } from "../lib/wikilinks"; import { countWords } from "../lib/word-count"; -import { useHoveredLinkStore } from "../lib/hovered-link"; +import { setHoveredLink, useHoveredLinkStore } from "../lib/hovered-link"; import { cloudSyncAttentionIsSettingsOnly, connectCloudAccountFromStatusBar, @@ -51,12 +51,22 @@ export function StatusBar({ note }: { note: NoteContent | null }): JSX.Element { // The target of the link the mouse is over (browser-style), shown on the left. const hoveredLink = useHoveredLinkStore((s) => s.href); + // A target belongs to the note it was hovered in. The preview root stays + // mounted across a note switch, so nothing else clears it when the note + // changes under a resting pointer, or after a tap on a touch screen, which + // synthesizes mousemove but never mouseleave. Keyed on the path, not the + // note object: typing must not blank a hover. (#820) + useEffect(() => { + setHoveredLink(null); + }, [note?.path]); + return (
diff --git a/packages/app-core/src/lib/cm-frontmatter-enter.test.ts b/packages/app-core/src/lib/cm-frontmatter-enter.test.ts new file mode 100644 index 00000000..1e5b20aa --- /dev/null +++ b/packages/app-core/src/lib/cm-frontmatter-enter.test.ts @@ -0,0 +1,142 @@ +// @vitest-environment jsdom +// #827: the frontmatter is carved out of the markdown parse, so the markdown +// Enter command no longer continues a `tags:` list there. This exercises the +// real dispatch chain the editors use (note grammar + vim-aware markdown +// keymap + default keymap) and pins what Enter does on every kind of +// frontmatter line, with Vim off and in Vim normal mode. +import { afterEach, describe, expect, it } from 'vitest' +import { EditorState } from '@codemirror/state' +import { EditorView, keymap } from '@codemirror/view' +import { vim } from '@replit/codemirror-vim' +import { noteMarkdown } from './cm-markdown-language' +import { vimAwareDefaultKeymap, vimAwareMarkdownKeymap } from './cm-vim-default-keymap' + +const views: EditorView[] = [] +afterEach(() => views.splice(0).forEach((v) => v.destroy())) + +function mount(doc: string, cursor: number, vimMode = false): EditorView { + const view = new EditorView({ + state: EditorState.create({ + doc, + selection: { anchor: cursor }, + extensions: [ + ...(vimMode ? [vim()] : []), + noteMarkdown(), + vimAwareMarkdownKeymap, + keymap.of([...vimAwareDefaultKeymap(vimMode)]) + ] + }), + parent: document.body + }) + views.push(view) + view.focus() + return view +} + +const pressEnter = (view: EditorView): void => { + view.contentDOM.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', keyCode: 13, bubbles: true, cancelable: true }) + ) +} + +const NOTE = `--- +title: InfSec +tags: + - todo + - +key: + nested: v +--- + +- item` + +// Absolute offset of the end of line `n` (1-based) in `doc`. +function lineEnd(doc: string, n: number): number { + const lines = doc.split('\n') + let pos = 0 + for (let i = 0; i < n; i++) pos += lines[i].length + (i < n - 1 ? 1 : 0) + return pos +} + +describe('Enter inside frontmatter (#827)', () => { + it('continues a YAML list item with the marker at the same indentation', () => { + const view = mount(NOTE, lineEnd(NOTE, 4)) // end of " - todo" + pressEnter(view) + expect(view.state.doc.line(5).text).toBe(' - ') + expect(view.state.selection.main.head).toBe(view.state.doc.line(5).to) + // The next line is the original empty item, untouched. + expect(view.state.doc.line(6).text).toBe(' - ') + }) + + it('ends the list on an empty item by clearing the marker', () => { + const view = mount(NOTE, lineEnd(NOTE, 5)) // end of " - " + pressEnter(view) + expect(view.state.doc.lines).toBe(NOTE.split('\n').length) + expect(view.state.doc.line(5).text).toBe('') + expect(view.state.selection.main.head).toBe(view.state.doc.line(5).from) + }) + + it('copies indentation on a nested key line (default Enter through indentNodeProp)', () => { + const view = mount(NOTE, lineEnd(NOTE, 7)) // end of " nested: v" + pressEnter(view) + expect(view.state.doc.line(8).text).toBe(' ') + expect(view.state.selection.main.head).toBe(view.state.doc.line(8).to) + }) + + it('inserts a plain newline on a top-level key line', () => { + const view = mount(NOTE, lineEnd(NOTE, 2)) // end of "title: InfSec" + pressEnter(view) + expect(view.state.doc.line(3).text).toBe('') + expect(view.state.doc.line(4).text).toBe('tags:') + }) + + it('leaves the closing fence to the default Enter', () => { + const view = mount(NOTE, lineEnd(NOTE, 8)) // end of the closing "---" + pressEnter(view) + expect(view.state.doc.line(8).text).toBe('---') + expect(view.state.doc.line(9).text).toBe('') + // A blank line was inserted; the body starts one line later. + expect(view.state.doc.line(11).text).toBe('- item') + }) + + it('does not treat a fence as a list item when the cursor is before the marker', () => { + const view = mount(NOTE, lineEnd(NOTE, 4) - '- todo'.length) // before "- todo" + pressEnter(view) + expect(view.state.doc.line(4).text).toBe(' ') + expect(view.state.doc.line(5).text).toBe(' - todo') + }) + + it('still lets markdown continue a list in the body', () => { + const view = mount(NOTE, NOTE.length) // end of "- item" + pressEnter(view) + expect(view.state.doc.line(view.state.doc.lines).text).toBe('- ') + }) + + it('does not fire on a `- item` line when the block is not closed', () => { + const open = '---\ntags:\n - todo\n\nbody' + const view = mount(open, lineEnd(open, 3)) // end of " - todo" + pressEnter(view) + // Without a closing fence the whole document is markdown and ` - todo` + // is a bullet, so the markdown command is the one continuing it here. + expect(view.state.doc.line(4).text).toBe(' - ') + }) + + it('in Vim normal mode Enter is a motion, not an edit', () => { + const view = mount(NOTE, lineEnd(NOTE, 4), true) // end of " - todo" + pressEnter(view) + // The j^ motion itself cannot be asserted: jsdom has no layout, so Vim's + // vertical motion cannot measure lines. What matters is that the list + // command deferred and nothing was edited. + expect(view.state.doc.toString()).toBe(NOTE) + }) + + it('in Vim insert mode Enter continues the list as with Vim off', () => { + const view = mount(NOTE, lineEnd(NOTE, 4), true) // end of " - todo" + view.contentDOM.dispatchEvent( + new KeyboardEvent('keydown', { key: 'a', keyCode: 65, bubbles: true, cancelable: true }) + ) // append after the cursor: insert mode at the end of the line + pressEnter(view) + expect(view.state.doc.line(5).text).toBe(' - ') + expect(view.state.doc.line(6).text).toBe(' - ') + }) +}) diff --git a/packages/app-core/src/lib/cm-frontmatter-tag-complete.ts b/packages/app-core/src/lib/cm-frontmatter-tag-complete.ts index 3807e82e..938613c3 100644 --- a/packages/app-core/src/lib/cm-frontmatter-tag-complete.ts +++ b/packages/app-core/src/lib/cm-frontmatter-tag-complete.ts @@ -5,7 +5,8 @@ */ import type { Completion, CompletionContext, CompletionResult } from '@codemirror/autocomplete' import type { EditorState } from '@codemirror/state' -import { collectTagCounts, rankTagCompletions } from './cm-hashtag-complete' +import { collectTagCounts } from './cm-hashtag-complete' +import { rankTagCompletions } from './tags' import { frontmatterTagsValue, isInsideFrontmatter } from './cm-frontmatter' /** Characters that terminate a tag token when scanning forward or backward diff --git a/packages/app-core/src/lib/cm-frontmatter.ts b/packages/app-core/src/lib/cm-frontmatter.ts index 3fbbdb5d..670e1cb1 100644 --- a/packages/app-core/src/lib/cm-frontmatter.ts +++ b/packages/app-core/src/lib/cm-frontmatter.ts @@ -1,11 +1,16 @@ /** * Render a note's leading YAML frontmatter block (the `---` … `---` at the very * top) as compact, muted "properties" instead of full-size body text. This is - * the in-editor counterpart to how the preview hides frontmatter — and it makes + * the in-editor counterpart to how the preview hides frontmatter, and it makes * database "record page" notes (whose properties live in frontmatter) read like * a property list rather than a wall of big text. + * + * The block itself is kept out of the markdown parser by the note grammar + * (cm-markdown-language.ts); this module only decorates it, plus one editing + * command (`insertNewlineContinueFrontmatterList`) for the list ergonomics + * that markdown used to provide by accident. */ -import { type EditorState, RangeSetBuilder } from '@codemirror/state' +import { EditorSelection, type EditorState, RangeSetBuilder } from '@codemirror/state' import { Decoration, type DecorationSet, @@ -13,23 +18,83 @@ import { ViewPlugin, type ViewUpdate } from '@codemirror/view' +import { isFrontmatterFence } from '@shared/markdown-lines' import { useStore } from '../store' /** Range of a closed leading `---` … `---` frontmatter block, or null if the * document does not start with one. Used by autocomplete to avoid offering * inline `#tags` inside frontmatter and to offer tags inside frontmatter - * `tags:` fields. */ + * `tags:` fields. The same predicate the note grammar scans with, so the + * card and the syntax tree always agree on where the block ends. */ export function frontmatterRange(state: EditorState): { from: number; to: number } | null { const doc = state.doc - if (doc.lines < 2 || doc.line(1).text.trim() !== '---') return null + if (doc.lines < 2 || !isFrontmatterFence(doc.line(1).text)) return null for (let i = 2; i <= doc.lines; i++) { - if (doc.line(i).text.trim() === '---') { + if (isFrontmatterFence(doc.line(i).text)) { return { from: doc.line(1).from, to: doc.line(i).to } } } return null } +/** A YAML sequence entry: optional indentation, `-`, then either nothing or a + * space and the value. `---` does not match (the second dash is not a space). */ +const FRONTMATTER_LIST_ITEM_RE = /^(\s*)-(?: +(.*))?$/ + +/** + * Enter on a `- item` line inside the frontmatter continues the YAML list. + * + * While the whole note was parsed as markdown, `tags:` followed by ` - todo` + * was a bullet list as far as the editor knew, so Enter added the next ` - ` + * for free and a second Enter on the empty item ended the list. The note + * grammar now keeps the frontmatter out of markdown (#827), which would have + * turned those two keystrokes back into plain line breaks. This command keeps + * the same two moves for YAML sequences: continue the item with the marker at + * the same indentation, or clear an empty item so the cursor is back at the + * key level. Everything else returns false and falls through to the default + * Enter, which copies the line's indentation. + */ +export function insertNewlineContinueFrontmatterList(view: EditorView): boolean { + const { state } = view + if (state.readOnly || state.selection.ranges.length > 1) return false + const range = state.selection.main + if (!range.empty) return false + const frontmatter = frontmatterRange(state) + if (!frontmatter) return false + const doc = state.doc + const line = doc.lineAt(range.head) + // Strictly between the fences: the fence lines belong to the default Enter. + if (line.number <= doc.lineAt(frontmatter.from).number) return false + if (line.number >= doc.lineAt(frontmatter.to).number) return false + const item = line.text.match(FRONTMATTER_LIST_ITEM_RE) + if (!item) return false + const indent = item[1] + const markerEnd = line.from + indent.length + 1 + // Cursor before the marker: a plain line break above the item. + if (range.head < markerEnd) return false + + if (!/\S/.test(line.text.slice(markerEnd - line.from))) { + // Second Enter on an empty `- ` ends the list, the way a markdown list + // does: the marker goes, and the cursor sits at the start of the line. + view.dispatch({ + changes: { from: line.from, to: line.to, insert: '' }, + selection: EditorSelection.cursor(line.from), + scrollIntoView: true, + userEvent: 'delete' + }) + return true + } + + const insert = state.lineBreak + indent + '- ' + view.dispatch({ + changes: { from: range.head, insert }, + selection: EditorSelection.cursor(range.head + insert.length), + scrollIntoView: true, + userEvent: 'input' + }) + return true +} + export function isInsideFrontmatter(state: EditorState, pos: number): boolean { const range = frontmatterRange(state) return range != null && pos >= range.from && pos <= range.to diff --git a/packages/app-core/src/lib/cm-hashtag-complete.ts b/packages/app-core/src/lib/cm-hashtag-complete.ts index fc78df73..401cbd38 100644 --- a/packages/app-core/src/lib/cm-hashtag-complete.ts +++ b/packages/app-core/src/lib/cm-hashtag-complete.ts @@ -13,7 +13,7 @@ import type { } from '@codemirror/autocomplete' import type { EditorView } from '@codemirror/view' import { useStore } from '../store' -import { noteTagsForCount } from './tags' +import { countVaultTags, rankTagCompletions } from './tags' import { resolveTypstPreambleFolder } from './typst-preamble' import { isTagSkippedContext } from './cm-hashtags' import { isInsideFrontmatter } from './cm-frontmatter' @@ -21,8 +21,6 @@ import { isInsideFrontmatter } from './cm-frontmatter' /** Completion carrying the `_icon` the shared slash renderer reads. */ type HashtagCompletion = Completion & { _icon?: string } -const MAX_SUGGESTIONS = 20 - /** * Match a `#tag` token immediately before the cursor. The `#` must follow the * start of the line or whitespace (the same boundary `extractTags` uses), so a @@ -41,8 +39,9 @@ function hashtagMatch(context: CompletionContext): { from: number; query: string /** * Unique tags across the vault (trash excluded), counted by how many notes use - * them. The active note is read live from its buffer so a tag just typed in the - * same note is offered too. Mirrors the aggregation in `TagView`. + * them, read from the store at completion time. The aggregation and the + * ranking live in `tags.ts` so the search palette's New note form can share + * them without pulling this module's CodeMirror imports into its chunk. */ export function collectTagCounts(): Map { const state = useStore.getState() @@ -52,32 +51,7 @@ export function collectTagCounts(): Map { state.vaultSettings?.typstPreambles?.folder ) const active = activePath && activeBody != null ? { path: activePath, body: activeBody } : null - const counter = new Map() - for (const note of state.notes) { - if (note.folder === 'trash') continue - for (const t of noteTagsForCount(note, active, preambleFolder)) { - counter.set(t, (counter.get(t) ?? 0) + 1) - } - } - return counter -} - -export interface RankedTag { tag: string; count: number } - -/** Rank vault tags for `query` so prefix matches beat substring matches, and - * more-used tags beat less-used ones. Excludes the exact tag already typed. */ -export function rankTagCompletions(query: string, counts: Map): RankedTag[] { - const q = query.toLowerCase() - return [...counts.entries()] - .map(([tag, count]) => { - const lower = tag.toLowerCase() - const rank = lower.startsWith(q) ? 0 : lower.includes(q) ? 1 : 2 - return { tag, lower, count, rank } - }) - .filter((t) => t.rank < 2 && t.lower !== q) - .sort((a, b) => a.rank - b.rank || b.count - a.count || a.tag.localeCompare(b.tag)) - .slice(0, MAX_SUGGESTIONS) - .map(({ tag, count }) => ({ tag, count })) + return countVaultTags(state.notes, active, preambleFolder) } export function hashtagSource(context: CompletionContext): CompletionResult | null { diff --git a/packages/app-core/src/lib/cm-markdown-language.test.ts b/packages/app-core/src/lib/cm-markdown-language.test.ts new file mode 100644 index 00000000..7b878021 --- /dev/null +++ b/packages/app-core/src/lib/cm-markdown-language.test.ts @@ -0,0 +1,274 @@ +import { markdownLanguage } from '@codemirror/lang-markdown' +import { ensureSyntaxTree, syntaxTree } from '@codemirror/language' +import { EditorState } from '@codemirror/state' +import type { SyntaxNode, Tree } from '@lezer/common' +import { describe, expect, it } from 'vitest' +import { frontmatterRange } from './cm-frontmatter' +import { noteMarkdown } from './cm-markdown-language' + +function parse(doc: string): { state: EditorState; tree: Tree } { + const state = EditorState.create({ doc, extensions: [noteMarkdown()] }) + const tree = ensureSyntaxTree(state, doc.length, 5000) + if (!tree) throw new Error('parse did not finish') + return { state, tree } +} + +/** Names of the nodes in the tree, pre-order, with absolute ranges. */ +function nodes(tree: Tree, from = 0, to = tree.length): string[] { + const out: string[] = [] + tree.iterate({ + from, + to, + enter: (node) => { + out.push(`${node.name}[${node.from},${node.to}]`) + } + }) + return out +} + +function names(tree: Tree): string[] { + return nodes(tree).map((entry) => entry.replace(/\[.*$/, '')) +} + +/** The markdown Document mounted on the body: the last child of the outer + * Document once the mount replaces the Body node. */ +function markdownDocument(tree: Tree): SyntaxNode { + const doc = tree.topNode.lastChild + if (!doc || doc.name !== 'Document') throw new Error(`no markdown document, got ${doc?.name}`) + return doc +} + +/** The markdown block (a direct child of the mounted Document) that contains + * `pos`. Its `.tree` is the object the incremental parser either reused from + * the previous tree or rebuilt, so identity between two trees means reuse. */ +function blockTreeAt(tree: Tree, pos: number): Tree { + for (let block = markdownDocument(tree).firstChild; block; block = block.nextSibling) { + if (block.from <= pos && pos < block.to) { + if (!block.tree) throw new Error(`block ${block.name} at ${pos} has no tree`) + return block.tree + } + } + throw new Error(`no block at ${pos}`) +} + +const REPORTED = `--- +title: InfSec +parent: [[Cyber Security]] +type: Uni Folder +tags: + - todo +--- +# InfSec + +Body text +Setext +------ + +--- +` + +describe('noteMarkdown: frontmatter is not markdown', () => { + it('parses the reported note without a setext heading in the frontmatter', () => { + const { tree } = parse(REPORTED) + const closing = REPORTED.indexOf('\n---\n#') + 1 + const all = nodes(tree) + expect(all[0]).toBe(`Document[0,${REPORTED.length}]`) + expect(all[1]).toBe(`Frontmatter[0,${closing + 3}]`) + expect(all[2]).toBe('FrontmatterMark[0,3]') + expect(all[3]).toBe(`FrontmatterMark[${closing},${closing + 3}]`) + // Nothing markdown-shaped inside the block: the outer Frontmatter node has + // exactly its two fences as children. + const frontmatter = nodes(tree, 0, closing + 3).filter((n) => !n.startsWith('Document')) + expect(frontmatter).toEqual([ + `Frontmatter[0,${closing + 3}]`, + 'FrontmatterMark[0,3]', + `FrontmatterMark[${closing},${closing + 3}]` + ]) + // The body keeps its markdown, at absolute positions. A setext underline + // turns the whole paragraph above it into the heading, so the real one + // starts at "Body text" and runs to the end of the dashes. + const heading = REPORTED.indexOf('# InfSec') + expect(all).toContain(`ATXHeading1[${heading},${heading + 8}]`) + const setextFrom = REPORTED.indexOf('Body text') + const setextTo = REPORTED.indexOf('------') + 6 + expect(all).toContain(`SetextHeading2[${setextFrom},${setextTo}]`) + expect(names(tree).filter((n) => n === 'SetextHeading2')).toHaveLength(1) + const rule = REPORTED.lastIndexOf('---') + expect(all).toContain(`HorizontalRule[${rule},${rule + 3}]`) + expect(names(tree).filter((n) => n === 'HorizontalRule')).toHaveLength(1) + }) + + it('agrees with frontmatterRange about where the block ends', () => { + for (const doc of [ + REPORTED, + '--- \nkey: value\n ---\nbody', + '---\nkey: value\n---', + '---\nkey: value\n---\n', + '---\n\nkey: value\n\n---\n\n# Title' + ]) { + const { state, tree } = parse(doc) + const range = frontmatterRange(state) + expect(range).not.toBeNull() + const frontmatter = tree.topNode.firstChild + expect(frontmatter?.name).toBe('Frontmatter') + expect({ from: frontmatter!.from, to: frontmatter!.to }).toEqual(range) + } + }) + + it('leaves an unclosed opening fence to markdown, as a horizontal rule', () => { + const doc = '---\ntitle: x\n\n# Heading' + const { state, tree } = parse(doc) + expect(frontmatterRange(state)).toBeNull() + const all = nodes(tree) + expect(all).not.toContain(expect.stringMatching(/^Frontmatter/)) + expect(all).toContain('HorizontalRule[0,3]') + expect(all).toContain(`ATXHeading1[${doc.indexOf('#')},${doc.length}]`) + }) + + it('only recognises a block that starts on line 1', () => { + const doc = '\n---\ntitle: x\n---\nbody' + const { tree } = parse(doc) + expect(names(tree)).not.toContain('Frontmatter') + expect(names(tree)).toContain('HorizontalRule') + }) + + it('does not mistake a longer dash run or a fence with text for a fence', () => { + for (const doc of ['----\ntitle: x\n---\nbody', '---\ntitle: x\n--- end\nbody']) { + const { state, tree } = parse(doc) + expect(frontmatterRange(state)).toBeNull() + expect(names(tree)).not.toContain('Frontmatter') + } + }) + + it('handles an empty document and a document that is only frontmatter', () => { + expect(nodes(parse('').tree)).toEqual(['Document[0,0]', 'Document[0,0]']) + const { tree } = parse('---\n---') + expect(nodes(tree)).toEqual([ + 'Document[0,7]', + 'Frontmatter[0,7]', + 'FrontmatterMark[0,3]', + 'FrontmatterMark[4,7]', + 'Document[7,7]' + ]) + }) + + it('finds a closing fence that straddles the read chunk boundary', () => { + const filler = 'k: ' + 'v'.repeat(4096 - 6) + '\n' + const doc = `---\n${filler}---\n# After` + const { state, tree } = parse(doc) + const range = frontmatterRange(state) + const frontmatter = tree.topNode.firstChild + expect(frontmatter?.name).toBe('Frontmatter') + expect({ from: frontmatter!.from, to: frontmatter!.to }).toEqual(range) + expect(names(tree)).toContain('ATXHeading1') + }) + + it('reports the markdown language active in the body and inactive in the frontmatter', () => { + const { state } = parse(REPORTED) + const inFrontmatter = REPORTED.indexOf('title') + 2 + const inBody = REPORTED.indexOf('Body') + 2 + expect(markdownLanguage.isActiveAt(state, inFrontmatter)).toBe(false) + expect(markdownLanguage.isActiveAt(state, inBody)).toBe(true) + expect(markdownLanguage.isActiveAt(state, REPORTED.length)).toBe(true) + }) + + it('still nests code fence languages in the body', () => { + const doc = '---\ntitle: x\n---\n\n```js\nconst a = 1\n```\n' + const { tree } = parse(doc) + expect(names(tree)).toContain('FencedCode') + // Code languages mount as overlays, which plain iteration skips but + // `resolveInner` enters: the `a` in `const a` is a JavaScript node. + const variable = doc.indexOf('const a') + 'const '.length + expect(tree.resolveInner(variable, 1).name).toBe('VariableDefinition') + }) +}) + +describe('noteMarkdown: incremental parsing', () => { + const paragraphs = Array.from({ length: 40 }, (_, i) => `Paragraph ${i} with some words.`) + const BODY = paragraphs.join('\n\n') + + function update(state: EditorState, from: number, to: number, insert: string): EditorState { + const next = state.update({ changes: { from, to, insert } }).state + // The state update parses for a bounded slice of time; make sure the + // tree is complete before comparing it. + ensureSyntaxTree(next, next.doc.length, 5000) + return next + } + + // A block well below the edit: the markdown parser never reuses the block + // touching the change or the final one, so those prove nothing either way. + const PROBE = 'Paragraph 20 ' + + it('reuses body blocks below an edit in the body', () => { + const doc = `---\ntitle: x\n---\n${BODY}` + const { state, tree } = parse(doc) + const probeAt = doc.indexOf(PROBE) + const probeTree = blockTreeAt(tree, probeAt) + + const editAt = doc.indexOf('Paragraph 3 ') + const next = update(state, editAt, editAt, 'Edited ') + const after = syntaxTree(next) + expect(blockTreeAt(after, probeAt + 'Edited '.length)).toBe(probeTree) + // And the edited block itself was reparsed with the new text. + const edited = blockTreeAt(after, editAt) + expect(edited).not.toBe(blockTreeAt(tree, editAt)) + expect(edited.length).toBe(blockTreeAt(tree, editAt).length + 'Edited '.length) + }) + + it('reuses body blocks below an edit inside the frontmatter', () => { + const doc = `---\ntitle: x\n---\n${BODY}` + const { state, tree } = parse(doc) + const probeAt = doc.indexOf(PROBE) + const probeTree = blockTreeAt(tree, probeAt) + + const editAt = doc.indexOf('title: x') + 'title: x'.length + const next = update(state, editAt, editAt, 'yz') + const after = syntaxTree(next) + expect(after.topNode.firstChild!.name).toBe('Frontmatter') + expect(after.topNode.firstChild!.to).toBe(doc.indexOf('\n---\n') + 1 + 3 + 2) + expect(blockTreeAt(after, probeAt + 2)).toBe(probeTree) + }) + + it('recognises the block the moment the closing fence is typed below an open one', () => { + // The "type frontmatter from scratch" flow: `---` on line 1 is a + // horizontal rule until the closing fence exists two lines further down. + const doc = `---\ntitle: x\n\n${BODY}` + const { state, tree } = parse(doc) + expect(names(tree)).toContain('HorizontalRule') + expect(names(tree)).not.toContain('Frontmatter') + const probeAt = doc.indexOf(PROBE) + const probeTree = blockTreeAt(tree, probeAt) + + const fenceAt = doc.indexOf('\n\n') + 1 + const next = update(state, fenceAt, fenceAt, '---') + const after = syntaxTree(next) + expect(nodes(after).slice(0, 4)).toEqual([ + `Document[0,${next.doc.length}]`, + `Frontmatter[0,${fenceAt + 3}]`, + 'FrontmatterMark[0,3]', + `FrontmatterMark[${fenceAt},${fenceAt + 3}]` + ]) + expect(names(after)).not.toContain('HorizontalRule') + expect(names(after)).not.toContain('SetextHeading2') + // This one transition reparses the body in full: the Body node moved from + // position 0 to the closing fence, and parseMixed (through @lezer/common + // 1.5.2) loses track of the old mount when the mounted node is the first + // child covering the new start. It costs one parse, the same as opening + // the note, and the very next edit is incremental again. + expect(blockTreeAt(after, probeAt + 3)).not.toBe(probeTree) + const settled = blockTreeAt(after, probeAt + 3) + const editAt = next.doc.toString().indexOf('Paragraph 3 ') + const again = update(next, editAt, editAt, 'Edited ') + expect(blockTreeAt(syntaxTree(again), probeAt + 3 + 'Edited '.length)).toBe(settled) + }) + + it('drops the block the moment its closing fence is broken', () => { + const doc = `---\ntitle: x\n---\n${BODY}` + const { state } = parse(doc) + const closing = doc.indexOf('\n---\n') + 1 + const next = update(state, closing + 3, closing + 3, 'x') + const after = syntaxTree(next) + expect(names(after)).not.toContain('Frontmatter') + expect(names(after)).toContain('HorizontalRule') + }) +}) diff --git a/packages/app-core/src/lib/cm-markdown-language.ts b/packages/app-core/src/lib/cm-markdown-language.ts new file mode 100644 index 00000000..a710db3b --- /dev/null +++ b/packages/app-core/src/lib/cm-markdown-language.ts @@ -0,0 +1,221 @@ +/** + * The note grammar every ZenNotes editor parses with: a leading YAML + * frontmatter block carved out of the document, markdown for everything + * after it. + * + * CommonMark knows nothing about frontmatter, so a note parsed as plain + * markdown reads `key: value` lines followed by the closing `---` as a setext + * heading (#827: the YAML header shows up in heading type) and the two fences + * as horizontal rules. The properties card (cm-frontmatter.ts) used to paper + * over that with CSS, but only inside the card, which does not load with Live + * Preview off (#616), so the raw view was the one that broke. + * + * Why not the two obvious tools: + * + * - `yamlFrontmatter` from @codemirror/lang-yaml treats an unclosed `---` as + * YAML to the end of the document (a note that opens with a horizontal rule + * loses all markdown) and disagrees with `frontmatterRange` about a fence + * with stray whitespace, so the card and the grammar would drift apart. + * - A markdown block-parser extension cannot see the closing fence when it is + * typed lines below an existing `---`: Lezer reuses the old HorizontalRule + * node from the previous tree and never re-runs the block parser on line 1, + * so the stale heading stays until the note is reopened. + * + * So this is a tiny outer parser. It re-scans the frontmatter range from the + * raw input on every parse (O(1) unless line 1 is a fence, one pass to the + * closing fence otherwise) and mounts the markdown parser onto the body + * through `parseMixed`. The mount hands the previous markdown tree to the + * inner parser as fragments, so body parsing stays incremental as long as the + * body keeps its start: edits anywhere in the body or inside the frontmatter + * reuse the untouched blocks. The two moments the body start moves (the + * closing fence typed for the first time, or broken) cost one full body + * parse, the same as opening the note; parseMixed cannot find the old mount + * across that move. The test file pins both behaviours. + * + * The frontmatter content is deliberately left untokenized: with Live Preview + * off a note reads as its raw text (#616), and the card styles it when on. + * Only the fences carry a token (`meta`), which the card already hides. + */ +import { markdown, markdownLanguage } from '@codemirror/lang-markdown' +import { + defineLanguageFacet, + indentNodeProp, + Language, + languageDataProp, + LanguageSupport +} from '@codemirror/language' +import { + type Input, + NodeType, + Parser, + type ParseWrapper, + type PartialParse, + parseMixed, + Tree, + type TreeFragment +} from '@lezer/common' +import { styleTags, tags as t } from '@lezer/highlight' +import { isFrontmatterFence } from '@shared/markdown-lines' +import { resolveCodeLanguage } from './cm-code-languages' + +type ParseRange = { from: number; to: number } + +const noteLanguageData = defineLanguageFacet() + +/** Top node. `indentNodeProp` mirrors the markdown Document (`() => null`): + * without it the tree's top-level fallback indents to column 0, and Enter on + * an indented frontmatter line would drop the indentation instead of + * copying it. */ +const documentType = NodeType.define({ + id: 0, + name: 'Document', + top: true, + props: [[languageDataProp, noteLanguageData], indentNodeProp.add({ Document: () => null })] +}) +/** The whole block, opening fence through the end of the closing fence line; + * the same range `frontmatterRange` (cm-frontmatter.ts) computes. */ +const frontmatterType = NodeType.define({ id: 1, name: 'Frontmatter' }) +/** One `---` fence. */ +const frontmatterMarkType = NodeType.define({ + id: 2, + name: 'FrontmatterMark', + props: [styleTags({ FrontmatterMark: t.meta })] +}) +/** Everything after the frontmatter (the whole document without one). The + * markdown tree is mounted here, so this node itself never shows up when + * iterating the syntax tree: the markdown Document takes its place. */ +const bodyType = NodeType.define({ id: 3, name: 'Body' }) + +interface FrontmatterSpan { + /** End of the opening fence line (its text, no line break). */ + openTo: number + closeFrom: number + closeTo: number +} + +const CHUNK = 4096 + +/** + * Find a closed frontmatter block at the start of `[from, to)`, reading the + * input in chunks so a large note is not sliced whole on every parse. Returns + * null as soon as line 1 is not a fence, or after reaching the end without a + * closing fence: an unclosed `---` stays markdown (a horizontal rule). + */ +export function scanFrontmatter(input: Input, from: number, to: number): FrontmatterSpan | null { + let text = '' + let textFrom = from + let lineFrom = from + let openTo = -1 + for (;;) { + let newline = text.indexOf('\n', lineFrom - textFrom) + while (newline < 0 && textFrom + text.length < to) { + // Drop the lines already consumed before appending, so the working + // string stays about one chunk long however far the scan goes. + if (lineFrom > textFrom) { + text = text.slice(lineFrom - textFrom) + textFrom = lineFrom + } + const end = textFrom + text.length + const next = input.read(end, Math.min(to, end + CHUNK)) + if (!next) break + text += next + newline = text.indexOf('\n', lineFrom - textFrom) + } + const lineTo = newline < 0 ? textFrom + text.length : textFrom + newline + const line = text.slice(lineFrom - textFrom, lineTo - textFrom) + if (isFrontmatterFence(line)) { + if (openTo >= 0) return { openTo, closeFrom: lineFrom, closeTo: lineTo } + openTo = lineTo + } else if (openTo < 0) { + return null + } + if (newline < 0) return null + lineFrom = lineTo + 1 + } +} + +function buildNoteTree(input: Input, from: number, to: number): Tree { + const span = scanFrontmatter(input, from, to) + const children: Tree[] = [] + const positions: number[] = [] + let bodyFrom = from + if (span) { + const fences = [ + new Tree(frontmatterMarkType, [], [], span.openTo - from), + new Tree(frontmatterMarkType, [], [], span.closeTo - span.closeFrom) + ] + children.push(new Tree(frontmatterType, fences, [0, span.closeFrom - from], span.closeTo - from)) + positions.push(0) + bodyFrom = span.closeTo + } + children.push(new Tree(bodyType, [], [], to - bodyFrom)) + positions.push(bodyFrom - from) + return new Tree(documentType, children, positions, to - from) +} + +/** The outer parse finishes in one step; `stopAt` is honoured by the mixed + * parse, which forwards it to the markdown parse of the body. */ +class NoteParse implements PartialParse { + parsedPos: number + stoppedAt: number | null = null + private readonly from: number + private readonly to: number + + constructor( + private readonly input: Input, + ranges: readonly ParseRange[] + ) { + this.from = ranges[0].from + this.to = ranges[ranges.length - 1].to + this.parsedPos = this.from + } + + advance(): Tree { + const tree = buildNoteTree(this.input, this.from, this.to) + this.parsedPos = this.to + return tree + } + + stopAt(pos: number): void { + this.stoppedAt = pos + } +} + +class NoteParser extends Parser { + private readonly wrap: ParseWrapper + + constructor(body: Parser) { + super() + this.wrap = parseMixed((node) => (node.type === bodyType ? { parser: body } : null)) + } + + createParse( + input: Input, + fragments: readonly TreeFragment[], + ranges: readonly ParseRange[] + ): PartialParse { + return this.wrap(new NoteParse(input, ranges), input, fragments, ranges) + } +} + +/** The markdown language the body is parsed with: GFM plus the vault's code + * fence languages. Its keymap is left off; `vimAwareMarkdownKeymap` adds + * the same bindings with Vim deference. */ +const noteBody = markdown({ + base: markdownLanguage, + codeLanguages: resolveCodeLanguage, + addKeymap: false +}) + +export const noteLanguage = new Language( + noteLanguageData, + new NoteParser(noteBody.language.parser), + [], + 'markdown' +) + +/** Language support for a note editor. Replaces `markdown({...})` in every + * editor so they all agree on where frontmatter ends and markdown begins. */ +export function noteMarkdown(): LanguageSupport { + return new LanguageSupport(noteLanguage, noteBody.support) +} diff --git a/packages/app-core/src/lib/cm-vim-default-keymap.ts b/packages/app-core/src/lib/cm-vim-default-keymap.ts index 486662e4..40bd1e9a 100644 --- a/packages/app-core/src/lib/cm-vim-default-keymap.ts +++ b/packages/app-core/src/lib/cm-vim-default-keymap.ts @@ -5,6 +5,7 @@ import { keymap, type EditorView, type KeyBinding } from '@codemirror/view' import { searchKeymap } from '@codemirror/search' import { getCM } from '@replit/codemirror-vim' import { insertNewlineContinueFencedCodeIndent } from './cm-code-fence-indent' +import { insertNewlineContinueFrontmatterList } from './cm-frontmatter' import { isMacPlatform } from './keymaps' /** @@ -203,8 +204,20 @@ const fencedCodeIndentEnter: KeyBinding = { run: insertNewlineContinueFencedCodeIndent } +// #827: the frontmatter is no longer markdown to the parser, so the markdown +// Enter command never sees a `tags:` list there. This keeps `- item` + Enter +// continuing the YAML sequence. It returns false outside the frontmatter and +// on non-list lines, so the markdown command runs as before. +const frontmatterListEnter: KeyBinding = { + key: 'Enter', + run: insertNewlineContinueFrontmatterList +} + export const vimAwareMarkdownKeymap: Extension = Prec.high( keymap.of( - deferKeysToVim([fencedCodeIndentEnter, ...markdownKeymap], new Set(['Enter', 'Backspace'])) + deferKeysToVim( + [frontmatterListEnter, fencedCodeIndentEnter, ...markdownKeymap], + new Set(['Enter', 'Backspace']) + ) ) ) diff --git a/packages/app-core/src/lib/cm-vim-display-line.ts b/packages/app-core/src/lib/cm-vim-display-line.ts index 8fb5fe40..f493f4f2 100644 --- a/packages/app-core/src/lib/cm-vim-display-line.ts +++ b/packages/app-core/src/lib/cm-vim-display-line.ts @@ -119,8 +119,10 @@ function warnPixelMotionFailure(err: unknown): void { * (seen on Linux fractional display scaling at certain zoom levels once a * line soft-wraps into 5+ display rows), so no exception may escape a motion. * The fallback keeps the cursor moving and the next press re-measures. + * Shared with the half-page motion (cm-vim-half-page-motion), which walks + * the same pixel-based display rows. */ -function pixelMotionFallback( +export function pixelMotionFallback( run: () => { line: number; ch: number }, fallback: () => { line: number; ch: number } ): { line: number; ch: number } { diff --git a/packages/app-core/src/lib/cm-vim-half-page-default-keys.test.ts b/packages/app-core/src/lib/cm-vim-half-page-default-keys.test.ts new file mode 100644 index 00000000..53e2439c --- /dev/null +++ b/packages/app-core/src/lib/cm-vim-half-page-default-keys.test.ts @@ -0,0 +1,123 @@ +// @vitest-environment jsdom +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest' +import { historyKeymap } from '@codemirror/commands' +import { searchKeymap } from '@codemirror/search' +import { EditorState } from '@codemirror/state' +import { EditorView, keymap } from '@codemirror/view' +import { vim } from '@replit/codemirror-vim' +import { mapDefaultHalfPageKeys, registerHalfPageMotion } from './cm-vim-half-page-motion' +import { vimHalfPageKeymap } from './vim-half-page-keymap' + +/** + * The wiring the floating note, Quick Note and external-file windows use: + * the default chords mapped on the global Vim plus the half-page keymap + * ahead of the search and history keymaps. The main editor maps the user's + * configured bindings through its keymap sync instead and is covered by + * vim-half-page-keymap.test.ts. + * + * jsdom has no layout: `coordsAtPos` throws, so the motion takes its + * logical-line fallback (one line per press, N lines with a count) and warns + * once. The pixel path is covered by the fake-view tests in + * cm-vim-half-page-motion.test.ts and by driving the built app. Unlike the + * tests that await timers, this one presses synchronously and destroys every + * view before the next frame, so the deferred measurement that the other Vim + * tests stub `Range.prototype` for never runs here, and stubbing it would + * hand the motion an empty geometry instead of the fallback under test. + */ +describe('default half-page keys in a secondary window', () => { + const views: EditorView[] = [] + const doc = ['one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight'].join('\n') + + beforeAll(() => { + // Both are the jsdom geometry failure: the motion's own fallback warning + // and CodeMirror logging the block-cursor measure it runs on the way in. + vi.spyOn(console, 'warn').mockImplementation(() => undefined) + vi.spyOn(console, 'error').mockImplementation(() => undefined) + registerHalfPageMotion() + mapDefaultHalfPageKeys() + }) + + afterEach(() => { + views.splice(0).forEach((view) => view.destroy()) + }) + + function mount(): EditorView { + // Same order as the windows: keymap before vim(), so CodeMirror's keymap + // handler sees the chord first, and multiple selections allowed as + // cm-vim-visual-highlight does, which is what let Ctrl+D add a cursor. + const view = new EditorView({ + state: EditorState.create({ + doc, + extensions: [ + EditorState.allowMultipleSelections.of(true), + keymap.of([...vimHalfPageKeymap(true, {}), ...historyKeymap, ...searchKeymap]), + vim() + ] + }), + parent: document.body + }) + views.push(view) + view.focus() + return view + } + + function press(view: EditorView, key: string, modifiers: KeyboardEventInit = {}): void { + view.contentDOM.dispatchEvent( + new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true, ...modifiers }) + ) + } + + it('moves the cursor in normal mode instead of selecting the word under it (#825)', () => { + const view = mount() + + press(view, 'd', { ctrlKey: true }) + // Left to the search keymap (Mod is Ctrl in jsdom, as on Linux) this would + // have selected "one" as {anchor: 0, head: 3}. + expect(view.state.selection.ranges).toHaveLength(1) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 4, head: 4 }) + + press(view, 'u', { ctrlKey: true }) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 0 }) + }) + + it('grows a visual selection as one range instead of adding cursors (#825)', () => { + const view = mount() + + press(view, 'v') + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 1 }) + + press(view, 'd', { ctrlKey: true }) + expect(view.state.selection.ranges).toHaveLength(1) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 5 }) + + press(view, 'd', { ctrlKey: true }) + expect(view.state.selection.ranges).toHaveLength(1) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 9 }) + + press(view, 'u', { ctrlKey: true }) + expect(view.state.selection.ranges).toHaveLength(1) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 5 }) + }) + + it('takes a count as a number of lines', () => { + const view = mount() + + press(view, '3') + press(view, 'd', { ctrlKey: true }) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 14, head: 14 }) + + press(view, '2') + press(view, 'u', { ctrlKey: true }) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 4, head: 4 }) + }) + + it('leaves insert mode alone', () => { + const view = mount() + + press(view, 'i') + press(view, 'd', { ctrlKey: true }) + // Vim's own insert-mode Ctrl+D (unindent) or the search keymap may act on + // the key, but the half-page motion must not move the cursor off line 1. + expect(view.state.doc.lineAt(view.state.selection.main.head).number).toBe(1) + }) +}) diff --git a/packages/app-core/src/lib/cm-vim-half-page-motion.test.ts b/packages/app-core/src/lib/cm-vim-half-page-motion.test.ts new file mode 100644 index 00000000..ce2d2d92 --- /dev/null +++ b/packages/app-core/src/lib/cm-vim-half-page-motion.test.ts @@ -0,0 +1,196 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { EditorSelection, Text } from '@codemirror/state' +import { halfPageDistance, zenMoveByHalfPage, type HalfPageView } from './cm-vim-half-page-motion' + +// Ten pixels per character, so a goal column in pixels is ten times the +// character column. The content starts 100px from the window's left edge to +// make sure the motion subtracts it like the codemirror-vim adapter does. +const CHAR_WIDTH = 10 +const CONTENT_LEFT = 100 + +type FakeView = HalfPageView & { moveVertically: ReturnType } + +/** + * A view whose vertical movement is one logical line per step at the same + * column (clipped to the target line's length) and whose edge behavior is + * CM6's: moving past the last line lands on the end of the document, past the + * first on offset 0, and from either of those the head does not move. + */ +function fakeView( + lines: string[], + geometry: { clientHeight: number; lineHeight: number; scrollTop?: number } +): FakeView { + const doc = Text.of(lines) + const view: FakeView = { + scrollDOM: { + clientHeight: geometry.clientHeight, + scrollHeight: lines.length * geometry.lineHeight, + scrollTop: geometry.scrollTop ?? 0 + }, + contentDOM: { getBoundingClientRect: () => ({ left: CONTENT_LEFT }) }, + defaultLineHeight: geometry.lineHeight, + state: { doc }, + coordsAtPos: (pos) => { + const line = doc.lineAt(pos) + return { left: CONTENT_LEFT + (pos - line.from) * CHAR_WIDTH } + }, + moveVertically: vi.fn((start, forward) => { + const edge = forward ? doc.length : 0 + if (start.head === edge) return start + const line = doc.lineAt(start.head) + const goal = start.goalColumn ?? (start.head - line.from) * CHAR_WIDTH + const targetNumber = forward ? line.number + 1 : line.number - 1 + if (targetNumber < 1 || targetNumber > doc.lines) { + return EditorSelection.cursor(edge, 1, undefined, goal) + } + const target = doc.line(targetNumber) + const col = Math.min(target.length, Math.round(goal / CHAR_WIDTH)) + return EditorSelection.cursor(target.from + col, 1, undefined, goal) + }) + } + return view +} + +function cmFor(view: HalfPageView | undefined, lineCount: number) { + return { firstLine: () => 0, lastLine: () => lineCount - 1, cm6: view } +} + +const LONG = 'abcdefghij' +const thirtyLines = Array.from({ length: 30 }, () => LONG) + +describe('halfPageDistance', () => { + it('is half the viewport without a count, in lines and pixels', () => { + expect(halfPageDistance(400, 20, 0)).toEqual({ lines: 10, pixels: 200 }) + // Odd viewports round rather than truncate. + expect(halfPageDistance(410, 20, 0)).toEqual({ lines: 10, pixels: 205 }) + }) + + it('is the typed count in lines and the same number of line heights in pixels', () => { + expect(halfPageDistance(400, 20, 3)).toEqual({ lines: 3, pixels: 60 }) + }) + + it('always moves at least one line and never divides by a zero line height', () => { + expect(halfPageDistance(0, 20, 0)).toEqual({ lines: 1, pixels: 1 }) + expect(halfPageDistance(400, 0, 0)).toEqual({ lines: 11, pixels: 200 }) + }) +}) + +describe('zenMoveByHalfPage', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('moves the head down half a page of display lines and scrolls the viewport as far', () => { + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20 }) + const vim = {} + + const target = zenMoveByHalfPage(cmFor(view, 30), { line: 0, ch: 2 }, { forward: true, repeat: 0 }, vim) + + expect(target).toMatchObject({ line: 10, ch: 2 }) + expect(view.scrollDOM.scrollTop).toBe(200) + expect(view.moveVertically).toHaveBeenCalledTimes(10) + }) + + it('moves back up and never scrolls above the top', () => { + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20, scrollTop: 120 }) + + const target = zenMoveByHalfPage(cmFor(view, 30), { line: 12, ch: 4 }, { forward: false, repeat: 0 }, {}) + + expect(target).toMatchObject({ line: 2, ch: 4 }) + expect(view.scrollDOM.scrollTop).toBe(0) + }) + + it('stops on the last line instead of wrapping, and clamps the scroll to the document', () => { + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20, scrollTop: 150 }) + + const target = zenMoveByHalfPage(cmFor(view, 30), { line: 25, ch: 3 }, { forward: true, repeat: 0 }, {}) + + expect(target.line).toBe(29) + // scrollHeight 600 minus clientHeight 400: the viewport cannot go past 200. + expect(view.scrollDOM.scrollTop).toBe(200) + }) + + it('stops on the first line instead of wrapping', () => { + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20, scrollTop: 40 }) + + const target = zenMoveByHalfPage(cmFor(view, 30), { line: 3, ch: 5 }, { forward: false, repeat: 0 }, {}) + + expect(target.line).toBe(0) + expect(view.scrollDOM.scrollTop).toBe(0) + }) + + it('moves and scrolls by the typed count instead of half a page', () => { + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20 }) + + const target = zenMoveByHalfPage( + cmFor(view, 30), + { line: 0, ch: 0 }, + { forward: true, repeat: 3, repeatIsExplicit: true }, + {} + ) + + expect(target).toMatchObject({ line: 3, ch: 0 }) + expect(view.scrollDOM.scrollTop).toBe(60) + }) + + it('ignores a leftover repeat that was not typed as a count', () => { + // codemirror-vim only marks the repeat explicit when digits were typed; + // a plain press arrives with repeat 0 and no flag and must page. + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20 }) + + const target = zenMoveByHalfPage(cmFor(view, 30), { line: 0, ch: 0 }, { forward: true, repeat: 7 }, {}) + + expect(target.line).toBe(10) + expect(view.scrollDOM.scrollTop).toBe(200) + }) + + it('keeps the goal column across consecutive presses through a short line', () => { + const lines = thirtyLines.slice() + lines[10] = 'ab' + const view = fakeView(lines, { clientHeight: 400, lineHeight: 20 }) + const vim: { lastMotion?: unknown; lastHSPos?: number } = {} + + const first = zenMoveByHalfPage(cmFor(view, 30), { line: 0, ch: 6 }, { forward: true, repeat: 0 }, vim) + expect(first).toMatchObject({ line: 10, ch: 2 }) + expect(vim.lastHSPos).toBe(60) + + // codemirror-vim records the motion that ran; the next press sees it. + vim.lastMotion = zenMoveByHalfPage + const second = zenMoveByHalfPage(cmFor(view, 30), first, { forward: true, repeat: 0 }, vim) + expect(second).toMatchObject({ line: 20, ch: 6 }) + + // Any other motion in between re-measures from the current head. + vim.lastMotion = () => undefined + const third = zenMoveByHalfPage(cmFor(view, 30), { line: 20, ch: 1 }, { forward: false, repeat: 0 }, vim) + expect(third).toMatchObject({ line: 10, ch: 1 }) + expect(vim.lastHSPos).toBe(10) + }) + + it('falls back to logical lines when the pixel path throws, and still scrolls', () => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined) + const view = fakeView(thirtyLines, { clientHeight: 400, lineHeight: 20 }) + view.moveVertically.mockImplementation(() => { + throw new Error('no layout') + }) + + const target = zenMoveByHalfPage(cmFor(view, 30), { line: 4, ch: 3 }, { forward: true, repeat: 0 }, {}) + + expect(target).toMatchObject({ line: 14, ch: 3 }) + expect(view.scrollDOM.scrollTop).toBe(200) + }) + + it('steps one logical line without a view to measure', () => { + expect(zenMoveByHalfPage(cmFor(undefined, 30), { line: 4, ch: 3 }, { forward: true, repeat: 0 }, {})).toMatchObject({ + line: 5, + ch: 3 + }) + expect( + zenMoveByHalfPage( + cmFor(undefined, 30), + { line: 4, ch: 3 }, + { forward: false, repeat: 9, repeatIsExplicit: true }, + {} + ) + ).toMatchObject({ line: 0, ch: 3 }) + }) +}) diff --git a/packages/app-core/src/lib/cm-vim-half-page-motion.ts b/packages/app-core/src/lib/cm-vim-half-page-motion.ts new file mode 100644 index 00000000..ef13cc5e --- /dev/null +++ b/packages/app-core/src/lib/cm-vim-half-page-motion.ts @@ -0,0 +1,210 @@ +import { EditorSelection, type SelectionRange } from '@codemirror/state' +import type { EditorView } from '@codemirror/view' +import { CodeMirror, Vim } from '@replit/codemirror-vim' +import { pixelMotionFallback } from './cm-vim-display-line' +import { getKeymapBinding } from './keymaps' +import { toVimSequence } from './vim-key-sequence' + +/** + * The parts of the CM6 view the motion reads and writes. Narrow on purpose so + * the unit test can stand in a fake with known geometry: jsdom has no layout, + * and the real layout is checked by driving the built app. + */ +export type HalfPageView = { + scrollDOM: { clientHeight: number; scrollHeight: number; scrollTop: number } + contentDOM: { getBoundingClientRect: () => { left: number } } + defaultLineHeight: number + state: { doc: EditorView['state']['doc'] } + coordsAtPos: (pos: number) => { left: number } | null + moveVertically: (start: SelectionRange, forward: boolean) => SelectionRange +} + +// Minimal shape of the CodeMirror-Vim adapter this motion touches. +type VimHalfPageCm = { + firstLine: () => number + lastLine: () => number + /** The underlying CodeMirror 6 view (set by the codemirror-vim adapter). */ + cm6?: HalfPageView +} + +type VimHalfPageMotionArgs = { + forward?: boolean + /** The typed count. With `explicitRepeat` on the mapping this is 0 when + * no count was typed, unlike most motions where it defaults to 1. */ + repeat?: number + repeatIsExplicit?: boolean +} + +type VimHalfPageState = { + lastMotion?: unknown + lastHSPos?: number +} + +/** + * How far one press goes. Without a count: half the visible editor, in + * display lines for the cursor and in pixels for the viewport, the same + * distance Vim's `scroll` option defaults to. `N` moves N lines and + * scrolls the viewport by N line heights, as Vim does with a count. + */ +export function halfPageDistance( + clientHeight: number, + lineHeight: number, + count: number +): { lines: number; pixels: number } { + const rowHeight = lineHeight > 0 ? lineHeight : 18 + if (count > 0) return { lines: count, pixels: count * rowHeight } + const pixels = Math.max(1, Math.round(clientHeight / 2)) + return { lines: Math.max(1, Math.round(pixels / rowHeight)), pixels } +} + +/** + * `` / `` as a Vim motion: move the cursor by half a page of + * display lines and scroll the viewport the same distance, both clamped to + * the note (#825). + * + * Replaces CodeMirror-Vim's built-in `moveByScroll`, which derives its + * scroll target from the cursor's pixel coordinates before and after the + * move. With live-preview decorations and folded headings shifting block + * heights, a position without coordinates reads as the top of the window, + * so that math could resolve to a negative offset and snap the cursor and + * viewport back to line 1 from the end of a note. Here the viewport moves by + * a fixed half-viewport (or N line heights with a count) and the cursor by + * display lines through `moveVertically`, which never wraps at either end. + * Mirrors the clamped preview scroll (`scrollPreviewBy`) in VimNav. + * + * This used to be an action mapped in normal mode only, which is why the + * keys did nothing useful with a selection: an action leaves Vim's own + * `vim.sel` untouched, so it cannot extend a visual selection, and the + * unmapped visual context left the key to CodeMirror's search and history + * keymaps on Linux and Windows (add a cursor, undo a selection). As a motion + * Vim itself moves the head, so `v` + `` grows the selection exactly as + * far as normal-mode `` moves, `V` grows it by whole lines, and a count + * works in both modes. + * + * The pixel path runs inside `pixelMotionFallback` (#574): if a coordinate + * query throws, the cursor still moves by logical lines and the viewport + * still scrolls, and the pressed key never lands in the note as text. + */ +export function zenMoveByHalfPage( + cm: VimHalfPageCm, + head: { line: number; ch: number }, + motionArgs: VimHalfPageMotionArgs, + vim: VimHalfPageState +): { line: number; ch: number } { + const forward = !!motionArgs.forward + const count = motionArgs.repeatIsExplicit ? Math.max(0, motionArgs.repeat || 0) : 0 + const view = cm.cm6 + if (!view) { + const step = count || 1 + return new CodeMirror.Pos( + clampLine(cm, forward ? head.line + step : head.line - step), + head.ch + ) + } + + const scroller = view.scrollDOM + const { lines, pixels } = halfPageDistance( + scroller.clientHeight, + view.defaultLineHeight, + count + ) + const logicalTarget = clampLine(cm, forward ? head.line + lines : head.line - lines) + + const target = pixelMotionFallback( + () => { + const doc = view.state.doc + const line = doc.line(Math.max(1, Math.min(doc.lines, head.line + 1))) + const from = Math.min(line.to, line.from + Math.max(0, head.ch)) + // Keep the horizontal goal column stable across consecutive presses, + // like j/k do, so passing a short line does not lose the column. + if (vim.lastMotion !== zenMoveByHalfPage || vim.lastHSPos == null) { + const coords = view.coordsAtPos(from) + vim.lastHSPos = coords + ? coords.left - view.contentDOM.getBoundingClientRect().left + : undefined + } + let range = EditorSelection.cursor(from, 1, undefined, vim.lastHSPos) + for (let i = 0; i < lines; i++) { + const next = view.moveVertically(range, forward) + // The first or last display line: stop, never wrap. + if (next.head === range.head) break + range = next + } + const landed = doc.lineAt(range.head) + return new CodeMirror.Pos(landed.number - 1, range.head - landed.from) + }, + () => new CodeMirror.Pos(logicalTarget, head.ch) + ) + + const maxTop = Math.max(0, scroller.scrollHeight - scroller.clientHeight) + scroller.scrollTop = Math.max( + 0, + Math.min(maxTop, scroller.scrollTop + (forward ? pixels : -pixels)) + ) + return target +} + +function clampLine(cm: VimHalfPageCm, line: number): number { + return Math.max(cm.firstLine(), Math.min(cm.lastLine(), line)) +} + +export const HALF_PAGE_MOTION = 'zenMoveByHalfPage' + +/** + * The mapping arguments for one direction. `explicitRepeat` hands the motion + * the typed count as is, 0 when none was typed, so `N` moves N lines + * while a bare press moves half a page. + */ +export function halfPageMotionArgs(forward: boolean): { forward: boolean; explicitRepeat: true } { + return { forward, explicitRepeat: true } +} + +let halfPageMotionRegistered = false + +/** + * Define the half-page motion on the (per-window) global Vim. Like the + * display-line motions, every renderer with an editor has its own Vim + * singleton, so each one calls this. Idempotent, so it is safe on HMR. The + * key mapping is separate: the main editor's keymap sync maps the user's + * configured `nav.halfPageDown` / `nav.halfPageUp` bindings, the other + * windows map the defaults through `mapDefaultHalfPageKeys`. + */ +export function registerHalfPageMotion(): void { + if (halfPageMotionRegistered) return + halfPageMotionRegistered = true + Vim.defineMotion( + HALF_PAGE_MOTION, + zenMoveByHalfPage as unknown as Parameters[1] + ) +} + +let defaultHalfPageKeysMapped = false + +/** + * Map the default half-page chords (Ctrl+D / Ctrl+U) to the motion in normal + * and visual context, for the windows that build their own editor and have + * no keymap overrides to consult (floating note, Quick Note, external file). + * Without this they were left with codemirror-vim's stock `moveByScroll`, + * and where Mod is Ctrl the search and history keymaps took the keys before + * Vim saw them at all: Ctrl+D selected the word under the cursor in normal + * mode and added a cursor per press in visual mode (#825). Pair with + * `vimHalfPageKeymap` in the window's CodeMirror keymap so the chords reach + * Vim first. Idempotent: each `Vim.mapCommand` call prepends a mapping. + */ +export function mapDefaultHalfPageKeys(): void { + if (defaultHalfPageKeysMapped) return + defaultHalfPageKeysMapped = true + const directions = [ + ['nav.halfPageDown', true], + ['nav.halfPageUp', false] + ] as const + for (const [id, forward] of directions) { + const sequence = toVimSequence(getKeymapBinding(null, id)) + if (!sequence) continue + for (const context of ['normal', 'visual'] as const) { + Vim.mapCommand(sequence, 'motion', HALF_PAGE_MOTION, halfPageMotionArgs(forward), { + context + }) + } + } +} diff --git a/packages/app-core/src/lib/cm-wikilink-render.ts b/packages/app-core/src/lib/cm-wikilink-render.ts index b60c5835..8bf6acd0 100644 --- a/packages/app-core/src/lib/cm-wikilink-render.ts +++ b/packages/app-core/src/lib/cm-wikilink-render.ts @@ -26,6 +26,7 @@ import { createNoteFromLinkNow, offerCreateNoteFromLink } from './create-note-fr import { openWikilinkAttachment } from './open-wikilink-attachment' import { resolveAssetPathAmong } from './asset-path-resolution' import { listDatabaseLinkTargets, resolveDatabaseWikilink } from './database-links' +import { setHoveredLink } from './hovered-link' // Same shape as the Preview pipeline (remarkWikilinks). const WIKILINK_RE = /(!?)\[\[([^\]|]+?)(?:\|([^\]]+))?\]\]/g @@ -278,6 +279,9 @@ const wikilinkClick = EditorView.domEventHandlers({ const target = el?.dataset.target if (!target) return false event.preventDefault() + // Following the link ends its status-bar hover; a tap never sends the + // mouseleave that would (#820). + setHoveredLink(null) openWikilink(target, { createWithoutAsking: event.button === 0 && (event.metaKey || event.ctrlKey) }) diff --git a/packages/app-core/src/lib/cm-wysiwyg-blocks.ts b/packages/app-core/src/lib/cm-wysiwyg-blocks.ts index 29a068b9..fdd2bb9c 100644 --- a/packages/app-core/src/lib/cm-wysiwyg-blocks.ts +++ b/packages/app-core/src/lib/cm-wysiwyg-blocks.ts @@ -7,7 +7,7 @@ * WYSIWYG-only: registered via `wysiwygExtensions()`; never loads in Split. */ import { syntaxTree } from '@codemirror/language' -import { RangeSetBuilder, type EditorState } from '@codemirror/state' +import { RangeSetBuilder } from '@codemirror/state' import { Decoration, type DecorationSet, @@ -17,18 +17,6 @@ import { WidgetType } from '@codemirror/view' import { calloutGroupFor } from './callout-types' -/** Line number (1-based) of the closing `---` of leading YAML frontmatter, - * or -1 when there is none. Lets us leave the frontmatter fences to the - * frontmatter styling rather than rendering them as horizontal rules. - * (Inlined: the PR's full frontmatter-properties module isn't ported.) */ -function frontmatterEndLine(state: EditorState): number { - const doc = state.doc - if (doc.lines < 2 || doc.line(1).text.trim() !== '---') return -1 - for (let i = 2; i <= doc.lines; i++) { - if (doc.line(i).text.trim() === '---') return i - } - return -1 -} const quoteLine = Decoration.line({ class: 'cm-wq-quote' }) @@ -121,10 +109,9 @@ function buildDecorations(view: EditorView): DecorationSet { const active = activeLineSet(view) const pending: Pending[] = [] const quotedLines = new Set() - // The properties widget owns the leading frontmatter (its `---` fences parse - // as HorizontalRule); skip that range so we don't emit an overlapping - // replace decoration over the same lines. - const fmEnd = frontmatterEndLine(state) + // The leading frontmatter is not part of the markdown tree (note grammar, + // cm-markdown-language.ts), so its fences never show up as HorizontalRule + // here; the properties widget owns those lines. for (const { from, to } of view.visibleRanges) { syntaxTree(state).iterate({ @@ -187,7 +174,6 @@ function buildDecorations(view: EditorView): DecorationSet { } if (node.name === 'HorizontalRule') { const lineNo = state.doc.lineAt(node.from).number - if (fmEnd >= 1 && lineNo <= fmEnd) return // leave frontmatter to the properties widget if (active.has(lineNo)) return // reveal `---` source on the active line pending.push({ from: node.from, to: node.to, deco: hrRule, line: false }) return diff --git a/packages/app-core/src/lib/harper-lint.test.ts b/packages/app-core/src/lib/harper-lint.test.ts index 95db9585..1f30be2a 100644 --- a/packages/app-core/src/lib/harper-lint.test.ts +++ b/packages/app-core/src/lib/harper-lint.test.ts @@ -97,6 +97,30 @@ describe.skipIf(process.platform === 'win32')('Harper session', () => { expect(await other.lint(text)).toEqual([]) }, 30_000) + it('keeps the dictionary and the ignored suggestions across a dialect change (#829)', async () => { + const current = await session() + const text = 'This is teh answer.' + await current.addWord('Zennotez') + const [first] = await current.lint(text) + await current.ignore(text, first) + const before = await current.exportState() + expect(before.words).toEqual(['Zennotez']) + expect(before.ignoredLints).toHaveLength(1) + + // harper.js answers a new dialect with a brand-new Linter and frees the + // old one, and the dictionary and the ignore list live in the Linter. + await current.configure({ dialect: 'british', lintConfig: {} }) + expect(await current.exportState()).toEqual(before) + expect((await current.lint('Open Zennotez today.')).map((lint) => lint.problem)).not.toContain( + 'Zennotez' + ) + expect((await current.lint(text)).map((lint) => lint.kind)).not.toContain(first.kind) + + // The same dialect again is not a change, so nothing is rebuilt or lost. + await current.configure({ dialect: 'british', lintConfig: {} }) + expect(await current.exportState()).toEqual(before) + }, 60_000) + it('does not lint a note past the size cap', async () => { const current = await session() const text = 'teh '.repeat(HARPER_LINT_CHAR_LIMIT / 4 + 1) diff --git a/packages/app-core/src/lib/harper-lint.ts b/packages/app-core/src/lib/harper-lint.ts index 34d43815..e08c65fe 100644 --- a/packages/app-core/src/lib/harper-lint.ts +++ b/packages/app-core/src/lib/harper-lint.ts @@ -123,29 +123,42 @@ export function harperSessionFromLinter( async ignore(text, lint) { await linter.ignoreLint(text, lint.raw) }, - async exportState() { - return { - words: await linter.exportWords(), - ignoredLints: harperIgnoredLintHashes(await linter.exportIgnoredLints()) - } - }, + exportState: () => exportState(linter), async configure(options) { - await linter.setDialect(toDialect(harper.Dialect, options.dialect)) + const dialect = toDialect(harper.Dialect, options.dialect) + if ((await linter.getDialect()) !== dialect) { + // harper.js answers a new dialect by freeing the Linter and building + // another, and the dictionary and the ignore list live inside the + // Linter. Carry them across, or the next export of this session would + // hand the vault an empty list in place of its words (#829). + const held = await exportState(linter) + await linter.setDialect(dialect) + await importState(linter, held) + } await linter.setLintConfig(options.lintConfig) }, - async importState(state) { - await linter.clearWords() - await linter.clearIgnoredLints() - if (state.words.length > 0) await linter.importWords(state.words) - const json = harperIgnoredLintsJson(state.ignoredLints) - if (json) await linter.importIgnoredLints(json) - }, + importState: (state) => importState(linter, state), dispose() { void linter.dispose?.() } } } +async function exportState(linter: Linter): Promise { + return { + words: await linter.exportWords(), + ignoredLints: harperIgnoredLintHashes(await linter.exportIgnoredLints()) + } +} + +async function importState(linter: Linter, state: HarperVaultState): Promise { + await linter.clearWords() + await linter.clearIgnoredLints() + if (state.words.length > 0) await linter.importWords(state.words) + const json = harperIgnoredLintsJson(state.ignoredLints) + if (json) await linter.importIgnoredLints(json) +} + async function createSession( options: HarperSessionOptions ): Promise { diff --git a/packages/app-core/src/lib/harper-runtime.test.ts b/packages/app-core/src/lib/harper-runtime.test.ts new file mode 100644 index 00000000..aad41f3f --- /dev/null +++ b/packages/app-core/src/lib/harper-runtime.test.ts @@ -0,0 +1,205 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { HarperVaultState } from '@shared/harper-settings' + +/** + * The runtime's glue is exercised against an in-memory stand-in for the + * Harper session, so these tests run in milliseconds and can hold the + * "compile" open for as long as the scenario needs. The real session is + * covered by harper-lint.test.ts. + */ +const harper = vi.hoisted(() => { + const words = new Set() + const ignored = new Set() + let release: (() => void) | null = null + let loading: Promise | null = null + const fakeSession = { + lint: vi.fn(async () => []), + addWord: vi.fn(async (word: string) => { + words.add(word) + }), + ignore: vi.fn(async () => undefined), + exportState: vi.fn( + async (): Promise => ({ words: [...words], ignoredLints: [...ignored] }) + ), + configure: vi.fn(async () => undefined), + importState: vi.fn(async (state: HarperVaultState) => { + words.clear() + ignored.clear() + for (const word of state.words) words.add(word) + for (const hash of state.ignoredLints) ignored.add(hash) + }) + } + return { + session: fakeSession, + words: () => [...words], + ignored: () => [...ignored], + /** The linter underneath dropped everything, as a rebuilt one does. */ + forget: () => { + words.clear() + ignored.clear() + }, + /** Let the pending `loadHarper` resolve. */ + release: () => release?.(), + reset: () => { + words.clear() + ignored.clear() + release = null + loading = null + for (const fn of Object.values(fakeSession)) fn.mockClear() + }, + loadHarper: vi.fn((options: { state: HarperVaultState }) => { + if (!loading) { + // Built from the first caller's options, like the real one, and held + // open until the test releases it, like a 15 MB compile. + for (const word of options.state.words) words.add(word) + for (const hash of options.state.ignoredLints) ignored.add(hash) + loading = new Promise((resolve) => { + release = resolve + }) + } + return loading.then(() => fakeSession) + }), + harperLoaded: () => loading !== null, + disposeHarper: vi.fn(() => { + loading = null + }) + } +}) + +const store = vi.hoisted(() => { + const state = { + harperEnabled: true, + harperDialect: 'american', + harperLintConfig: {}, + vaultSettings: { harper: undefined as HarperVaultState | undefined }, + saveHarperVaultState: vi.fn(async (next: HarperVaultState) => { + state.vaultSettings = { harper: next } + }) + } + return { state } +}) + +vi.mock('../store', () => ({ + useStore: { getState: () => store.state, subscribe: vi.fn(() => () => undefined) } +})) + +vi.mock('./harper-lint', () => ({ + loadHarper: harper.loadHarper, + harperLoaded: harper.harperLoaded, + disposeHarper: harper.disposeHarper +})) + +const VAULT: HarperVaultState = { words: ['Zennotez', 'Flurbish'], ignoredLints: ['12'] } + +async function runtime(): Promise { + // `applied` and `seenVaultState` are module state; every test starts fresh. + vi.resetModules() + return import('./harper-runtime') +} + +describe('Harper runtime (#829)', () => { + beforeEach(() => { + harper.reset() + store.state.harperEnabled = true + store.state.harperDialect = 'american' + store.state.harperLintConfig = {} + store.state.vaultSettings = { harper: undefined } + store.state.saveHarperVaultState.mockClear() + vi.stubGlobal('window', { zen: { getCapabilities: () => ({ supportsHarper: true }) } }) + }) + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('imports the words the vault loads while the session is still compiling', async () => { + const { harperEditorConfig } = await runtime() + const config = harperEditorConfig() + // The warm-up runs before `store.init()` has read vault.json, so the + // session is built from empty settings. + const pending = config.session() + expect(harper.loadHarper).toHaveBeenCalledWith( + expect.objectContaining({ state: { words: [], ignoredLints: [] } }) + ) + // The vault lands while the compile is in flight. + store.state.vaultSettings = { harper: VAULT } + harper.release() + expect(await pending).not.toBeNull() + + expect(harper.words()).toEqual(VAULT.words) + expect(harper.ignored()).toEqual(VAULT.ignoredLints) + // A `zg` on a third word writes all three, not the one the session knew. + await config.addWord('Glorpish') + expect(store.state.saveHarperVaultState).toHaveBeenLastCalledWith({ + words: ['Zennotez', 'Flurbish', 'Glorpish'], + ignoredLints: ['12'] + }) + }) + + it('never writes a shorter list than the vault holds, and teaches the session the difference', async () => { + store.state.vaultSettings = { harper: VAULT } + const { harperEditorConfig } = await runtime() + const config = harperEditorConfig() + const pending = config.session() + harper.release() + expect(await pending).not.toBeNull() + expect(harper.words()).toEqual(VAULT.words) + + harper.forget() + await config.addWord('Glorpish') + expect(store.state.saveHarperVaultState).toHaveBeenLastCalledWith({ + words: ['Zennotez', 'Flurbish', 'Glorpish'], + ignoredLints: ['12'] + }) + expect(harper.words()).toEqual(['Zennotez', 'Flurbish', 'Glorpish']) + expect(harper.ignored()).toEqual(['12']) + }) + + it('imports a change from outside once, and never the echo of its own write', async () => { + store.state.vaultSettings = { harper: VAULT } + const { harperEditorConfig, harperSeenVaultState } = await runtime() + const config = harperEditorConfig() + const pending = config.session() + harper.release() + await pending + // Built from the vault's state: nothing to import on the first pass, or + // on any later pass while the store stands still. + expect(harper.session.importState).not.toHaveBeenCalled() + await config.session() + expect(harper.session.importState).not.toHaveBeenCalled() + + await config.addWord('Glorpish') + expect(harperSeenVaultState()).toBe(JSON.stringify(store.state.vaultSettings.harper)) + await config.session() + expect(harper.session.importState).not.toHaveBeenCalled() + + // Another window (or device) added a word: the store moves, and the + // session follows. + const outside: HarperVaultState = { + words: ['Zennotez', 'Flurbish', 'Glorpish', 'Kanata'], + ignoredLints: ['12', '9722060015410969502'] + } + store.state.vaultSettings = { harper: outside } + await config.session() + expect(harper.session.importState).toHaveBeenCalledTimes(1) + expect(harper.session.importState).toHaveBeenCalledWith(outside) + expect(harper.words()).toEqual(outside.words) + await config.session() + expect(harper.session.importState).toHaveBeenCalledTimes(1) + }) + + it('starts over cleanly when the load fails', async () => { + store.state.vaultSettings = { harper: VAULT } + const { harperEditorConfig, harperSeenVaultState } = await runtime() + const config = harperEditorConfig() + harper.loadHarper.mockRejectedValueOnce(new Error('wasm refused')) + const error = vi.spyOn(console, 'error').mockImplementation(() => undefined) + expect(await config.session()).toBeNull() + error.mockRestore() + expect(harperSeenVaultState()).toBeNull() + + const pending = config.session() + harper.release() + expect(await pending).not.toBeNull() + expect(harper.words()).toEqual(VAULT.words) + }) +}) diff --git a/packages/app-core/src/lib/harper-runtime.ts b/packages/app-core/src/lib/harper-runtime.ts index cd1f7c33..3d8165f9 100644 --- a/packages/app-core/src/lib/harper-runtime.ts +++ b/packages/app-core/src/lib/harper-runtime.ts @@ -10,10 +10,20 @@ * starts from scratch, and an enabled setting warms the session at idle after * boot so the first note does not wait for a 15 MB compile. */ -import { EMPTY_HARPER_VAULT_STATE, type HarperVaultState } from '@shared/harper-settings' +import { + EMPTY_HARPER_VAULT_STATE, + mergeHarperVaultState, + type HarperVaultState +} from '@shared/harper-settings' import { useStore } from '../store' import type { HarperEditorConfig } from './cm-harper' -import { disposeHarper, loadHarper, type HarperLint, type HarperSession } from './harper-lint' +import { + disposeHarper, + harperLoaded, + loadHarper, + type HarperLint, + type HarperSession +} from './harper-lint' interface Applied { dialect: string @@ -21,10 +31,20 @@ interface Applied { } let applied: Applied | null = null -/** The store's vault state as of the last import or our own last write. A - * store value equal to this is either already in the session or an echo of - * what the session exported, so it is never imported again; anything else - * came from outside (a vault switch, Cloud sync, another device) and is. */ +/** The store's vault state as the session holds it: what it was built from, + * the last import, or our own last write. A store value equal to this is + * either already in the session or an echo of what the session exported, so + * it is never imported again; anything else came from outside (the vault + * finishing its load after boot, a vault switch, Cloud sync, another window + * or device) and is. + * + * Both are recorded the moment a session starts building, before the 15 MB + * compile is awaited. The store keeps loading the vault while that compile + * runs, and a session built from the still-empty settings must read the + * words that land meanwhile as a change to import, not as the state it was + * born with. A session that never learned the vault's words underlined them + * all over again and, on the next `zg`, wrote its own short list over the + * vault's (#829). */ let seenVaultState: string | null = null let reconciling: Promise | null = null @@ -52,14 +72,27 @@ export function harperSeenVaultState(): string | null { async function session(): Promise { const state = useStore.getState() if (!state.harperEnabled || !harperSupported()) return null + const options = { + dialect: state.harperDialect, + lintConfig: state.harperLintConfig, + state: currentVaultState() + } + // `loadHarper` memoizes on its first caller's options, so only that caller + // builds the session, and it records what the session is built from here, + // synchronously, before the compile is awaited (see `seenVaultState`). + const creating = !harperLoaded() + if (creating) { + applied = { dialect: options.dialect, lintConfig: JSON.stringify(options.lintConfig) } + seenVaultState = JSON.stringify(options.state) + } let loaded: HarperSession try { - loaded = await loadHarper({ - dialect: state.harperDialect, - lintConfig: state.harperLintConfig, - state: currentVaultState() - }) + loaded = await loadHarper(options) } catch (error) { + if (creating) { + applied = null + seenVaultState = null + } console.error('[zen:harper] failed to load Harper', error) return null } @@ -68,15 +101,17 @@ async function session(): Promise { } /** Bring the session in line with the store. Serialized so two callers never - * race their `configure` and `importState` calls against each other. */ + * race their `configure` and `importState` calls against each other; a + * waiter re-reads the store once the pass ahead of it is done, and a failure + * in that pass belongs to its own caller, not to the waiter. */ async function reconcile(loaded: HarperSession): Promise { - if (reconciling) await reconciling + while (reconciling) await reconciling.catch(() => undefined) const state = useStore.getState() const next: Applied = { dialect: state.harperDialect, lintConfig: JSON.stringify(state.harperLintConfig) } const vaultState = currentVaultState() const vaultJson = JSON.stringify(vaultState) const configChanged = !applied || applied.dialect !== next.dialect || applied.lintConfig !== next.lintConfig - const vaultChanged = seenVaultState === null || seenVaultState !== vaultJson + const vaultChanged = seenVaultState !== vaultJson if (!configChanged && !vaultChanged) return reconciling = (async () => { if (configChanged) { @@ -84,9 +119,7 @@ async function reconcile(loaded: HarperSession): Promise { applied = next } if (vaultChanged) { - // The session was created from the store's state, so the very first - // pass only records what it already holds. - if (seenVaultState !== null) await loaded.importState(vaultState) + await loaded.importState(vaultState) seenVaultState = vaultJson } })() @@ -97,10 +130,27 @@ async function reconcile(loaded: HarperSession): Promise { } } +/** + * Write the session's dictionary and ignore list back to the vault. The vault + * side is the union of what it already holds and what the session exports: + * both lists are append-only from inside the app, so a session that knows + * fewer entries than the vault has lost some (it was built before the vault + * loaded, or a dialect change rebuilt harper's linter underneath it), and the + * one place that writes vault.json must never trade the vault's list for that + * shorter one. When the vault knew more, the session learns it here too, so + * the re-lint that follows a `zg` clears every word the vault has. + */ async function persist(loaded: HarperSession): Promise { - const next = await loaded.exportState() + const exported = await loaded.exportState() + const next = mergeHarperVaultState(currentVaultState(), exported) await useStore.getState().saveHarperVaultState(next) - // Whatever the store now holds is what the session just exported. + if ( + next.words.length !== exported.words.length || + next.ignoredLints.length !== exported.ignoredLints.length + ) { + await loaded.importState(next) + } + // Whatever the store now holds is what the session holds. seenVaultState = JSON.stringify(currentVaultState()) } diff --git a/packages/app-core/src/lib/help.ts b/packages/app-core/src/lib/help.ts index c3641601..dd08d3c4 100644 --- a/packages/app-core/src/lib/help.ts +++ b/packages/app-core/src/lib/help.ts @@ -61,12 +61,12 @@ export const HELP_QUICK_START: HelpCard[] = [ { title: 'Switch between write and read modes', body: - 'Use Edit when you want raw markdown control, Split when you want source and rendered output together, and Preview when you want a clean reading surface with keyboard navigation. Your editor cursor stays where you left it when you return from Preview, and switching into Preview opens the reading view at the line you were editing instead of the top of the note. Each note remembers its own last mode; pick what notes open in before that with Settings → Editor → Default view mode (Edit, Split, or Preview), which travels with your portable config.' + 'Use Edit when you want raw markdown control, Split when you want source and rendered output together, and Preview when you want a clean reading surface with keyboard navigation. Switching into Preview opens the reading view at the line you were editing instead of the top of the note. Coming back follows where you read: if you only peeked and your cursor line is still in view, the cursor stays put; if you scrolled on to another part of the note, Edit or Split opens with the cursor on the section you were reading (Split also re-aligns the reading view beside it). To edit a specific passage, double-click it in Preview and the editor opens right there; the “Edit this block” button on an image embed does the same. Each note remembers its own last mode; pick what notes open in before that with Settings → Editor → Default view mode (Edit, Split, or Preview), which travels with your portable config.' }, { title: 'Find things in the right place', body: - 'Use note search when you know the note title or path, vault text search when you know a phrase inside the note, and the command palette when you know the action you want but not where it lives.' + 'Use note search when you know the note title or path, vault text search when you know a phrase inside the note, and the command palette when you know the action you want but not where it lives. When note search turns up nothing, Shift+Enter opens a small New note form with your search as the name; pick a folder and tags if you like, press Enter, and the note opens in the editor. A search that comes up empty is two keystrokes from becoming the note.' }, { title: 'Keep supporting material nearby', @@ -484,6 +484,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ items: [ { keys: 'Mod+P', action: 'Search notes', detail: 'Open the note search palette, from the editor too: on Linux and Windows this wins over Vim\u2019s Ctrl+P (cursor up).' }, { keys: 'Ctrl+D (in Search notes)', action: 'Move the highlighted note to Trash', detail: 'Trash a note straight from the search results, with the usual confirmation; the palette stays open, so a clean-up pass is search, Ctrl+D, search, Ctrl+D.' }, + { keys: 'Shift+Enter (in Search notes)', action: 'Create a note named after your search', detail: 'The last row of the results offers to create the note you typed: Shift+Enter (or Enter on that row) opens a New note form with three fields. Name starts as your search text, selected so you can retype it. Folder starts empty (your Inbox) or as the path you typed, like projects/roadmap; landing in it lists every folder, typing narrows the list, Enter picks the highlighted one and ArrowUp keeps a folder that does not exist yet. Tags starts with any #tag words from your search; type to pick an existing tag or add a new one, Space or comma commits, Backspace on an empty field removes the last one. The line under the fields tells you what will happen. A note with the same name in that folder blocks Create until you change the name, and Shift+↵ opens the existing note instead; the same name elsewhere only warns. Enter in Name, or Ctrl/Cmd+Enter anywhere, creates the note and opens it; Escape goes back to the results.' }, { keys: 'Mod+F', action: 'Search notes (non-Vim mode)', detail: 'Open the note search palette directly when Vim mode is off.' }, { keys: 'Mod+F (in the editor)', action: 'Find and replace in the note', detail: 'With Vim mode on, Linux and Windows keep Ctrl+F as Vim\u2019s page-forward (search with / instead); on macOS and with Vim off the bar opens as usual. In Edit and Split, open the editor’s find-and-replace bar: Tab moves between the Find and Replace fields, with match-case, whole-word, and regex toggles. Esc closes it.' }, { keys: 'Shift+Mod+P', action: 'Open commands', detail: 'Open the command palette.' }, @@ -504,7 +505,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ { keys: 'Alt+1 … Alt+9', action: 'Go to tab 1 through 9', detail: 'Jump straight to a tab by position, browser-style (Ctrl+1 … Ctrl+9 on macOS, where Option types characters and the ⌘ digits are taken). Tab numbers count across panes in the same order gt cycles; rebindable under Settings → Keymaps. Vim users get the same jump as {count}gt. Heads-up for macOS with multiple Spaces: Mission Control claims Ctrl+digit for Switch to Desktop, so rebind here or free the key under System Settings → Keyboard Shortcuts.' }, { keys: 'Shift+Mod+T', action: 'Reopen closed tab', detail: 'Reopen the most recently closed tab, restoring its position and pinned state. Repeat to walk back through your close history.' }, { keys: 'Mod+O', action: 'Open file', detail: 'Desktop only: pick a Markdown file with the native dialog. A file inside a known vault opens against that vault; anything else opens in a standalone external-file window. Links in that window follow from the file\'s own folder: a relative link such as `../README.md` opens the file it names, and a `[[wikilink]]` finds a page of that name in the folder or below it, each in its own window (or in its vault, when the target lives in one).' }, - { keys: 'Mod+4 / Mod+5 / Mod+6', action: 'Edit / Split / Preview mode', detail: 'Switch the active note between the raw editor, side-by-side split, and rendered preview.' }, + { keys: 'Mod+4 / Mod+5 / Mod+6', action: 'Edit / Split / Preview mode', detail: 'Switch the active note between the raw editor, side-by-side split, and rendered preview. Preview opens at the line you were editing; Edit and Split open on the section you were reading when you scrolled away from the cursor in Preview, and keep the cursor where it was when you only peeked.' }, { keys: 'Mod+L', action: 'Toggle checkbox', detail: 'Turn the current line into a checkbox and toggle it on repeat. See the “Any line becomes a checkbox” card in Core concepts for the full state rules.' }, { keys: 'Alt+Q (macOS: Ctrl+Q)', action: 'Reflow paragraph', detail: 'Join the hard-wrapped lines of the paragraph under the cursor (or every paragraph in the selection) into one line, so the editor wraps it to the pane. Headings, lists, tables, code, and explicit line breaks are untouched. See the “Reflow a hard-wrapped paragraph” card. Remappable as editor.reflowParagraph.' }, { keys: 'Shift+Mod+E', action: 'Export note as PDF', detail: 'Export the active note as a PDF file.' }, @@ -537,7 +538,7 @@ export const HELP_SHORTCUT_SECTIONS: HelpShortcutSection[] = [ { keys: 'Ctrl-w s', action: 'Split down', detail: 'Clone the current tab into a pane below.' }, { keys: '[b / ]b', action: 'Previous / next buffer', detail: 'Move across open buffers, falling back to recent notes when only one buffer is open. Both take a count: `3]b` jumps three buffers forward, wrapping around the ring.' }, { keys: 'Space o', action: 'Open buffers', detail: 'Show a searchable list of every open buffer across every pane. Press Ctrl+D to close the highlighted buffer without leaving the list.' }, - { keys: 'Space f', action: 'Search notes', detail: 'Open the vault-wide note search palette.' }, + { keys: 'Space f', action: 'Search notes', detail: 'Open the vault-wide note search palette. Inside it, Shift+Enter opens a New note form named after your search, with a folder picker and tags; Enter creates the note and opens it.' }, { keys: 'Space s t', action: 'Search vault text', detail: 'Fuzzy-search matching text lines across notes in Inbox, Quick Notes, and Archive.' }, { keys: 'Space e', action: 'Toggle left sidebar', detail: 'Show or hide the folder/tag sidebar without touching the mouse.' }, { keys: ']] / [[', action: 'Next / previous heading', detail: 'Jump the cursor to the next or previous markdown heading in the note, the way Vim’s section motions move between sections. It is a motion, so it composes: `d]]` deletes to the next heading, `v]]` selects to it, `3]]` skips three, and `Ctrl+O` jumps back. Headings inside code fences and frontmatter are skipped, matching the outline. With no heading left that way, the cursor goes to the end or start of the note.' }, @@ -1253,6 +1254,6 @@ export const HELP_CLI: HelpCard[] = [ { title: 'MCP for AI agents', body: - '`zn mcp` starts the ZenNotes MCP server in stdio mode, the same one Claude Code, Claude Desktop, and Codex use under the hood. Once `zn` is installed, Settings → MCP installs configure the clients to launch `zn mcp` directly, so the install path is one stable absolute path that survives app moves. The server works on the vault the app has open: a folder on this machine, or a self-hosted ZenNotes server you connected from Settings → Vault. `vault_info` says which. A server that requires a token needs it in the MCP client\'s environment as `ZENNOTES_REMOTE_TOKEN` (the app keeps its own copy in the OS secret store, which `zn` cannot read); `ZENNOTES_SERVER` or `ZENNOTES_VAULT` in that environment point the MCP at another vault instead. Beyond reading and writing notes, the server can hold a review with you through comments: ask the assistant to read a note\'s comments and answer them, and its replies land in the Comments panel under yours, signed with its name (`list_comments`, `add_comment`, `reply_to_comment`, `resolve_comment`).' + '`zn mcp` starts the ZenNotes MCP server in stdio mode, the same one Claude Code, Claude Desktop, and Codex use under the hood. Once `zn` is installed, Settings → MCP installs configure the clients to launch `zn mcp` directly, so the install path is one stable absolute path that survives app moves. The server works on the vault the app has open: a folder on this machine, or a self-hosted ZenNotes server you connected from Settings → Vault. `vault_info` says which. A server that requires a token needs it in the MCP client\'s environment as `ZENNOTES_REMOTE_TOKEN` (the app keeps its own copy in the OS secret store, which `zn` cannot read). To point an agent somewhere else, give the command the same flags every other `zn` command takes: `zn mcp --vault work` or `zn mcp --server home --token ` in the client\'s config, or `ZENNOTES_SERVER` / `ZENNOTES_VAULT` in its environment. Beyond reading and writing notes, the server can hold a review with you through comments: ask the assistant to read a note\'s comments and answer them, and its replies land in the Comments panel under yours, signed with its name (`list_comments`, `add_comment`, `reply_to_comment`, `resolve_comment`).' } ] diff --git a/packages/app-core/src/lib/local-assets-image-edit-block.test.ts b/packages/app-core/src/lib/local-assets-image-edit-block.test.ts new file mode 100644 index 00000000..af9f30c7 --- /dev/null +++ b/packages/app-core/src/lib/local-assets-image-edit-block.test.ts @@ -0,0 +1,66 @@ +// @vitest-environment jsdom + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// The reading view stamps every top-level block with its source line so the +// split scroll sync and the reading position carried into Edit (#822) can +// find the source. A standalone image paragraph is replaced by a figure, which +// used to drop that stamp: the image was the one block Edit could not find, +// and its "Edit this block" button opened the editor wherever the caret was. + +function installZen(): void { + Object.defineProperty(window, 'zen', { + configurable: true, + value: { + resolveLocalAssetUrl: vi.fn((_r: string, _n: string, href: string) => `zen-asset://v/${href}`), + resolveVaultAssetUrl: vi.fn((_r: string, rel: string) => `zen-asset://v/${rel}`) + } + }) +} + +async function load() { + vi.resetModules() + localStorage.clear() + installZen() + const { useStore } = await import('../store') + const { enhanceLocalAssetNodes } = await import('./local-assets') + return { useStore, enhanceLocalAssetNodes } +} + +beforeEach(() => { + vi.restoreAllMocks() +}) + +describe('image embed keeps its block source line', () => { + it('moves the paragraph stamp onto the figure and hands it to "Edit this block"', async () => { + const { useStore, enhanceLocalAssetNodes } = await load() + useStore.setState({ assetFiles: [{ path: 'assets/shot.png' }] } as never) + const root = document.createElement('div') + root.innerHTML = '

shot

' + const onRequestEdit = vi.fn() + enhanceLocalAssetNodes(root, { vaultRoot: '/v', notePath: 'inbox/Gallery.md', onRequestEdit }) + + const figure = root.querySelector('figure.local-image-embed')! + expect(figure.dataset.sourceLine).toBe('17') + figure.getBoundingClientRect = () => ({ top: 240 } as DOMRect) + + figure.querySelector('button[aria-label="Edit this block"]')!.click() + expect(onRequestEdit).toHaveBeenCalledWith({ sourceLine: 17, blockClientTop: 240 }) + }) + + it('reports no line for an image paragraph that was never stamped', async () => { + const { useStore, enhanceLocalAssetNodes } = await load() + useStore.setState({ assetFiles: [{ path: 'assets/shot.png' }] } as never) + const root = document.createElement('div') + root.innerHTML = '

shot

' + const onRequestEdit = vi.fn() + enhanceLocalAssetNodes(root, { vaultRoot: '/v', notePath: 'inbox/Gallery.md', onRequestEdit }) + + const figure = root.querySelector('figure.local-image-embed')! + expect(figure.dataset.sourceLine).toBeUndefined() + figure.querySelector('button[aria-label="Edit this block"]')!.click() + expect(onRequestEdit).toHaveBeenCalledWith( + expect.objectContaining({ sourceLine: null }) + ) + }) +}) diff --git a/packages/app-core/src/lib/local-assets.ts b/packages/app-core/src/lib/local-assets.ts index 2b50f781..25ef8631 100644 --- a/packages/app-core/src/lib/local-assets.ts +++ b/packages/app-core/src/lib/local-assets.ts @@ -3,6 +3,9 @@ import { externalLinkUrl } from './internal-links' import { openVaultAssetExternally } from './external-file-link' import { isExcalidrawPath, isObsidianExcalidrawPath } from '@shared/excalidraw' import { resolveAssetPathAmong, stripQueryAndHash } from './asset-path-resolution' +import type { PreviewEditRequest } from './preview-outline-jump' + +type RequestEdit = (request?: PreviewEditRequest | null) => void const IMAGE_EXTENSIONS = new Set([ '.apng', @@ -131,7 +134,8 @@ function buildImageEmbed( img: HTMLImageElement, rawHref: string, resolvedUrl: string, - onRequestEdit?: (() => void) | null, + sourceLine: number | null, + onRequestEdit?: RequestEdit | null, onOpenAsset?: (() => void) | null ): HTMLElement { const figure = document.createElement('figure') @@ -144,6 +148,10 @@ function buildImageEmbed( figure.dataset.localAssetUrl = resolvedUrl figure.dataset.localAssetKind = 'image' figure.dataset.localAssetHref = rawHref + // The figure stands in for the stamped paragraph it replaces, so the + // split scroll sync and the reading position carried into Edit still see + // this block's source line. (#822) + if (sourceLine != null) figure.dataset.sourceLine = String(sourceLine) const frame = document.createElement('div') frame.className = 'local-image-embed-frame' @@ -156,7 +164,7 @@ function buildImageEmbed( editButton.addEventListener('click', (e) => { e.preventDefault() e.stopPropagation() - onRequestEdit() + onRequestEdit({ sourceLine, blockClientTop: figure.getBoundingClientRect().top }) }) controlsTop.append(editButton) } @@ -374,7 +382,7 @@ export function enhanceLocalAssetNodes( options: { vaultRoot: string | null | undefined notePath: string | null | undefined - onRequestEdit?: (() => void) | null + onRequestEdit?: RequestEdit | null /** When set, PDF embeds matching this vault-relative path are * collapsed to a compact placeholder instead of a full iframe. */ pinnedAssetPath?: string | null @@ -448,11 +456,13 @@ export function enhanceLocalAssetNodes( const paragraph = isStandaloneImageParagraph(img) if (!paragraph || paragraph.dataset.assetEmbed === 'true') return paragraph.dataset.assetEmbed = 'true' + const sourceLine = Number(paragraph.dataset.sourceLine) paragraph.replaceWith( buildImageEmbed( img, raw, resolved, + Number.isFinite(sourceLine) && sourceLine >= 1 ? sourceLine : null, onRequestEdit, assetVaultRel && onOpenAsset ? () => onOpenAsset(assetVaultRel) : null ) diff --git a/packages/app-core/src/lib/preview-outline-jump.test.ts b/packages/app-core/src/lib/preview-outline-jump.test.ts index bc85ac18..6d3a5724 100644 --- a/packages/app-core/src/lib/preview-outline-jump.test.ts +++ b/packages/app-core/src/lib/preview-outline-jump.test.ts @@ -2,13 +2,17 @@ import { describe, expect, it } from 'vitest' import { + editorLandingTopMargin, findOutlineHeadingIndex, findRenderedHeadingForOutlineLine, nextOutlinePreviewSyncLockUntil, outlineHeadingTextOffset, planPreviewJump, + previewEditRequestForTarget, previewScrollTopForHeading, previewShowsNote, + previewShowsSourceLine, + previewVisibleSourceLines, scrollTopForElementRelativeTop, scrollTopForScrollRatio, shouldSyncPreviewAfterMarkdownSettles, @@ -168,6 +172,141 @@ describe('planPreviewJump (a jump landing in a pane that is reading)', () => { }) }) +// A reading view whose scroller spans `viewportTop..viewportBottom` on screen, +// with stamped blocks laid out at the given client rects. +function renderedPreview( + blocks: Array<{ line: number | string | null; top: number; bottom: number }>, + viewportTop: number, + viewportBottom: number +): HTMLDivElement { + const scroller = document.createElement('div') + scroller.getBoundingClientRect = () => + ({ top: viewportTop, bottom: viewportBottom, height: viewportBottom - viewportTop } as DOMRect) + for (const block of blocks) { + const el = document.createElement('p') + if (block.line != null) el.setAttribute('data-source-line', String(block.line)) + el.getBoundingClientRect = () => + ({ top: block.top, bottom: block.bottom, height: block.bottom - block.top } as DOMRect) + scroller.appendChild(el) + } + return scroller +} + +describe('previewVisibleSourceLines (what the reader has on screen, #822)', () => { + const layout = [ + { line: 1, top: 0, bottom: 100 }, + { line: 5, top: 100, bottom: 300 }, + { line: 12, top: 300, bottom: 500 }, + { line: 20, top: 500, bottom: 700 }, + { line: 30, top: 700, bottom: 900 } + ] + + it('starts at the block still partly in view and ends at the first block below the fold', () => { + // Line 5 is only visible in its lower half; line 20 pokes in from below. + // Both count as on screen; line 30 starts past the bottom edge. + expect(previewVisibleSourceLines(renderedPreview(layout, 250, 650))).toEqual({ top: 5, end: 30 }) + }) + + it('reports an open end when the view reaches the end of the note', () => { + expect(previewVisibleSourceLines(renderedPreview(layout, 250, 1000))).toEqual({ top: 5, end: null }) + }) + + it('does not count a block whose bottom edge merely touches the top of the view', () => { + const touching = [ + { line: 1, top: 0, bottom: 251 }, + { line: 5, top: 251, bottom: 600 } + ] + expect(previewVisibleSourceLines(renderedPreview(touching, 250, 650))).toEqual({ top: 5, end: null }) + }) + + it('skips unstamped and malformed blocks and reports nothing for an empty render', () => { + const mixed = [ + { line: null, top: 0, bottom: 400 }, + { line: 'nope', top: 0, bottom: 400 }, + { line: 8, top: 100, bottom: 400 } + ] + expect(previewVisibleSourceLines(renderedPreview(mixed, 0, 500))).toEqual({ top: 8, end: null }) + expect(previewVisibleSourceLines(renderedPreview([], 0, 500))).toBeNull() + expect(previewVisibleSourceLines(null)).toBeNull() + }) + + it('treats the line range as half open', () => { + const visible = { top: 5, end: 30 } + expect(previewShowsSourceLine(visible, 5)).toBe(true) + expect(previewShowsSourceLine(visible, 4)).toBe(false) + expect(previewShowsSourceLine(visible, 29)).toBe(true) + expect(previewShowsSourceLine(visible, 30)).toBe(false) + expect(previewShowsSourceLine({ top: 5, end: null }, 999)).toBe(true) + expect(previewShowsSourceLine(null, 5)).toBe(false) + }) +}) + +describe('previewEditRequestForTarget (double-click in the reading view, #822)', () => { + function article(): HTMLElement { + const root = document.createElement('article') + root.innerHTML = [ + '

bold text

', + '

link

', + '
', + '

embedded

', + '
code
', + '
', + '

Heading

', + '

bad stamp

', + '

zero

' + ].join('') + return root + } + + it('resolves inline content to its top-level block and reports where the block is', () => { + const root = article() + const paragraph = root.querySelector('[data-source-line="7"]')! + paragraph.getBoundingClientRect = () => ({ top: 321 } as DOMRect) + + expect(previewEditRequestForTarget(root.querySelector('strong'))).toEqual({ + sourceLine: 7, + blockClientTop: 321 + }) + expect(previewEditRequestForTarget(root.querySelector('code'))?.sourceLine).toBe(15) + expect(previewEditRequestForTarget(root.querySelector('h2'))?.sourceLine).toBe(30) + }) + + it('leaves links, controls, embeds, diagrams and transcluded notes alone', () => { + const root = article() + expect(previewEditRequestForTarget(root.querySelector('a'))).toBeNull() + expect(previewEditRequestForTarget(root.querySelector('img'))).toBeNull() + expect(previewEditRequestForTarget(root.querySelector('.note-embed p'))).toBeNull() + expect(previewEditRequestForTarget(root.querySelector('svg'))).toBeNull() + expect(previewEditRequestForTarget(root.querySelector('h2 button'))).toBeNull() + }) + + it('ignores clicks off any stamped block and stamps it cannot read', () => { + const root = article() + expect(previewEditRequestForTarget(root)).toBeNull() + expect(previewEditRequestForTarget(null)).toBeNull() + expect(previewEditRequestForTarget(root.querySelector('[data-source-line="nope"]'))).toBeNull() + expect(previewEditRequestForTarget(root.querySelector('[data-source-line="0"]'))).toBeNull() + }) +}) + +describe('editorLandingTopMargin', () => { + it('keeps the line at the height its block had on screen', () => { + expect(editorLandingTopMargin(420, 100, 800, 24)).toBe(320) + expect(editorLandingTopMargin(350.4, 100, 800, 24)).toBe(250) + }) + + it('clamps so the line neither hugs the top nor drops off the bottom', () => { + expect(editorLandingTopMargin(90, 100, 800, 24)).toBe(24) + expect(editorLandingTopMargin(880, 100, 800, 24)).toBe(752) + // A viewport too short for the clamp still gets the minimum margin. + expect(editorLandingTopMargin(30, 0, 40, 24)).toBe(24) + }) + + it('falls back to the outline-jump margin when the block position is unknown', () => { + expect(editorLandingTopMargin(null, 100, 800, 24)).toBe(24) + }) +}) + describe('previewShowsNote', () => { it('is true only when the rendered article carries the note path', () => { const scroller = document.createElement('div') diff --git a/packages/app-core/src/lib/preview-outline-jump.ts b/packages/app-core/src/lib/preview-outline-jump.ts index d2bc5b12..e0077637 100644 --- a/packages/app-core/src/lib/preview-outline-jump.ts +++ b/packages/app-core/src/lib/preview-outline-jump.ts @@ -46,6 +46,116 @@ export function previewShowsNote(previewScrollEl: ParentNode | null, notePath: s return article?.dataset.notePath === notePath } +/** + * The stretch of source the reading view has on screen. `top` is the stamped + * line of the first block still (partly) in view, `end` the line of the first + * block below the fold, or null when the view reaches the end of the note. + */ +export interface PreviewVisibleSourceLines { + top: number + end: number | null +} + +/** + * Read the visible source range off the rendered blocks. Null when nothing + * stamped is on screen: an empty note, or a render of the previous note + * (check `previewShowsNote` first). + */ +export function previewVisibleSourceLines( + previewScrollEl: HTMLElement | null +): PreviewVisibleSourceLines | null { + if (!previewScrollEl) return null + const viewport = previewScrollEl.getBoundingClientRect() + let top: number | null = null + for (const block of previewScrollEl.querySelectorAll('[data-source-line]')) { + const line = Number(block.dataset.sourceLine) + if (!Number.isFinite(line)) continue + const rect = block.getBoundingClientRect() + if (top == null) { + if (rect.bottom > viewport.top + 1) top = line + } else if (rect.top >= viewport.bottom) { + return { top, end: line } + } + } + return top == null ? null : { top, end: null } +} + +/** + * Whether `line` falls inside what the reader can see. The block that starts + * at `top` counts even when its first pixels are scrolled off, so a caret + * left on a heading whose section is on screen is still "in view". + */ +export function previewShowsSourceLine( + visible: PreviewVisibleSourceLines | null, + line: number +): boolean { + if (!visible) return false + return line >= visible.top && (visible.end == null || line < visible.end) +} + +/** A rendered block the reader pointed at, resolved to where it came from. */ +export interface PreviewEditRequest { + /** 1-based source line of the block, null when the pointer sat off any stamped block. */ + sourceLine: number | null + /** Where the block's top edge is on screen (client coordinates), when known. */ + blockClientTop: number | null +} + +// Things in the reading view that own their double-click, or whose lines are +// not this note's: links (the first click already navigated), controls, asset +// embeds (the image embed offers its own "Edit this block" button), diagrams +// (double-click resets their pan and zoom), Excalidraw frames, and transcluded +// notes (their stamps count lines of the expanded markdown, not of this file). +const PREVIEW_EDIT_INERT_SELECTOR = [ + 'a', + 'button', + 'input', + 'textarea', + 'select', + 'summary', + 'label', + 'iframe', + 'video', + 'audio', + 'canvas', + '[data-local-asset-kind]', + '[data-zen-diagram-kind]', + '[data-excalidraw-embed]', + '.note-embed' +].join(', ') + +/** + * The block a double-click in the reading view opens for editing, or null when + * the click landed on something that should keep its own behaviour. + */ +export function previewEditRequestForTarget(target: EventTarget | null): PreviewEditRequest | null { + if (!(target instanceof Element)) return null + if (target.closest(PREVIEW_EDIT_INERT_SELECTOR)) return null + const block = target.closest('[data-source-line]') + if (!block) return null + const line = Number(block.dataset.sourceLine) + if (!Number.isFinite(line) || line < 1) return null + return { sourceLine: line, blockClientTop: block.getBoundingClientRect().top } +} + +/** + * Where the landed line should sit in the editor so it stays at the height its + * rendered block had on screen and the eye does not have to travel. Clamped so + * the line neither hugs the top edge nor falls off the bottom; a block whose + * position is unknown lands at the minimum margin, like an outline jump. + */ +export function editorLandingTopMargin( + blockClientTop: number | null, + viewportClientTop: number, + viewportHeight: number, + minMargin: number +): number { + if (blockClientTop == null) return minMargin + const maxMargin = Math.max(minMargin, viewportHeight - 2 * minMargin) + const offset = Math.round(blockClientTop - viewportClientTop) + return Math.max(minMargin, Math.min(maxMargin, offset)) +} + const ATX_HEADING_TEXT_OFFSET_RE = /^(#{1,6})[ \t]+/ export function outlineHeadingTextOffset(lineText: string): number { diff --git a/packages/app-core/src/lib/search-create.test.ts b/packages/app-core/src/lib/search-create.test.ts new file mode 100644 index 00000000..b3407225 --- /dev/null +++ b/packages/app-core/src/lib/search-create.test.ts @@ -0,0 +1,316 @@ +import { describe, expect, it } from 'vitest' +import type { FolderEntry, NoteFolder, NoteMeta, VaultSettings } from '@shared/ipc' +import { + addTag, + buildDestinationChoices, + checkNoteName, + composeNewNoteBody, + destinationLabel, + destinationText, + filterDestinationChoices, + findNameCollision, + normalizeTag, + parseDestinationText, + rankTagChoices, + searchCreateDraft, + type AreaLabels +} from './search-create' + +// #826: the note search palette offers to create the note the query names. +// This is the pure half of the New note form: the draft read off the query, +// the Name and Folder fields, the "already exists" check, the folder picker +// and the tags. + +function note(path: string, folder: NoteFolder = 'inbox', tags: string[] = []): NoteMeta { + const file = path.split('/').pop() ?? path + return { + path, + title: file.replace(/\.md$/, ''), + folder, + siblingOrder: 0, + createdAt: 0, + updatedAt: 0, + size: 0, + tags, + wikilinks: [], + hasAttachments: false, + assetEmbeds: [], + excerpt: '' + } +} + +const notes = [ + note('inbox/Alpha.md'), + note('inbox/projects/Roadmap.md'), + note('quick/Scratch.md', 'quick'), + note('archive/Alpha.md', 'archive'), + note('trash/Gone.md', 'trash') +] + +const LABELS: AreaLabels = { inbox: 'Inbox', quick: 'Quick Notes', archive: 'Archive', trash: 'Trash' } + +describe('searchCreateDraft', () => { + it('drafts nothing for an empty or tag-only query', () => { + expect(searchCreateDraft('', [])).toBeNull() + expect(searchCreateDraft(' ', ['ops'])).toBeNull() + }) + + it('splits a path into the Folder and Name fields, in the `:e` dialect', () => { + expect(searchCreateDraft('Meeting notes', [])).toEqual({ + name: 'Meeting notes', + folderText: '', + tags: [] + }) + expect(searchCreateDraft('projects/ideas/Q4', [])).toMatchObject({ + name: 'Q4', + folderText: 'projects/ideas' + }) + expect(searchCreateDraft('archive/Old plan', [])).toMatchObject({ + name: 'Old plan', + folderText: 'archive' + }) + expect(searchCreateDraft('quick/Scratch 2', [])).toMatchObject({ folderText: 'quick' }) + expect(searchCreateDraft('/Meeting notes.md', [])).toMatchObject({ + name: 'Meeting notes', + folderText: '' + }) + }) + + it('keeps text that cannot be a path, so the form can say why', () => { + // No slash: the whole thing is the name, and the Name field complains. + expect(searchCreateDraft('what?', [])).toMatchObject({ name: 'what?', folderText: '' }) + // With a slash, each field gets its own part to complain about. + expect(searchCreateDraft('projects/what?', [])).toMatchObject({ + name: 'what?', + folderText: 'projects' + }) + expect(searchCreateDraft('../escape', [])).toMatchObject({ name: 'escape', folderText: '..' }) + // The Trash parses fine as a path; the Folder field is what rejects it. + expect(searchCreateDraft('trash/Gone', [])).toMatchObject({ name: 'Gone', folderText: 'trash' }) + }) + + it('carries the query tags over, deduplicated and without the ones that are not tags', () => { + expect(searchCreateDraft('Runbook', ['ops', 'Ops', 'prod', '9lives'])?.tags).toEqual([ + 'ops', + 'prod' + ]) + }) +}) + +describe('parseDestinationText', () => { + it('reads empty text and the Inbox name as the Inbox root', () => { + const root = { destination: { folder: 'inbox', subpath: '' }, error: null } + expect(parseDestinationText('')).toEqual(root) + expect(parseDestinationText(' / ')).toEqual(root) + expect(parseDestinationText('inbox')).toEqual(root) + expect(parseDestinationText('Inbox/')).toEqual(root) + }) + + it('nests under Inbox unless a top folder leads, and tolerates slashes either way', () => { + expect(parseDestinationText('projects/ideas/').destination).toEqual({ + folder: 'inbox', + subpath: 'projects/ideas' + }) + expect(parseDestinationText('projects\\ideas').destination).toEqual({ + folder: 'inbox', + subpath: 'projects/ideas' + }) + expect(parseDestinationText('Inbox/projects').destination).toEqual({ + folder: 'inbox', + subpath: 'projects' + }) + expect(parseDestinationText('archive').destination).toEqual({ folder: 'archive', subpath: '' }) + expect(parseDestinationText('archive/old').destination).toEqual({ + folder: 'archive', + subpath: 'old' + }) + expect(parseDestinationText('quick').destination).toEqual({ folder: 'quick', subpath: '' }) + }) + + it('refuses the Trash, databases, dot segments and characters a folder cannot carry', () => { + expect(parseDestinationText('trash').error).toBe('Notes cannot be created in the Trash.') + expect(parseDestinationText('trash/old').error).toBe('Notes cannot be created in the Trash.') + expect(parseDestinationText('Tasks.base').error).toMatch(/Databases/) + expect(parseDestinationText('team/Tasks.base/pages').error).toMatch(/Databases/) + expect(parseDestinationText('a/../b').error).toMatch(/"\." or "\.\."/) + expect(parseDestinationText('bad:name').error).toMatch(/^Folder names cannot contain/) + }) +}) + +describe('checkNoteName', () => { + it('trims, drops a .md suffix, and lets a name spell a folder without becoming one', () => { + expect(checkNoteName(' Plan ')).toEqual({ title: 'Plan', error: null }) + expect(checkNoteName('Plan.md')).toEqual({ title: 'Plan', error: null }) + expect(checkNoteName('Inbox')).toEqual({ title: 'Inbox', error: null }) + expect(checkNoteName('archive')).toEqual({ title: 'archive', error: null }) + }) + + it('names what is wrong: nothing, a slash, a forbidden character, a dot segment', () => { + expect(checkNoteName(' ').error).toBe('Enter a name.') + expect(checkNoteName('a/b').error).toMatch(/slash/) + expect(checkNoteName('a\\b').error).toMatch(/slash/) + expect(checkNoteName('what?').error).toMatch(/cannot contain/) + expect(checkNoteName('..').error).toMatch(/"\." or "\.\."/) + }) +}) + +describe('findNameCollision', () => { + const inboxRoot = { folder: 'inbox' as const, subpath: '' } + + it('prefers the note in the chosen folder, ignoring case', () => { + expect(findNameCollision('alpha', inboxRoot, notes, null)).toEqual({ + note: notes[0], + sameFolder: true + }) + expect(findNameCollision('Alpha', { folder: 'archive', subpath: '' }, notes, null)).toEqual({ + note: notes[3], + sameFolder: true + }) + // Folder names compare case-insensitively too, as the default macOS file + // system does: creating in `Projects` would land on `projects/Roadmap.md`. + expect(findNameCollision('roadmap', { folder: 'inbox', subpath: 'Projects' }, notes, null)).toEqual({ + note: notes[1], + sameFolder: true + }) + }) + + it('reports a same-named note elsewhere without calling it a twin', () => { + expect(findNameCollision('Roadmap', inboxRoot, notes, null)).toEqual({ + note: notes[1], + sameFolder: false + }) + expect(findNameCollision('scratch', inboxRoot, notes, null)).toEqual({ + note: notes[2], + sameFolder: false + }) + }) + + it('never counts a partial match, an empty name, or a trashed note', () => { + expect(findNameCollision('Alph', inboxRoot, notes, null)).toBeNull() + expect(findNameCollision(' ', inboxRoot, notes, null)).toBeNull() + expect(findNameCollision('Gone', inboxRoot, notes, null)).toBeNull() + }) + + it('reads folders the way the vault lays them out, not from the literal path', () => { + // Notes kept at the vault root: `projects/Roadmap.md` is the `projects` + // subfolder of the notes area, and `Alpha.md` sits in its root. + const rootMode = { primaryNotesLocation: 'root' } as unknown as VaultSettings + const rooted = [note('Alpha.md'), note('projects/Roadmap.md')] + expect(findNameCollision('alpha', inboxRoot, rooted, rootMode)?.sameFolder).toBe(true) + expect( + findNameCollision('roadmap', { folder: 'inbox', subpath: 'projects' }, rooted, rootMode) + ?.sameFolder + ).toBe(true) + expect(findNameCollision('roadmap', inboxRoot, rooted, rootMode)?.sameFolder).toBe(false) + }) +}) + +describe('the folder picker', () => { + const entry = (folder: NoteFolder, subpath: string): FolderEntry => ({ + folder, + subpath, + siblingOrder: 0 + }) + const folders = [ + entry('inbox', 'projects/ideas'), + entry('inbox', 'projects'), + entry('inbox', 'Tasks.base'), + entry('inbox', 'a'), + entry('archive', 'old'), + entry('quick', 'scratch'), + entry('trash', 'x') + ] + + it('lists each area root then its subfolders by depth and name, skipping databases and the Trash', () => { + const choices = buildDestinationChoices(folders, LABELS) + expect(choices.map((c) => c.value)).toEqual([ + '', + 'a', + 'projects', + 'projects/ideas', + 'quick', + 'quick/scratch', + 'archive', + 'archive/old' + ]) + expect(choices.map((c) => c.label)).toEqual([ + 'Inbox', + 'Inbox › a', + 'Inbox › projects', + 'Inbox › projects/ideas', + 'Quick Notes', + 'Quick Notes › scratch', + 'Archive', + 'Archive › old' + ]) + expect(choices[3].destination).toEqual({ folder: 'inbox', subpath: 'projects/ideas' }) + }) + + it('speaks the labels it is given, so renamed folders and root-mode vaults read right', () => { + const labels = { ...LABELS, inbox: 'My Vault' } + expect(buildDestinationChoices(folders, labels)[2].label).toBe('My Vault › projects') + expect(destinationLabel({ folder: 'inbox', subpath: '' }, labels)).toBe('My Vault') + expect(destinationLabel({ folder: 'archive', subpath: 'old' }, labels)).toBe('Archive › old') + }) + + it('narrows by value first, then label, then anything containing the text', () => { + const choices = buildDestinationChoices(folders, LABELS) + const values = (text: string): string[] => + filterDestinationChoices(choices, text).map((c) => c.value) + expect(values('proj')).toEqual(['projects', 'projects/ideas']) + expect(values('projects')).toEqual(['projects', 'projects/ideas']) + expect(values('ideas')).toEqual(['projects/ideas']) + expect(values('Inbox')).toEqual(['', 'a', 'projects', 'projects/ideas']) + expect(values('old')).toEqual(['archive/old']) + expect(values('/archive/')).toEqual(['archive', 'archive/old']) + expect(values('nothing-here')).toEqual([]) + expect(values('')).toHaveLength(choices.length) + }) + + it('round-trips a destination through its field text', () => { + for (const destination of [ + { folder: 'inbox' as const, subpath: '' }, + { folder: 'inbox' as const, subpath: 'projects/ideas' }, + { folder: 'quick' as const, subpath: '' }, + { folder: 'archive' as const, subpath: 'old' } + ]) { + expect(parseDestinationText(destinationText(destination)).destination).toEqual(destination) + } + }) +}) + +describe('tags', () => { + const counts = new Map([ + ['Ops', 3], + ['devops', 1], + ['op', 1], + ['other', 5] + ]) + + it('normalizes a typed tag the way extractTags reads one', () => { + expect(normalizeTag('#Ops')).toBe('Ops') + expect(normalizeTag(' ops/infra ')).toBe('ops/infra') + expect(normalizeTag('9lives')).toBeNull() + expect(normalizeTag('two words')).toBeNull() + expect(normalizeTag('#')).toBeNull() + }) + + it('adds a tag once, in the spelling the vault already uses', () => { + expect(addTag([], 'ops', counts)).toEqual(['Ops']) + expect(addTag(['Ops'], '#OPS', counts)).toEqual(['Ops']) + expect(addTag(['a'], 'b')).toEqual(['a', 'b']) + expect(addTag(['a'], 'not a tag')).toEqual(['a']) + }) + + it('ranks the exact tag first, then prefixes, then substrings, minus the chosen ones', () => { + expect(rankTagChoices('op', counts, []).map((t) => t.tag)).toEqual(['op', 'Ops', 'devops']) + expect(rankTagChoices('op', counts, ['op', 'devops']).map((t) => t.tag)).toEqual(['Ops']) + // No text: the vault's tags, most used first. + expect(rankTagChoices('', counts, []).map((t) => t.tag)).toEqual(['other', 'Ops', 'devops', 'op']) + }) + + it('writes the body the CLI would', () => { + expect(composeNewNoteBody('Runbook', [])).toBe('# Runbook\n\n') + expect(composeNewNoteBody('Runbook', ['ops', 'prod'])).toBe('# Runbook\n\n#ops #prod\n\n') + }) +}) diff --git a/packages/app-core/src/lib/search-create.ts b/packages/app-core/src/lib/search-create.ts new file mode 100644 index 00000000..3ca62e32 --- /dev/null +++ b/packages/app-core/src/lib/search-create.ts @@ -0,0 +1,307 @@ +import type { FolderEntry, NoteFolder, NoteMeta, VaultSettings } from '@shared/ipc' +import { formDirContaining } from '@shared/databases' +import { parseCreateNotePath } from './wikilinks' +import { noteFolderSubpath } from './vault-layout' +import { rankTagCompletions, type RankedTag } from './tags' + +/** + * Creating a note from the search palette (#826). The pure half of the flow: + * turning the query into a draft, reading the Folder field, checking the Name + * field, spotting a note that already has the name, listing the folders the + * picker offers, and shaping the tags. The form itself lives in + * `SearchCreateForm.tsx`. + */ + +export interface NoteDestination { + folder: NoteFolder + subpath: string +} + +/** The user's answer to the Folder field, or why it is not one. */ +export type DestinationParse = + | { destination: NoteDestination; error: null } + | { destination: null; error: string } + +/** The user's answer to the Name field, or why it is not one. */ +export type NameCheck = { title: string; error: null } | { title: null; error: string } + +/** What the New note form opens with, read off the search query. */ +export interface SearchCreateDraft { + /** Prefilled Name field. Raw query text when it cannot be a name, so the + * form can show the reason instead of the palette silently doing nothing. */ + name: string + /** Prefilled Folder field, in the text form `destinationText` produces. */ + folderText: string + /** Prefilled tags: the `#tag` words of the query, so a search like + * `#ops runbook` becomes a note that already carries #ops. */ + tags: string[] +} + +/** + * The Folder field speaks the `:e` dialect the rest of the app uses for paths: + * empty is the Inbox root, `projects/ideas` nests under Inbox, and a leading + * top folder (`archive`, `quick/x`) picks that folder. Never a literal on-disk + * path: system folders can be remapped, so `inbox/x` as a string means nothing. + */ +export function destinationText(destination: NoteDestination): string { + if (destination.folder === 'inbox') return destination.subpath + return destination.subpath ? `${destination.folder}/${destination.subpath}` : destination.folder +} + +const TRASH_ERROR = 'Notes cannot be created in the Trash.' +const DATABASE_ERROR = 'Databases hold rows, not notes. Pick a folder.' + +/** Read the Folder field. Typed folders need not exist yet: creating makes them. */ +export function parseDestinationText(text: string): DestinationParse { + const trimmed = text.trim().replace(/\\/g, '/').replace(/^\/+|\/+$/g, '') + if (!trimmed) return { destination: { folder: 'inbox', subpath: '' }, error: null } + // The path parser wants a file at the end; a placeholder name turns the + // folder text into "that folder, any file" and its checks apply unchanged. + let parsed: ReturnType + try { + parsed = parseCreateNotePath(`${trimmed}/-`) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + return { destination: null, error: message.replace(/^File names/, 'Folder names') } + } + if (parsed.folder === 'trash') return { destination: null, error: TRASH_ERROR } + if (formDirContaining(parsed.subpath)) return { destination: null, error: DATABASE_ERROR } + return { destination: { folder: parsed.folder, subpath: parsed.subpath }, error: null } +} + +/** + * Read the Name field. The name is one file name, never a path: the Folder + * field owns the directory, so a slash here is a mistake worth naming. Leading + * the name with `/` when parsing keeps a note called "Inbox" or "archive" a + * plain title instead of a top-folder prefix. + */ +export function checkNoteName(name: string): NameCheck { + const trimmed = name.trim() + if (!trimmed) return { title: null, error: 'Enter a name.' } + if (/[\\/]/.test(trimmed)) { + return { title: null, error: 'Names cannot contain a slash. Pick the folder in the Folder field.' } + } + try { + return { title: parseCreateNotePath(`/${trimmed}`).title, error: null } + } catch (err) { + return { title: null, error: err instanceof Error ? err.message : String(err) } + } +} + +/** + * What the search query asks to create. Null when there is nothing to name a + * note after (an empty or tag-only query). A query that parses as a path is + * split into its folder and name; one that does not is split at its last + * slash so each field shows its own complaint. + */ +export function searchCreateDraft( + freeText: string, + tagTokens: readonly string[] +): SearchCreateDraft | null { + const text = freeText.trim() + if (!text) return null + const tags = tagTokens.reduce((acc, tag) => addTag(acc, tag), []) + try { + const parsed = parseCreateNotePath(text) + return { + name: parsed.title, + folderText: destinationText({ folder: parsed.folder, subpath: parsed.subpath }), + tags + } + } catch { + const normalized = text.replace(/\\/g, '/') + const slash = normalized.lastIndexOf('/') + if (slash < 0) return { name: normalized, folderText: '', tags } + return { name: normalized.slice(slash + 1).trim(), folderText: normalized.slice(0, slash), tags } + } +} + +export interface NameCollision { + note: NoteMeta + /** True when the note sits in the chosen folder, where creating would only + * make a "Title 2" twin; false for a same-named note somewhere else. */ + sameFolder: boolean +} + +/** + * The live note that already carries `title`, if any. Same-folder matches win + * over matches elsewhere; the Trash never counts. Titles and folders compare + * case-insensitively because that is how the default macOS file system sees + * them, and a false alarm on Linux costs less than a missed twin on a Mac. + */ +export function findNameCollision( + title: string, + destination: NoteDestination, + notes: readonly NoteMeta[], + settings: VaultSettings | null | undefined +): NameCollision | null { + const wanted = title.trim().toLowerCase() + if (!wanted) return null + const subpath = destination.subpath.toLowerCase() + let elsewhere: NoteMeta | null = null + for (const note of notes) { + if (note.folder === 'trash' || note.title.trim().toLowerCase() !== wanted) continue + if ( + note.folder === destination.folder && + noteFolderSubpath(note, settings).toLowerCase() === subpath + ) { + return { note, sameFolder: true } + } + elsewhere ??= note + } + return elsewhere ? { note: elsewhere, sameFolder: false } : null +} + +export interface DestinationChoice { + /** Folder field text this row fills in (`destinationText`). */ + value: string + /** `Inbox`, `Inbox › projects/ideas`, `Archive › old`. */ + label: string + destination: NoteDestination +} + +/** Areas the picker offers, in the order they appear. The Trash never does. */ +const PICKER_AREAS: readonly Exclude[] = ['inbox', 'quick', 'archive'] + +/** What each top folder is called on screen. The caller resolves renamed + * system folders, and a vault that keeps its notes at the root passes the + * vault's name for `inbox`, the way the note list heading does. */ +export type AreaLabels = Record + +/** `Inbox`, `Inbox › projects/ideas`, `Archive › old`: how the form names a + * destination. Never the on-disk path, which remapped folders make wrong. */ +export function destinationLabel(destination: NoteDestination, labels: AreaLabels): string { + const area = labels[destination.folder] + return destination.subpath ? `${area} › ${destination.subpath}` : area +} + +/** + * The folders the picker offers: each area's root, then its subfolders by + * depth and name. `folders` is the store's list of real subfolders; database + * folders (`.base`) are skipped because they hold rows, not notes. + */ +export function buildDestinationChoices( + folders: readonly FolderEntry[], + labels: AreaLabels +): DestinationChoice[] { + const choices: DestinationChoice[] = [] + const push = (destination: NoteDestination): void => { + choices.push({ + value: destinationText(destination), + label: destinationLabel(destination, labels), + destination + }) + } + for (const area of PICKER_AREAS) { + push({ folder: area, subpath: '' }) + const seen = new Set() + const subs = folders + .filter((entry) => entry.folder === area && entry.subpath && !formDirContaining(entry.subpath)) + .filter((entry) => !seen.has(entry.subpath) && seen.add(entry.subpath)) + .sort((a, b) => { + const depth = a.subpath.split('/').length - b.subpath.split('/').length + return depth || a.subpath.localeCompare(b.subpath) + }) + for (const entry of subs) push({ folder: area, subpath: entry.subpath }) + } + return choices +} + +const MAX_DESTINATION_ROWS = 12 + +/** + * Narrow the picker to what the Folder field says, ranked the way the other + * folder prompts rank: an exact value, then values and labels that start + * with the text, then anything containing it. Empty text lists everything. + */ +export function filterDestinationChoices( + choices: readonly DestinationChoice[], + text: string +): DestinationChoice[] { + const query = text.trim().toLowerCase().replace(/\\/g, '/').replace(/^\/+|\/+$/g, '') + if (!query) return choices.slice(0, MAX_DESTINATION_ROWS) + return choices + .map((choice, index) => { + const value = choice.value.toLowerCase() + const label = choice.label.toLowerCase() + const rank = + value === query + ? 0 + : value.startsWith(query) + ? 1 + : label.startsWith(query) + ? 2 + : value.includes(query) || label.includes(query) + ? 3 + : null + return rank === null ? null : { choice, rank, index } + }) + .filter((entry): entry is { choice: DestinationChoice; rank: number; index: number } => !!entry) + .sort((a, b) => a.rank - b.rank || a.index - b.index) + .slice(0, MAX_DESTINATION_ROWS) + .map((entry) => entry.choice) +} + +/** The same shape `extractTags` accepts: a letter, then letters, digits, `_`, `-`, `/`. */ +const TAG_RE = /^\p{L}[\p{L}\d_/-]*$/u + +/** A typed tag without its `#`, or null when it could never be one. */ +export function normalizeTag(raw: string): string | null { + const tag = raw.trim().replace(/^#+/, '') + return TAG_RE.test(tag) ? tag : null +} + +/** + * Add a typed tag to the chips. Nothing changes for text that is not a tag or + * a tag already present (case-insensitively). When the vault already spells + * the tag some way, that spelling wins over the typed one, so `ops` and `Ops` + * never become two tags. + */ +export function addTag( + tags: readonly string[], + raw: string, + known?: ReadonlyMap +): string[] { + const normalized = normalizeTag(raw) + if (!normalized) return [...tags] + const lower = normalized.toLowerCase() + if (tags.some((tag) => tag.toLowerCase() === lower)) return [...tags] + const canonical = known ? [...known.keys()].find((tag) => tag.toLowerCase() === lower) : undefined + return [...tags, canonical ?? normalized] +} + +/** + * Suggestions for the Tags field. The tag the text spells exactly leads (so + * Enter picks the vault's spelling), then the usual prefix-then-substring + * ranking, minus tags already chosen. + */ +export function rankTagChoices( + text: string, + counts: ReadonlyMap, + chosen: readonly string[] +): RankedTag[] { + const query = text.trim().replace(/^#+/, '') + const lower = query.toLowerCase() + const taken = new Set(chosen.map((tag) => tag.toLowerCase())) + const ranked: RankedTag[] = [] + if (lower) { + for (const [tag, count] of counts) { + if (tag.toLowerCase() === lower && !taken.has(lower)) ranked.push({ tag, count }) + } + } + for (const entry of rankTagCompletions(query, counts)) { + if (!taken.has(entry.tag.toLowerCase())) ranked.push(entry) + } + return ranked +} + +/** + * The body a note created with tags starts from: the heading every new note + * gets, then one line of `#tags`. Mirrors `composeBody` in the CLI's capture + * and notes commands so a note made in the app and one made from a terminal + * look the same. + */ +export function composeNewNoteBody(title: string, tags: readonly string[]): string { + const tagLine = tags.length > 0 ? tags.map((tag) => `#${tag}`).join(' ') + '\n\n' : '' + return `# ${title}\n\n${tagLine}` +} diff --git a/packages/app-core/src/lib/tags.ts b/packages/app-core/src/lib/tags.ts index ea8ae6f5..7f59135c 100644 --- a/packages/app-core/src/lib/tags.ts +++ b/packages/app-core/src/lib/tags.ts @@ -110,6 +110,53 @@ export function noteTagsForCount( const EMPTY_TAGS: readonly string[] = [] +/** + * Unique tags across the vault (trash excluded), counted by how many notes use + * them. The active note is read live from its buffer so a tag just typed in the + * same note is offered too. One aggregation for the editor's `#` completion, + * the frontmatter `tags:` completion and the New note form in search. + */ +export function countVaultTags( + notes: readonly { path: string; folder: string; tags: readonly string[] }[], + active: { path: string; body: string } | null | undefined, + preambleFolder: string +): Map { + const counter = new Map() + for (const note of notes) { + if (note.folder === 'trash') continue + for (const t of noteTagsForCount(note, active, preambleFolder)) { + counter.set(t, (counter.get(t) ?? 0) + 1) + } + } + return counter +} + +export interface RankedTag { + tag: string + count: number +} + +const MAX_TAG_COMPLETIONS = 20 + +/** Rank vault tags for `query` so prefix matches beat substring matches, and + * more-used tags beat less-used ones. Excludes the exact tag already typed. */ +export function rankTagCompletions( + query: string, + counts: ReadonlyMap +): RankedTag[] { + const q = query.toLowerCase() + return [...counts.entries()] + .map(([tag, count]) => { + const lower = tag.toLowerCase() + const rank = lower.startsWith(q) ? 0 : lower.includes(q) ? 1 : 2 + return { tag, lower, count, rank } + }) + .filter((t) => t.rank < 2 && t.lower !== q) + .sort((a, b) => a.rank - b.rank || b.count - a.count || a.tag.localeCompare(b.tag)) + .slice(0, MAX_TAG_COMPLETIONS) + .map(({ tag, count }) => ({ tag, count })) +} + /** A node in the hierarchical (`/`-separated) tag tree. (#439) */ export interface TagTreeNode { /** The last path segment shown as the row label, e.g. `compiler`. */ diff --git a/packages/app-core/src/lib/vim-half-page-keymap.test.ts b/packages/app-core/src/lib/vim-half-page-keymap.test.ts index fe59a21f..937a6916 100644 --- a/packages/app-core/src/lib/vim-half-page-keymap.test.ts +++ b/packages/app-core/src/lib/vim-half-page-keymap.test.ts @@ -4,30 +4,37 @@ import { historyKeymap } from '@codemirror/commands' import { searchKeymap } from '@codemirror/search' import { EditorState } from '@codemirror/state' import { EditorView, keymap } from '@codemirror/view' -import { Vim, vim } from '@replit/codemirror-vim' +import { CodeMirror, Vim, vim } from '@replit/codemirror-vim' import type { KeymapOverrides } from './keymaps' import { keyBindingsFor, vimHalfPageKeymap } from './vim-half-page-keymap' describe('vimHalfPageKeymap', () => { const views: EditorView[] = [] - const mapped: string[] = [] + const mapped: Array<{ binding: string; context: 'normal' | 'visual' }> = [] afterEach(() => { views.splice(0).forEach((view) => view.destroy()) - mapped.splice(0).forEach((binding) => Vim.unmap(binding, 'normal')) + mapped.splice(0).forEach(({ binding, context }) => Vim.unmap(binding, context)) }) function mount(overrides: KeymapOverrides = {}): EditorView { + // Extension order mirrors EditorPane: a keymap precedes the Vim plugin + // there (the snippet keymap comes first), so CodeMirror's keymap handler + // runs before Vim sees the key. Listing vim() first would hand every + // key to Vim and leave the keymap under test unexercised. Multiple + // selections are allowed as in the app (cm-vim-visual-highlight), which + // is what lets the search keymap's Ctrl+D add a range. const view = new EditorView({ state: EditorState.create({ doc: 'one\ntwo\nthree', extensions: [ - vim(), + EditorState.allowMultipleSelections.of(true), keymap.of([ ...vimHalfPageKeymap(true, overrides), ...historyKeymap, ...searchKeymap - ]) + ]), + vim() ] }), parent: document.body @@ -40,7 +47,18 @@ describe('vimHalfPageKeymap', () => { function mapAction(binding: string, action: string, callback: () => void): void { Vim.defineAction(action, callback) Vim.mapCommand(binding, 'action', action, {}, { context: 'normal' }) - mapped.push(binding) + mapped.push({ binding, context: 'normal' }) + } + + /** A stand-in for the half-page motion: one logical line in the given direction. */ + function mapVisualMotion(binding: string, motion: string, forward: boolean): void { + Vim.defineMotion(motion, ((_cm: unknown, head: { line: number; ch: number }) => + new CodeMirror.Pos( + forward ? head.line + 1 : Math.max(0, head.line - 1), + head.ch + )) as unknown as Parameters[1]) + Vim.mapCommand(binding, 'motion', motion, { forward }, { context: 'visual' }) + mapped.push({ binding, context: 'visual' }) } function press(view: EditorView, key: string, modifiers: KeyboardEventInit): void { @@ -71,7 +89,28 @@ describe('vimHalfPageKeymap', () => { expect(calls).toBe(1) }) - it('defers outside Vim normal mode', () => { + it('runs the visual-context mapping in visual mode instead of the search keymap (#825)', () => { + // jsdom is not a Mac, so Mod is Ctrl and searchKeymap's Mod-d + // (selectNextOccurrence) competes for the key exactly as on Linux. + mapVisualMotion('', 'testVisualHalfPageDown', true) + mapVisualMotion('', 'testVisualHalfPageUp', false) + const view = mount() + + press(view, 'v', {}) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 1 }) + + press(view, 'd', { ctrlKey: true }) + // The selection grew to the next line's first character (inclusive). + // Left to the search keymap it would instead have gained a second range. + expect(view.state.selection.ranges).toHaveLength(1) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 5 }) + + press(view, 'u', { ctrlKey: true }) + expect(view.state.selection.ranges).toHaveLength(1) + expect(view.state.selection.main.toJSON()).toEqual({ anchor: 0, head: 1 }) + }) + + it('defers in insert mode', () => { let calls = 0 mapAction('', 'testNormalHalfPageDown', () => calls++) const view = mount() diff --git a/packages/app-core/src/lib/vim-half-page-keymap.ts b/packages/app-core/src/lib/vim-half-page-keymap.ts index be2606c3..9c619938 100644 --- a/packages/app-core/src/lib/vim-half-page-keymap.ts +++ b/packages/app-core/src/lib/vim-half-page-keymap.ts @@ -20,6 +20,18 @@ export function keyBindingsFor(binding: string, run: Command): KeyBinding[] { return [{ key: toCodeMirrorKey(binding), run }] } +/** + * Hand the half-page keys to Vim ahead of CodeMirror's own keymaps. + * + * Where Mod is Ctrl (Linux, Windows) the search keymap binds Ctrl+D to + * "select next occurrence" and the history keymap binds Ctrl+U to "undo + * selection", and both run before the Vim plugin sees the key. This binding + * sits ahead of them and feeds the configured sequence to Vim in normal and + * visual mode, so the same half-page motion runs in both: a visual selection + * used to gain an extra cursor per press instead of growing (#825). Insert + * mode falls through untouched: the half-page motion has no business there, + * and Vim's own insert-mode Ctrl+D (unindent) keeps its place in the order. + */ export function vimHalfPageKeymap( vimMode: boolean, overrides: KeymapOverrides @@ -32,7 +44,7 @@ export function vimHalfPageKeymap( return keyBindingsFor(binding, (view): boolean => { const cm = getCM(view) const vim = cm?.state.vim - if (!cm || !vim || vim.insertMode || vim.visualMode) return false + if (!cm || !vim || vim.insertMode) return false return !!Vim.handleKey(cm, sequence, 'user') }) }) diff --git a/packages/app-core/src/store-note-integrity.test.ts b/packages/app-core/src/store-note-integrity.test.ts index 7adcbd35..57318d63 100644 --- a/packages/app-core/src/store-note-integrity.test.ts +++ b/packages/app-core/src/store-note-integrity.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' // Reproduction harness for #202 ("Notes show the wrong content" → files // overwritten with another note's body). Drives the REAL store over an @@ -97,6 +97,9 @@ beforeEach(() => { vi.restoreAllMocks() installZen() }) +afterEach(() => { + vi.useRealTimers() +}) function seedRootVault(useStore: { setState: (s: Record) => void }): void { useStore.setState({ @@ -417,5 +420,143 @@ describe('#585 — dirty buffers survive watcher change events', () => { await flush() expect(useStore.getState().noteContents[target]?.body).toBe('INDEX_BODY with unsaved edits') + // The edit above armed a real debounced save. Settle it here: a timer that + // outlives its test fires into whichever test is running 350 ms later and + // shows up there as a write nobody asked for. + await useStore.getState().persistNote(target) + expect(vault.get(target)).toBe('INDEX_BODY with unsaved edits') + }) +}) + +// #828: a custom Vim insert-mode escape such as `jk` types the `j` into the +// document and removes it again once the `k` completes the sequence. The +// buffer ends where it started, but the first change had marked the note +// dirty, so the debounced save rewrote identical bytes, the file's mtime +// moved, and {{modified_*}} tokens updated for a note nobody changed. The +// same shape covers a typed character that is backspaced and an undo back to +// the saved text. +describe('#828: a buffer back on its saved bytes is not rewritten', () => { + async function openIndex() { + const { useStore } = await loadStore() + seedRootVault(useStore) + const target = 'index.md' + await useStore.getState().openNoteInPane(useStore.getState().activePaneId, target) + await flush() + return { useStore, target } + } + + it('cancels the save when an inserted character is removed again', async () => { + const { useStore, target } = await openIndex() + vi.useFakeTimers() + + useStore.getState().updateNoteBody(target, 'INDEX_BODYj') + expect(useStore.getState().noteDirty[target]).toBe(true) + useStore.getState().updateNoteBody(target, 'INDEX_BODY') + await vi.advanceTimersByTimeAsync(1000) + + expect(writeCalls).toEqual([]) + expect(useStore.getState().noteDirty[target]).toBe(false) + expect(useStore.getState().activeDirty).toBe(false) + expect(useStore.getState().noteContents[target]?.body).toBe('INDEX_BODY') + }) + + it('still saves once when real typing ends with the escape sequence', async () => { + const { useStore, target } = await openIndex() + vi.useFakeTimers() + + useStore.getState().updateNoteBody(target, 'INDEX_BODY typed') + useStore.getState().updateNoteBody(target, 'INDEX_BODY typedj') + useStore.getState().updateNoteBody(target, 'INDEX_BODY typed') + await vi.advanceTimersByTimeAsync(1000) + + expect(writeCalls).toEqual([{ path: target, body: 'INDEX_BODY typed' }]) + expect(useStore.getState().noteDirty[target]).toBe(false) + }) + + it('measures a revert against the last save, not the body the note opened with', async () => { + const { useStore, target } = await openIndex() + vi.useFakeTimers() + + useStore.getState().updateNoteBody(target, 'FIRST') + await vi.advanceTimersByTimeAsync(1000) + expect(writeCalls).toEqual([{ path: target, body: 'FIRST' }]) + + // Back to what disk holds now: nothing to write. + useStore.getState().updateNoteBody(target, 'FIRST more') + useStore.getState().updateNoteBody(target, 'FIRST') + await vi.advanceTimersByTimeAsync(1000) + expect(writeCalls).toHaveLength(1) + expect(useStore.getState().noteDirty[target]).toBe(false) + + // Back to the body it opened with: disk has moved on, so this is an edit. + useStore.getState().updateNoteBody(target, 'INDEX_BODY') + await vi.advanceTimersByTimeAsync(1000) + expect(writeCalls).toEqual([ + { path: target, body: 'FIRST' }, + { path: target, body: 'INDEX_BODY' } + ]) + expect(vault.get(target)).toBe('INDEX_BODY') + }) + + it('a revert while a write is in flight still lands on disk', async () => { + const { useStore, target } = await openIndex() + let release!: () => void + const gate = new Promise((r) => { + release = r + }) + const zen = window.zen as unknown as { + writeNote: (p: string, b: string) => Promise + } + const realWrite = zen.writeNote + zen.writeNote = async (p: string, b: string) => { + await gate + return realWrite(p, b) + } + + useStore.getState().updateNoteBody(target, 'FIRST') + const persisting = useStore.getState().persistNote(target) + // The disk is about to hold FIRST, so going back to the opening body is + // not a return to the saved bytes even though it matches them right now. + useStore.getState().updateNoteBody(target, 'INDEX_BODY') + release() + await persisting + + expect(vault.get(target)).toBe('FIRST') + expect(useStore.getState().noteDirty[target]).toBe(true) + await useStore.getState().persistNote(target) + expect(vault.get(target)).toBe('INDEX_BODY') + expect(useStore.getState().noteDirty[target]).toBe(false) + }) + + it('typing ahead of a write and then returning to the written body is clean', async () => { + const { useStore, target } = await openIndex() + // Installed before the first edit so every debounce timer this test arms + // is a fake one that the fake clearTimeout can actually cancel. + vi.useFakeTimers() + let release!: () => void + const gate = new Promise((r) => { + release = r + }) + const zen = window.zen as unknown as { + writeNote: (p: string, b: string) => Promise + } + const realWrite = zen.writeNote + zen.writeNote = async (p: string, b: string) => { + await gate + return realWrite(p, b) + } + + useStore.getState().updateNoteBody(target, 'FIRST') + const persisting = useStore.getState().persistNote(target) + useStore.getState().updateNoteBody(target, 'FIRST AND SECOND') // typed mid-write + release() + await persisting + expect(useStore.getState().noteDirty[target]).toBe(true) + + useStore.getState().updateNoteBody(target, 'FIRST') + await vi.advanceTimersByTimeAsync(1000) + + expect(writeCalls).toEqual([{ path: target, body: 'FIRST' }]) + expect(useStore.getState().noteDirty[target]).toBe(false) }) }) diff --git a/packages/app-core/src/store.test.ts b/packages/app-core/src/store.test.ts index 8ee6506f..23fd7555 100644 --- a/packages/app-core/src/store.test.ts +++ b/packages/app-core/src/store.test.ts @@ -2659,3 +2659,40 @@ describe('file-task lifecycle coordination', () => { expect(useStore.getState().noteContents[source.path]).toBeUndefined() }) }) + +describe('createAndOpen with tags (#826 follow-up)', () => { + it('writes a tag line under the heading, using the title the vault settled on', async () => { + const createNote = vi.fn().mockResolvedValue(makeNote('# Runbook 2\n\n', 'inbox/Runbook 2.md')) + const writeNote = vi.fn().mockResolvedValue(undefined) + installZen({ + createNote, + writeNote, + listNotes: vi.fn().mockResolvedValue([makeNote('# Runbook 2\n\n', 'inbox/Runbook 2.md')]), + readNote: vi.fn().mockResolvedValue(makeNote('# Runbook 2\n\n#ops #prod\n\n', 'inbox/Runbook 2.md')) + }) + const { useStore } = await loadStore() + + await useStore.getState().createAndOpen('inbox', '', { title: 'Runbook', tags: ['ops', 'prod'] }) + + expect(createNote).toHaveBeenCalledWith('inbox', 'Runbook', '') + expect(writeNote).toHaveBeenCalledTimes(1) + expect(writeNote).toHaveBeenCalledWith('inbox/Runbook 2.md', '# Runbook 2\n\n#ops #prod\n\n') + expect(useStore.getState().selectedPath).toBe('inbox/Runbook 2.md') + }) + + it('leaves the body the vault wrote when there are no tags', async () => { + const writeNote = vi.fn().mockResolvedValue(undefined) + installZen({ + createNote: vi.fn().mockResolvedValue(makeNote('# T\n\n', 'inbox/T.md')), + writeNote, + listNotes: vi.fn().mockResolvedValue([makeNote('# T\n\n', 'inbox/T.md')]), + readNote: vi.fn().mockResolvedValue(makeNote('# T\n\n', 'inbox/T.md')) + }) + const { useStore } = await loadStore() + + await useStore.getState().createAndOpen('inbox', '', { title: 'T', tags: [] }) + await useStore.getState().createAndOpen('inbox', '', { title: 'T' }) + + expect(writeNote).not.toHaveBeenCalled() + }) +}) diff --git a/packages/app-core/src/store.ts b/packages/app-core/src/store.ts index e8f15b0f..9b70b5bb 100644 --- a/packages/app-core/src/store.ts +++ b/packages/app-core/src/store.ts @@ -131,6 +131,7 @@ import { buildTemplateDestinationPrompt, parseTemplateDestination } from './lib/move-note' +import { composeNewNoteBody } from './lib/search-create' import type { KeymapId, KeymapOverrides } from './lib/keymaps' import { normalizeKeymapOverrides } from './lib/keymaps' import { @@ -3395,10 +3396,12 @@ interface Store { formatActiveNote: () => Promise renameNote: (oldPath: string, nextTitle: string, hostIsCurrent?: () => boolean) => Promise renameActive: (nextTitle: string) => Promise + /** Create a note and open it. `tags` seeds the body with one line of + * `#tags` under the heading, the way `zn capture --tag` does. */ createAndOpen: ( folder: NoteFolder, subpath?: string, - options?: { focusTitle?: boolean; title?: string } + options?: { focusTitle?: boolean; title?: string; tags?: readonly string[] } ) => Promise createDrawingAndOpen: (folder: NoteFolder, subpath?: string) => Promise /** Quick-add a whole-note task file (`#task`-tagged, TaskNotes-style). Prompts @@ -3791,6 +3794,15 @@ const pathSaveTimers = new Map>() * older one to the final rename. */ const pathSaveQueues = new Map>() const PATH_SAVE_DEBOUNCE_MS = 350 +/** The on-disk body of every dirty note, taken from the buffer the moment it + * first drifted from disk (a clean buffer equals disk) and moved forward by + * each completed write. An edit that brings the buffer back to these bytes + * is not a change: a custom Vim insert-mode escape such as `jk` types and + * removes its `j`, and saving the identical text only moved the file's mtime + * and the {{modified_*}} tokens with it (#828). Entries are consulted only + * while `noteDirty[path]` is true and are replaced by the note's next edit + * once it is clean again, so a leftover for a clean note is never read. */ +const savedBodies = new Map() // Only the latest watcher read may apply, and a newer local save invalidates // older reads even if it finishes or returns to the same starting body. const noteContentVersions = new Map() @@ -7671,12 +7683,18 @@ export const useStore = create((set, get) => { updateNoteBody: (path, body) => { if (isNoteEditingLocked(get().vault, path)) return + let backOnDisk = false set((s) => { const existing = s.noteContents[path] if (existing) body = rewriteRenamingBody(path, body, existing.folder) if (!existing || existing.body === body) return s + if (!s.noteDirty[path]) savedBodies.set(path, existing.body) + // While a write is in flight the bytes on disk are changing under us, + // so only a settled note can be declared back on them; the completion + // below records what actually landed for the next comparison. + backOnDisk = !pathSaveQueues.has(path) && savedBodies.get(path) === body const contents = { ...s.noteContents, [path]: { ...existing, body } } - const dirty = { ...s.noteDirty, [path]: true } + const dirty = { ...s.noteDirty, [path]: !backOnDisk } // Editing a preview tab promotes it to a permanent tab (VS Code // behavior) so the edit can't be displaced by the next preview. // Cheap guard first: this runs on every keystroke. @@ -7691,6 +7709,15 @@ export const useStore = create((set, get) => { ...activeFieldsFrom(layout, s.activePaneId, contents, dirty) } }) + if (backOnDisk) { + savedBodies.delete(path) + const pending = pathSaveTimers.get(path) + if (pending) { + clearTimeout(pending) + pathSaveTimers.delete(path) + } + return + } if (folderMutationBlocks(path)) return // Debounced disk write. const existing = pathSaveTimers.get(path) @@ -7743,8 +7770,11 @@ export const useStore = create((set, get) => { } set((cur) => { // Keystrokes that landed while the write was in flight leave the - // buffer ahead of disk. The queued caller will persist them next. + // buffer ahead of disk. The queued caller will persist them next, + // unless they take the buffer back to the bytes just written. const stillCurrent = cur.noteContents[path]?.body === writtenBody + if (stillCurrent || !cur.noteContents[path]) savedBodies.delete(path) + else savedBodies.set(path, writtenBody) const dirty = stillCurrent ? { ...cur.noteDirty, [path]: false } : cur.noteDirty return { noteDirty: dirty, @@ -7913,6 +7943,11 @@ export const useStore = create((set, get) => { try { const meta = await window.zen.createNote(folder, options?.title, subpath) rememberEditModeForCreatedNote(meta.path) + // The heading uses the title the vault settled on, which may carry a + // " 2" suffix the requested one did not. + if (options?.tags && options.tags.length > 0) { + await window.zen.writeNote(meta.path, composeNewNoteBody(meta.title, options.tags)) + } await get().refreshNotes() set({ view: { kind: 'folder', folder, subpath }, diff --git a/packages/app-core/src/styles/index.css b/packages/app-core/src/styles/index.css index 36a51a7f..a789a8cc 100644 --- a/packages/app-core/src/styles/index.css +++ b/packages/app-core/src/styles/index.css @@ -2159,7 +2159,7 @@ html[data-completed-task-style="gray-strikethrough"] .cm-editor .cm-task-done * color: rgb(var(--z-fg)); } -/* Leading YAML frontmatter — render as a compact "properties" metadata card +/* Leading YAML frontmatter: rendered as a compact "properties" metadata card * (see cm-frontmatter.ts). For database record pages these values mirror the * database fields, so it should read like a tidy property list, not body text. */ .cm-editor .cm-frontmatter-line { @@ -2172,38 +2172,17 @@ html[data-completed-task-style="gray-strikethrough"] .cm-editor .cm-task-done * border-left: 1px solid rgb(var(--z-grey-0) / 0.13); border-right: 1px solid rgb(var(--z-grey-0) / 0.13); } -.cm-editor .cm-frontmatter-line - :is( - .tok-meta, - .tok-string, - .tok-keyword, - .tok-atom, - .tok-heading, - .tok-heading1, - .tok-heading2, - .tok-heading3, - .tok-heading4, - .tok-heading5, - .tok-heading6 - ) { +/* The note grammar (cm-markdown-language.ts) keeps the frontmatter out of the + * markdown parse, so its lines carry no heading or list tokens; the only token + * inside the card is the `meta` mark on the two `---` fences, which must + * inherit the fence rows' transparent color below. */ +.cm-editor .cm-frontmatter-line .tok-meta { color: inherit; } -/* Without a blank line before the closing `---`, CodeMirror's markdown parser - * can tokenize the previous frontmatter line as a setext heading. The metadata - * card owns frontmatter presentation, so suppress body heading typography here. */ -.cm-editor .cm-frontmatter-line - :is(.tok-heading, .tok-heading1, .tok-heading2, .tok-heading3, .tok-heading4, .tok-heading5, .tok-heading6) { - font-size: inherit; - font-weight: inherit; - line-height: inherit; - text-transform: none; - letter-spacing: 0; -} /* The key (before the `:`) is a muted-but-legible label; the value renders at * normal text brightness (see the line rule above), so an all-frontmatter note * like a task file stays readable instead of fading into the background. */ -.cm-editor .cm-frontmatter-key, -.cm-editor .cm-frontmatter-key :is(.tok-meta, .tok-string, .tok-keyword, .tok-atom) { +.cm-editor .cm-frontmatter-key { color: rgb(var(--z-grey-1)); font-weight: 500; } diff --git a/packages/bridge-contract/package.json b/packages/bridge-contract/package.json index 9fe41e59..ab5a6af8 100644 --- a/packages/bridge-contract/package.json +++ b/packages/bridge-contract/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/bridge-contract", "private": true, - "version": "2.53.0", + "version": "2.54.0", "type": "module", "exports": { "./bridge": "./src/bridge.ts", diff --git a/packages/shared-domain/package.json b/packages/shared-domain/package.json index dad04fb3..4a1eca7a 100644 --- a/packages/shared-domain/package.json +++ b/packages/shared-domain/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/shared-domain", "private": true, - "version": "2.53.0", + "version": "2.54.0", "type": "module", "exports": { "./*": "./src/*.ts" diff --git a/packages/shared-domain/src/harper-settings.test.ts b/packages/shared-domain/src/harper-settings.test.ts index dc417e79..56b6e160 100644 --- a/packages/shared-domain/src/harper-settings.test.ts +++ b/packages/shared-domain/src/harper-settings.test.ts @@ -3,6 +3,7 @@ import { harperIgnoredLintHashes, harperIgnoredLintsJson, isHarperDialect, + mergeHarperVaultState, normalizeHarperLintConfig, normalizeHarperVaultState } from './harper-settings' @@ -24,6 +25,30 @@ describe('normalizeHarperVaultState', () => { }) }) +describe('mergeHarperVaultState (#829)', () => { + it('keeps every entry of both sides, the base order first, new ones appended', () => { + expect( + mergeHarperVaultState( + { words: ['Zennotez', 'Kanata'], ignoredLints: ['12'] }, + { words: ['Kanata', 'Flurbish'], ignoredLints: ['9722060015410969502', '12'] } + ) + ).toEqual({ + words: ['Zennotez', 'Kanata', 'Flurbish'], + ignoredLints: ['12', '9722060015410969502'] + }) + }) + + it('never shrinks to the side that holds less', () => { + const vault = { words: ['Zennotez', 'Kanata'], ignoredLints: ['12'] } + // A session that lost its imports exports one freshly added word. + expect(mergeHarperVaultState(vault, { words: ['Flurbish'], ignoredLints: [] })).toEqual({ + words: ['Zennotez', 'Kanata', 'Flurbish'], + ignoredLints: ['12'] + }) + expect(mergeHarperVaultState(vault, { words: [], ignoredLints: [] })).toEqual(vault) + }) +}) + describe('ignored lint hashes', () => { it('round-trips a 64-bit hash without parsing it as a number', () => { const exported = '{"context_hashes":[9722060015410969502,18446744073709551615]}' diff --git a/packages/shared-domain/src/harper-settings.ts b/packages/shared-domain/src/harper-settings.ts index cb723a84..ef53513f 100644 --- a/packages/shared-domain/src/harper-settings.ts +++ b/packages/shared-domain/src/harper-settings.ts @@ -61,6 +61,27 @@ export function normalizeHarperVaultState(value: unknown): HarperVaultState | un return { words, ignoredLints } } +/** + * The union of two states, `base` first: a vault's list keeps its order and + * anything only `extra` knows lands at the end. The dictionary and the ignore + * list are append-only from inside the app (there is no UI that removes an + * entry), so when the vault and a live session disagree the answer is + * everything both hold, never the shorter list; a persist built on this + * cannot lose a word the vault already had. A future "remove word" feature + * has to revisit the callers of this function. + */ +export function mergeHarperVaultState( + base: HarperVaultState, + extra: HarperVaultState +): HarperVaultState { + return { + words: uniqueStrings([...base.words, ...extra.words]), + ignoredLints: uniqueStrings([...base.ignoredLints, ...extra.ignoredLints]).filter((hash) => + /^\d+$/.test(hash) + ) + } +} + /** Harper exports ignored lints as `{"context_hashes":[, ...]}`. Pull the * digit runs out as strings without ever parsing the JSON. */ export function harperIgnoredLintHashes(exported: string): string[] { diff --git a/packages/shared-domain/src/markdown-lines.ts b/packages/shared-domain/src/markdown-lines.ts index a3120919..32e1f159 100644 --- a/packages/shared-domain/src/markdown-lines.ts +++ b/packages/shared-domain/src/markdown-lines.ts @@ -13,16 +13,28 @@ export const FENCE_OPEN_RE = /^\s*(`{3,}|~{3,})(.*)$/ // trailing whitespace (no info string). export const FENCE_CLOSE_RE = /^\s*(`{3,}|~{3,})[ \t]*$/ +/** + * A YAML frontmatter fence: `---` alone on its line, surrounding whitespace + * tolerated. Every frontmatter consumer in the editor decides "is this line a + * fence" with this one predicate: this walker, the properties card + * (`frontmatterRange` in app-core's cm-frontmatter.ts), and the note grammar + * that keeps the block out of the markdown parser (cm-markdown-language.ts). + * They must agree, or the card styles a range the grammar still parses as + * markdown, and a `key: value` line comes back as a setext heading (#827). + */ +export function isFrontmatterFence(line: string): boolean { + return line.trim() === '---' +} + /** * 0-based index of the closing `---` of a leading YAML frontmatter block, - * or -1 when the body has none. Matches the editor's frontmatter detection - * (cm-wysiwyg-blocks): the very first line must be `---`, and the block runs - * to the next `---` line. + * or -1 when the body has none. The very first line must be a fence, and the + * block runs to the next fence line. */ export function frontmatterEndIndex(lines: string[]): number { - if (lines.length < 2 || lines[0].trim() !== '---') return -1 + if (lines.length < 2 || !isFrontmatterFence(lines[0])) return -1 for (let i = 1; i < lines.length; i++) { - if (lines[i].trim() === '---') return i + if (isFrontmatterFence(lines[i])) return i } return -1 } diff --git a/packages/shared-ui/package.json b/packages/shared-ui/package.json index 264624e1..73cfab5c 100644 --- a/packages/shared-ui/package.json +++ b/packages/shared-ui/package.json @@ -1,7 +1,7 @@ { "name": "@zennotes/shared-ui", "private": true, - "version": "2.53.0", + "version": "2.54.0", "type": "module", "exports": { ".": "./src/index.ts"