From a8de6af641a9d15bfc9a2e80e8845bbc10a3d9ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Marcos?= Date: Tue, 18 Feb 2025 00:23:48 -0300 Subject: [PATCH] Fix `editor::SplitSelectionIntoLines` adding an extra line at the end (#25053) Closes #4795 Release Notes: - Fixed `editor::SplitSelectionIntoLines` adding an extra line at end of selection --- crates/editor/src/editor.rs | 21 +++++-- crates/editor/src/editor_tests.rs | 100 ++++++++++++++++++++++++------ crates/rope/src/rope.rs | 5 +- 3 files changed, 100 insertions(+), 26 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c927b16948..e17236a71d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -9122,21 +9122,32 @@ impl Editor { window: &mut Window, cx: &mut Context, ) { - let mut to_unfold = Vec::new(); + let selections = self + .selections + .all::(cx) + .into_iter() + .map(|selection| selection.start..selection.end) + .collect::>(); + self.unfold_ranges(&selections, true, true, cx); + let mut new_selection_ranges = Vec::new(); { - let selections = self.selections.all::(cx); let buffer = self.buffer.read(cx).read(cx); for selection in selections { for row in selection.start.row..selection.end.row { let cursor = Point::new(row, buffer.line_len(MultiBufferRow(row))); new_selection_ranges.push(cursor..cursor); } - new_selection_ranges.push(selection.end..selection.end); - to_unfold.push(selection.start..selection.end); + + let is_multiline_selection = selection.start.row != selection.end.row; + // Don't insert last one if it's a multi-line selection ending at the start of a line, + // so this action feels more ergonomic when paired with other selection operations + let should_skip_last = is_multiline_selection && selection.end.column == 0; + if !should_skip_last { + new_selection_ranges.push(selection.end..selection.end); + } } } - self.unfold_ranges(&to_unfold, true, true, cx); self.change_selections(Some(Autoscroll::fit()), window, cx, |s| { s.select_ranges(new_selection_ranges); }); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 153de66726..c3778e6f72 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -4874,13 +4874,76 @@ fn test_select_line(cx: &mut TestAppContext) { } #[gpui::test] -fn test_split_selection_into_lines(cx: &mut TestAppContext) { +async fn test_split_selection_into_lines(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorTestContext::new(cx).await; + + #[track_caller] + fn test(cx: &mut EditorTestContext, initial_state: &'static str, expected_state: &'static str) { + cx.set_state(initial_state); + cx.update_editor(|e, window, cx| { + e.split_selection_into_lines(&SplitSelectionIntoLines, window, cx) + }); + cx.assert_editor_state(expected_state); + } + + // Selection starts and ends at the middle of lines, left-to-right + test( + &mut cx, + "aa\nb«ˇb\ncc\ndd\ne»e\nff", + "aa\nbbˇ\nccˇ\nddˇ\neˇe\nff", + ); + // Same thing, right-to-left + test( + &mut cx, + "aa\nb«b\ncc\ndd\neˇ»e\nff", + "aa\nbbˇ\nccˇ\nddˇ\neˇe\nff", + ); + + // Whole buffer, left-to-right, last line *doesn't* end with newline + test( + &mut cx, + "«ˇaa\nbb\ncc\ndd\nee\nff»", + "aaˇ\nbbˇ\nccˇ\nddˇ\neeˇ\nffˇ", + ); + // Same thing, right-to-left + test( + &mut cx, + "«aa\nbb\ncc\ndd\nee\nffˇ»", + "aaˇ\nbbˇ\nccˇ\nddˇ\neeˇ\nffˇ", + ); + + // Whole buffer, left-to-right, last line ends with newline + test( + &mut cx, + "«ˇaa\nbb\ncc\ndd\nee\nff\n»", + "aaˇ\nbbˇ\nccˇ\nddˇ\neeˇ\nffˇ\n", + ); + // Same thing, right-to-left + test( + &mut cx, + "«aa\nbb\ncc\ndd\nee\nff\nˇ»", + "aaˇ\nbbˇ\nccˇ\nddˇ\neeˇ\nffˇ\n", + ); + + // Starts at the end of a line, ends at the start of another + test( + &mut cx, + "aa\nbb«ˇ\ncc\ndd\nee\n»ff\n", + "aa\nbbˇ\nccˇ\nddˇ\neeˇ\nff\n", + ); +} + +#[gpui::test] +async fn test_split_selection_into_lines_interacting_with_creases(cx: &mut TestAppContext) { init_test(cx, |_| {}); let editor = cx.add_window(|window, cx| { let buffer = MultiBuffer::build_simple(&sample_text(9, 5, 'a'), cx); build_editor(buffer, window, cx) }); + + // setup _ = editor.update(cx, |editor, window, cx| { editor.fold_creases( vec![ @@ -4892,6 +4955,13 @@ fn test_split_selection_into_lines(cx: &mut TestAppContext) { window, cx, ); + assert_eq!( + editor.display_text(cx), + "aa⋯bbb\nccc⋯eeee\nfffff\nggggg\n⋯i" + ); + }); + + _ = editor.update(cx, |editor, window, cx| { editor.change_selections(None, window, cx, |s| { s.select_display_ranges([ DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(0), 1), @@ -4900,28 +4970,15 @@ fn test_split_selection_into_lines(cx: &mut TestAppContext) { DisplayPoint::new(DisplayRow(4), 4)..DisplayPoint::new(DisplayRow(4), 4), ]) }); - assert_eq!( - editor.display_text(cx), - "aa⋯bbb\nccc⋯eeee\nfffff\nggggg\n⋯i" - ); - }); - - _ = editor.update(cx, |editor, window, cx| { editor.split_selection_into_lines(&SplitSelectionIntoLines, window, cx); assert_eq!( editor.display_text(cx), "aaaaa\nbbbbb\nccc⋯eeee\nfffff\nggggg\n⋯i" ); - assert_eq!( - editor.selections.display_ranges(cx), - [ - DisplayPoint::new(DisplayRow(0), 1)..DisplayPoint::new(DisplayRow(0), 1), - DisplayPoint::new(DisplayRow(0), 2)..DisplayPoint::new(DisplayRow(0), 2), - DisplayPoint::new(DisplayRow(2), 0)..DisplayPoint::new(DisplayRow(2), 0), - DisplayPoint::new(DisplayRow(5), 4)..DisplayPoint::new(DisplayRow(5), 4) - ] - ); }); + EditorTestContext::for_editor(editor, cx) + .await + .assert_editor_state("aˇaˇaaa\nbbbbb\nˇccccc\nddddd\neeeee\nfffff\nggggg\nhhhhh\niiiiiˇ"); _ = editor.update(cx, |editor, window, cx| { editor.change_selections(None, window, cx, |s| { @@ -4943,11 +5000,15 @@ fn test_split_selection_into_lines(cx: &mut TestAppContext) { DisplayPoint::new(DisplayRow(3), 5)..DisplayPoint::new(DisplayRow(3), 5), DisplayPoint::new(DisplayRow(4), 5)..DisplayPoint::new(DisplayRow(4), 5), DisplayPoint::new(DisplayRow(5), 5)..DisplayPoint::new(DisplayRow(5), 5), - DisplayPoint::new(DisplayRow(6), 5)..DisplayPoint::new(DisplayRow(6), 5), - DisplayPoint::new(DisplayRow(7), 0)..DisplayPoint::new(DisplayRow(7), 0) + DisplayPoint::new(DisplayRow(6), 5)..DisplayPoint::new(DisplayRow(6), 5) ] ); }); + EditorTestContext::for_editor(editor, cx) + .await + .assert_editor_state( + "aaaaaˇ\nbbbbbˇ\ncccccˇ\ndddddˇ\neeeeeˇ\nfffffˇ\ngggggˇ\nhhhhh\niiiii", + ); } #[gpui::test] @@ -4956,7 +5017,6 @@ async fn test_add_selection_above_below(cx: &mut TestAppContext) { let mut cx = EditorTestContext::new(cx).await; - // let buffer = MultiBuffer::build_simple("abc\ndefghi\n\njk\nlmno\n", cx); cx.set_state(indoc!( r#"abc defˇghi diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index 7f9f8f7503..919b3ea809 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -975,7 +975,10 @@ pub struct TextSummary { pub chars: usize, /// Length in UTF-16 code units pub len_utf16: OffsetUtf16, - /// A point representing the number of lines and the length of the last line + /// A point representing the number of lines and the length of the last line. + /// + /// In other words, it marks the point after the last byte in the text, (if + /// EOF was a character, this would be its position). pub lines: Point, /// How many `char`s are in the first line pub first_line_chars: u32,