fix bug in marked_range utils

crates/editor/src/editor.rs

@@ -2735,8 +2735,9 @@ impl Editor {
     pub fn backspace(&mut self, _: &Backspace, cx: &mut ViewContext<Self>) {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
         let mut selections = self.selections.all::<Point>(cx);
+        if !self.selections.line_mode {
             for selection in &mut selections {
-            if selection.is_empty() && !self.selections.line_mode {
+                if selection.is_empty() {
                     let old_head = selection.head();
                     let mut new_head =
                         movement::left(&display_map, old_head.to_display_point(&display_map))
@@ -2745,7 +2746,8 @@ impl Editor {
                         .buffer_snapshot
                         .buffer_line_for_row(old_head.row)
                     {
-                    let indent_column = buffer.indent_column_for_line(line_buffer_range.start.row);
+                        let indent_column =
+                            buffer.indent_column_for_line(line_buffer_range.start.row);
                         let language_name = buffer.language().map(|language| language.name());
                         let indent = cx.global::<Settings>().tab_size(language_name.as_deref());
                         if old_head.column <= indent_column && old_head.column > 0 {
@@ -2759,6 +2761,7 @@ impl Editor {
                     selection.set_head(new_head, SelectionGoal::None);
                 }
             }
+        }
 
         self.transact(cx, |this, cx| {
             this.change_selections(Some(Autoscroll::Fit), cx, |s| s.select(selections));
@@ -7492,8 +7495,7 @@ mod tests {
             |The qu[ick b}rown"});
         cx.update_editor(|e, cx| e.backspace(&Backspace, cx));
         cx.assert_editor_state(indoc! {"
-            |
-            fox jumps over
+            |fox jumps over
             the lazy dog|"});
     }
 
@@ -7516,13 +7518,11 @@ mod tests {
         cx.update_editor(|e, _| e.selections.line_mode = true);
         cx.set_state(indoc! {"
             The |quick |brown
-            fox {jum]ps over|
+            fox {jum]ps over
             the lazy dog
             |The qu[ick b}rown"});
         cx.update_editor(|e, cx| e.backspace(&Backspace, cx));
-        cx.assert_editor_state(indoc! {"
-            |
-            the lazy dog|"});
+        cx.assert_editor_state("|the lazy dog|");
     }
 
     #[gpui::test]
@@ -7830,131 +7830,79 @@ mod tests {
     }
 
     #[gpui::test]
-    fn test_clipboard(cx: &mut gpui::MutableAppContext) {
-        cx.set_global(Settings::test(cx));
-        let buffer = MultiBuffer::build_simple("one✅ two three four five six ", cx);
-        let view = cx
-            .add_window(Default::default(), |cx| build_editor(buffer.clone(), cx))
-            .1;
+    async fn test_clipboard(cx: &mut gpui::TestAppContext) {
+        let mut cx = EditorTestContext::new(cx).await;
 
-        // Cut with three selections. Clipboard text is divided into three slices.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_ranges(vec![0..7, 11..17, 22..27]));
-            view.cut(&Cut, cx);
-            assert_eq!(view.display_text(cx), "two four six ");
-        });
+        cx.set_state("[one✅ }two [three }four [five }six ");
+        cx.update_editor(|e, cx| e.cut(&Cut, cx));
+        cx.assert_editor_state("|two |four |six ");
 
         // Paste with three cursors. Each cursor pastes one slice of the clipboard text.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_ranges(vec![4..4, 9..9, 13..13]));
-            view.paste(&Paste, cx);
-            assert_eq!(view.display_text(cx), "two one✅ four three six five ");
-            assert_eq!(
-                view.selections.display_ranges(cx),
-                &[
-                    DisplayPoint::new(0, 11)..DisplayPoint::new(0, 11),
-                    DisplayPoint::new(0, 22)..DisplayPoint::new(0, 22),
-                    DisplayPoint::new(0, 31)..DisplayPoint::new(0, 31)
-                ]
-            );
-        });
+        cx.set_state("two |four |six |");
+        cx.update_editor(|e, cx| e.paste(&Paste, cx));
+        cx.assert_editor_state("two one✅ |four three |six five |");
 
         // Paste again but with only two cursors. Since the number of cursors doesn't
         // match the number of slices in the clipboard, the entire clipboard text
         // is pasted at each cursor.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_ranges(vec![0..0, 31..31]));
-            view.handle_input(&Input("( ".into()), cx);
-            view.paste(&Paste, cx);
-            view.handle_input(&Input(") ".into()), cx);
-            assert_eq!(
-                view.display_text(cx),
-                "( one✅ \nthree \nfive ) two one✅ four three six five ( one✅ \nthree \nfive ) "
-            );
-        });
-
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_ranges(vec![0..0]));
-            view.handle_input(&Input("123\n4567\n89\n".into()), cx);
-            assert_eq!(
-                view.display_text(cx),
-                "123\n4567\n89\n( one✅ \nthree \nfive ) two one✅ four three six five ( one✅ \nthree \nfive ) "
-            );
+        cx.set_state("|two one✅ four three six five |");
+        cx.update_editor(|e, cx| {
+            e.handle_input(&Input("( ".into()), cx);
+            e.paste(&Paste, cx);
+            e.handle_input(&Input(") ".into()), cx);
         });
+        cx.assert_editor_state(indoc! {"
+            ( one✅ 
+            three 
+            five ) |two one✅ four three six five ( one✅ 
+            three 
+            five ) |"});
 
         // Cut with three selections, one of which is full-line.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_display_ranges(
-                [
-                    DisplayPoint::new(0, 1)..DisplayPoint::new(0, 2),
-                    DisplayPoint::new(1, 1)..DisplayPoint::new(1, 1),
-                    DisplayPoint::new(2, 0)..DisplayPoint::new(2, 1),
-                ],
-            ));
-            view.cut(&Cut, cx);
-            assert_eq!(
-                view.display_text(cx),
-                "13\n9\n( one✅ \nthree \nfive ) two one✅ four three six five ( one✅ \nthree \nfive ) "
-            );
-        });
+        cx.set_state(indoc! {"
+            1[2}3
+            4|567
+            [8}9"});
+        cx.update_editor(|e, cx| e.cut(&Cut, cx));
+        cx.assert_editor_state(indoc! {"
+            1|3
+            |9"});
 
         // Paste with three selections, noticing how the copied selection that was full-line
         // gets inserted before the second cursor.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_display_ranges(
-                [
-                    DisplayPoint::new(0, 1)..DisplayPoint::new(0, 1),
-                    DisplayPoint::new(1, 1)..DisplayPoint::new(1, 1),
-                    DisplayPoint::new(2, 2)..DisplayPoint::new(2, 3),
-                ],
-            ));
-            view.paste(&Paste, cx);
-            assert_eq!(
-                view.display_text(cx),
-                "123\n4567\n9\n( 8ne✅ \nthree \nfive ) two one✅ four three six five ( one✅ \nthree \nfive ) "
-            );
-            assert_eq!(
-                view.selections.display_ranges(cx),
-                &[
-                    DisplayPoint::new(0, 2)..DisplayPoint::new(0, 2),
-                    DisplayPoint::new(2, 1)..DisplayPoint::new(2, 1),
-                    DisplayPoint::new(3, 3)..DisplayPoint::new(3, 3),
-                ]
-            );
-        });
+        cx.set_state(indoc! {"
+            1|3
+            9|
+            [o}ne"});
+        cx.update_editor(|e, cx| e.paste(&Paste, cx));
+        cx.assert_editor_state(indoc! {"
+            12|3
+            4567
+            9|
+            8|ne"});
 
         // Copy with a single cursor only, which writes the whole line into the clipboard.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| {
-                s.select_display_ranges([DisplayPoint::new(0, 1)..DisplayPoint::new(0, 1)])
-            });
-            view.copy(&Copy, cx);
-        });
+        cx.set_state(indoc! {"
+            The quick brown
+            fox ju|mps over
+            the lazy dog"});
+        cx.update_editor(|e, cx| e.copy(&Copy, cx));
+        cx.assert_clipboard_content(Some("fox jumps over\n"));
 
         // Paste with three selections, noticing how the copied full-line selection is inserted
         // before the empty selections but replaces the selection that is non-empty.
-        view.update(cx, |view, cx| {
-            view.change_selections(None, cx, |s| s.select_display_ranges(
-                [
-                    DisplayPoint::new(0, 1)..DisplayPoint::new(0, 1),
-                    DisplayPoint::new(1, 0)..DisplayPoint::new(1, 2),
-                    DisplayPoint::new(2, 1)..DisplayPoint::new(2, 1),
-                ],
-            ));
-            view.paste(&Paste, cx);
-            assert_eq!(
-                view.display_text(cx),
-                "123\n123\n123\n67\n123\n9\n( 8ne✅ \nthree \nfive ) two one✅ four three six five ( one✅ \nthree \nfive ) "
-            );
-            assert_eq!(
-                view.selections.display_ranges(cx),
-                &[
-                    DisplayPoint::new(1, 1)..DisplayPoint::new(1, 1),
-                    DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0),
-                    DisplayPoint::new(5, 1)..DisplayPoint::new(5, 1),
-                ]
-            );
-        });
+        cx.set_state(indoc! {"
+            T|he quick brown
+            [fo}x jumps over
+            t|he lazy dog"});
+        cx.update_editor(|e, cx| e.paste(&Paste, cx));
+        cx.assert_editor_state(indoc! {"
+            fox jumps over
+            T|he quick brown
+            fox jumps over
+            |x jumps over
+            fox jumps over
+            t|he lazy dog"});
     }
 
     #[gpui::test]
@@ -8693,8 +8641,10 @@ mod tests {
             fn assert(editor: &mut Editor, cx: &mut ViewContext<Editor>, marked_text_ranges: &str) {
                 let range_markers = ('<', '>');
                 let (expected_text, mut selection_ranges_lookup) =
-                    marked_text_ranges_by(marked_text_ranges, vec![range_markers.clone()]);
-                let selection_ranges = selection_ranges_lookup.remove(&range_markers).unwrap();
+                    marked_text_ranges_by(marked_text_ranges, vec![range_markers.clone().into()]);
+                let selection_ranges = selection_ranges_lookup
+                    .remove(&range_markers.into())
+                    .unwrap();
                 assert_eq!(editor.text(cx), expected_text);
                 assert_eq!(editor.selections.ranges::<usize>(cx), selection_ranges);
             }

crates/editor/src/test.rs

@@ -4,7 +4,6 @@ use indoc::indoc;
 
 use collections::BTreeMap;
 use gpui::{keymap::Keystroke, ModelHandle, ViewContext, ViewHandle};
-use itertools::{Either, Itertools};
 use language::Selection;
 use settings::Settings;
 use util::{
@@ -138,17 +137,26 @@ impl<'a> EditorTestContext<'a> {
     // `{` to `]` represents a non empty selection with the head at `{`
     pub fn set_state(&mut self, text: &str) {
         self.editor.update(self.cx, |editor, cx| {
-            let (text_with_ranges, empty_selections) = marked_text(&text);
-            let (unmarked_text, mut selection_ranges) =
-                marked_text_ranges_by(&text_with_ranges, vec![('[', '}'), ('{', ']')]);
+            let (unmarked_text, mut selection_ranges) = marked_text_ranges_by(
+                &text,
+                vec!['|'.into(), ('[', '}').into(), ('{', ']').into()],
+            );
             editor.set_text(unmarked_text, cx);
 
-            let mut selections: Vec<Range<usize>> = empty_selections
+            let mut selections: Vec<Range<usize>> =
+                selection_ranges.remove(&'|'.into()).unwrap_or_default();
+            selections.extend(
+                selection_ranges
+                    .remove(&('{', ']').into())
+                    .unwrap_or_default()
                     .into_iter()
-                .map(|offset| offset..offset)
-                .collect();
-            selections.extend(selection_ranges.remove(&('{', ']')).unwrap_or_default());
-            selections.extend(selection_ranges.remove(&('[', '}')).unwrap_or_default());
+                    .map(|range| range.end..range.start),
+            );
+            selections.extend(
+                selection_ranges
+                    .remove(&('[', '}').into())
+                    .unwrap_or_default(),
+            );
 
             editor.change_selections(Some(Autoscroll::Fit), cx, |s| s.select_ranges(selections));
         })
@@ -159,17 +167,23 @@ impl<'a> EditorTestContext<'a> {
     // `[` to `}` represents a non empty selection with the head at `}`
     // `{` to `]` represents a non empty selection with the head at `{`
     pub fn assert_editor_state(&mut self, text: &str) {
-        let (text_with_ranges, expected_empty_selections) = marked_text(&text);
-        let (unmarked_text, mut selection_ranges) =
-            marked_text_ranges_by(&text_with_ranges, vec![('[', '}'), ('{', ']')]);
+        let (unmarked_text, mut selection_ranges) = marked_text_ranges_by(
+            &text,
+            vec!['|'.into(), ('[', '}').into(), ('{', ']').into()],
+        );
         let editor_text = self.editor_text();
         assert_eq!(
             editor_text, unmarked_text,
             "Unmarked text doesn't match editor text"
         );
 
-        let expected_reverse_selections = selection_ranges.remove(&('{', ']')).unwrap_or_default();
-        let expected_forward_selections = selection_ranges.remove(&('[', '}')).unwrap_or_default();
+        let expected_empty_selections = selection_ranges.remove(&'|'.into()).unwrap_or_default();
+        let expected_reverse_selections = selection_ranges
+            .remove(&('{', ']').into())
+            .unwrap_or_default();
+        let expected_forward_selections = selection_ranges
+            .remove(&('[', '}').into())
+            .unwrap_or_default();
 
         self.assert_selections(
             expected_empty_selections,
@@ -180,65 +194,53 @@ impl<'a> EditorTestContext<'a> {
     }
 
     pub fn assert_editor_selections(&mut self, expected_selections: Vec<Selection<usize>>) {
-        let (expected_empty_selections, expected_non_empty_selections): (Vec<_>, Vec<_>) =
-            expected_selections.into_iter().partition_map(|selection| {
-                if selection.is_empty() {
-                    Either::Left(selection.head())
-                } else {
-                    Either::Right(selection)
-                }
-            });
+        let mut empty_selections = Vec::new();
+        let mut reverse_selections = Vec::new();
+        let mut forward_selections = Vec::new();
 
-        let (expected_reverse_selections, expected_forward_selections): (Vec<_>, Vec<_>) =
-            expected_non_empty_selections
-                .into_iter()
-                .partition_map(|selection| {
-                    let range = selection.start..selection.end;
-                    if selection.reversed {
-                        Either::Left(range)
+        for selection in expected_selections {
+            let range = selection.range();
+            if selection.is_empty() {
+                empty_selections.push(range);
+            } else if selection.reversed {
+                reverse_selections.push(range);
             } else {
-                        Either::Right(range)
+                forward_selections.push(range)
+            }
         }
-                });
 
         self.assert_selections(
-            expected_empty_selections,
-            expected_reverse_selections,
-            expected_forward_selections,
+            empty_selections,
+            reverse_selections,
+            forward_selections,
             None,
         )
     }
 
     fn assert_selections(
         &mut self,
-        expected_empty_selections: Vec<usize>,
+        expected_empty_selections: Vec<Range<usize>>,
         expected_reverse_selections: Vec<Range<usize>>,
         expected_forward_selections: Vec<Range<usize>>,
         asserted_text: Option<String>,
     ) {
         let (empty_selections, reverse_selections, forward_selections) =
             self.editor.read_with(self.cx, |editor, cx| {
-                let (empty_selections, non_empty_selections): (Vec<_>, Vec<_>) = editor
-                    .selections
-                    .all::<usize>(cx)
-                    .into_iter()
-                    .partition_map(|selection| {
-                        if selection.is_empty() {
-                            Either::Left(selection.head())
-                        } else {
-                            Either::Right(selection)
-                        }
-                    });
+                let mut empty_selections = Vec::new();
+                let mut reverse_selections = Vec::new();
+                let mut forward_selections = Vec::new();
 
-                let (reverse_selections, forward_selections): (Vec<_>, Vec<_>) =
-                    non_empty_selections.into_iter().partition_map(|selection| {
-                        let range = selection.start..selection.end;
-                        if selection.reversed {
-                            Either::Left(range)
+                for selection in editor.selections.all::<usize>(cx) {
+                    let range = selection.range();
+                    if selection.is_empty() {
+                        empty_selections.push(range);
+                    } else if selection.reversed {
+                        reverse_selections.push(range);
                     } else {
-                            Either::Right(range)
+                        forward_selections.push(range)
                     }
-                    });
+                }
+
                 (empty_selections, reverse_selections, forward_selections)
             });
 
@@ -258,7 +260,7 @@ impl<'a> EditorTestContext<'a> {
                 .map_err(|err| {
                     err.map(|missing| {
                         let mut error_text = unmarked_text.clone();
-                        error_text.insert(missing, '|');
+                        error_text.insert(missing.start, '|');
                         error_text
                     })
                 })
@@ -316,14 +318,14 @@ impl<'a> EditorTestContext<'a> {
 
     fn insert_markers(
         &mut self,
-        empty_selections: &Vec<usize>,
+        empty_selections: &Vec<Range<usize>>,
         reverse_selections: &Vec<Range<usize>>,
         forward_selections: &Vec<Range<usize>>,
     ) -> String {
         let mut editor_text_with_selections = self.editor_text();
         let mut selection_marks = BTreeMap::new();
-        for offset in empty_selections {
-            selection_marks.insert(offset, '|');
+        for range in empty_selections {
+            selection_marks.insert(&range.start, '|');
         }
         for range in reverse_selections {
             selection_marks.insert(&range.start, '{');

crates/util/src/test/marked_text.rs

@@ -24,20 +24,55 @@ pub fn marked_text(marked_text: &str) -> (String, Vec<usize>) {
     (unmarked_text, markers.remove(&'|').unwrap_or_default())
 }
 
+#[derive(Eq, PartialEq, Hash)]
+pub enum TextRangeMarker {
+    Empty(char),
+    Range(char, char),
+}
+
+impl TextRangeMarker {
+    fn markers(&self) -> Vec<char> {
+        match self {
+            Self::Empty(m) => vec![*m],
+            Self::Range(l, r) => vec![*l, *r],
+        }
+    }
+}
+
+impl From<char> for TextRangeMarker {
+    fn from(marker: char) -> Self {
+        Self::Empty(marker)
+    }
+}
+
+impl From<(char, char)> for TextRangeMarker {
+    fn from((left_marker, right_marker): (char, char)) -> Self {
+        Self::Range(left_marker, right_marker)
+    }
+}
+
 pub fn marked_text_ranges_by(
     marked_text: &str,
-    delimiters: Vec<(char, char)>,
-) -> (String, HashMap<(char, char), Vec<Range<usize>>>) {
-    let all_markers = delimiters
-        .iter()
-        .flat_map(|(start, end)| [*start, *end])
-        .collect();
-    let (unmarked_text, mut markers) = marked_text_by(marked_text, all_markers);
-    let range_lookup = delimiters
+    markers: Vec<TextRangeMarker>,
+) -> (String, HashMap<TextRangeMarker, Vec<Range<usize>>>) {
+    let all_markers = markers.iter().flat_map(|m| m.markers()).collect();
+
+    let (unmarked_text, mut marker_offsets) = marked_text_by(marked_text, all_markers);
+    let range_lookup = markers
         .into_iter()
-        .map(|(start_marker, end_marker)| {
-            let starts = markers.remove(&start_marker).unwrap_or_default();
-            let ends = markers.remove(&end_marker).unwrap_or_default();
+        .map(|marker| match marker {
+            TextRangeMarker::Empty(empty_marker_char) => {
+                let ranges = marker_offsets
+                    .remove(&empty_marker_char)
+                    .unwrap_or_default()
+                    .into_iter()
+                    .map(|empty_index| empty_index..empty_index)
+                    .collect::<Vec<Range<usize>>>();
+                (marker, ranges)
+            }
+            TextRangeMarker::Range(start_marker, end_marker) => {
+                let starts = marker_offsets.remove(&start_marker).unwrap_or_default();
+                let ends = marker_offsets.remove(&end_marker).unwrap_or_default();
                 assert_eq!(starts.len(), ends.len(), "marked ranges are unbalanced");
 
                 let ranges = starts
@@ -48,7 +83,8 @@ pub fn marked_text_ranges_by(
                         start..end
                     })
                     .collect::<Vec<Range<usize>>>();
-            ((start_marker, end_marker), ranges)
+                (marker, ranges)
+            }
         })
         .collect();
 
@@ -58,14 +94,16 @@ pub fn marked_text_ranges_by(
 // Returns ranges delimited by (), [], and <> ranges. Ranges using the same markers
 // must not be overlapping. May also include | for empty ranges
 pub fn marked_text_ranges(full_marked_text: &str) -> (String, Vec<Range<usize>>) {
-    let (range_marked_text, empty_offsets) = marked_text(full_marked_text);
-    let (unmarked, range_lookup) =
-        marked_text_ranges_by(&range_marked_text, vec![('[', ']'), ('(', ')'), ('<', '>')]);
-    let mut combined_ranges: Vec<_> = range_lookup
-        .into_values()
-        .flatten()
-        .chain(empty_offsets.into_iter().map(|offset| offset..offset))
-        .collect();
+    let (unmarked, range_lookup) = marked_text_ranges_by(
+        &full_marked_text,
+        vec![
+            '|'.into(),
+            ('[', ']').into(),
+            ('(', ')').into(),
+            ('<', '>').into(),
+        ],
+    );
+    let mut combined_ranges: Vec<_> = range_lookup.into_values().flatten().collect();
 
     combined_ranges.sort_by_key(|range| range.start);
     (unmarked, combined_ranges)