ui: Tab nests a list item, ⇧Tab lifts it back out - #130
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015oc7gyoL7AccTEF2picayR
|
Warning Review limit reached
Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe editor now supports Markdown list nesting with Tab and lifting with Shift-Tab. A pure list-editing engine parses list structure, generates text and selection changes, preserves numbering and markers, and integrates with editor bindings, keymap validation, shortcuts, and tests. ChangesList indentation editing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BindingRegistry
participant runListShift
participant ListEngine
participant EditorState
User->>BindingRegistry: press Tab or Shift-Tab
BindingRegistry->>runListShift: invoke configured list command
runListShift->>ListEngine: indentList or outdentList with selection
ListEngine-->>runListShift: return ListEdit or inert result
runListShift->>EditorState: apply changes and update selection and scroll
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/src/list.ts`:
- Around line 252-256: Update Marker to retain the list marker identity, then in
scan assign kind from the bullet character or delimiter using bullet ?? delim.
Apply the same marker-based kind comparison in the downward walk alongside
ordered() so runs stop when ordered delimiters or bullet characters change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d40d07d9-ec90-43f9-a6a9-65e5bf216235
📒 Files selected for processing (7)
ui/src/bindings.tsui/src/editorkeys.test.tsui/src/editorkeys.tsui/src/list.test.tsui/src/list.tsui/src/main.tsui/src/shortcuts.ts
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015oc7gyoL7AccTEF2picayR
2 participants
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
- ba child of- ameant 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
indentListreturns null, the binding declines, and Tab goes on steppingthe focus ring — which is why this isn't CodeMirror's own
indentWithTab, aone-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.
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. aby two and the nesting simply doesn't parse. An item's subtreetravels 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 of1. 2. 3.is a list whose first itemsays "2.", and renders as "2.". The start number stays the author's where
they chose it (a list opening at
5.goes on opening at5.), and the lazy1. 1. 1.style is left alone — it renders identically and nothing movedinto or out of it.
runListShift, the CodeMirror half. A caret inside code declinesbefore the engine is asked: a
- itemline in a fence is text, notstructure, and
inCodeContextis the same read the rich paste makes.chords are rebindable and findable like every other (obligation 4).
editorkeys.ts gains
markdownKeymapin STOCK_KEYMAPS, which was a real gap:markdown()installs it at Prec.high, above B2's own chords, and nothingcompared 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
indentWithTabarriving indefaultKeymapas 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 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_015oc7gyoL7AccTEF2picayR
Summary by CodeRabbit
New Features
Documentation