Skip to content

[VSCodeSweeper MVP — test only] Copying multiple empty selections adds blank lines when pasted into a single cursor#6

Draft
egamma wants to merge 2 commits into
mainfrom
vscodesweeper/fix-324290
Draft

[VSCodeSweeper MVP — test only] Copying multiple empty selections adds blank lines when pasted into a single cursor#6
egamma wants to merge 2 commits into
mainfrom
vscodesweeper/fix-324290

Conversation

@egamma

@egamma egamma commented Jul 9, 2026

Copy link
Copy Markdown
Owner

⚠️ VSCodeSweeper MVP — test only. This draft PR was generated by an AI experiment
(vscodesweeper) inside a personal fork. It is
not intended for microsoft/vscode and is not a contribution proposal.

Upstream issue: microsoft#324290

Change summary

Copying multiple empty selections and pasting into a single cursor produces spurious blank lines between entries (regression since 1.108.0).

Validation

Write a unit test in src/vs/editor/test/common/viewModel/viewModelImpl.test.ts (or the existing clipboard test suite) that creates a model with 3 lines, places empty selections on each, calls getPlainTextToCopy, joins the result array with the EOL character, and asserts the output equals 'line1\nline2\nline3\n' with no double newlines.


Branch vscodesweeper/fix-324290 · generated fix · review loop runs separately (codex review).

@egamma egamma added the vscodesweeper-mvp VSCodeSweeper MVP test PR (fork-only) label Jul 9, 2026
@egamma

egamma commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 codex review — round 1

VSCodeSweeper MVP codex cross-review of a copilot-generated fix (fork-only experiment).

The patch fixes the targeted all-empty-selection case but broadens the behavior to all multi-selection copies whose fragments end in EOL, which can change text/plain clipboard contents for normal selections.

Review comment:

  • [P2] Preserve delimiters for non-line-copy selections — src/vs/editor/browser/controller/editContext/clipboardUtils.ts:52-52
    When every copied fragment happens to end with the model EOL, this now removes the separator for all multi-selection copies, not just the empty-selection full-line case. For example, if a user selects two non-contiguous snippets that both intentionally include their trailing newline, the text/plain clipboard payload changes from joining the selections with an extra delimiter to simple concatenation, so a single-cursor paste or paste into another app loses the selection boundary. The newline elision should be limited to fragments where the trailing newline was added for empty-selection line copying, or handled per fragment rather than by every(...).

@egamma

egamma commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

🔧 Round 1: pushed commits addressing the review above; re-reviewing.

@egamma

egamma commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

🤖 codex review — round 2

VSCodeSweeper MVP codex cross-review of a copilot-generated fix (fork-only experiment).

The change narrowly adjusts clipboard text joining for multiple empty selections whose copied fragments already include line endings, preventing extra blank lines without affecting mixed or non-empty selections. I did not find any introduced correctness issues in the diff.

@egamma

egamma commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

✅ Round 2: no actionable findings — the review loop converged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vscodesweeper-mvp VSCodeSweeper MVP test PR (fork-only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant