diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index a852424aa..296a0a3b3 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -2276,6 +2276,9 @@ export class Editor implements DiffsEditor { if (this.#selections !== undefined) { const edits: TextEdit[] = []; const nextSelections: EditorSelection[] = []; + // Line-based indentation is resolved per selection, so overlapping + // line coverage must share the same leading-whitespace edit. + const editedLines = new Set(); // Single-line indent inserts text at each caret. When several carets // share a line, indentation inserted by carets to their left shifts // them right, so record each one here and offset its resulting @@ -2299,7 +2302,13 @@ export class Editor implements DiffsEditor { this.#metrics.tabSize, outdent ); - edits.push(...ret[0]); + for (const edit of ret[0]) { + const line = edit.range.start.line; + if (!editedLines.has(line)) { + editedLines.add(line); + edits.push(edit); + } + } nextSelections.push(ret[1]); } else { const lineChar0 = textDocument.charAt({ diff --git a/packages/diffs/src/editor/selection.ts b/packages/diffs/src/editor/selection.ts index d57b9845b..f676370e9 100644 --- a/packages/diffs/src/editor/selection.ts +++ b/packages/diffs/src/editor/selection.ts @@ -89,6 +89,7 @@ export function resolveIndentEdits( const { start, end } = selection; const edits: TextEdit[] = []; let newSelection: EditorSelection = { ...selection }; + const blockIndent = start.line !== end.line; let endLine = end.line; if (start.line < end.line && end.character === 0) { endLine--; @@ -98,6 +99,9 @@ export function resolveIndentEdits( if (lineText === undefined) { continue; } + if (blockIndent && lineText.trim().length === 0) { + continue; + } const indentUnit = lineText.startsWith('\t') ? '\t' : ' '.repeat(tabSize); let deleteLength = 0; let newText = indentUnit; diff --git a/packages/diffs/test/README.md b/packages/diffs/test/README.md index cc2b9e12f..4509dc530 100644 --- a/packages/diffs/test/README.md +++ b/packages/diffs/test/README.md @@ -125,14 +125,6 @@ order. If you touch one of these areas, consider adding the missing coverage: - **Batch edit ordering sensitivity** (1) — accepting a batch containing a delete and an insert at the same offset depends on the caller's array order: delete-first throws an overlap error, insert-first succeeds. -- **Line indent/outdent multi-selection dedupe** (3) — indent dispatch - concatenates per-selection edits with no shared-line dedupe, so a line under - two carets/ranges indents twice; the outdent variant emits two identical - deletes that fail overlap validation and the whole command throws. -- **Block indent dirties blank lines** (1) — a multi-line block indent inserts - the indent unit on empty and whitespace-only lines inside the selection, - injecting trailing whitespace on lines the user never touched (outdent - round-trips it away, bounding the damage). - **History equivalence across non-history edits** (7) — edits applied with `updateHistory=false` are an implementation detail of how an edit reaches `applyEdits`, not a separate semantic class: a mixed programmatic/local diff --git a/packages/diffs/test/editorApplyEdits.test.ts b/packages/diffs/test/editorApplyEdits.test.ts index e9aeae0f5..b4be37e3d 100644 --- a/packages/diffs/test/editorApplyEdits.test.ts +++ b/packages/diffs/test/editorApplyEdits.test.ts @@ -1660,32 +1660,6 @@ function makeRandom(seed: number): () => number { }; } -// Mirrors the Editor's line-based indent dispatch (src/editor/editor.ts, case -// 'indentMore'/'indentLess' at ~line 1891): resolveIndentEdits runs once per -// selection and the resulting edits are concatenated into a single applyEdits -// batch, with no dedupe when two selections touch the same line. -function dispatchLineIndent( - d: TextDocument, - selections: EditorSelection[], - tabSize: number, - outdent: boolean -): EditorSelection[] { - const edits: TextEdit[] = []; - const nextSelections: EditorSelection[] = []; - for (const selection of selections) { - const [selectionEdits, nextSelection] = resolveIndentEdits( - d, - selection, - tabSize, - outdent - ); - edits.push(...selectionEdits); - nextSelections.push(nextSelection); - } - d.applyEdits(edits, true, selections); - return nextSelections; -} - // Mirrors Editor#moveSelectedLines (src/editor/editor.ts ~line 2267): // getSelectedLineBlocks merges the selections into line blocks, each block is // rotated with its neighbor line in one edit, and every selection is remapped @@ -2205,43 +2179,54 @@ describe('applyEdits: out-of-bounds edit ranges clamp instead of throwing', () = }); describe('line-based indent commands with selections sharing a line', () => { - // KNOWN BUG: the editor's indent dispatch concatenates each selection's - // resolveIndentEdits output with no shared-line dedupe, so two carets on one - // line emit two identical zero-length inserts at column 0. Both pass the - // overlap validation (equal start offsets never compare as overlapping) and - // the line is indented twice. - test.failing('two carets on one line indent that line exactly once', () => { - const d = doc('quartz vein'); - const next = dispatchLineIndent(d, [caret(0, 2), caret(0, 6)], 2, false); - expect(d.getText()).toBe(' quartz vein'); - expect(next).toEqual([caret(0, 4), caret(0, 8)]); - }); - - // KNOWN BUG: same missing dedupe with range selections — the line shared by - // both ranges receives one indent edit per selection and ends up indented - // twice while its neighbors indent once. - test.failing( - 'two ranges sharing a line indent every line exactly once', - () => { - const d = doc('ada\nberyl\ncobalt'); - dispatchLineIndent(d, [range(0, 1, 1, 2), range(1, 3, 2, 1)], 2, false); - expect(d.getText()).toBe(' ada\n beryl\n cobalt'); + test('two carets on one line indent that line exactly once', async () => { + const { cleanup, content, editor, window } = + await createEditorFixture('quartz vein'); + + try { + editor.setState({ selections: [caret(0, 2), caret(0, 6)] }); + + pressKey(window, content, { key: ']', metaKey: true }); + + expect(editor.getText()).toBe(' quartz vein'); + expect(editor.getState().selections).toEqual([caret(0, 4), caret(0, 8)]); + } finally { + cleanup(); } - ); + }); - // KNOWN BUG: the outdent variant of the same composition produces two - // identical single-character delete edits for the shared line; unlike the - // zero-length inserts these DO fail overlap validation, so the whole command - // throws 'Overlapping text edits are not supported' and nothing is applied. - test.failing( - 'two carets on one tab-indented line outdent it exactly once', - () => { - const d = doc('\tquartz vein'); - const next = dispatchLineIndent(d, [caret(0, 3), caret(0, 7)], 2, true); - expect(d.getText()).toBe('quartz vein'); - expect(next).toEqual([caret(0, 2), caret(0, 6)]); + test('two ranges sharing a line indent every line exactly once', async () => { + const { cleanup, content, editor, window } = + await createEditorFixture('ada\nberyl\ncobalt'); + + try { + editor.setState({ + selections: [range(0, 1, 1, 2), range(1, 3, 2, 1)], + }); + + pressKey(window, content, { key: ']', metaKey: true }); + + expect(editor.getText()).toBe(' ada\n beryl\n cobalt'); + } finally { + cleanup(); } - ); + }); + + test('two carets on one tab-indented line outdent it exactly once', async () => { + const { cleanup, content, editor, window } = + await createEditorFixture('\tquartz vein'); + + try { + editor.setState({ selections: [caret(0, 3), caret(0, 7)] }); + + pressKey(window, content, { key: 'Tab', shiftKey: true }); + + expect(editor.getText()).toBe('quartz vein'); + expect(editor.getState().selections).toEqual([caret(0, 2), caret(0, 6)]); + } finally { + cleanup(); + } + }); }); describe('indentLess on tab and mixed indentation', () => { @@ -2302,30 +2287,29 @@ describe('indentLess on tab and mixed indentation', () => { }); describe('block indent over blank and whitespace-only lines', () => { - // KNOWN BUG: resolveIndentEdits emits an insert at column 0 of every line - // the selection touches, including empty and whitespace-only lines, so a - // block indent injects trailing whitespace on lines the user never typed - // on. The conventional behavior is to skip lines with no content. Outdent - // strips the injected unit back off, so a full indent/outdent round trip - // self-heals — the damage is bounded to the indented state. - test.failing( - 'block indent leaves blank and whitespace-only lines untouched', - () => { - const d = doc('alpha\n\n \nbeta'); - const [edits] = resolveIndentEdits( - d, - { - start: { line: 0, character: 0 }, - end: { line: 3, character: 4 }, - direction: DirectionForward, - }, - 2, - false - ); - d.applyEdits(edits); - expect(d.getText()).toBe(' alpha\n\n \n beta'); - } - ); + test('block indent and outdent leave blank lines untouched', () => { + const original = 'alpha\n\n \nbeta'; + const d = doc(original); + const [indentEdits, indentedSelection] = resolveIndentEdits( + d, + range(0, 0, 3, 4), + 2, + false + ); + d.applyEdits(indentEdits); + expect(d.getText()).toBe(' alpha\n\n \n beta'); + + const [outdentEdits] = resolveIndentEdits(d, indentedSelection, 2, true); + d.applyEdits(outdentEdits); + expect(d.getText()).toBe(original); + }); + + test('single-line indent still inserts whitespace on a blank line', () => { + const d = doc(''); + const [edits] = resolveIndentEdits(d, caret(0, 0), 2, false); + d.applyEdits(edits); + expect(d.getText()).toBe(' '); + }); }); describe('move line commands with merged and same-line multi-cursor blocks', () => {