From e104cb94e77592bca13ce461108e6c566c67bcae Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Wed, 25 May 2022 14:13:18 -0700 Subject: [PATCH] fix bug in marked_range utils --- crates/editor/src/editor.rs | 220 +++++++++++----------------- crates/editor/src/test.rs | 126 ++++++++-------- crates/util/src/test/marked_text.rs | 98 +++++++++---- 3 files changed, 217 insertions(+), 227 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index bb181a6ea1..e7af70f303 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2735,28 +2735,31 @@ impl Editor { pub fn backspace(&mut self, _: &Backspace, cx: &mut ViewContext) { let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); let mut selections = self.selections.all::(cx); - for selection in &mut selections { - if selection.is_empty() && !self.selections.line_mode { - let old_head = selection.head(); - let mut new_head = - movement::left(&display_map, old_head.to_display_point(&display_map)) - .to_point(&display_map); - if let Some((buffer, line_buffer_range)) = display_map - .buffer_snapshot - .buffer_line_for_row(old_head.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::().tab_size(language_name.as_deref()); - if old_head.column <= indent_column && old_head.column > 0 { - new_head = cmp::min( - new_head, - Point::new(old_head.row, ((old_head.column - 1) / indent) * indent), - ); + if !self.selections.line_mode { + for selection in &mut selections { + if selection.is_empty() { + let old_head = selection.head(); + let mut new_head = + movement::left(&display_map, old_head.to_display_point(&display_map)) + .to_point(&display_map); + if let Some((buffer, line_buffer_range)) = display_map + .buffer_snapshot + .buffer_line_for_row(old_head.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::().tab_size(language_name.as_deref()); + if old_head.column <= indent_column && old_head.column > 0 { + new_head = cmp::min( + new_head, + Point::new(old_head.row, ((old_head.column - 1) / indent) * indent), + ); + } } - } - selection.set_head(new_head, SelectionGoal::None); + selection.set_head(new_head, SelectionGoal::None); + } } } @@ -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, 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::(cx), selection_ranges); } diff --git a/crates/editor/src/test.rs b/crates/editor/src/test.rs index a8bcc94ee2..4c9ceed9ae 100644 --- a/crates/editor/src/test.rs +++ b/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> = empty_selections - .into_iter() - .map(|offset| offset..offset) - .collect(); - selections.extend(selection_ranges.remove(&('{', ']')).unwrap_or_default()); - selections.extend(selection_ranges.remove(&('[', '}')).unwrap_or_default()); + let mut selections: Vec> = + selection_ranges.remove(&'|'.into()).unwrap_or_default(); + selections.extend( + selection_ranges + .remove(&('{', ']').into()) + .unwrap_or_default() + .into_iter() + .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>) { - 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) - } else { - Either::Right(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 { + 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, + expected_empty_selections: Vec>, expected_reverse_selections: Vec>, expected_forward_selections: Vec>, asserted_text: Option, ) { 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::(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(); + + for selection in editor.selections.all::(cx) { + let range = selection.range(); + if selection.is_empty() { + empty_selections.push(range); + } else if selection.reversed { + reverse_selections.push(range); + } else { + forward_selections.push(range) + } + } - 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) - } else { - Either::Right(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, + empty_selections: &Vec>, reverse_selections: &Vec>, forward_selections: &Vec>, ) -> 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, '{'); diff --git a/crates/util/src/test/marked_text.rs b/crates/util/src/test/marked_text.rs index 23ac35ce86..733feeb3f8 100644 --- a/crates/util/src/test/marked_text.rs +++ b/crates/util/src/test/marked_text.rs @@ -24,31 +24,67 @@ pub fn marked_text(marked_text: &str) -> (String, Vec) { (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 { + match self { + Self::Empty(m) => vec![*m], + Self::Range(l, r) => vec![*l, *r], + } + } +} + +impl From 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>>) { - 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 - .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(); - assert_eq!(starts.len(), ends.len(), "marked ranges are unbalanced"); + markers: Vec, +) -> (String, HashMap>>) { + let all_markers = markers.iter().flat_map(|m| m.markers()).collect(); - let ranges = starts - .into_iter() - .zip(ends) - .map(|(start, end)| { - assert!(end >= start, "marked ranges must be disjoint"); - start..end - }) - .collect::>>(); - ((start_marker, end_marker), ranges) + let (unmarked_text, mut marker_offsets) = marked_text_by(marked_text, all_markers); + let range_lookup = markers + .into_iter() + .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::>>(); + (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 + .into_iter() + .zip(ends) + .map(|(start, end)| { + assert!(end >= start, "marked ranges must be disjoint"); + start..end + }) + .collect::>>(); + (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>) { - 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)