From cb76b2a6ad4371eec12cbee612d476f91fd59b97 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 10 Oct 2023 08:53:30 -0600 Subject: [PATCH] Make vim visual block work better --- crates/editor/src/display_map.rs | 11 +-- crates/editor/src/element.rs | 2 +- crates/editor/src/movement.rs | 2 - crates/editor/src/selections_collection.rs | 6 +- crates/text/src/selection.rs | 1 - crates/vim/src/motion.rs | 1 - crates/vim/src/test.rs | 1 + crates/vim/src/visual.rs | 73 ++++++++++++++----- .../test_visual_block_issue_2123.json | 5 ++ .../vim/test_data/test_wrapped_motions.json | 15 ++++ 10 files changed, 85 insertions(+), 32 deletions(-) create mode 100644 crates/vim/test_data/test_visual_block_issue_2123.json create mode 100644 crates/vim/test_data/test_wrapped_motions.json diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 0f2b5665c6..a92c46e072 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -509,11 +509,12 @@ impl DisplaySnapshot { pub fn highlighted_chunks<'a>( &'a self, display_rows: Range, + language_aware: bool, style: &'a EditorStyle, ) -> impl Iterator> { self.chunks( display_rows, - true, + language_aware, Some(style.theme.hint), Some(style.theme.suggestion), ) @@ -562,7 +563,7 @@ impl DisplaySnapshot { }) } - fn layout_line_for_row( + pub fn lay_out_line_for_row( &self, display_row: u32, TextLayoutDetails { @@ -575,7 +576,7 @@ impl DisplaySnapshot { let mut line = String::new(); let range = display_row..display_row + 1; - for chunk in self.highlighted_chunks(range, editor_style) { + for chunk in self.highlighted_chunks(range, false, editor_style) { line.push_str(chunk.chunk); let text_style = if let Some(style) = chunk.style { @@ -607,7 +608,7 @@ impl DisplaySnapshot { display_point: DisplayPoint, text_layout_details: &TextLayoutDetails, ) -> f32 { - let layout_line = self.layout_line_for_row(display_point.row(), text_layout_details); + let layout_line = self.lay_out_line_for_row(display_point.row(), text_layout_details); layout_line.x_for_index(display_point.column() as usize) } @@ -617,7 +618,7 @@ impl DisplaySnapshot { x_coordinate: f32, text_layout_details: &TextLayoutDetails, ) -> u32 { - let layout_line = self.layout_line_for_row(display_row, text_layout_details); + let layout_line = self.lay_out_line_for_row(display_row, text_layout_details); layout_line.closest_index_for_x(x_coordinate) as u32 } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 30015eb760..39d91fe899 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1583,7 +1583,7 @@ impl EditorElement { .collect() } else { let style = &self.style; - let chunks = snapshot.highlighted_chunks(rows.clone(), style); + let chunks = snapshot.highlighted_chunks(rows.clone(), true, style); LineWithInvisibles::from_chunks( chunks, diff --git a/crates/editor/src/movement.rs b/crates/editor/src/movement.rs index 7e75ae5e5d..0305d853e6 100644 --- a/crates/editor/src/movement.rs +++ b/crates/editor/src/movement.rs @@ -107,7 +107,6 @@ pub fn up_by_rows( SelectionGoal::HorizontalPosition(x) => x, SelectionGoal::WrappedHorizontalPosition((_, x)) => x, SelectionGoal::HorizontalRange { end, .. } => end, - SelectionGoal::WrappedHorizontalRange { end: (_, end), .. } => end, _ => map.x_for_point(start, text_layout_details), }; @@ -144,7 +143,6 @@ pub fn down_by_rows( SelectionGoal::HorizontalPosition(x) => x, SelectionGoal::WrappedHorizontalPosition((_, x)) => x, SelectionGoal::HorizontalRange { end, .. } => end, - SelectionGoal::WrappedHorizontalRange { end: (_, end), .. } => end, _ => map.x_for_point(start, text_layout_details), }; diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index 2fa8ffe408..148604bd23 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -313,10 +313,12 @@ impl SelectionsCollection { let is_empty = positions.start == positions.end; let line_len = display_map.line_len(row); - let start_col = display_map.column_for_x(row, positions.start, text_layout_details); + let layed_out_line = display_map.lay_out_line_for_row(row, &text_layout_details); + + let start_col = layed_out_line.closest_index_for_x(positions.start) as u32; if start_col < line_len || (is_empty && start_col == line_len) { let start = DisplayPoint::new(row, start_col); - let end_col = display_map.column_for_x(row, positions.end, text_layout_details); + let end_col = layed_out_line.closest_index_for_x(positions.end) as u32; let end = DisplayPoint::new(row, end_col); Some(Selection { diff --git a/crates/text/src/selection.rs b/crates/text/src/selection.rs index e127083112..480cb99d74 100644 --- a/crates/text/src/selection.rs +++ b/crates/text/src/selection.rs @@ -8,7 +8,6 @@ pub enum SelectionGoal { HorizontalPosition(f32), HorizontalRange { start: f32, end: f32 }, WrappedHorizontalPosition((u32, f32)), - WrappedHorizontalRange { start: (u32, f32), end: (u32, f32) }, } #[derive(Clone, Debug, PartialEq)] diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 36514f8cc4..e8d954bc13 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -568,7 +568,6 @@ fn up_down_buffer_rows( let (goal_wrap, goal_x) = match goal { SelectionGoal::WrappedHorizontalPosition((row, x)) => (row, x), - SelectionGoal::WrappedHorizontalRange { end: (row, x), .. } => (row, x), SelectionGoal::HorizontalRange { end, .. } => (select_nth_wrapped_row, end), SelectionGoal::HorizontalPosition(x) => (select_nth_wrapped_row, x), _ => { diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index d6d9a19360..3c48679212 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -653,6 +653,7 @@ async fn test_selection_goal(cx: &mut gpui::TestAppContext) { .await; } +#[gpui::test] async fn test_wrapped_motions(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; diff --git a/crates/vim/src/visual.rs b/crates/vim/src/visual.rs index ac4c5478a1..f4a450c932 100644 --- a/crates/vim/src/visual.rs +++ b/crates/vim/src/visual.rs @@ -145,16 +145,18 @@ pub fn visual_block_motion( let map = &s.display_map(); let mut head = s.newest_anchor().head().to_display_point(map); let mut tail = s.oldest_anchor().tail().to_display_point(map); + dbg!(head, tail); + dbg!(s.newest_anchor().goal); let (start, end) = match s.newest_anchor().goal { SelectionGoal::HorizontalRange { start, end } if preserve_goal => (start, end), - SelectionGoal::HorizontalPosition(start) if preserve_goal => (start, start + 10.0), + SelectionGoal::HorizontalPosition(start) if preserve_goal => (start, start), _ => ( map.x_for_point(tail, &text_layout_details), map.x_for_point(head, &text_layout_details), ), }; - let goal = SelectionGoal::HorizontalRange { start, end }; + let mut goal = SelectionGoal::HorizontalRange { start, end }; let was_reversed = tail.column() > head.column(); if !was_reversed && !preserve_goal { @@ -179,35 +181,44 @@ pub fn visual_block_motion( let positions = if is_reversed { map.x_for_point(head, &text_layout_details)..map.x_for_point(tail, &text_layout_details) } else if head.column() == tail.column() { - map.x_for_point(head, &text_layout_details) - ..map.x_for_point(head, &text_layout_details) + 10.0 + let head_forward = movement::saturating_right(map, head); + map.x_for_point(head, &text_layout_details)..map.x_for_point(head, &text_layout_details) } else { map.x_for_point(tail, &text_layout_details)..map.x_for_point(head, &text_layout_details) }; + if !preserve_goal { + goal = SelectionGoal::HorizontalRange { + start: positions.start, + end: positions.end, + }; + } + let mut selections = Vec::new(); let mut row = tail.row(); loop { - let start = map.clip_point( - DisplayPoint::new( - row, - map.column_for_x(row, positions.start, &text_layout_details), - ), - Bias::Left, + let layed_out_line = map.lay_out_line_for_row(row, &text_layout_details); + let start = DisplayPoint::new( + row, + layed_out_line.closest_index_for_x(positions.start) as u32, ); - let end = map.clip_point( - DisplayPoint::new( - row, - map.column_for_x(row, positions.end, &text_layout_details), - ), - Bias::Left, + let mut end = DisplayPoint::new( + row, + layed_out_line.closest_index_for_x(positions.end) as u32, ); + if end <= start { + if start.column() == map.line_len(start.row()) { + end = start; + } else { + end = movement::saturating_right(map, start); + } + } + if positions.start - <= map.x_for_point( - DisplayPoint::new(row, map.line_len(row)), - &text_layout_details, - ) + <= + //map.x_for_point(DisplayPoint::new(row, map.line_len(row)), &text_layout_details) + layed_out_line.width() { let selection = Selection { id: s.new_selection_id(), @@ -915,6 +926,28 @@ mod test { .await; } + #[gpui::test] + async fn test_visual_block_issue_2123(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_shared_state(indoc! { + "The ˇquick brown + fox jumps over + the lazy dog + " + }) + .await; + cx.simulate_shared_keystrokes(["ctrl-v", "right", "down"]) + .await; + cx.assert_shared_state(indoc! { + "The «quˇ»ick brown + fox «juˇ»mps over + the lazy dog + " + }) + .await; + } + #[gpui::test] async fn test_visual_block_insert(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; diff --git a/crates/vim/test_data/test_visual_block_issue_2123.json b/crates/vim/test_data/test_visual_block_issue_2123.json new file mode 100644 index 0000000000..0f48bcc890 --- /dev/null +++ b/crates/vim/test_data/test_visual_block_issue_2123.json @@ -0,0 +1,5 @@ +{"Put":{"state":"The ˇquick brown\nfox jumps over\nthe lazy dog\n"}} +{"Key":"ctrl-v"} +{"Key":"right"} +{"Key":"down"} +{"Get":{"state":"The «quˇ»ick brown\nfox «juˇ»mps over\nthe lazy dog\n","mode":"VisualBlock"}} diff --git a/crates/vim/test_data/test_wrapped_motions.json b/crates/vim/test_data/test_wrapped_motions.json new file mode 100644 index 0000000000..195a58f6b5 --- /dev/null +++ b/crates/vim/test_data/test_wrapped_motions.json @@ -0,0 +1,15 @@ +{"SetOption":{"value":"wrap"}} +{"SetOption":{"value":"columns=12"}} +{"Put":{"state":"aaˇaa\n😃😃"}} +{"Key":"j"} +{"Get":{"state":"aaaa\n😃ˇ😃","mode":"Normal"}} +{"Put":{"state":"123456789012aaˇaa\n123456789012😃😃"}} +{"Key":"j"} +{"Get":{"state":"123456789012aaaa\n123456789012😃ˇ😃","mode":"Normal"}} +{"Put":{"state":"123456789012aaˇaa\n123456789012😃😃"}} +{"Key":"j"} +{"Get":{"state":"123456789012aaaa\n123456789012😃ˇ😃","mode":"Normal"}} +{"Put":{"state":"123456789012aaaaˇaaaaaaaa123456789012\nwow\n123456789012😃😃😃😃😃😃123456789012"}} +{"Key":"j"} +{"Key":"j"} +{"Get":{"state":"123456789012aaaaaaaaaaaa123456789012\nwow\n123456789012😃😃ˇ😃😃😃😃123456789012","mode":"Normal"}}