From a2255a33d143ddd3496ea965fa87f326f5036317 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 03:45:02 +0000 Subject: [PATCH 1/2] =?UTF-8?q?ui:=20Tab=20nests=20a=20list=20item,=20?= =?UTF-8?q?=E2=87=A7Tab=20lifts=20it=20back=20out?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nesting a list was the one structural edit the editor had no gesture for. Enter continues a marker and ⌘B wraps a word, but making `- b` a child of `- a` meant counting spaces by hand — and the key everyone reaches for first, Tab, was unbound, so it did what an unbound Tab does in a webview: walked the focus ring straight out of the buffer and into the next pane. Tab is claimed **only with the caret in a list item**. Everywhere else in the note `indentList` returns null, the binding declines, and Tab goes on stepping the focus ring — which is why this isn't CodeMirror's own `indentWithTab`, a one-liner that takes the key outright and takes the keyboard's way out of the editor with it. Indenting a paragraph would make a code block anyway, which nobody means by Tab. Inside a list the key is swallowed even when nothing can move (the first item of a list has nothing to nest under): a gesture that *sometimes* ejects you from the buffer is worse than one that sometimes does nothing. - ui/src/list.ts — the engine, pure and node-testable (the format.ts pattern). It edits leading whitespace and the digits of an ordered marker, and nothing else. The new indent is the previous sibling's **content column**, not a fixed two spaces, because that is where CommonMark puts a child — indent under `1. a` by two and the nesting simply doesn't parse. An item's subtree travels with it, or ⇧Tab would re-parent the children onto whatever the item landed beside. Ordered runs are renumbered, both the one an item left and the one it joined: `2.` nested out of `1. 2. 3.` is a list whose first item says "2.", and renders as "2.". The start number stays the author's where they chose it (a list opening at `5.` goes on opening at `5.`), and the lazy `1. 1. 1.` style is left alone — it renders identically and nothing moved into or out of it. - main.ts — `runListShift`, the CodeMirror half. A caret inside code declines before the engine is asked: a `- item` line in a fence is text, not structure, and `inCodeContext` is the same read the rich paste makes. - bindings.ts / shortcuts.ts — declared once, with a row in the sheet, so the chords are rebindable and findable like every other (obligation 4). editorkeys.ts gains `markdownKeymap` in STOCK_KEYMAPS, which was a real gap: `markdown()` installs it at Prec.high, *above* B2's own chords, and nothing compared it against them. Its ⏎ and ⌫ overlap nothing, but "CodeMirror leaves Tab alone" is only an assertion if every keymap the editor installs is in that list — so editorkeys.test.ts now asserts it directly, and would catch an `indentWithTab` arriving in `defaultKeymap` as much as a markdown binding. list.test.ts covers the gesture in 29 checks, asserting on the Markdown that comes out rather than the change list — including the two shapes that read as bugs when they're wrong: the ordered renumbering, and Tab declining outside a list. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015oc7gyoL7AccTEF2picayR --- ui/src/bindings.ts | 18 +- ui/src/editorkeys.test.ts | 42 +++- ui/src/editorkeys.ts | 8 + ui/src/list.test.ts | 233 +++++++++++++++++++ ui/src/list.ts | 459 ++++++++++++++++++++++++++++++++++++++ ui/src/main.ts | 47 ++++ ui/src/shortcuts.ts | 4 + 7 files changed, 802 insertions(+), 9 deletions(-) create mode 100644 ui/src/list.test.ts create mode 100644 ui/src/list.ts diff --git a/ui/src/bindings.ts b/ui/src/bindings.ts index 7d1b84b..d596f43 100644 --- a/ui/src/bindings.ts +++ b/ui/src/bindings.ts @@ -188,14 +188,26 @@ export const DEFAULT_BINDINGS = [ { id: "find.next", label: "Next match", keys: ["Mod-g"], scope: "find" }, { id: "find.prev", label: "Previous match", keys: ["Mod-Shift-g"], scope: "find" }, - // The editor. The first four are handed to CodeMirror's own keymap (ahead of its - // defaults, so they win); ⌘S is the document handler's, and reaches it only because - // CodeMirror leaves Mod-s unbound — editorkeys.test.ts is what keeps that true. + // The editor. All but ⌘S are handed to CodeMirror's own keymap (ahead of its defaults, + // so they win); ⌘S is the document handler's, and reaches it only because CodeMirror + // leaves Mod-s unbound — editorkeys.test.ts is what keeps that true. { id: "format.bold", label: "Bold", keys: ["Mod-b"], scope: "editor" }, { id: "format.italic", label: "Italic", keys: ["Mod-i"], scope: "editor" }, { id: "editor.table", label: "Insert a table", keys: ["Mod-t"], scope: "editor" }, { id: "editor.paste-plain", label: "Paste as plain text", keys: ["Mod-Shift-v"], scope: "editor" }, { id: "editor.save", label: "Save now", keys: ["Mod-s"], scope: "editor" }, + // Tab, the one chord here that is also the *platform's*. It is claimed only with the + // caret in a list item (list.ts declines otherwise), so Tab still walks the focus ring + // everywhere else in the buffer. The overlay layer's own `Any-Tab` trap is a sibling + // scope rather than a clash: a dialog holds the keyboard while it's up, so CodeMirror + // never sees the key it would take — which is exactly what the trap is for. + { id: "editor.list.indent", label: "Nest a list item", keys: ["Tab"], scope: "editor" }, + { + id: "editor.list.outdent", + label: "Lift a list item out", + keys: ["Shift-Tab"], + scope: "editor", + }, // The frontmatter drawer — a separate surface from the body editor, hence its own // scope: ⌘S means "save this drawer" here and "flush the note" there. diff --git a/ui/src/editorkeys.test.ts b/ui/src/editorkeys.test.ts index d0bff6d..55076bc 100644 --- a/ui/src/editorkeys.test.ts +++ b/ui/src/editorkeys.test.ts @@ -12,7 +12,7 @@ // that binds Mod-e would break ⌘E in the one path a user hits constantly — no error, no // failing test, just a chord that stopped working. So the assumption is a list now, and // the list is asserted. -import { DEFAULT_BINDINGS, chordFor, parseChord } from "./bindings.ts"; +import { DEFAULT_BINDINGS, chordFor, keystrokes, parseChord } from "./bindings.ts"; import { STOCK_EDITOR_KEYMAP, STOCK_KEYMAPS, @@ -137,17 +137,40 @@ check("B2 and the editor never meet on a ⌃ keystroke — the emacs bindings ar }); check("the editor's own B2 chords are the ones installed ahead of the stock keymap", () => { - // ⌘B / ⌘I / ⌘T / ⇧⌘V go into the editor's keymap; ⌘S is the document handler's. All - // five are scope `editor`, which is what makes the overlap check consider them at all - // — a chord filed under the wrong scope would be compared against the wrong keyboard. + // ⌘B / ⌘I / ⌘T / ⇧⌘V and Tab / ⇧Tab go into the editor's keymap; ⌘S is the document + // handler's. All are scope `editor`, which is what makes the overlap check consider + // them at all — a chord filed under the wrong scope would be compared against the + // wrong keyboard. const editorIds = DEFAULT_BINDINGS.filter((b) => b.scope === "editor").map((b) => b.id); assertEq( editorIds, - ["format.bold", "format.italic", "editor.table", "editor.paste-plain", "editor.save"], + [ + "format.bold", + "format.italic", + "editor.table", + "editor.paste-plain", + "editor.save", + "editor.list.indent", + "editor.list.outdent", + ], "the editor's chords", ); }); +check("Tab reaches the editor's list commands — nothing in the editor binds it first", () => { + // The assumption `editor.list.indent` rests on, and the reason `markdownKeymap` is in + // STOCK_KEYMAPS now: `markdown()` installs it at Prec.high, *above* B2's own chords, + // so a release that gave it a Tab binding would take the key without a word. The stock + // list is also where `indentWithTab` would show up if it were ever added to + // `defaultKeymap` — that one would silently turn Tab back into plain indentation. + const onTab = editorChords().filter((c) => keystrokes(c.spec).some((f) => f.endsWith("Tab"))); + assertEq( + onTab.map((c) => `${c.spec} — ${c.source} ${c.command}`), + [], + "stock chords over Tab", + ); +}); + check("chords handed to CodeMirror are ones CodeMirror can parse", () => { // `chordFor` gives main.ts the registry's spelling and it goes straight into // `keymap.of` — which works because the syntax is CodeMirror's, with one exception. @@ -155,7 +178,14 @@ check("chords handed to CodeMirror are ones CodeMirror can parse", () => { // a chord nothing presses. Nothing installed in the editor uses it today; this is what // notices if that changes, since the failure is otherwise a chord that silently // stops working. - const installed = ["format.bold", "format.italic", "editor.table", "editor.paste-plain"]; + const installed = [ + "format.bold", + "format.italic", + "editor.table", + "editor.paste-plain", + "editor.list.indent", + "editor.list.outdent", + ]; for (const id of installed) { const spec = chordFor(id); parseChord(spec); // our side diff --git a/ui/src/editorkeys.ts b/ui/src/editorkeys.ts index ff0a81b..a5ad5da 100644 --- a/ui/src/editorkeys.ts +++ b/ui/src/editorkeys.ts @@ -24,6 +24,7 @@ // and left edit mode. The alias is gone (see `Chord.mod`); this is the check that noticed. import { completionKeymap } from "@codemirror/autocomplete"; import { defaultKeymap, historyKeymap } from "@codemirror/commands"; +import { markdownKeymap } from "@codemirror/lang-markdown"; import type { KeyBinding } from "@codemirror/view"; import { type Binding, type Scope, activeBindings, allKeys, keystrokes } from "./bindings.ts"; @@ -42,6 +43,13 @@ export const STOCK_KEYMAPS: readonly StockKeymap[] = [ // It declines unless the completion menu is actually open, which is what keeps ⏎ a // newline the rest of the time. { source: "completionKeymap", keymap: completionKeymap }, + // Installed by `markdown()` itself (its `addKeymap` default), at Prec.high — so above + // B2's own chords. It was missing here, which is the gap this module exists to close: + // ⏎ continuing a list marker and ⌫ eating one are bindings the editor really runs, and + // nothing checked them. It matters more now that B2 binds Tab (`editor.list.indent`) — + // "CodeMirror leaves Tab alone" is only an assertion if every keymap the editor + // installs is in this list. + { source: "markdownKeymap", keymap: markdownKeymap }, ]; /** What main.ts spreads into the editor's keymap after B2's own chords. `completionKeymap` diff --git a/ui/src/list.test.ts b/ui/src/list.test.ts new file mode 100644 index 0000000..278dc47 --- /dev/null +++ b/ui/src/list.test.ts @@ -0,0 +1,233 @@ +// Tests for the nested-list engine (list.ts) — the Tab / ⇧Tab commands. +// Run directly: node --experimental-strip-types src/list.test.ts +// Hand-rolled asserts, the format.test.ts / panes.test.ts idiom. +// +// The assertions are on the **Markdown that comes out**, not on the change list: what +// matters is the note on disk, and a change list is one of several ways to spell the same +// document. `applyChanges` is list.ts's own mirror of what CodeMirror does with them. +import { applyChanges, indentList, outdentList } from "./list.ts"; + +let passed = 0; + +function assertEq(actual: unknown, expected: unknown, msg: string): void { + const [a, b] = [JSON.stringify(actual), JSON.stringify(expected)]; + if (a !== b) throw new Error(`assertion failed: ${msg}\n actual: ${a}\n expected: ${b}`); +} +function assert(cond: boolean, msg: string): void { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} +function check(name: string, fn: () => void): void { + fn(); + passed++; + console.log(` ok ${name}`); +} + +/** The document with `|` marking the caret, or `[`…`]` marking a selection — the shape + * a reader can check against the prose without counting offsets. */ +function at(marked: string): { doc: string; from: number; to: number } { + if (marked.includes("|")) { + const from = marked.indexOf("|"); + return { doc: marked.replace("|", ""), from, to: from }; + } + const from = marked.indexOf("["); + const to = marked.indexOf("]") - 1; + return { doc: marked.replace("[", "").replace("]", ""), from, to }; +} + +/** Run a command over a marked-up document; returns the Markdown, and where the + * selection landed in it. */ +function run( + cmd: typeof indentList, + marked: string, +): { doc: string; from: number; to: number } | null { + const { doc, from, to } = at(marked); + const r = cmd(doc, from, to); + if (!r) return null; + return { doc: applyChanges(doc, r.changes), from: r.selFrom, to: r.selTo }; +} + +// --- when the gesture applies at all ------------------------------------------------- + +check("Tab outside a list is not the editor's to take", () => { + // The null is what leaves Tab meaning "next control" everywhere else — the whole + // reason this isn't `indentWithTab`. + assertEq(indentList("A plain paragraph.", 3, 3), null, "a paragraph"); + assertEq(indentList("# A heading\n\ntext", 4, 4), null, "a heading"); + assertEq(outdentList("A plain paragraph.", 3, 3), null, "⇧Tab, the same"); +}); + +check("a thematic break is not a one-item list", () => { + // `* * *` and `---` parse as bullets under a naive reading, and nesting one would turn + // a rule into a list. + assertEq(indentList("- a\n\n* * *", 6, 6), null, "* * *"); + assertEq(indentList("- a\n\n---", 6, 6), null, "---"); +}); + +check("Tab in a list is claimed even when nothing can move", () => { + // Claimed, not declined: a gesture that sometimes throws you out of the buffer is + // worse than one that sometimes does nothing (list.ts's header). + const first = run(indentList, "- |a\n- b"); + assertEq(first, { doc: "- a\n- b", from: 2, to: 2 }, "the first item has nothing to nest under"); + const top = run(outdentList, "- a\n- |b"); + assertEq(top, { doc: "- a\n- b", from: 6, to: 6 }, "a top-level item has nothing to leave"); +}); + +// --- nesting ------------------------------------------------------------------------- + +check("Tab nests an item under the one above it", () => { + assertEq(run(indentList, "- a\n- |b")?.doc, "- a\n - b", "two spaces, the bullet's content column"); +}); + +check("the new indent is the previous sibling's content column, not a fixed step", () => { + // `1. ` is three columns wide, so a child of it starts at three — indent by a fixed + // two and the nesting simply doesn't parse. + assertEq(run(indentList, "1. a\n2. |b")?.doc, "1. a\n 1. b", "an ordered parent"); + assertEq(run(indentList, "10. a\n11. |b")?.doc, "10. a\n 1. b", "a two-digit one"); +}); + +check("an item already nested goes one level deeper, not back to the top", () => { + assertEq(run(indentList, "- a\n - b\n - |c")?.doc, "- a\n - b\n - c", "c under b"); +}); + +check("nesting carries the item's own children with it", () => { + const r = run(indentList, "- a\n- |b\n - c\n continuation\n- d"); + assertEq(r?.doc, "- a\n - b\n - c\n continuation\n- d", "the subtree moves as one"); +}); + +check("a blank line inside the subtree does not end it", () => { + const r = run(indentList, "- a\n- |b\n\n - c\n- d"); + assertEq(r?.doc, "- a\n - b\n\n - c\n- d", "the loose item's child came along"); +}); + +check("the sibling search stops at the list it is in", () => { + // Without a block boundary this would reach back over the paragraph, adopt the first + // list's `- a` as a sibling and nest under a list it isn't part of. + assertEq(run(indentList, "- a\n\nA paragraph.\n\n- |b")?.doc, "- a\n\nA paragraph.\n\n- b", "no reach"); +}); + +// --- lifting out --------------------------------------------------------------------- + +check("⇧Tab lifts an item out to its parent's column", () => { + assertEq(run(outdentList, "- a\n - |b")?.doc, "- a\n- b", "back to the top level"); + assertEq(run(outdentList, "- a\n - b\n - |c")?.doc, "- a\n - b\n - c", "one level, not all of them"); +}); + +check("lifting out carries the children too", () => { + const r = run(outdentList, "- a\n - |b\n - c\n- d"); + assertEq(r?.doc, "- a\n- b\n - c\n- d", "c is still b's child"); +}); + +check("an item's continuation lines do not hide its parent", () => { + // `parentOf` has to read past a paragraph: an item's own wrapped text sits between it + // and its children, and stopping there would leave ⇧Tab inert. + const r = run(outdentList, "- a\n more about a\n - |b"); + assertEq(r?.doc, "- a\n more about a\n- b", "a is still the parent"); +}); + +check("indent then outdent is the document you started with", () => { + const start = "- a\n- b\n - c\n- d"; + const there = indentList(start, 6, 6); + assert(there !== null, "indent applied"); + const mid = applyChanges(start, there?.changes ?? []); + const back = outdentList(mid, there?.selFrom ?? 0, there?.selTo ?? 0); + assert(back !== null, "outdent applied"); + assertEq(applyChanges(mid, back?.changes ?? []), start, "round trip"); +}); + +// --- ordered lists ------------------------------------------------------------------- + +check("nesting an ordered item renumbers what it left and what it joined", () => { + // Without this the nested list opens at "2." — which renders as "2." — and the run it + // left counts 1, 3. + assertEq(run(indentList, "1. a\n2. |b\n3. c")?.doc, "1. a\n 1. b\n2. c", "both runs"); +}); + +check("an item joining an existing nested run takes the next number", () => { + assertEq(run(indentList, "1. a\n 1. x\n2. |b")?.doc, "1. a\n 1. x\n 2. b", "x then b"); +}); + +check("a list that opens at 5 goes on opening at 5", () => { + // The start number is the author's; only a run that is newly *headed* restarts at 1. + assertEq(run(indentList, "5. a\n6. |b")?.doc, "5. a\n 1. b", "a keeps its 5"); +}); + +check("lifting an ordered item out renumbers the run it lands in", () => { + assertEq(run(outdentList, "1. a\n 1. x\n 2. |y\n2. b")?.doc, "1. a\n 1. x\n2. y\n3. b", "y joins the top run"); +}); + +check("the run left behind restarts when its first item moved away", () => { + // `y` did not move, but it is the first item of that nested list now, and a list whose + // first item says "2." renders as "2.". + assertEq(run(outdentList, "1. a\n 1. |x\n 2. y")?.doc, "1. a\n2. x\n 1. y", "y restarts at 1"); +}); + +check("the lazy 1. 1. 1. style is left as the author wrote it", () => { + // It renders identically to 1, 2, 3 and is a deliberate way to write Markdown. Nothing + // moved into or out of that run, so nothing about it is wrong. + assertEq(run(indentList, "1. a\n1. b\n1. |c")?.doc, "1. a\n1. b\n 1. c", "a and b untouched"); +}); + +check("a bullet run is not renumbered into an ordered one", () => { + assertEq(run(indentList, "- a\n- b\n- |c")?.doc, "- a\n- b\n - c", "bullets stay bullets"); +}); + +check("the bullet character is the author's", () => { + // `-` and `*` start *different* lists in CommonMark, so rewriting one to match its new + // neighbours would restructure the note behind the author's back. + assertEq(run(indentList, "- a\n - x\n* |b")?.doc, "- a\n - x\n * b", "b is still a `*`"); +}); + +// --- selections ---------------------------------------------------------------------- + +check("a selection over several items moves them as a block", () => { + const r = run(indentList, "- a\n- [b\n- c]\n- d"); + assertEq(r?.doc, "- a\n - b\n - c\n- d", "both shifted by the head's step"); +}); + +check("the selection survives the edit, so Tab Tab nests twice", () => { + const start = "- a\n - x\n- b"; + const once = indentList(start, 12, 12); + const mid = applyChanges(start, once?.changes ?? []); + assertEq(mid, "- a\n - x\n - b", "one step"); + const twice = indentList(mid, once?.selFrom ?? 0, once?.selTo ?? 0); + assertEq(applyChanges(mid, twice?.changes ?? []), "- a\n - x\n - b", "two steps"); +}); + +check("a caret keeps its distance from the content it sits in", () => { + const r = run(indentList, "- a\n- b|"); + assertEq(r, { doc: "- a\n - b", from: 9, to: 9 }, "still after the b"); +}); + +check("a caret inside the indentation rides to the front of the text", () => { + const r = run(outdentList, "- a\n | - b"); + assertEq(r?.doc, "- a\n- b", "outdented"); + assertEq(r?.from, 4, "at the line's new start"); +}); + +check("a selection ending at a line start stops short of that line", () => { + // The line-wise reading every editor uses: `- c` is not in the selection, so it does + // not move, and it is not what the step is measured from. + const r = run(indentList, "- a\n- [b\n]- c"); + assertEq(r?.doc, "- a\n - b\n- c", "only b moved"); +}); + +// --- whitespace ---------------------------------------------------------------------- + +check("a tab of indentation is measured at four columns, and rewritten as spaces", () => { + // CommonMark §2.2 is the measuring rule; spaces are what B2 writes, so the note stays + // one thing rather than a mix. + const r = run(outdentList, "- a\n\t- |b"); + assertEq(r?.doc, "- a\n- b", "a four-column tab, lifted to the top level"); +}); + +check("an item with no content still offers a column to nest into", () => { + assertEq(run(indentList, "-\n- |b")?.doc, "-\n - b", "one past the bare marker"); +}); + +check("five spaces after a marker is code indentation, not a deeper content column", () => { + // CommonMark: a gap of five or more puts the content one column past the marker, and + // the rest is an indented code block inside the item. + assertEq(run(indentList, "- a\n- |b")?.doc, "- a\n - b", "two, not six"); +}); + +console.log(`\n${passed} checks passed`); diff --git a/ui/src/list.ts b/ui/src/list.ts new file mode 100644 index 0000000..e219d08 --- /dev/null +++ b/ui/src/list.ts @@ -0,0 +1,459 @@ +// Nested lists, the pure half — the Tab / ⇧Tab engine. main.ts wires it into the +// editor's keymap through the registry (`editor.list.indent` / `editor.list.outdent`, +// bindings.ts); this module never touches CodeMirror, so node runs its test straight off +// the source (`npm test`), like format.ts / newentry.ts / treenav.ts. +// +// Why it exists. Nesting a list was the one structural edit the editor had no gesture +// for: Enter continues a list (`markdownKeymap`, installed by `markdown()`), ⌘B wraps a +// word, but making `- b` a child of `- a` meant counting spaces by hand. Tab is the +// instinct — and Tab, unbound, is the *browser's*: it walked focus out of the buffer and +// into the next pane, which is the bug this closes. +// +// **Tab keeps its own meaning outside a list.** `indentList` returns null when the +// cursor isn't in a list item, and the binding declines, so Tab still steps the focus +// ring everywhere else — indenting a paragraph would make a code block, which nobody +// means by Tab. Inside a list it is claimed even when nothing can move (the first item of +// a list has nothing to nest under, a top-level item has nothing to lift out of): a +// gesture that sometimes ejects you from the buffer is worse than one that sometimes does +// nothing. K1's promise is unaffected either way — ⌘E leaves edit mode and ⌘1/⌘2/⌘3 move +// between panes, so the keyboard is never stuck in the editor. +// +// What it edits, and what it leaves alone. Leading whitespace, and the digits of an +// ordered marker. Nothing else: the bullet character is the author's (`-` vs `*` starts a +// *different* list in CommonMark, so rewriting one would silently restructure the note), +// and so is every byte of content. Renumbering is not a flourish — indenting `2.` out of +// `1. 2. 3.` leaves a nested list whose first item says "2.", which renders as "2.". + +/** One text edit in original-document coordinates (CodeMirror's change shape). */ +export interface ListChange { + from: number; + to: number; + insert: string; +} + +/** The edits, and the selection to land on (post-edit coordinates). Empty `changes` is a + * claimed-but-inert gesture — see the header. */ +export interface ListEdit { + changes: ListChange[]; + selFrom: number; + selTo: number; +} + +/** CommonMark §2.2: where tabs help define block structure they behave as if replaced by + * spaces to a tab stop of 4. Measuring follows the spec; what B2 *writes* is spaces. */ +const TAB_STOP = 4; + +/** A list item's marker, and the two columns that decide nesting. `\d{1,9}` is + * CommonMark's own limit on an ordered marker. */ +const ITEM = /^([ \t]*)(?:([-*+])|(\d{1,9})([.)]))(?:([ \t]+)|$)/; + +/** `* * *`, `---`, `___` — a thematic break, which the item pattern would otherwise read + * as a bullet whose content is more bullets. */ +const RULE = /^[ \t]*(?:(?:\*[ \t]*){3,}|(?:-[ \t]*){3,}|(?:_[ \t]*){3,})$/; + +const LEADING_WS = /^[ \t]*/; + +interface Marker { + /** Column the marker starts at — an item's own nesting level. */ + indent: number; + /** Characters of leading whitespace: what an indent edit replaces. */ + indentLen: number; + /** Column the item's content starts at — where a child of this item must sit. */ + content: number; + /** The ordered number and the characters spelling it; absent on a bullet. */ + num?: number; + numLen?: number; +} + +interface Ln { + /** Document offset of the line's first character. */ + from: number; + text: string; + blank: boolean; + /** Column the first non-whitespace character sits at; 0 on a blank line. */ + indent: number; + item?: Marker; +} + +/** The column a whitespace-and-marker prefix ends at, tabs expanded per CommonMark. */ +function measure(prefix: string): number { + let col = 0; + for (const ch of prefix) col = ch === "\t" ? col + TAB_STOP - (col % TAB_STOP) : col + 1; + return col; +} + +function leadingWs(text: string): string { + return LEADING_WS.exec(text)?.[0] ?? ""; +} + +/** Read the document as lines, each classified as a list item or not. */ +function scan(doc: string): Ln[] { + const out: Ln[] = []; + let from = 0; + for (const text of doc.split("\n")) { + const ws = leadingWs(text); + const blank = ws.length === text.length; + const indent = measure(ws); + const m = blank || RULE.test(text) ? null : ITEM.exec(text); + if (!m) { + out.push({ from, text, blank, indent }); + } else { + // An unmatched group is undefined at runtime, which `RegExpExecArray`'s `string[]` + // index signature doesn't say — hence the explicit types rather than a destructure + // that would read as total. + const lead: string = m[1]; + const bullet: string | undefined = m[2]; + const digits: string | undefined = m[3]; + const delim: string | undefined = m[4]; + const gap: string | undefined = m[5]; + const markerText = bullet ?? `${digits}${delim}`; + const afterMarker = measure(lead + markerText); + // CommonMark: content sits one column past the marker plus the gap — except that a + // gap of five or more spaces is code indentation, and an item with no content at + // all has no gap to measure. Both cases put the content column one past the marker. + const gapped = gap === undefined ? afterMarker + 1 : measure(lead + markerText + gap); + const item: Marker = { + indent, + indentLen: lead.length, + content: gapped - afterMarker > 4 ? afterMarker + 1 : gapped, + }; + if (digits !== undefined) { + item.num = Number(digits); + item.numLen = digits.length; + } + out.push({ from, text, blank, indent, item }); + } + from += text.length + 1; + } + return out; +} + +function lineIndexAt(lines: readonly Ln[], pos: number): number { + let lo = 0; + let hi = lines.length - 1; + while (lo < hi) { + const mid = (lo + hi + 1) >> 1; + if (lines[mid].from <= pos) lo = mid; + else hi = mid - 1; + } + return lo; +} + +/** The virtual nesting level of a line — the document's own, or the one the pending edit + * gives it. Every structural walk reads the list through one of these, which is what + * lets the same code answer "who is my sibling?" before and after the move. */ +type Level = (i: number) => number; + +/** The run of lines the list around `i` occupies: items, their indented continuations, + * and the single blank lines between them. Two blank lines, or an unindented paragraph, + * end it. + * + * Bounding the walks below to a block is what keeps them honest across the whole note: a + * sibling search that ran to the top of the document would happily adopt an unrelated + * list three paragraphs up. */ +function blockOf(lines: readonly Ln[], i: number): [number, number] { + const listish = (j: number): boolean => + lines[j].item !== undefined || (!lines[j].blank && lines[j].indent > 0); + let start = i; + for (let j = i - 1; j >= 0; ) { + if (listish(j)) { + start = j; + j--; + } else if (lines[j].blank && j > 0 && listish(j - 1)) { + start = j - 1; + j -= 2; + } else break; + } + let end = i; + for (let j = i + 1; j < lines.length; ) { + if (listish(j)) { + end = j; + j++; + } else if (lines[j].blank && j + 1 < lines.length && listish(j + 1)) { + end = j + 1; + j += 2; + } else break; + } + return [start, end]; +} + +/** The item directly above `i` at the same level, or -1 when `i` is the first of its run. + * + * A shallower item is the parent (so there is no previous sibling), a deeper one belongs + * to an earlier sibling, and a paragraph at or above our level ends the run. */ +function prevSibling( + lines: readonly Ln[], + level: Level, + block: [number, number], + i: number, +): number { + const mine = level(i); + for (let j = i - 1; j >= block[0]; j--) { + const l = lines[j]; + if (l.blank) continue; + if (l.item) { + const at = level(j); + if (at === mine) return j; + if (at < mine) return -1; + continue; + } + if (level(j) <= mine) return -1; + } + return -1; +} + +/** The mirror of `prevSibling`, downwards — only the renumbering needs it. */ +function nextSibling( + lines: readonly Ln[], + level: Level, + block: [number, number], + i: number, +): number { + const mine = level(i); + for (let j = i + 1; j <= block[1]; j++) { + const l = lines[j]; + if (l.blank) continue; + if (l.item) { + const at = level(j); + if (at === mine) return j; + if (at < mine) return -1; + continue; + } + if (level(j) <= mine) return -1; + } + return -1; +} + +/** The nearest enclosing item — what ⇧Tab lifts out to. Unlike `prevSibling` this reads + * past a paragraph: an item's own continuation lines sit between it and its children. */ +function parentOf( + lines: readonly Ln[], + level: Level, + block: [number, number], + i: number, +): number { + const mine = level(i); + for (let j = i - 1; j >= block[0]; j--) { + if (lines[j].item && level(j) < mine) return j; + } + return -1; +} + +/** The consecutive siblings of `i`, `i` included, in document order. + * + * A change of marker kind ends the run, because in CommonMark it ends the *list* — a + * bullet after a number is a new list, not a fourth item, so its numbering is its own. */ +function groupOf( + lines: readonly Ln[], + level: Level, + block: [number, number], + i: number, +): number[] { + const ordered = (j: number): boolean => lines[j].item?.num !== undefined; + const kind = ordered(i); + const out = [i]; + for (let j = prevSibling(lines, level, block, i); j >= 0; ) { + if (ordered(j) !== kind) break; + out.unshift(j); + j = prevSibling(lines, level, block, j); + } + for (let j = nextSibling(lines, level, block, i); j >= 0; ) { + if (ordered(j) !== kind) break; + out.push(j); + j = nextSibling(lines, level, block, j); + } + return out; +} + +/** + * Indent (`dir` 1) or outdent (-1) the list item(s) the selection covers. + * + * Returns null when the selection touches no list item at all — the caller's cue to + * decline the keystroke and leave Tab to the platform. + */ +function listEdit(doc: string, from: number, to: number, dir: 1 | -1): ListEdit | null { + const lines = scan(doc); + const first = lineIndexAt(lines, from); + let last = lineIndexAt(lines, to); + // A selection ending at column 0 stops short of that line, the way every editor's + // line-wise command reads it. + if (last > first && to === lines[last].from) last--; + + let head = -1; + for (let i = first; i <= last && head < 0; i++) if (lines[i].item) head = i; + if (head < 0) return null; + + const inert: ListEdit = { changes: [], selFrom: from, selTo: to }; + const item = lines[head].item; + if (!item) return null; + + const block = blockOf(lines, head); + const before: Level = (i) => lines[i].indent; + + // Where the head item is going: under its previous sibling (to that item's content + // column, which is where CommonMark puts a child), or out to its parent's own column. + let target: number; + if (dir === 1) { + const sib = prevSibling(lines, before, block, head); + const sibItem = sib >= 0 ? lines[sib].item : undefined; + if (!sibItem) return inert; // the first item of a list has nothing to nest under + target = sibItem.content; + } else { + const par = parentOf(lines, before, block, head); + const parItem = par >= 0 ? lines[par].item : undefined; + if (!parItem) return inert; // already at the top level + target = parItem.indent; + } + const delta = target - item.indent; + if (delta === 0) return inert; + + // The move carries the item's own subtree with it: the lines below the selection that + // are indented past its last item are its children and continuations, and leaving them + // behind would re-parent them onto whatever the head landed beside. + let tail = head; + for (let i = head; i <= last; i++) if (lines[i].item) tail = i; + let end = last; + for (let j = last + 1; j <= block[1]; j++) { + if (lines[j].blank) continue; + if (lines[j].indent <= lines[tail].indent) break; + end = j; + } + + // Planned per line, emitted below in one pass — a line can be both re-indented and + // re-numbered, and the two rewrites *touch*: they would go to CodeMirror as an + // insertion at the line start and a replacement starting at the same offset, whose + // relative order a ChangeSet does not promise. One edit over the whole prefix has no + // order to get wrong. + const wsShift: number[] = Array.from(lines, () => 0); + const newWs: (string | undefined)[] = Array.from(lines, () => undefined); + for (let i = head; i <= end; i++) { + const l = lines[i]; + if (l.blank) continue; + const oldWs = leadingWs(l.text); + const next = " ".repeat(Math.max(0, l.indent + delta)); + if (next !== oldWs) newWs[i] = next; + wsShift[i] = next.length - oldWs.length; + } + + // The list as it will be, so the renumbering below reasons about the structure the + // author is about to see rather than the one they had. + const after: Level = (i) => + i >= head && i <= end && !lines[i].blank + ? Math.max(0, lines[i].indent + delta) + : lines[i].indent; + const moved = (i: number): boolean => i >= head && i <= end && lines[i].item !== undefined; + + // Two runs can come out mis-numbered: the one the head joined, and the one it left. + // `groupOf` reads them off the post-move structure; the anchors are just a line known + // to be in each. The head's old neighbours stay where they were, so they still name the + // run it left — unless the selection took them along, in which case there is no run + // left behind to fix. + const anchors = [head]; + const oldPrev = prevSibling(lines, before, block, head); + const oldNext = nextSibling(lines, before, block, head); + if (oldPrev >= 0 && !moved(oldPrev)) anchors.push(oldPrev); + if (oldNext >= 0 && !moved(oldNext)) anchors.push(oldNext); + + const wanted = new Map(); + const done = new Set(); + for (const anchor of anchors) { + const group = groupOf(lines, after, block, anchor); + if (done.has(group[0])) continue; + done.add(group[0]); + renumber(lines, group, before, block, moved, wanted); + } + + const numShift: number[] = Array.from(lines, () => 0); + const changes: ListChange[] = []; + for (let i = 0; i < lines.length; i++) { + const l = lines[i]; + const ws = newWs[i]; + const want = wanted.get(i); + const it = l.item; + if (want !== undefined && it?.numLen !== undefined) { + const num = String(want); + numShift[i] = num.length - it.numLen; + const past = l.from + it.indentLen + it.numLen; + changes.push( + ws === undefined + ? { from: l.from + it.indentLen, to: past, insert: num } + : { from: l.from, to: past, insert: ws + num }, + ); + } else if (ws !== undefined) { + changes.push({ from: l.from, to: l.from + leadingWs(l.text).length, insert: ws }); + } + } + + const shift = (i: number): number => wsShift[i] + numShift[i]; + const mapPos = (pos: number): number => { + const i = lineIndexAt(lines, pos); + let acc = 0; + for (let j = 0; j < i; j++) acc += shift(j); + const wsLen = leadingWs(lines[i].text).length; + const off = pos - lines[i].from; + const wsNow = wsLen + wsShift[i]; + // A caret inside the indentation has no column of its own to keep — it rides to the + // front of the text. Past it, the caret keeps its distance from the content. + const inLine = off <= wsLen ? wsNow : Math.max(wsNow, off + shift(i)); + return lines[i].from + acc + inLine; + }; + + return { changes, selFrom: mapPos(from), selTo: mapPos(to) }; +} + +/** + * Give one run of sibling items the numbers it should carry, appending the edits. + * + * The start number is the author's where they chose it (a list opening at `5.` keeps + * opening at `5.`) and 1 where the run is newly headed — an item that had a sibling above + * it before the move and doesn't now is the first item of a list that didn't exist a + * keystroke ago, and a list that starts at "2." renders as "2.". + * + * The one run left untouched is the lazy `1. 1. 1.` style, which renders identically and + * is a deliberate way to write Markdown; nothing moved in or out of it, so nothing about + * it is wrong. + */ +function renumber( + lines: readonly Ln[], + group: readonly number[], + before: Level, + block: [number, number], + moved: (i: number) => boolean, + wanted: Map, +): void { + const nums = group.map((i) => lines[i].item?.num); + if (nums.some((n) => n === undefined)) return; // a bullet run numbers nothing + const settled = !group.some(moved); + if (settled && group.length > 1 && nums.every((n) => n === nums[0])) return; + + const lead = group[0]; + const wasFirst = prevSibling(lines, before, block, lead) < 0; + const start = wasFirst ? (nums[0] ?? 1) : 1; + group.forEach((i, k) => { + const want = start + k; + if (lines[i].item?.num !== want) wanted.set(i, want); + }); +} + +/** Tab — nest the list item(s) the selection covers one level deeper. */ +export function indentList(doc: string, from: number, to: number): ListEdit | null { + return listEdit(doc, from, to, 1); +} + +/** ⇧Tab — lift the list item(s) the selection covers out one level. */ +export function outdentList(doc: string, from: number, to: number): ListEdit | null { + return listEdit(doc, from, to, -1); +} + +/** Apply an edit to a document — the suite's mirror of what CodeMirror does with the + * changes, and the only honest way to assert on the Markdown that comes out. Exported + * for the test; the app dispatches the changes instead. */ +export function applyChanges(doc: string, changes: readonly ListChange[]): string { + let out = ""; + let at = 0; + for (const c of changes) { + out += doc.slice(at, c.from) + c.insert; + at = c.to; + } + return out + doc.slice(at); +} diff --git a/ui/src/main.ts b/ui/src/main.ts index e5ea9bd..0ab6359 100644 --- a/ui/src/main.ts +++ b/ui/src/main.ts @@ -57,6 +57,7 @@ import { livePreview, wikilink } from "./livepreview"; import { b2Highlighter, highlightCodeBlocks, resolveLang } from "./highlight"; import { wikiCandidates, wikiInsertion, wikiQueryAt } from "./wikicomplete"; import { FORMATS, insertTable, toggleInline, type InlineFormat } from "./format"; +import { indentList, outdentList, type ListEdit } from "./list"; import { activeBindings, canonicalKey, @@ -2498,6 +2499,44 @@ function runFormat(view: EditorView, fmt: InlineFormat): boolean { ); return true; } + +/** + * Tab / ⇧Tab — nest or lift out the list item(s) the selection covers. The engine is + * list.ts; this is the CodeMirror half, the `runFormat` pattern one construct up. + * + * Declining matters twice over. A `null` from the engine means the caret is not in a + * list, and returning false there is what leaves Tab walking the focus ring through the + * rest of the app — the reason this isn't `indentWithTab`, which claims the key outright + * and takes the keyboard's way out of the buffer with it. And a caret inside code + * declines before the engine is asked at all: a `- item` line in a fence is text, not + * structure, and `inCodeContext` is the same read the rich paste makes to keep its hands + * off code. + * + * One range rather than `changeByRange`: an indent moves every offset after it, so a + * second cursor's edit would be computed against a document the first has already + * shifted. Multi-cursor nesting is a gesture nobody makes; ⌘B's is one they do. + */ +function runListShift( + view: EditorView, + shift: (doc: string, from: number, to: number) => ListEdit | null, +): boolean { + if (inCodeContext(view.state)) return false; + const { from, to } = view.state.selection.main; + const r = shift(view.state.doc.toString(), from, to); + if (!r) return false; + // Claimed but inert — the first item of a list has nothing to nest under. Swallowing + // the key is the point (list.ts's header): a gesture that sometimes ejects you from + // the buffer is worse than one that sometimes does nothing. + if (r.changes.length > 0) { + view.dispatch({ + changes: r.changes, + selection: EditorSelection.range(r.selFrom, r.selTo), + scrollIntoView: true, + }); + } + return true; +} + /** * B2's own chords inside the editor, read from the **live** registry each time. * @@ -2515,6 +2554,14 @@ function b2EditorKeymap(): KeyBinding[] { run: (view: EditorView) => runFormat(view, f), })), { key: chordFor("editor.table"), run: runInsertTable }, + { + key: chordFor("editor.list.indent"), + run: (view: EditorView) => runListShift(view, indentList), + }, + { + key: chordFor("editor.list.outdent"), + run: (view: EditorView) => runListShift(view, outdentList), + }, { key: chordFor("editor.paste-plain"), run: (view: EditorView) => { diff --git a/ui/src/shortcuts.ts b/ui/src/shortcuts.ts index 0772207..3856e05 100644 --- a/ui/src/shortcuts.ts +++ b/ui/src/shortcuts.ts @@ -124,6 +124,10 @@ const OWN_SHEET: readonly SheetGroup[] = [ { ids: ["edit.toggle"], action: "Enter or leave edit mode" }, { ids: ["editor.save"], action: "Save now (editing autosaves anyway)" }, { ids: ["format.bold", "format.italic"], action: "Bold / italic" }, + { + ids: ["editor.list.indent", "editor.list.outdent"], + action: "Nest / lift out the list item under the cursor (elsewhere, Tab moves on)", + }, { ids: ["editor.table"], action: "Insert a table" }, { ids: ["editor.paste-plain"], action: "Paste as plain text" }, { keys: "[[", action: "Wikilink completion — ↑↓ then ⏎" }, From 6209b4467abb1468bb236237d06130e39a53916b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 05:20:56 +0000 Subject: [PATCH 2/2] ui: numbering stops at a change of list marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch on #130. `groupOf` collected a renumbering run by asking only "ordered or bullet?", but CommonMark starts a new list at every change of *marker* — `1.` then `2)`, or `-` then `*` — so a run could span two lists and the renumbering would walk straight across the boundary. Two failures, one boundary: 1. a 1. a 1) x -> 2) x <- rewritten; `1) x` heads its own list 2) y 3) y 3) z 1) z 1. a 1. a 5) x -> 2) x <- the author's start number, lost 6) y 1) y The second is why the fix isn't only in `groupOf`. `renumber` decides a run's start number by asking whether its lead item *was* the head of its run, and that question has to be about the list as well: `5)` under a `1.` item is a head — its own — and reading the line above as a sibling restarts it at 1. So `Marker` keeps the marker's identity (`kind`), and the two sibling walks gain run-scoped twins that stop at a change of it. `prevSibling` itself is left alone on purpose: "which item am I nested under?" is a question about columns, and `* b` landing under `- a` is the shape the author asked for by pressing Tab (pinned by "the bullet character is the author's"). "Which items share my numbering?" is a question about the list. The two answers part company exactly here, which is why they are now two functions. The bullet half of the boundary was already inert — a run containing a bullet has no numbers, so `renumber` returns before it can do anything — but the walks read the same either way and the comment above them claimed the rule already. list.test.ts pins both documents above. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015oc7gyoL7AccTEF2picayR --- ui/src/list.test.ts | 15 +++++++++++++++ ui/src/list.ts | 47 +++++++++++++++++++++++++++++++++------------ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/ui/src/list.test.ts b/ui/src/list.test.ts index 278dc47..921cf76 100644 --- a/ui/src/list.test.ts +++ b/ui/src/list.test.ts @@ -167,6 +167,21 @@ check("the lazy 1. 1. 1. style is left as the author wrote it", () => { assertEq(run(indentList, "1. a\n1. b\n1. |c")?.doc, "1. a\n1. b\n 1. c", "a and b untouched"); }); +check("a change of ordered delimiter is a new list, and numbering stops at it", () => { + // `1.` then `1)` is two lists in CommonMark, not one list of two. Reading them as one + // run had the renumbering walk straight over the boundary and rewrite `1) x` to `2) x` + // — an edit to a list the author never touched, three lines from the caret. + const r = run(indentList, "1. a\n1) x\n2) y\n3) |z"); + assertEq(r?.doc, "1. a\n1) x\n2) y\n 1) z", "the `)` list keeps its own count"); +}); + +check("a second list's deliberate start number survives the list above it", () => { + // The other half of the same boundary, and the one a run-level fix alone misses: `5)` + // *is* the head of its list, so "was this item the head of its run?" has to ask about + // the list too, or the 5 the author chose restarts at 1. + assertEq(run(indentList, "1. a\n5) x\n6) |y")?.doc, "1. a\n5) x\n 1) y", "x keeps its 5"); +}); + check("a bullet run is not renumbered into an ordered one", () => { assertEq(run(indentList, "- a\n- b\n- |c")?.doc, "- a\n- b\n - c", "bullets stay bullets"); }); diff --git a/ui/src/list.ts b/ui/src/list.ts index e219d08..51d3281 100644 --- a/ui/src/list.ts +++ b/ui/src/list.ts @@ -60,6 +60,10 @@ interface Marker { indentLen: number; /** Column the item's content starts at — where a child of this item must sit. */ content: number; + /** The bullet character, or the ordered delimiter — the marker's *identity*. Changing + * either starts a new list in CommonMark (`1.` then `2)` is two lists, not one list of + * two), which is what bounds a run of shared numbering. */ + kind: string; /** The ordered number and the characters spelling it; absent on a bullet. */ num?: number; numLen?: number; @@ -116,6 +120,8 @@ function scan(doc: string): Ln[] { indent, indentLen: lead.length, content: gapped - afterMarker > 4 ? afterMarker + 1 : gapped, + // The alternation matched one branch or the other, so one of these is a string. + kind: bullet ?? delim ?? "", }; if (digits !== undefined) { item.num = Number(digits); @@ -239,28 +245,42 @@ function parentOf( return -1; } -/** The consecutive siblings of `i`, `i` included, in document order. +/** The two sibling walks, narrowed to the same *list*. * - * A change of marker kind ends the run, because in CommonMark it ends the *list* — a - * bullet after a number is a new list, not a fourth item, so its numbering is its own. */ + * A neighbour at my level is not necessarily in my list: CommonMark starts a new one at + * every change of marker — `1.` then `2)`, or `-` then `*` — so numbering must stop at + * that boundary even though the indentation doesn't. + * + * Deliberately **not** what the nesting target uses. "Which item am I nested under?" is + * a question about columns, and `* b` landing under `- a` is the shape the author asked + * for by pressing Tab; "which items share my numbering?" is a question about the list, + * and the two answers part company exactly here. */ +function prevInRun(lines: readonly Ln[], level: Level, block: [number, number], i: number): number { + const j = prevSibling(lines, level, block, i); + return j >= 0 && lines[j].item?.kind === lines[i].item?.kind ? j : -1; +} + +function nextInRun(lines: readonly Ln[], level: Level, block: [number, number], i: number): number { + const j = nextSibling(lines, level, block, i); + return j >= 0 && lines[j].item?.kind === lines[i].item?.kind ? j : -1; +} + +/** The consecutive items of `i`'s own list at `i`'s level, `i` included, in document + * order — the run a numbering sequence runs over. */ function groupOf( lines: readonly Ln[], level: Level, block: [number, number], i: number, ): number[] { - const ordered = (j: number): boolean => lines[j].item?.num !== undefined; - const kind = ordered(i); const out = [i]; - for (let j = prevSibling(lines, level, block, i); j >= 0; ) { - if (ordered(j) !== kind) break; + for (let j = prevInRun(lines, level, block, i); j >= 0; ) { out.unshift(j); - j = prevSibling(lines, level, block, j); + j = prevInRun(lines, level, block, j); } - for (let j = nextSibling(lines, level, block, i); j >= 0; ) { - if (ordered(j) !== kind) break; + for (let j = nextInRun(lines, level, block, i); j >= 0; ) { out.push(j); - j = nextSibling(lines, level, block, j); + j = nextInRun(lines, level, block, j); } return out; } @@ -426,8 +446,11 @@ function renumber( const settled = !group.some(moved); if (settled && group.length > 1 && nums.every((n) => n === nums[0])) return; + // `prevInRun`, not `prevSibling`: "was this the head of its list?" has to ask about the + // list. A `5)` item under a `1.` one *is* a head — its own — and reading the neighbour + // above as a sibling would restart it at 1 and lose the number the author chose. const lead = group[0]; - const wasFirst = prevSibling(lines, before, block, lead) < 0; + const wasFirst = prevInRun(lines, before, block, lead) < 0; const start = wasFirst ? (nums[0] ?? 1) : 1; group.forEach((i, k) => { const want = start + k;