feat: structural list Tab/Shift-Tab — marker adoption, cross-list nesting, width-aware renumber#207
Merged
Merged
Conversation
…width-crossing renumber child re-indent Relocates the ordered-renumber logic out of Enter-continuation into the shared list-transform module and makes it marker-width-aware: when a renumber crosses a digit-width boundary (9. -> 10.), the sibling's nested children are now re-indented to track the new content column, fixing a latent bug where a width-crossing renumber dropped a child out of the list. isEmptyItem and the Enter marker-construction logic also move to list-transform.ts (continuationMarkerFor), so Enter-continuation now composes the same primitives a later structural-transform planner will use.
Introduce `orderedShape(number, delim)` + `MAX_LIST_NUMBER` as the sole cap-enforcement point for ordered ListMark numbers: it returns null when the number falls outside 1..999_999_999. Route continuationMarkerFor, renumberRun's per-sibling number, adoptedShapeFrom, adoptedShapeForJoin, newRunShapeFor, and the forced-child renumber through it. This closes two bugs at once and removes four scattered naked `> 999_999_999` guards: - renumberRun had no lower bound, so a `0.`-based run whose delta drove a follower negative emitted `-1.` (rejected by ORDERED_RE = corrupt Markdown). It now fails closed (returns []). - continuationMarkerFor lacked the 9-digit cap every other producer enforces, so continuing `999999999.` produced a 10-digit non-marker. The planner-inline cap re-checks (adopted.number > MAX in both planners) become dead once `adopted` flows from orderedShape and are removed; the existing `adopted === null` no-op covers the overflow path. Tests: (codex) indenting the middle of a `0.`-based run never emits a negative marker; renumberRun width-SHRINK de-indents a narrowed sibling's nested child (the negative-deltaWidth removal branch).
Two live data-corruption bugs in planOutdentItem: fable-1 (empty-item caret): the caret was placed at `markerLine.from + parentMarkCol + newMarker.length`, adding a COLUMN count to a byte offset. On a tab-indented line surviving chars differ from surviving columns, so with a tab indent at EOF the target overshot the shortened document and view.dispatch threw RangeError — swallowed by applyShift's catch, dropping the entire outdent. Derive the caret from the CHARS actually removed ahead of the marker (leadingCharsForColumns) so it stays in range on tab-mixed indent. fable-2 (forced-child geometry): promotedContentCol hard-coded a 1-space marker gap. The non-empty path replaces only the ListMark span, preserving the item's original marker->content gap, so an aligned run (`1. a`, gap 2) computed a content column one short. A forced child at the true column then failed to nest and the outer run corrupted (1,2,1,3). Use the item's real gap (contentColumnOf - marker end) for the non-empty path; the empty path keeps its synthesized single-space gap. Tests: tab-indent empty outdent at EOF composes without RangeError; aligned-gap outdent nests the forced child and keeps the outer run intact; empty item WITH forced children composes with caret after the marker; tab-indented nested item outdents by whole-tab removal.
Replace the IndentPlan / OutdentPlan `{ changes: ChangeSpec[] }` structs —
where an empty `changes: []` overloaded as an "intentional no-op" sentinel
the caller had to recognise — with a discriminated `ListEditPlan`:
{ kind: "noop" } | { kind: "edit"; changes; selection? }
The `edit` variant couples `changes` with the empty-item-caret `selection`,
so a nonsensical `{ changes: [], selection }` (which applyShift would
early-return on, dropping the selection) is now unrepresentable. applyShift
branches on `plan.kind === "noop"` instead of `changes.length === 0`, and
takes the plan directly. Fixes the stale OutdentPlan doc comment (said the
no-op was "null").
error-handler: distinguish parse-budget exhaustion from a genuine
structural no-op. resolveItemAtEof / renumberRun returned the same no-op
whether ensureSyntaxTree missed budget (retryable on a large doc) or the
caret simply was not in a list item. Add a QUOLL_PERF-gated warnBudgetMiss
at the two ensureSyntaxTree-null sites — dead-coded out of production and
silent in the unit suite, so the Tab/Shift-Tab result is unchanged; it only
surfaces the retryable case to a dev build.
Tests pin the discriminant: noop on a top-level outdent / first-item
indent, edit (with non-empty changes) on a real promotion.
- list-indent-keymap: the dispatch catch logged only `err`, so a planner regression (overlapping ChangeSpec = ChangeSet.of throw, or an out-of-range selection) gave no clue which transaction failed. Log userEvent, docLength, selection, and changeCount. - Add a disjointness regression test: a width-crossing forced child whose renumber + re-indent fold into ONE net per-line change dispatches cleanly, proving the catch is an unreachable path. - Fix three stale comments: contentColumnOf no longer claims a list-indent-keymap.ts dupe (that module is a thin shell); enclosingListItem narrows its "consolidates dupes" claim to the one surviving copy in list-geometry.ts and drops the dangling task-brief pointer; the module doc restates present tense (the transforms live in this file) and records the file-split as a deferred follow-up TODO.
The ternary built two near-identical edit variants only to omit an undefined selection; the optional selection field carries it either way and the dispatch shell drops an undefined selection before view.dispatch.
…er no longer stales at 0 The cycle-1 lower bound >= 1 over-rejected 0., which ORDERED_RE accepts and CommonMark permits as an ordered-list start. A vacated-run renumber whose correct target was 0 failed the whole run closed, leaving a stale/split-brain outer run. Lower the bound to >= 0 so only NEGATIVE markers (the original cycle-1 bug) fail closed. Update the two doc-comments that mis-stated the reject condition as < 1. Add a regression test pinning the 0.-follower path.
The weak toBeLessThanOrEqual(doc.length) assertion passed even with the caret parked before the synthesized marker (in range but wrong) — the exact tab-line regression condition where surviving chars != surviving columns. Pin the exact post-transform column with toBe(doc.line(3).to), matching the space-indent tests.
…parent The empty-item outdent + destination-renumber confluence was only covered with BULLET parents (renumber never fires). Add a test for an empty ordered child under an ordered parent with a following top-level sibling, so the destination renumber fires in the same transaction as the empty-path caret. Pins both the renumbered run (2. q -> 3. q) and the surviving caret coordinate.
The planner's speculative-no-op contract on a null EOF re-parse (renumberRun -> [], resolveItemAtEof -> null -> planner noop) was unobserved: every other test forceParses to EOF. A future rewrite of the fail-close to an early-return could run the planner over a null tree and corrupt/throw while the suite stayed green. Isolated file mocks only ensureSyntaxTree (return null), keeping every other @codemirror/language export real, and pins doc-unchanged + no-throw.
The cycle where orderedShape's lower bound became >= 0 (0. is a valid CommonMark ordered start) corrected the renumberRun JSDoc but left two sibling inline comments (renumberRun's fail-close return and planOutdentItem's adopted-shape note) asserting the old 1..9-digit range. Align them with the 0..9-digit bound the code now enforces.
mtskf
added a commit
that referenced
this pull request
Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Upgrades the list
Tab/Shift-Tabcommands from pure whitespace shifts to structural moves into a destination sibling run. A CommonMark list run is only valid when every item shares marker type + glyph + delimiter, so marker adoption here is a correctness requirement, not styling.User-visible changes:
Shift-Tab(outdent) adopts the destination run's marker + renumbers. Outdenting a bullet nested under a numbered list now joins that list as the next number (- ddd→3. ddd) and renumbers the following items. Previously it stayed a-at the wrong level.Tab(nest) is content-column aware and works across adjacent lists. Nesting a bullet under a numbered item now indents to the parent's content column (so it actually nests instead of breaking the list), and a document already broken by a too-shallow 2-space nest heals onTabinstead of dragging its tail along.EnterthenShift-Tabon an empty item makes it match the destination:- [ ] aaa→ a new- [ ], a numbered parent → the next number. Checkboxes are always synthesized unchecked; non-empty items are never silently converted, and an existing checkbox is never stripped from a non-empty item.1.and renumbers the vacated outer run to close the gap (Notion-style), instead of leaving a stale gap. One existing test's expectation was updated accordingly.9.→10.) now re-indents that item's nested children, which previously fell out of the list.How
New pure planner module
src/webview/cm/list/list-transform.ts(marker primitives, line classifier,renumberRun,planIndentItem/planOutdentItem) returnsChangeSpec[]; the keymapCommands are thin shells.list-tree.tsholds the grammar/destination-resolution adapters so node-shape knowledge stays in one place. Every transform is oneisolateHistorytransaction through the normal edit-sync → host write-lock pipeline — no raw write path, byte-identical round-trip, whole-transform undo. All whitespace shifts funnel through a single per-line net-delta map so ChangeSpecs never overlap; ordered output is capped at 9 digits (fail-closed).Testing
pnpm compile+pnpm build+pnpm lintgreen; fullpnpm test(unit + browser + e2e) green (e2e 82 passing). New unit tests cover the primary scenarios plus edge cases (delimiter/glyph preservation, task-ness adoption, checked→unchecked, ordered-task combos, forced-children re-homing, multi-digit width crossings, lazy-tail healing, caret placement). Manual GUI smoke (typing round-trip) still recommended before release.