editor: Fix DeleteToPreviousSubwordStart and DeleteToNextSubwordEnd interaction with newlines (#46235)

Ruben Fricke created

Closes #40110

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

These modifications align the subword commands with their word
counterparts and with other popular editors like VSCode and Sublime.

Related to: https://github.com/zed-industries/zed/pull/16848

  Release Notes:

- Improved `DeleteToPreviousSubwordStart` and `DeleteToNextSubwordEnd`
interactions around newlines. You can opt-in into the previous behavior
by adding `{"ignore_newlines": true}` to either action's binds in your
keymap.

  ---

This is my first contribution to Zed! If anything should be done
differently, please let me know. Happy to learn :)

Change summary

crates/editor/src/actions.rs      | 30 +++++++++-
crates/editor/src/editor.rs       | 32 +++++++++---
crates/editor/src/editor_tests.rs | 86 +++++++++++++++++++++++++++++++++
crates/editor/src/movement.rs     | 46 ++++++++++++++++-
4 files changed, 179 insertions(+), 15 deletions(-)

Detailed changes

crates/editor/src/actions.rs 🔗

@@ -244,6 +244,32 @@ pub struct DeleteToPreviousWordStart {
     pub ignore_brackets: bool,
 }
 
+/// Deletes from the cursor to the end of the next subword.
+/// Stops before the end of the next subword, if whitespace sequences of length >= 2 are encountered.
+#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema, Action)]
+#[action(namespace = editor)]
+#[serde(deny_unknown_fields)]
+pub struct DeleteToNextSubwordEnd {
+    #[serde(default)]
+    pub ignore_newlines: bool,
+    // Whether to stop before the start of the previous word, if language-defined bracket is encountered.
+    #[serde(default)]
+    pub ignore_brackets: bool,
+}
+
+/// Deletes from the cursor to the start of the previous subword.
+/// Stops before the start of the previous subword, if whitespace sequences of length >= 2 are encountered.
+#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema, Action)]
+#[action(namespace = editor)]
+#[serde(deny_unknown_fields)]
+pub struct DeleteToPreviousSubwordStart {
+    #[serde(default)]
+    pub ignore_newlines: bool,
+    // Whether to stop before the start of the previous word, if language-defined bracket is encountered.
+    #[serde(default)]
+    pub ignore_brackets: bool,
+}
+
 /// Cuts from cursor to end of line.
 #[derive(PartialEq, Clone, Deserialize, Default, JsonSchema, Action)]
 #[action(namespace = editor)]
@@ -450,10 +476,6 @@ actions!(
         DeleteLine,
         /// Deletes from cursor to end of line.
         DeleteToEndOfLine,
-        /// Deletes to the end of the next subword.
-        DeleteToNextSubwordEnd,
-        /// Deletes to the start of the previous subword.
-        DeleteToPreviousSubwordStart,
         /// Diffs the text stored in the clipboard against the current selection.
         DiffClipboardWithSelection,
         /// Displays names of all active cursors.

crates/editor/src/editor.rs 🔗

@@ -14158,7 +14158,7 @@ impl Editor {
 
     pub fn delete_to_previous_subword_start(
         &mut self,
-        _: &DeleteToPreviousSubwordStart,
+        action: &DeleteToPreviousSubwordStart,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
@@ -14168,9 +14168,17 @@ impl Editor {
             this.change_selections(Default::default(), window, cx, |s| {
                 s.move_with(|map, selection| {
                     if selection.is_empty() {
-                        let mut cursor = movement::previous_subword_start(map, selection.head());
-                        cursor =
-                            movement::adjust_greedy_deletion(map, selection.head(), cursor, false);
+                        let mut cursor = if action.ignore_newlines {
+                            movement::previous_subword_start(map, selection.head())
+                        } else {
+                            movement::previous_subword_start_or_newline(map, selection.head())
+                        };
+                        cursor = movement::adjust_greedy_deletion(
+                            map,
+                            selection.head(),
+                            cursor,
+                            action.ignore_brackets,
+                        );
                         selection.set_head(cursor, SelectionGoal::None);
                     }
                 });
@@ -14267,7 +14275,7 @@ impl Editor {
 
     pub fn delete_to_next_subword_end(
         &mut self,
-        _: &DeleteToNextSubwordEnd,
+        action: &DeleteToNextSubwordEnd,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
@@ -14276,9 +14284,17 @@ impl Editor {
             this.change_selections(Default::default(), window, cx, |s| {
                 s.move_with(|map, selection| {
                     if selection.is_empty() {
-                        let mut cursor = movement::next_subword_end(map, selection.head());
-                        cursor =
-                            movement::adjust_greedy_deletion(map, selection.head(), cursor, false);
+                        let mut cursor = if action.ignore_newlines {
+                            movement::next_subword_end(map, selection.head())
+                        } else {
+                            movement::next_subword_end_or_newline(map, selection.head())
+                        };
+                        cursor = movement::adjust_greedy_deletion(
+                            map,
+                            selection.head(),
+                            cursor,
+                            action.ignore_brackets,
+                        );
                         selection.set_head(cursor, SelectionGoal::None);
                     }
                 });

crates/editor/src/editor_tests.rs 🔗

@@ -3028,6 +3028,48 @@ fn test_delete_to_previous_word_start_or_newline(cx: &mut TestAppContext) {
     });
 }
 
+#[gpui::test]
+fn test_delete_to_previous_subword_start_or_newline(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let editor = cx.add_window(|window, cx| {
+        let buffer = MultiBuffer::build_simple("fooBar\n\nbazQux", cx);
+        build_editor(buffer, window, cx)
+    });
+    let del_to_prev_sub_word_start = DeleteToPreviousSubwordStart {
+        ignore_newlines: false,
+        ignore_brackets: false,
+    };
+    let del_to_prev_sub_word_start_ignore_newlines = DeleteToPreviousSubwordStart {
+        ignore_newlines: true,
+        ignore_brackets: false,
+    };
+
+    _ = editor.update(cx, |editor, window, cx| {
+        editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
+            s.select_display_ranges([
+                DisplayPoint::new(DisplayRow(2), 6)..DisplayPoint::new(DisplayRow(2), 6)
+            ])
+        });
+        editor.delete_to_previous_subword_start(&del_to_prev_sub_word_start, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "fooBar\n\nbaz");
+        editor.delete_to_previous_subword_start(&del_to_prev_sub_word_start, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "fooBar\n\n");
+        editor.delete_to_previous_subword_start(&del_to_prev_sub_word_start, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "fooBar\n");
+        editor.delete_to_previous_subword_start(&del_to_prev_sub_word_start, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "fooBar");
+        editor.delete_to_previous_subword_start(&del_to_prev_sub_word_start, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "foo");
+        editor.delete_to_previous_subword_start(
+            &del_to_prev_sub_word_start_ignore_newlines,
+            window,
+            cx,
+        );
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "");
+    });
+}
+
 #[gpui::test]
 fn test_delete_to_next_word_end_or_newline(cx: &mut TestAppContext) {
     init_test(cx, |_| {});
@@ -3077,6 +3119,50 @@ fn test_delete_to_next_word_end_or_newline(cx: &mut TestAppContext) {
     });
 }
 
+#[gpui::test]
+fn test_delete_to_next_subword_end_or_newline(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let editor = cx.add_window(|window, cx| {
+        let buffer = MultiBuffer::build_simple("\nfooBar\n   bazQux", cx);
+        build_editor(buffer, window, cx)
+    });
+    let del_to_next_subword_end = DeleteToNextSubwordEnd {
+        ignore_newlines: false,
+        ignore_brackets: false,
+    };
+    let del_to_next_subword_end_ignore_newlines = DeleteToNextSubwordEnd {
+        ignore_newlines: true,
+        ignore_brackets: false,
+    };
+
+    _ = editor.update(cx, |editor, window, cx| {
+        editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
+            s.select_display_ranges([
+                DisplayPoint::new(DisplayRow(0), 0)..DisplayPoint::new(DisplayRow(0), 0)
+            ])
+        });
+        // Delete "\n" (empty line)
+        editor.delete_to_next_subword_end(&del_to_next_subword_end, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "fooBar\n   bazQux");
+        // Delete "foo" (subword boundary)
+        editor.delete_to_next_subword_end(&del_to_next_subword_end, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "Bar\n   bazQux");
+        // Delete "Bar"
+        editor.delete_to_next_subword_end(&del_to_next_subword_end, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "\n   bazQux");
+        // Delete "\n   " (newline + leading whitespace)
+        editor.delete_to_next_subword_end(&del_to_next_subword_end, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "bazQux");
+        // Delete "baz" (subword boundary)
+        editor.delete_to_next_subword_end(&del_to_next_subword_end, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "Qux");
+        // With ignore_newlines, delete "Qux"
+        editor.delete_to_next_subword_end(&del_to_next_subword_end_ignore_newlines, window, cx);
+        assert_eq!(editor.buffer.read(cx).read(cx).text(), "");
+    });
+}
+
 #[gpui::test]
 fn test_newline(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/editor/src/movement.rs 🔗

@@ -412,6 +412,21 @@ pub fn previous_subword_start(map: &DisplaySnapshot, point: DisplayPoint) -> Dis
     })
 }
 
+/// 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 or newline.
+pub fn previous_subword_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| {
+        (is_subword_start(left, right, &classifier)) || left == '\n' || right == '\n'
+    })
+}
+
 pub fn is_subword_start(left: char, right: char, classifier: &CharClassifier) -> bool {
     let is_word_start = classifier.kind(left) != classifier.kind(right) && !right.is_whitespace();
     let is_subword_start = classifier.is_word('-') && left == '-' && right != '-'
@@ -473,13 +488,38 @@ pub fn next_subword_end(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPo
     })
 }
 
+/// 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 or newline.
+pub fn next_subword_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)
+            || is_subword_boundary_end(left, right, &classifier))
+            && ((on_starting_row && !left.is_whitespace())
+                || (!on_starting_row && !right.is_whitespace())))
+            || right == '\n'
+    })
+}
+
 pub fn is_subword_end(left: char, right: char, classifier: &CharClassifier) -> bool {
     let is_word_end =
         (classifier.kind(left) != classifier.kind(right)) && !classifier.is_whitespace(left);
-    let is_subword_end = classifier.is_word('-') && left != '-' && right == '-'
+    is_word_end || is_subword_boundary_end(left, right, classifier)
+}
+
+/// Returns true if the transition from `left` to `right` is a subword boundary,
+/// such as case changes, underscores, or dashes. Does not include word boundaries like whitespace.
+fn is_subword_boundary_end(left: char, right: char, classifier: &CharClassifier) -> bool {
+    classifier.is_word('-') && left != '-' && right == '-'
         || left != '_' && right == '_'
-        || left.is_lowercase() && right.is_uppercase();
-    is_word_end || is_subword_end
+        || left.is_lowercase() && right.is_uppercase()
 }
 
 /// Returns a position of the start of the current paragraph, where a paragraph