From 20e65fd2ff3753918d34968d260b5ed1a071980d Mon Sep 17 00:00:00 2001 From: BartolHrg <78815047+BartolHrg@users.noreply.github.com> Date: Tue, 15 Jul 2025 18:59:01 +0200 Subject: [PATCH 1/3] fix copy with multiple cursors and empty selections --- src/vs/editor/common/viewModel/viewModelImpl.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 32bfed905ef..5f20b579cbd 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -963,17 +963,6 @@ export class ViewModel extends Disposable implements IViewModel { if (!emptySelectionClipboard) { return ''; } - - const modelLineNumbers = modelRanges.map((r) => r.startLineNumber); - - let result = ''; - for (let i = 0; i < modelLineNumbers.length; i++) { - if (i > 0 && modelLineNumbers[i - 1] === modelLineNumbers[i]) { - continue; - } - result += this.model.getLineContent(modelLineNumbers[i]) + newLineCharacter; - } - return result; } if (hasEmptyRange && emptySelectionClipboard) { @@ -984,7 +973,7 @@ export class ViewModel extends Disposable implements IViewModel { const modelLineNumber = modelRange.startLineNumber; if (modelRange.isEmpty()) { if (modelLineNumber !== prevModelLineNumber) { - result.push(this.model.getLineContent(modelLineNumber)); + result.push(this.model.getLineContent(modelLineNumber) + newLineCharacter); } } else { result.push(this.model.getValueInRange(modelRange, forceCRLF ? EndOfLinePreference.CRLF : EndOfLinePreference.TextDefined)); From defdcd4e5dc2f22df1666a183bad6e70a45ca369 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Mon, 15 Dec 2025 22:43:11 +0100 Subject: [PATCH 2/3] Add unit tests and also allow for this scenario in `_distributePasteToCursors` --- .../common/cursor/cursorTypeEditOperations.ts | 6 +- .../editor/common/viewModel/viewModelImpl.ts | 2 +- .../test/browser/controller/cursor.test.ts | 83 +++++++++++++++++++ .../browser/viewModel/viewModelImpl.test.ts | 17 ++++ 4 files changed, 104 insertions(+), 4 deletions(-) diff --git a/src/vs/editor/common/cursor/cursorTypeEditOperations.ts b/src/vs/editor/common/cursor/cursorTypeEditOperations.ts index e250bb74431..d1290a3e651 100644 --- a/src/vs/editor/common/cursor/cursorTypeEditOperations.ts +++ b/src/vs/editor/common/cursor/cursorTypeEditOperations.ts @@ -664,15 +664,15 @@ export class PasteOperation { } private static _distributePasteToCursors(config: CursorConfiguration, selections: Selection[], text: string, pasteOnNewLine: boolean, multicursorText: string[]): string[] | null { - if (pasteOnNewLine) { - return null; - } if (selections.length === 1) { return null; } if (multicursorText && multicursorText.length === selections.length) { return multicursorText; } + if (pasteOnNewLine) { + return null; + } if (config.multiCursorPaste === 'spread') { // Try to spread the pasted text in case the line count matches the cursor count // Remove trailing \n if present diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 586155bd446..429e5d4a11e 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -967,7 +967,7 @@ export class ViewModel extends Disposable implements IViewModel { } if (hasEmptyRange && emptySelectionClipboard) { - // mixed empty selections and non-empty selections + // some (maybe all) empty selections const result: string[] = []; let prevModelLineNumber = 0; for (const modelRange of modelRanges) { diff --git a/src/vs/editor/test/browser/controller/cursor.test.ts b/src/vs/editor/test/browser/controller/cursor.test.ts index 1322aae8fba..ce1a2b5e4af 100644 --- a/src/vs/editor/test/browser/controller/cursor.test.ts +++ b/src/vs/editor/test/browser/controller/cursor.test.ts @@ -2239,6 +2239,89 @@ suite('Editor Controller', () => { }); }); + test('issue #256039: paste from multiple cursors with empty selections and multiCursorPaste full', () => { + // Bug scenario: User copies 2 lines from 2 cursors (with empty selections) + // and pastes to 2 cursors with multiCursorPaste: "full". + // + // Expected: Each cursor receives its respective line. + // Bug (without fix): Both cursors receive all lines because multicursorText + // was not being passed as an array when all selections were empty. + // + // This test verifies the correct behavior when multicursorText is properly + // provided as an array (which is what the fix ensures happens). + usingCursor({ + text: [ + 'line1', + 'line2', + 'line3' + ], + editorOpts: { + multiCursorPaste: 'full' + } + }, (editor, model, viewModel) => { + // 2 cursors on lines 1 and 2 + viewModel.setSelections('test', [new Selection(1, 1, 1, 1), new Selection(2, 1, 2, 1)]); + + // Paste with multicursorText properly set (the fix ensures this) + viewModel.paste( + 'line1\nline2\n', + true, + ['line1\n', 'line2\n'] // This array enables proper distribution + ); + + // Each cursor gets its respective line + assert.strictEqual(model.getValue(), [ + 'line1', + 'line1', + 'line2', + 'line2', + 'line3' + ].join('\n')); + }); + }); + + test('issue #256083: bug reproduction - paste without multicursorText array with multiCursorPaste full', () => { + // This test demonstrates the BUG behavior (before the fix): + // When copying from multiple cursors with empty selections, pasteOnNewLine is true. + // The bug is that _distributePasteToCursors returns null when pasteOnNewLine=true, + // ignoring the multicursorText array entirely. This causes _simplePaste to be used, + // which pastes the FULL text at the beginning of EACH cursor's line. + usingCursor({ + text: [ + 'line1', + 'line2', + 'line3' + ], + editorOpts: { + multiCursorPaste: 'full' + } + }, (editor, model, viewModel) => { + // 2 cursors on lines 1 and 2 + viewModel.setSelections('test', [new Selection(1, 1, 1, 1), new Selection(2, 1, 2, 1)]); + + // Paste with pasteOnNewLine=true (copied from empty selections) + // Even with multicursorText provided, it's ignored due to the bug + viewModel.paste( + 'line1\nline2\n', + true, // pasteOnNewLine - this triggers the early return in _distributePasteToCursors + ['line1\n', 'line2\n'] // This is ignored because pasteOnNewLine=true! + ); + + // BUG BEHAVIOR: _simplePaste is used, which pastes full text at start of each line + // Cursor 1 (line 1): inserts "line1\nline2\n" at position (1,1) + // Cursor 2 (line 2): inserts "line1\nline2\n" at position (2,1) - but line numbers shift! + assert.strictEqual(model.getValue(), [ + 'line1', + 'line2', + 'line1', // original line1 pushed down + 'line1', + 'line2', + 'line2', // original line2 pushed down + 'line3' + ].join('\n')); + }); + }); + test('issue #3071: Investigate why undo stack gets corrupted', () => { const model = createTextModel( [ diff --git a/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts b/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts index 8afa07a8b56..6eae80f817b 100644 --- a/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts +++ b/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts @@ -202,6 +202,23 @@ suite('ViewModel', () => { ); }); + test('issue #256039: getPlainTextToCopy with multiple cursors and empty selections should return array', () => { + // Bug: When copying with multiple cursors (empty selections) with emptySelectionClipboard enabled, + // the result should be an array so that pasting with "editor.multiCursorPaste": "full" + // correctly distributes each line to the corresponding cursor. + // Without the fix, this returns 'line2\nline3\n' (a single string). + // With the fix, this returns ['line2\n', 'line3\n'] (an array). + assertGetPlainTextToCopy( + USUAL_TEXT, + [ + new Range(2, 1, 2, 1), + new Range(3, 1, 3, 1), + ], + true, + ['line2\n', 'line3\n'] + ); + }); + test('getPlainTextToCopy 1/2', () => { assertGetPlainTextToCopy( USUAL_TEXT, From 1bb90a3b40b121d94ddcade763ec936a59e63e89 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Wed, 17 Dec 2025 23:01:52 +0100 Subject: [PATCH 3/3] Update unit tests --- .../editor/common/viewModel/viewModelImpl.ts | 6 +-- .../test/browser/controller/cursor.test.ts | 54 +------------------ .../browser/viewModel/viewModelImpl.test.ts | 12 +++-- 3 files changed, 12 insertions(+), 60 deletions(-) diff --git a/src/vs/editor/common/viewModel/viewModelImpl.ts b/src/vs/editor/common/viewModel/viewModelImpl.ts index 429e5d4a11e..284bbc487a4 100644 --- a/src/vs/editor/common/viewModel/viewModelImpl.ts +++ b/src/vs/editor/common/viewModel/viewModelImpl.ts @@ -959,11 +959,9 @@ export class ViewModel extends Disposable implements IViewModel { } } - if (!hasNonEmptyRange) { + if (!hasNonEmptyRange && !emptySelectionClipboard) { // all ranges are empty - if (!emptySelectionClipboard) { - return ''; - } + return ''; } if (hasEmptyRange && emptySelectionClipboard) { diff --git a/src/vs/editor/test/browser/controller/cursor.test.ts b/src/vs/editor/test/browser/controller/cursor.test.ts index 1609b98a9ea..61ddc81e06d 100644 --- a/src/vs/editor/test/browser/controller/cursor.test.ts +++ b/src/vs/editor/test/browser/controller/cursor.test.ts @@ -2240,15 +2240,6 @@ suite('Editor Controller', () => { }); test('issue #256039: paste from multiple cursors with empty selections and multiCursorPaste full', () => { - // Bug scenario: User copies 2 lines from 2 cursors (with empty selections) - // and pastes to 2 cursors with multiCursorPaste: "full". - // - // Expected: Each cursor receives its respective line. - // Bug (without fix): Both cursors receive all lines because multicursorText - // was not being passed as an array when all selections were empty. - // - // This test verifies the correct behavior when multicursorText is properly - // provided as an array (which is what the fix ensures happens). usingCursor({ text: [ 'line1', @@ -2262,11 +2253,10 @@ suite('Editor Controller', () => { // 2 cursors on lines 1 and 2 viewModel.setSelections('test', [new Selection(1, 1, 1, 1), new Selection(2, 1, 2, 1)]); - // Paste with multicursorText properly set (the fix ensures this) viewModel.paste( 'line1\nline2\n', true, - ['line1\n', 'line2\n'] // This array enables proper distribution + ['line1\n', 'line2\n'] ); // Each cursor gets its respective line @@ -2280,48 +2270,6 @@ suite('Editor Controller', () => { }); }); - test('issue #256083: bug reproduction - paste without multicursorText array with multiCursorPaste full', () => { - // This test demonstrates the BUG behavior (before the fix): - // When copying from multiple cursors with empty selections, pasteOnNewLine is true. - // The bug is that _distributePasteToCursors returns null when pasteOnNewLine=true, - // ignoring the multicursorText array entirely. This causes _simplePaste to be used, - // which pastes the FULL text at the beginning of EACH cursor's line. - usingCursor({ - text: [ - 'line1', - 'line2', - 'line3' - ], - editorOpts: { - multiCursorPaste: 'full' - } - }, (editor, model, viewModel) => { - // 2 cursors on lines 1 and 2 - viewModel.setSelections('test', [new Selection(1, 1, 1, 1), new Selection(2, 1, 2, 1)]); - - // Paste with pasteOnNewLine=true (copied from empty selections) - // Even with multicursorText provided, it's ignored due to the bug - viewModel.paste( - 'line1\nline2\n', - true, // pasteOnNewLine - this triggers the early return in _distributePasteToCursors - ['line1\n', 'line2\n'] // This is ignored because pasteOnNewLine=true! - ); - - // BUG BEHAVIOR: _simplePaste is used, which pastes full text at start of each line - // Cursor 1 (line 1): inserts "line1\nline2\n" at position (1,1) - // Cursor 2 (line 2): inserts "line1\nline2\n" at position (2,1) - but line numbers shift! - assert.strictEqual(model.getValue(), [ - 'line1', - 'line2', - 'line1', // original line1 pushed down - 'line1', - 'line2', - 'line2', // original line2 pushed down - 'line3' - ].join('\n')); - }); - }); - test('issue #3071: Investigate why undo stack gets corrupted', () => { const model = createTextModel( [ diff --git a/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts b/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts index 6eae80f817b..c5ae0e5ac9c 100644 --- a/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts +++ b/src/vs/editor/test/browser/viewModel/viewModelImpl.test.ts @@ -198,7 +198,10 @@ suite('ViewModel', () => { new Range(3, 2, 3, 2), ], true, - 'line2\nline3\n' + [ + 'line2\n', + 'line3\n' + ] ); }); @@ -239,7 +242,7 @@ suite('ViewModel', () => { new Range(3, 2, 3, 2), ], true, - ['ine2', 'line3'] + ['ine2', 'line3\n'] ); }); @@ -276,7 +279,10 @@ suite('ViewModel', () => { new Range(3, 2, 3, 2), ], true, - 'line2\nline3\n' + [ + 'line2\n', + 'line3\n' + ] ); });