Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/diffs/src/editor/editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2276,6 +2276,9 @@ export class Editor<LAnnotation> implements DiffsEditor<LAnnotation> {
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<number>();
// 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
Expand All @@ -2299,7 +2302,13 @@ export class Editor<LAnnotation> implements DiffsEditor<LAnnotation> {
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({
Expand Down
4 changes: 4 additions & 0 deletions packages/diffs/src/editor/selection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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--;
Expand All @@ -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;
Expand Down
8 changes: 0 additions & 8 deletions packages/diffs/test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
152 changes: 68 additions & 84 deletions packages/diffs/test/editorApplyEdits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown>,
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
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down