From edff78e722a505fd068bad61d0b466bcaafa4a08 Mon Sep 17 00:00:00 2001 From: Tarun Verghis Date: Thu, 2 May 2024 02:30:45 -0700 Subject: [PATCH] Allow ignoring soft wraps when moving to line ends (#11153) Release Notes: - Fixed #10888 This patch addresses behavior of `Editor::move_to_{beginning|end}_of_line`. It adds a setting, `stop_at_soft_wraps` when defining a keymap for the `editor::MoveToBeginningOfLine` and `editor::MoveToEndOfLine` actions. When `true`, it causes movement to the either end of the line (via, for example Home or End), to go to the logical end, as opposed to the nearest soft wrap point in the respective direction. --------- Co-authored-by: Kirill Bulatov --- crates/editor/src/actions.rs | 41 ++++++--- crates/editor/src/editor.rs | 11 ++- crates/editor/src/editor_tests.rs | 106 +++++++++++++++++++++-- crates/language/src/language_settings.rs | 5 +- crates/util/src/serde.rs | 3 + crates/util/src/util.rs | 1 + 6 files changed, 140 insertions(+), 27 deletions(-) create mode 100644 crates/util/src/serde.rs diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index b050894bc7..160b8a1e1b 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -1,5 +1,6 @@ //! This module contains all actions supported by [`Editor`]. use super::*; +use util::serde::default_true; #[derive(PartialEq, Clone, Deserialize, Default)] pub struct SelectNext { @@ -13,6 +14,12 @@ pub struct SelectPrevious { pub replace_newest: bool, } +#[derive(PartialEq, Clone, Deserialize, Default)] +pub struct MoveToBeginningOfLine { + #[serde(default = "default_true")] + pub(super) stop_at_soft_wraps: bool, +} + #[derive(PartialEq, Clone, Deserialize, Default)] pub struct SelectToBeginningOfLine { #[serde(default)] @@ -31,6 +38,12 @@ pub struct MovePageDown { pub(super) center_cursor: bool, } +#[derive(PartialEq, Clone, Deserialize, Default)] +pub struct MoveToEndOfLine { + #[serde(default = "default_true")] + pub(super) stop_at_soft_wraps: bool, +} + #[derive(PartialEq, Clone, Deserialize, Default)] pub struct SelectToEndOfLine { #[serde(default)] @@ -103,23 +116,25 @@ pub struct ExpandExcerpts { impl_actions!( editor, [ + ConfirmCodeAction, + ConfirmCompletion, + ExpandExcerpts, + FoldAt, + MoveDownByLines, + MovePageDown, + MovePageUp, + MoveToBeginningOfLine, + MoveToEndOfLine, + MoveUpByLines, + SelectDownByLines, SelectNext, SelectPrevious, SelectToBeginningOfLine, - ExpandExcerpts, - MovePageUp, - MovePageDown, SelectToEndOfLine, - ToggleCodeActions, - ConfirmCompletion, - ConfirmCodeAction, - ToggleComments, - FoldAt, - UnfoldAt, - MoveUpByLines, - MoveDownByLines, SelectUpByLines, - SelectDownByLines, + ToggleCodeActions, + ToggleComments, + UnfoldAt, ] ); @@ -190,10 +205,8 @@ gpui::actions!( MoveLineUp, MoveRight, MoveToBeginning, - MoveToBeginningOfLine, MoveToEnclosingBracket, MoveToEnd, - MoveToEndOfLine, MoveToEndOfParagraph, MoveToNextSubwordEnd, MoveToNextWordEnd, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 423c35d148..76f75aae70 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6240,13 +6240,13 @@ impl Editor { pub fn move_to_beginning_of_line( &mut self, - _: &MoveToBeginningOfLine, + action: &MoveToBeginningOfLine, cx: &mut ViewContext, ) { self.change_selections(Some(Autoscroll::fit()), cx, |s| { s.move_cursors_with(|map, head, _| { ( - movement::indented_line_beginning(map, head, true), + movement::indented_line_beginning(map, head, action.stop_at_soft_wraps), SelectionGoal::None, ) }); @@ -6290,10 +6290,13 @@ impl Editor { }); } - pub fn move_to_end_of_line(&mut self, _: &MoveToEndOfLine, cx: &mut ViewContext) { + pub fn move_to_end_of_line(&mut self, action: &MoveToEndOfLine, cx: &mut ViewContext) { self.change_selections(Some(Autoscroll::fit()), cx, |s| { s.move_cursors_with(|map, head, _| { - (movement::line_end(map, head, true), SelectionGoal::None) + ( + movement::line_end(map, head, action.stop_at_soft_wraps), + SelectionGoal::None, + ) }); }) } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index d10870ee67..17f2a3c363 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -1045,6 +1045,13 @@ fn test_move_cursor_different_line_lengths(cx: &mut TestAppContext) { #[gpui::test] fn test_beginning_end_of_line(cx: &mut TestAppContext) { init_test(cx, |_| {}); + let move_to_beg = MoveToBeginningOfLine { + stop_at_soft_wraps: true, + }; + + let move_to_end = MoveToEndOfLine { + stop_at_soft_wraps: true, + }; let view = cx.add_window(|cx| { let buffer = MultiBuffer::build_simple("abc\n def", cx); @@ -1060,7 +1067,7 @@ fn test_beginning_end_of_line(cx: &mut TestAppContext) { }); _ = view.update(cx, |view, cx| { - view.move_to_beginning_of_line(&MoveToBeginningOfLine, cx); + view.move_to_beginning_of_line(&move_to_beg, cx); assert_eq!( view.selections.display_ranges(cx), &[ @@ -1071,7 +1078,7 @@ fn test_beginning_end_of_line(cx: &mut TestAppContext) { }); _ = view.update(cx, |view, cx| { - view.move_to_beginning_of_line(&MoveToBeginningOfLine, cx); + view.move_to_beginning_of_line(&move_to_beg, cx); assert_eq!( view.selections.display_ranges(cx), &[ @@ -1082,7 +1089,7 @@ fn test_beginning_end_of_line(cx: &mut TestAppContext) { }); _ = view.update(cx, |view, cx| { - view.move_to_beginning_of_line(&MoveToBeginningOfLine, cx); + view.move_to_beginning_of_line(&move_to_beg, cx); assert_eq!( view.selections.display_ranges(cx), &[ @@ -1093,7 +1100,7 @@ fn test_beginning_end_of_line(cx: &mut TestAppContext) { }); _ = view.update(cx, |view, cx| { - view.move_to_end_of_line(&MoveToEndOfLine, cx); + view.move_to_end_of_line(&move_to_end, cx); assert_eq!( view.selections.display_ranges(cx), &[ @@ -1105,7 +1112,7 @@ fn test_beginning_end_of_line(cx: &mut TestAppContext) { // Moving to the end of line again is a no-op. _ = view.update(cx, |view, cx| { - view.move_to_end_of_line(&MoveToEndOfLine, cx); + view.move_to_end_of_line(&move_to_end, cx); assert_eq!( view.selections.display_ranges(cx), &[ @@ -1205,6 +1212,95 @@ fn test_beginning_end_of_line(cx: &mut TestAppContext) { }); } +#[gpui::test] +fn test_beginning_end_of_line_ignore_soft_wrap(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let move_to_beg = MoveToBeginningOfLine { + stop_at_soft_wraps: false, + }; + + let move_to_end = MoveToEndOfLine { + stop_at_soft_wraps: false, + }; + + let view = cx.add_window(|cx| { + let buffer = MultiBuffer::build_simple("thequickbrownfox\njumpedoverthelazydogs", cx); + build_editor(buffer, cx) + }); + + _ = view.update(cx, |view, cx| { + view.set_wrap_width(Some(140.0.into()), cx); + + // We expect the following lines after wrapping + // ``` + // thequickbrownfox + // jumpedoverthelazydo + // gs + // ``` + // The final `gs` was soft-wrapped onto a new line. + assert_eq!( + "thequickbrownfox\njumpedoverthelaz\nydogs", + view.display_text(cx), + ); + + // First, let's assert behavior on the first line, that was not soft-wrapped. + // Start the cursor at the `k` on the first line + view.change_selections(None, cx, |s| { + s.select_display_ranges([DisplayPoint::new(0, 7)..DisplayPoint::new(0, 7)]); + }); + + // Moving to the beginning of the line should put us at the beginning of the line. + view.move_to_beginning_of_line(&move_to_beg, cx); + assert_eq!( + vec![DisplayPoint::new(0, 0)..DisplayPoint::new(0, 0),], + view.selections.display_ranges(cx) + ); + + // Moving to the end of the line should put us at the end of the line. + view.move_to_end_of_line(&move_to_end, cx); + assert_eq!( + vec![DisplayPoint::new(0, 16)..DisplayPoint::new(0, 16),], + view.selections.display_ranges(cx) + ); + + // Now, let's assert behavior on the second line, that ended up being soft-wrapped. + // Start the cursor at the last line (`y` that was wrapped to a new line) + view.change_selections(None, cx, |s| { + s.select_display_ranges([DisplayPoint::new(2, 0)..DisplayPoint::new(2, 0)]); + }); + + // Moving to the beginning of the line should put us at the start of the second line of + // display text, i.e., the `j`. + view.move_to_beginning_of_line(&move_to_beg, cx); + assert_eq!( + vec![DisplayPoint::new(1, 0)..DisplayPoint::new(1, 0),], + view.selections.display_ranges(cx) + ); + + // Moving to the beginning of the line again should be a no-op. + view.move_to_beginning_of_line(&move_to_beg, cx); + assert_eq!( + vec![DisplayPoint::new(1, 0)..DisplayPoint::new(1, 0),], + view.selections.display_ranges(cx) + ); + + // Moving to the end of the line should put us right after the `s` that was soft-wrapped to the + // next display line. + view.move_to_end_of_line(&move_to_end, cx); + assert_eq!( + vec![DisplayPoint::new(2, 5)..DisplayPoint::new(2, 5),], + view.selections.display_ranges(cx) + ); + + // Moving to the end of the line again should be a no-op. + view.move_to_end_of_line(&move_to_end, cx); + assert_eq!( + vec![DisplayPoint::new(2, 5)..DisplayPoint::new(2, 5),], + view.selections.display_ranges(cx) + ); + }); +} + #[gpui::test] fn test_prev_next_word_boundary(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 940e7ad18e..bea5344be2 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -13,6 +13,7 @@ use schemars::{ use serde::{Deserialize, Serialize}; use settings::{Settings, SettingsLocation, SettingsSources}; use std::{num::NonZeroU32, path::Path, sync::Arc}; +use util::serde::default_true; impl<'a> Into> for &'a dyn File { fn into(self) -> SettingsLocation<'a> { @@ -438,10 +439,6 @@ pub struct InlayHintSettings { pub scroll_debounce_ms: u64, } -fn default_true() -> bool { - true -} - fn edit_debounce_ms() -> u64 { 700 } diff --git a/crates/util/src/serde.rs b/crates/util/src/serde.rs new file mode 100644 index 0000000000..be948c659f --- /dev/null +++ b/crates/util/src/serde.rs @@ -0,0 +1,3 @@ +pub const fn default_true() -> bool { + true +} diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 82387af752..9ff5087e89 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -5,6 +5,7 @@ mod git_author; pub mod github; pub mod http; pub mod paths; +pub mod serde; #[cfg(any(test, feature = "test-support"))] pub mod test;