Reverse lines: apply to whole document when selection is single line #257031
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as off-topic.
This comment was marked as off-topic.
hickford
force-pushed
the
line-operations
branch
from
bc4b4d9 to
474d180
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes the behavior of three line operation actions ('reverse lines', 'sort lines ascending/descending', and 'delete duplicate lines') to be more intuitive. Previously, when a selection started and ended on the same line, these actions did nothing. Now, they apply to the entire document when the selection is within a single line, making the actions more useful when users intend to operate on the whole document.
Key Changes
- Changed condition from
isEmpty()toisSingleLine()so actions apply to whole document when selection is on a single line - Added
isSingleLine()method to Range class to test if a range starts and ends on the same line - Updated tests to reflect and validate the new behavior
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/vs/editor/common/core/range.ts | Adds isSingleLine() method to Range class that checks if startLineNumber equals endLineNumber |
| src/vs/monaco.d.ts | Adds JSDoc documentation for the new isSingleLine() method in the public API |
| src/vs/editor/contrib/linesOperations/browser/linesOperations.ts | Updates AbstractSortLinesAction, DeleteDuplicateLinesAction, and ReverseLinesAction to use isSingleLine() instead of isEmpty() |
| src/vs/editor/contrib/linesOperations/test/browser/linesOperations.test.ts | Adds tests for the new behavior and updates an existing test description |
Comments suppressed due to low confidence (1)
src/vs/editor/contrib/linesOperations/test/browser/linesOperations.test.ts:187
- Missing test coverage for the new single-line selection behavior. The SortLinesDescendingAction suite should include a test similar to the one added for SortLinesAscendingAction (lines 110-128, though that test has a separate bug) and DeleteDuplicateLinesAction (lines 272-287) to verify that the action applies to the whole document when the selection is a single line.
suite('SortLinesDescendingAction', () => {
test('should sort selected lines in descending order', function () {
withTestCodeEditor(
[
'alpha',
'beta',
'omicron'
], {}, (editor) => {
const model = editor.getModel()!;
const sortLinesDescendingAction = new SortLinesDescendingAction();
editor.setSelection(new Selection(1, 1, 3, 7));
executeAction(sortLinesDescendingAction, editor);
assert.deepStrictEqual(model.getLinesContent(), [
'omicron',
'beta',
'alpha'
]);
assertSelection(editor, new Selection(1, 1, 3, 5));
});
});
test('should sort multiple selections in descending order', function () {
withTestCodeEditor(
[
'alpha',
'beta',
'omicron',
'',
'alpha',
'beta',
'omicron'
], {}, (editor) => {
const model = editor.getModel()!;
const sortLinesDescendingAction = new SortLinesDescendingAction();
editor.setSelections([new Selection(1, 1, 3, 7), new Selection(5, 1, 7, 7)]);
executeAction(sortLinesDescendingAction, editor);
assert.deepStrictEqual(model.getLinesContent(), [
'omicron',
'beta',
'alpha',
'',
'omicron',
'beta',
'alpha'
]);
const expectedSelections = [
new Selection(1, 1, 3, 5),
new Selection(5, 1, 7, 5)
];
editor.getSelections()!.forEach((actualSelection, index) => {
assert.deepStrictEqual(actualSelection.toString(), expectedSelections[index].toString());
});
});
});
});
Comment on lines
110
to
128
| test('applies to whole document when selection is single line', function () { | ||
| withTestCodeEditor( | ||
| [ | ||
| 'omicron', | ||
| 'beta', | ||
| 'alpha' | ||
| ], {}, (editor) => { | ||
| const model = editor.getModel()!; | ||
| const reverseLinesAction = new ReverseLinesAction(); | ||
|
|
||
| editor.setSelection(new Selection(2, 1, 2, 4)); | ||
| executeAction(reverseLinesAction, editor); | ||
| assert.deepStrictEqual(model.getLinesContent(), [ | ||
| 'alpha', | ||
| 'beta', | ||
| 'omicron' | ||
| ]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This test is using ReverseLinesAction but is placed in the SortLinesAscendingAction suite. It should either test SortLinesAscendingAction or be moved to the ReverseLinesAction suite. Since there's already an identical test in the ReverseLinesAction suite (line 769), this test should be updated to test SortLinesAscendingAction instead.
Likewise for actions 'Delete duplicate lines' and 'Sort lines'
hickford
force-pushed
the
line-operations
branch
from
474d180 to
d0e4f97
Compare
aiday-mar
approved these changes
Dec 26, 2025
Tyriar
approved these changes
Dec 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, actions 'reverse lines', 'sort lines' and 'delete duplicate lines' did nothing when the selection starts and ends on the same line. This could be confusing if the user was trying to sort the document and nothing happened.
Instead, apply the action to the whole document. This is more useful.