From 9a297dcb5f5ced6c19c55e91fe10bf0f08fcca37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ernesto=20Garc=C3=ADa?= Date: Mon, 26 Oct 2026 11:08:29 -0300 Subject: [PATCH] Keep spaces after the caret out of a new list item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enter in a list item moved the spaces after the caret into the new item: "1. one‸ two" gave "2. ‸ two". The old editor trimmed them. The move to CodeMirror lost that, and no test noticed. CodeMirror's insertNewlineContinueMarkup drops the spaces before the caret, but not those after it. Enter now deletes those first, then lets CodeMirror continue the list or quote. Layers map through the two steps one at a time. So a ghost that began with a space moves into the new item, not over the line break. - An empty item with spaces after the caret now ends the list, as in the old editor. - A caret right after a marker, as in "1.‸ a", keeps the space that ends the marker. - keys.test.ts has every old list test, plus cases for quotes, tabs, a ghost at the caret, undo, and a selection. - The fix-bug skill no longer calls this bug open. Two old renumbering cases stay out of the tests for now. Today's editor follows CommonMark there and renumbers further than the old one did. That choice is open. --- .claude/skills/fix-bug/SKILL.md | 3 +- src/editor/keys.test.ts | 84 ++++++++++++++++++++++++++++++++- src/editor/keys.ts | 28 ++++++++++- 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/.claude/skills/fix-bug/SKILL.md b/.claude/skills/fix-bug/SKILL.md index 8b2c233..1ce602d 100644 --- a/.claude/skills/fix-bug/SKILL.md +++ b/.claude/skills/fix-bug/SKILL.md @@ -10,8 +10,7 @@ description: Fixes a bug so it stays fixed, with a regression test that proves t 2. **Reproduce it at the lowest layer that shows it.** A test on `EditorState` in `src/editor/*.test.ts` beats one through React. A test in the Rust file's `mod tests` beats the app. Run it: it must fail, for the reason in the report. Done when you have that failing test. - - Example, for "Enter in a list item leaves a stray space", in `src/editor/keys.test.ts`. This - bug is real and still open: + - Example, for "Enter in a list item leaves a stray space", in `src/editor/keys.test.ts`: `expect(run(stateFor("1. one‸ two"), enter).text).toBe("1. one\n2. ‸two")` - Some bugs show only in WebKit: caret, IME, dead keys. Reproduce those in `pnpm tauri dev` and write the steps down. Still add a test for any part the editor state can show. diff --git a/src/editor/keys.test.ts b/src/editor/keys.test.ts index 3d8979a..e165100 100644 --- a/src/editor/keys.test.ts +++ b/src/editor/keys.test.ts @@ -1,7 +1,8 @@ +import { redo, undo } from "@codemirror/commands"; import { EditorSelection, EditorState, StateCommand } from "@codemirror/state"; import { EditorView } from "@codemirror/view"; import { describe, expect, it } from "vitest"; -import { parsePage } from "./format"; +import { parsePage, serializePage } from "./format"; import { backspace, enter, setHeading, toggleBold, toggleItalic, toggleLink, toggleList } from "./keys"; import { layersOf } from "./layers"; import { createEditorState } from "./setup"; @@ -58,6 +59,87 @@ describe("Enter", () => { }); }); +describe("Enter in a list", () => { + it("starts the next numbered item on the next line", () => { + expect(run(stateFor("1. First list item‸"), enter).text).toBe("1. First list item\n2. ‸"); + }); + + it.each([ + ["- a‸", "- a\n- ‸"], + [" * a‸", " * a\n * ‸"], + ["9) a‸", "9) a\n10) ‸"], + ])("continues %j", (before, after) => { + expect(run(stateFor(before), enter).text).toBe(after); + }); + + it.each([ + ["1. one‸ two", "1. one\n2. ‸two"], + ["- one ‸ two", "- one\n- ‸two"], + ["1. a‸ ", "1. a\n2. ‸"], + ["> a‸ b", "> a\n> ‸b"], + ["- one‸\t two", "- one\n- ‸two"], + ])("splits %j without stray spaces", (before, after) => { + expect(run(stateFor(before), enter).text).toBe(after); + }); + + it.each([ + ["1.‸ a", "1.\n2. ‸ a"], + ["- [ ]‸ a", "- [ ]\n- [ ] ‸ a"], + [">‸ a", ">\n> ‸ a"], + ])("keeps the space that ends the marker in %j", (before, after) => { + expect(run(stateFor(before), enter).text).toBe(after); + }); + + it("undoes a split in one step, apart from the typing before it", () => { + const typed = stateFor("1. on‸ two").update({ + changes: { from: 5, insert: "e" }, + selection: EditorSelection.cursor(6), + userEvent: "input.type", + }).state; + const split = run(typed, enter); + expect(split.text).toBe("1. one\n2. ‸two"); + expect(run(split.state, undo).text).toBe("1. one‸ two"); + }); + + it("moves a ghost that starts at the caret into the new item", () => { + const page = parsePage("1. Cut this{- , frankly,-} now.\n"); + const state = createEditorState(page.text, page).update({ selection: EditorSelection.cursor(11) }).state; + const save = (s: EditorState) => serializePage({ text: s.doc.toString(), ...layersOf(s), overflow: "" }); + const split = run(state, enter).state; + expect(save(split)).toBe("1. Cut this\n2. {-, frankly,-} now.\n"); + const undone = run(split, undo).state; + expect(save(undone)).toBe("1. Cut this{- , frankly,-} now.\n"); + expect(save(run(undone, redo).state)).toBe("1. Cut this\n2. {-, frankly,-} now.\n"); + }); + + it("leaves a selection to the default Enter", () => { + expect(enter({ state: stateFor("- one «two» three"), dispatch: () => {} })).toBe(false); + }); + + it("renumbers the items below", () => { + expect(run(stateFor("1. a‸\n2. b\n3. c"), enter).text).toBe("1. a\n2. ‸\n3. b\n4. c"); + expect(run(stateFor("8. a‸\n9. b"), enter).text).toBe("8. a\n9. ‸\n10. b"); + }); + + it("renumbers past nested lines and stops at the end of the list", () => { + expect(run(stateFor("1. a‸\n - x\n2. b"), enter).text).toBe("1. a\n2. ‸\n - x\n3. b"); + expect(run(stateFor("1. a‸\n2) b"), enter).text).toBe("1. a\n2. ‸\n2) b"); + }); + + it("drops an empty item and leaves a paragraph to type in", () => { + expect(run(stateFor("1. a\n2. ‸"), enter).text).toBe("1. a\n\n‸"); + expect(run(stateFor("1. a\n2. ‸ "), enter).text).toBe("1. a\n\n‸"); + expect(run(stateFor("- a\n- ‸ \n- b"), enter).text).toBe("- a\n\n‸\n\n- b"); + }); + + it("leaves plain text and the marker itself alone", () => { + expect(run(stateFor("Just‸ text"), enter).text).toBe("Just\n\n‸text"); + expect(run(stateFor("1.5‸ million"), enter).text).toBe("1.5\n\n‸million"); + expect(run(stateFor("1‸. a"), enter).text).toBe("1‸. a"); + expect(enter({ state: stateFor("1‸. a"), dispatch: () => {} })).toBe(false); + }); +}); + describe("Backspace", () => { it("joins a paragraph to the one above", () => { expect(runView(stateFor("One.\n\n‸Two."), backspace)).toBe("One.‸Two."); diff --git a/src/editor/keys.ts b/src/editor/keys.ts index edb913d..958323b 100644 --- a/src/editor/keys.ts +++ b/src/editor/keys.ts @@ -22,6 +22,8 @@ function nodeAt(state: EditorState, pos: number, names: string[]): SyntaxNode | const continueMarkup = insertNewlineContinueMarkupCommand({ nonTightLists: false }); const EMPTY_ITEM = /^\s*(?:>\s*)*(?:[-*+]|\d+[.)])\s*(?:\[[ xX]\]\s*)?$/; +/** Quote and list markup up to, not including, the space after it. */ +const BARE_MARKER = /^[\s>]*(?:(?:[-*+]|\d+[.)])(?:\s+\[[ xX]\])?)?$/; /** Enter: continue a list or quote, a plain line in code, else a new paragraph. */ export const enter: StateCommand = (target) => { @@ -31,10 +33,15 @@ export const enter: StateCommand = (target) => { const range = state.selection.main; const line = state.doc.lineAt(range.from); if (nodeAt(state, head, ["ListItem", "Blockquote"])) { + // CodeMirror drops the spaces before the caret, but not those after it. A + // caret right after a marker, as in `1.‸ a`, keeps the space that ends it. + let to = head; + const bare = BARE_MARKER.test(state.doc.sliceString(line.from, head)); + while (!bare && to < line.to && /[ \t]/.test(state.doc.sliceString(to, to + 1))) to++; // An empty top-level item ends the list: drop it, and leave a paragraph to // type in, with blank lines around it. Items below become a list of their // own. A nested one goes to CodeMirror, which outdents it. - if (range.empty && EMPTY_ITEM.test(line.text) && !/^\s/.test(line.text) && head === line.to) { + if (range.empty && EMPTY_ITEM.test(line.text) && !/^\s/.test(line.text) && to === line.to) { const hasAfter = line.number < state.doc.lines && state.doc.line(line.number + 1).text.trim() !== ""; dispatch( state.update({ @@ -46,7 +53,24 @@ export const enter: StateCommand = (target) => { ); return true; } - return continueMarkup(target); + // Delete the spaces after the caret first, so the new item doesn't begin with one. + if (to === head) return continueMarkup(target); + const spaces = state.changes({ from: head, to }); + const trimmed = state.update({ changes: spaces }).state; + return continueMarkup({ + state: trimmed, + dispatch: (tr) => + dispatch( + state.update({ + changes: spaces.compose(tr.changes), + selection: tr.selection, + // Mapped in two steps, so a ghost that began with those spaces starts in the new item. + effects: setLayers.of(mapLayers(layersOf(trimmed), tr.changes)), + scrollIntoView: true, + userEvent: "input", + }), + ), + }); } if (!line.text.trim()) return insertNewline(target);