editor: Fix DeleteToPreviousWordStart and DeleteToNextWordEnd interaction with newlines (#16848)

Kajus created

Closes #5285, #14389

Changes:
- `DeleteToPreviousWordStart` now deletes '\n' separately from preceding
words and whitespace.
- `DeleteToNextWordEnd` now deletes '\n' and any following whitespace
separately from subsequent words.
- Added an `ignore_newlines` flag to both actions to optionally retain
the old behavior.

These modifications align the behavior more closely with other popular
editors like VSCode and Sublime:
- `DeleteToPreviousWordStart` now matches the default <Ctrl+Backspace>
action in those editors.
- `DeleteToNextWordEnd` becomes more intuitive and closely resembles the
default <Ctrl+Delete> behavior in those editors.

Release Notes:

- Improved `DeleteToPreviousWordStart` and `DeleteToNextWordEnd`
interactions around newlines. You can opt-in into the previous behavior
by adding {"ignore_newlines": true} to either action's binds in your
keymap. ([#5285](https://github.com/zed-industries/zed/issues/5285),
[#14389](https://github.com/zed-industries/zed/issues/14389))

Change summary

crates/editor/src/actions.rs      | 16 +++++
crates/editor/src/editor.rs       | 20 +++++-
crates/editor/src/editor_tests.rs | 92 ++++++++++++++++++++++++++++++++
crates/editor/src/movement.rs     | 31 +++++++++++
4 files changed, 151 insertions(+), 8 deletions(-)

Detailed changes

crates/editor/src/actions.rs 🔗

@@ -141,12 +141,26 @@ pub struct ShowCompletions {
 #[derive(PartialEq, Clone, Deserialize, Default)]
 pub struct HandleInput(pub String);
 
+#[derive(PartialEq, Clone, Deserialize, Default)]
+pub struct DeleteToNextWordEnd {
+    #[serde(default)]
+    pub ignore_newlines: bool,
+}
+
+#[derive(PartialEq, Clone, Deserialize, Default)]
+pub struct DeleteToPreviousWordStart {
+    #[serde(default)]
+    pub ignore_newlines: bool,
+}
+
 impl_actions!(
     editor,
     [
         ConfirmCodeAction,
         ConfirmCompletion,
         ComposeCompletion,
+        DeleteToNextWordEnd,
+        DeleteToPreviousWordStart,
         ExpandExcerpts,
         ExpandExcerptsUp,
         ExpandExcerptsDown,
@@ -208,9 +222,7 @@ gpui::actions!(
         DeleteToBeginningOfLine,
         DeleteToEndOfLine,
         DeleteToNextSubwordEnd,
-        DeleteToNextWordEnd,
         DeleteToPreviousSubwordStart,
-        DeleteToPreviousWordStart,
         DisplayCursorNames,
         DuplicateLineDown,
         DuplicateLineUp,

crates/editor/src/editor.rs 🔗

@@ -7327,7 +7327,7 @@ impl Editor {
 
     pub fn delete_to_previous_word_start(
         &mut self,
-        _: &DeleteToPreviousWordStart,
+        action: &DeleteToPreviousWordStart,
         cx: &mut ViewContext<Self>,
     ) {
         self.transact(cx, |this, cx| {
@@ -7336,7 +7336,11 @@ impl Editor {
                 let line_mode = s.line_mode;
                 s.move_with(|map, selection| {
                     if selection.is_empty() && !line_mode {
-                        let cursor = movement::previous_word_start(map, selection.head());
+                        let cursor = if action.ignore_newlines {
+                            movement::previous_word_start(map, selection.head())
+                        } else {
+                            movement::previous_word_start_or_newline(map, selection.head())
+                        };
                         selection.set_head(cursor, SelectionGoal::None);
                     }
                 });
@@ -7405,13 +7409,21 @@ impl Editor {
         })
     }
 
-    pub fn delete_to_next_word_end(&mut self, _: &DeleteToNextWordEnd, cx: &mut ViewContext<Self>) {
+    pub fn delete_to_next_word_end(
+        &mut self,
+        action: &DeleteToNextWordEnd,
+        cx: &mut ViewContext<Self>,
+    ) {
         self.transact(cx, |this, cx| {
             this.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 let line_mode = s.line_mode;
                 s.move_with(|map, selection| {
                     if selection.is_empty() && !line_mode {
-                        let cursor = movement::next_word_end(map, selection.head());
+                        let cursor = if action.ignore_newlines {
+                            movement::next_word_end(map, selection.head())
+                        } else {
+                            movement::next_word_end_or_newline(map, selection.head())
+                        };
                         selection.set_head(cursor, SelectionGoal::None);
                     }
                 });

crates/editor/src/editor_tests.rs 🔗

@@ -2102,7 +2102,12 @@ fn test_delete_to_word_boundary(cx: &mut TestAppContext) {
                 DisplayPoint::new(DisplayRow(0), 9)..DisplayPoint::new(DisplayRow(0), 12),
             ])
         });
-        view.delete_to_previous_word_start(&DeleteToPreviousWordStart, cx);
+        view.delete_to_previous_word_start(
+            &DeleteToPreviousWordStart {
+                ignore_newlines: false,
+            },
+            cx,
+        );
         assert_eq!(view.buffer.read(cx).read(cx).text(), "e two te four");
     });
 
@@ -2115,11 +2120,94 @@ fn test_delete_to_word_boundary(cx: &mut TestAppContext) {
                 DisplayPoint::new(DisplayRow(0), 9)..DisplayPoint::new(DisplayRow(0), 10),
             ])
         });
-        view.delete_to_next_word_end(&DeleteToNextWordEnd, cx);
+        view.delete_to_next_word_end(
+            &DeleteToNextWordEnd {
+                ignore_newlines: false,
+            },
+            cx,
+        );
         assert_eq!(view.buffer.read(cx).read(cx).text(), "e t te our");
     });
 }
 
+#[gpui::test]
+fn test_delete_to_previous_word_start_or_newline(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let view = cx.add_window(|cx| {
+        let buffer = MultiBuffer::build_simple("one\n2\nthree\n4", cx);
+        build_editor(buffer.clone(), cx)
+    });
+    let del_to_prev_word_start = DeleteToPreviousWordStart {
+        ignore_newlines: false,
+    };
+    let del_to_prev_word_start_ignore_newlines = DeleteToPreviousWordStart {
+        ignore_newlines: true,
+    };
+
+    _ = view.update(cx, |view, cx| {
+        view.change_selections(None, cx, |s| {
+            s.select_display_ranges([
+                DisplayPoint::new(DisplayRow(3), 1)..DisplayPoint::new(DisplayRow(3), 1)
+            ])
+        });
+        view.delete_to_previous_word_start(&del_to_prev_word_start, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "one\n2\nthree\n");
+        view.delete_to_previous_word_start(&del_to_prev_word_start, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "one\n2\nthree");
+        view.delete_to_previous_word_start(&del_to_prev_word_start, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "one\n2\n");
+        view.delete_to_previous_word_start(&del_to_prev_word_start, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "one\n2");
+        view.delete_to_previous_word_start(&del_to_prev_word_start_ignore_newlines, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "one\n");
+        view.delete_to_previous_word_start(&del_to_prev_word_start_ignore_newlines, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "");
+    });
+}
+
+#[gpui::test]
+fn test_delete_to_next_word_end_or_newline(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let view = cx.add_window(|cx| {
+        let buffer = MultiBuffer::build_simple("\none\n   two\nthree\n   four", cx);
+        build_editor(buffer.clone(), cx)
+    });
+    let del_to_next_word_end = DeleteToNextWordEnd {
+        ignore_newlines: false,
+    };
+    let del_to_next_word_end_ignore_newlines = DeleteToNextWordEnd {
+        ignore_newlines: true,
+    };
+
+    _ = view.update(cx, |view, cx| {
+        view.change_selections(None, cx, |s| {
+            s.select_display_ranges([
+                DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(0), 0)
+            ])
+        });
+        view.delete_to_next_word_end(&del_to_next_word_end, cx);
+        assert_eq!(
+            view.buffer.read(cx).read(cx).text(),
+            "one\n   two\nthree\n   four"
+        );
+        view.delete_to_next_word_end(&del_to_next_word_end, cx);
+        assert_eq!(
+            view.buffer.read(cx).read(cx).text(),
+            "\n   two\nthree\n   four"
+        );
+        view.delete_to_next_word_end(&del_to_next_word_end, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "two\nthree\n   four");
+        view.delete_to_next_word_end(&del_to_next_word_end, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "\nthree\n   four");
+        view.delete_to_next_word_end(&del_to_next_word_end_ignore_newlines, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "\n   four");
+        view.delete_to_next_word_end(&del_to_next_word_end_ignore_newlines, cx);
+        assert_eq!(view.buffer.read(cx).read(cx).text(), "");
+    });
+}
+
 #[gpui::test]
 fn test_newline(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/movement.rs 🔗

@@ -270,6 +270,19 @@ pub fn previous_word_start(map: &DisplaySnapshot, point: DisplayPoint) -> Displa
     })
 }
 
+/// Returns a position of the previous word boundary, where a word character is defined as either
+/// uppercase letter, lowercase letter, '_' character, language-specific word character (like '-' in CSS) or newline.
+pub fn previous_word_start_or_newline(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
+    let raw_point = point.to_point(map);
+    let classifier = map.buffer_snapshot.char_classifier_at(raw_point);
+
+    find_preceding_boundary_display_point(map, point, FindRange::MultiLine, |left, right| {
+        (classifier.kind(left) != classifier.kind(right) && !right.is_whitespace())
+            || left == '\n'
+            || right == '\n'
+    })
+}
+
 /// Returns a position of the previous subword boundary, where a subword is defined as a run of
 /// word characters of the same "subkind" - where subcharacter kinds are '_' character,
 /// lowerspace characters and uppercase characters.
@@ -299,6 +312,24 @@ pub fn next_word_end(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint
     })
 }
 
+/// Returns a position of the next word boundary, where a word character is defined as either
+/// uppercase letter, lowercase letter, '_' character, language-specific word character (like '-' in CSS) or newline.
+pub fn next_word_end_or_newline(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
+    let raw_point = point.to_point(map);
+    let classifier = map.buffer_snapshot.char_classifier_at(raw_point);
+
+    let mut on_starting_row = true;
+    find_boundary(map, point, FindRange::MultiLine, |left, right| {
+        if left == '\n' {
+            on_starting_row = false;
+        }
+        (classifier.kind(left) != classifier.kind(right)
+            && ((on_starting_row && !left.is_whitespace())
+                || (!on_starting_row && !right.is_whitespace())))
+            || right == '\n'
+    })
+}
+
 /// Returns a position of the next subword boundary, where a subword is defined as a run of
 /// word characters of the same "subkind" - where subcharacter kinds are '_' character,
 /// lowerspace characters and uppercase characters.