Add newline above and improve newline below

Add a new action for inserting a new line above the current line. @ForLoveOfCats  also helped fix a bug among other things. When two collaborators had their cursors at the end of a line, and one collaborator performed a newline below action, the second collaborator's cursor would be dragged to the new line. This is also fixing that.

Co-Authored-By: Julia <30666851+ForLoveOfCats@users.noreply.github.com>

Cargo.lock

@@ -1213,6 +1213,7 @@ dependencies = [
  "git",
  "gpui",
  "hyper",
+ "indoc",
  "language",
  "lazy_static",
  "lipsum",

assets/keymaps/default.json

@@ -163,6 +163,7 @@
         "context": "Editor && mode == full",
         "bindings": {
             "enter": "editor::Newline",
+            "cmd-shift-enter": "editor::NewlineAbove",
             "cmd-enter": "editor::NewlineBelow",
             "alt-z": "editor::ToggleSoftWrap",
             "cmd-f": [

crates/collab/Cargo.toml

@@ -55,6 +55,7 @@ toml = "0.5.8"
 tracing = "0.1.34"
 tracing-log = "0.1.3"
 tracing-subscriber = { version = "0.3.11", features = ["env-filter", "json"] }
+indoc = "1.0.4"
 
 [dev-dependencies]
 collections = { path = "../collections", features = ["test-support"] }

crates/collab/src/tests/integration_tests.rs

@@ -6,8 +6,9 @@ use call::{room, ActiveCall, ParticipantLocation, Room};
 use client::{User, RECEIVE_TIMEOUT};
 use collections::HashSet;
 use editor::{
-    ConfirmCodeAction, ConfirmCompletion, ConfirmRename, Editor, ExcerptRange, MultiBuffer, Redo,
+    test::editor_test_context::EditorTestContext, ConfirmCodeAction, ConfirmCompletion,
-    Rename, ToOffset, ToggleCodeActions, Undo,
+    ConfirmRename, Editor, ExcerptRange, MultiBuffer, Redo, Rename, ToOffset, ToggleCodeActions,
+    Undo,
 };
 use fs::{FakeFs, Fs as _, LineEnding, RemoveOptions};
 use futures::StreamExt as _;
@@ -15,6 +16,7 @@ use gpui::{
     executor::Deterministic, geometry::vector::vec2f, test::EmptyView, ModelHandle, TestAppContext,
     ViewHandle,
 };
+use indoc::indoc;
 use language::{
     tree_sitter_rust, Anchor, Diagnostic, DiagnosticEntry, FakeLspAdapter, Language,
     LanguageConfig, OffsetRangeExt, Point, Rope,
@@ -3042,6 +3044,104 @@ async fn test_editing_while_guest_opens_buffer(
     buffer_b.read_with(cx_b, |buf, _| assert_eq!(buf.text(), text));
 }
 
+#[gpui::test]
+async fn test_newline_above_or_below_does_not_move_guest_cursor(
+    deterministic: Arc<Deterministic>,
+    cx_a: &mut TestAppContext,
+    cx_b: &mut TestAppContext,
+) {
+    deterministic.forbid_parking();
+    let mut server = TestServer::start(&deterministic).await;
+    let client_a = server.create_client(cx_a, "user_a").await;
+    let client_b = server.create_client(cx_b, "user_b").await;
+    server
+        .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
+        .await;
+    let active_call_a = cx_a.read(ActiveCall::global);
+
+    client_a
+        .fs
+        .insert_tree("/dir", json!({ "a.txt": "Some text\n" }))
+        .await;
+    let (project_a, worktree_id) = client_a.build_local_project("/dir", cx_a).await;
+    let project_id = active_call_a
+        .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
+        .await
+        .unwrap();
+
+    let project_b = client_b.build_remote_project(project_id, cx_b).await;
+
+    // Open a buffer as client A
+    let buffer_a = project_a
+        .update(cx_a, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
+        .await
+        .unwrap();
+    let (_, window_a) = cx_a.add_window(|_| EmptyView);
+    let editor_a = cx_a.add_view(&window_a, |cx| {
+        Editor::for_buffer(buffer_a, Some(project_a), cx)
+    });
+    let mut editor_cx_a = EditorTestContext {
+        cx: cx_a,
+        window_id: window_a.id(),
+        editor: editor_a,
+    };
+
+    // Open a buffer as client B
+    let buffer_b = project_b
+        .update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
+        .await
+        .unwrap();
+    let (_, window_b) = cx_b.add_window(|_| EmptyView);
+    let editor_b = cx_b.add_view(&window_b, |cx| {
+        Editor::for_buffer(buffer_b, Some(project_b), cx)
+    });
+    let mut editor_cx_b = EditorTestContext {
+        cx: cx_b,
+        window_id: window_b.id(),
+        editor: editor_b,
+    };
+
+    // Test newline above
+    editor_cx_a.set_selections_state(indoc! {"
+        Some textˇ
+    "});
+    editor_cx_b.set_selections_state(indoc! {"
+        Some textˇ
+    "});
+    editor_cx_a.update_editor(|editor, cx| editor.newline_above(&editor::NewlineAbove, cx));
+    deterministic.run_until_parked();
+    editor_cx_a.assert_editor_state(indoc! {"
+        ˇ
+        Some text
+    "});
+    editor_cx_b.assert_editor_state(indoc! {"
+
+        Some textˇ
+    "});
+
+    // Test newline below
+    editor_cx_a.set_selections_state(indoc! {"
+
+        Some textˇ
+    "});
+    editor_cx_b.set_selections_state(indoc! {"
+
+        Some textˇ
+    "});
+    editor_cx_a.update_editor(|editor, cx| editor.newline_below(&editor::NewlineBelow, cx));
+    deterministic.run_until_parked();
+    editor_cx_a.assert_editor_state(indoc! {"
+
+        Some text
+        ˇ
+    "});
+    editor_cx_b.assert_editor_state(indoc! {"
+
+        Some textˇ
+
+    "});
+}
+
 #[gpui::test(iterations = 10)]
 async fn test_leaving_worktree_while_opening_buffer(
     deterministic: Arc<Deterministic>,

crates/editor/src/editor.rs

@@ -184,6 +184,7 @@ actions!(
         Backspace,
         Delete,
         Newline,
+        NewlineAbove,
         NewlineBelow,
         GoToDiagnostic,
         GoToPrevDiagnostic,
@@ -301,6 +302,7 @@ pub fn init(cx: &mut AppContext) {
     cx.add_action(Editor::select);
     cx.add_action(Editor::cancel);
     cx.add_action(Editor::newline);
+    cx.add_action(Editor::newline_above);
     cx.add_action(Editor::newline_below);
     cx.add_action(Editor::backspace);
     cx.add_action(Editor::delete);
@@ -2118,7 +2120,7 @@ impl Editor {
         });
     }
 
-    pub fn newline_below(&mut self, _: &NewlineBelow, cx: &mut ViewContext<Self>) {
+    pub fn newline_above(&mut self, _: &NewlineAbove, cx: &mut ViewContext<Self>) {
         let buffer = self.buffer.read(cx);
         let snapshot = buffer.snapshot(cx);
 
@@ -2130,19 +2132,17 @@ impl Editor {
             let cursor = selection.head();
             let row = cursor.row;
 
-            let end_of_line = snapshot
+            let start_of_line = snapshot.clip_point(Point::new(row, 0), Bias::Left);
-                .clip_point(Point::new(row, snapshot.line_len(row)), Bias::Left)
-                .to_point(&snapshot);
 
             let newline = "\n".to_string();
-            edits.push((end_of_line..end_of_line, newline));
+            edits.push((start_of_line..start_of_line, newline));
 
-            rows_inserted += 1;
             rows.push(row + rows_inserted);
+            rows_inserted += 1;
         }
 
         self.transact(cx, |editor, cx| {
-            editor.edit_with_autoindent(edits, cx);
+            editor.edit(edits, cx);
 
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 let mut index = 0;
@@ -2157,6 +2157,85 @@ impl Editor {
                     (clipped, SelectionGoal::None)
                 });
             });
+
+            let mut indent_edits = Vec::new();
+            let multibuffer_snapshot = editor.buffer.read(cx).snapshot(cx);
+            for row in rows {
+                let indents = multibuffer_snapshot.suggested_indents(row..row + 1, cx);
+                for (row, indent) in indents {
+                    if indent.len == 0 {
+                        continue;
+                    }
+
+                    let text = match indent.kind {
+                        IndentKind::Space => " ".repeat(indent.len as usize),
+                        IndentKind::Tab => "\t".repeat(indent.len as usize),
+                    };
+                    let point = Point::new(row, 0);
+                    indent_edits.push((point..point, text));
+                }
+            }
+            editor.edit(indent_edits, cx);
+        });
+    }
+
+    pub fn newline_below(&mut self, _: &NewlineBelow, cx: &mut ViewContext<Self>) {
+        let buffer = self.buffer.read(cx);
+        let snapshot = buffer.snapshot(cx);
+
+        let mut edits = Vec::new();
+        let mut rows = Vec::new();
+        let mut rows_inserted = 0;
+
+        for selection in self.selections.all_adjusted(cx) {
+            let cursor = selection.head();
+            let row = cursor.row;
+
+            let point = Point::new(row + 1, 0);
+            let start_of_line = snapshot.clip_point(point, Bias::Left);
+
+            let newline = "\n".to_string();
+            edits.push((start_of_line..start_of_line, newline));
+
+            rows_inserted += 1;
+            rows.push(row + rows_inserted);
+        }
+
+        self.transact(cx, |editor, cx| {
+            editor.edit(edits, cx);
+
+            editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                let mut index = 0;
+                s.move_cursors_with(|map, _, _| {
+                    let row = rows[index];
+                    index += 1;
+
+                    let point = Point::new(row, 0);
+                    let boundary = map.next_line_boundary(point).1;
+                    let clipped = map.clip_point(boundary, Bias::Left);
+
+                    (clipped, SelectionGoal::None)
+                });
+            });
+
+            let mut indent_edits = Vec::new();
+            let multibuffer_snapshot = editor.buffer.read(cx).snapshot(cx);
+            for row in rows {
+                let indents = multibuffer_snapshot.suggested_indents(row..row + 1, cx);
+                for (row, indent) in indents {
+                    if indent.len == 0 {
+                        continue;
+                    }
+
+                    let text = match indent.kind {
+                        IndentKind::Space => " ".repeat(indent.len as usize),
+                        IndentKind::Tab => "\t".repeat(indent.len as usize),
+                    };
+                    let point = Point::new(row, 0);
+                    indent_edits.push((point..point, text));
+                }
+            }
+            editor.edit(indent_edits, cx);
         });
     }
 

crates/editor/src/editor_tests.rs

@@ -1467,6 +1467,55 @@ fn test_newline_with_old_selections(cx: &mut gpui::AppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_newline_above(cx: &mut gpui::TestAppContext) {
+    let mut cx = EditorTestContext::new(cx);
+    cx.update(|cx| {
+        cx.update_global::<Settings, _, _>(|settings, _| {
+            settings.editor_overrides.tab_size = Some(NonZeroU32::new(4).unwrap());
+        });
+    });
+
+    let language = Arc::new(
+        Language::new(
+            LanguageConfig::default(),
+            Some(tree_sitter_rust::language()),
+        )
+        .with_indents_query(r#"(_ "(" ")" @end) @indent"#)
+        .unwrap(),
+    );
+    cx.update_buffer(|buffer, cx| buffer.set_language(Some(language), cx));
+
+    cx.set_state(indoc! {"
+        const a: ˇA = (
+            (ˇ
+                «const_functionˇ»(ˇ),
+                so«mˇ»et«hˇ»ing_ˇelse,ˇ
+            )ˇ
+        ˇ);ˇ
+    "});
+    cx.update_editor(|e, cx| e.newline_above(&NewlineAbove, cx));
+    cx.assert_editor_state(indoc! {"
+        ˇ
+        const a: A = (
+            ˇ
+            (
+                ˇ
+                ˇ
+                const_function(),
+                ˇ
+                ˇ
+                ˇ
+                ˇ
+                something_else,
+                ˇ
+            )
+            ˇ
+            ˇ
+        );
+    "});
+}
+
 #[gpui::test]
 async fn test_newline_below(cx: &mut gpui::TestAppContext) {
     let mut cx = EditorTestContext::new(cx);

crates/editor/src/test/editor_test_context.rs

@@ -167,7 +167,7 @@ impl<'a> EditorTestContext<'a> {
     ///
     /// See the `util::test::marked_text_ranges` function for more information.
     pub fn set_state(&mut self, marked_text: &str) -> ContextHandle {
-        let _state_context = self.add_assertion_context(format!(
+        let state_context = self.add_assertion_context(format!(
             "Initial Editor State: \"{}\"",
             marked_text.escape_debug().to_string()
         ));
@@ -178,7 +178,22 @@ impl<'a> EditorTestContext<'a> {
                 s.select_ranges(selection_ranges)
             })
         });
-        _state_context
+        state_context
+    }
+
+    /// Only change the editor's selections
+    pub fn set_selections_state(&mut self, marked_text: &str) -> ContextHandle {
+        let state_context = self.add_assertion_context(format!(
+            "Initial Editor State: \"{}\"",
+            marked_text.escape_debug().to_string()
+        ));
+        let (_, selection_ranges) = marked_text_ranges(marked_text, true);
+        self.editor.update(self.cx, |editor, cx| {
+            editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                s.select_ranges(selection_ranges)
+            })
+        });
+        state_context
     }
 
     /// Make an assertion about the editor's text and the ranges and directions
@@ -189,10 +204,11 @@ impl<'a> EditorTestContext<'a> {
     pub fn assert_editor_state(&mut self, marked_text: &str) {
         let (unmarked_text, expected_selections) = marked_text_ranges(marked_text, true);
         let buffer_text = self.buffer_text();
-        assert_eq!(
+
-            buffer_text, unmarked_text,
+        if buffer_text != unmarked_text {
-            "Unmarked text doesn't match buffer text"
+            panic!("Unmarked text doesn't match buffer text\nBuffer text: {buffer_text:?}\nUnmarked text: {unmarked_text:?}\nRaw buffer text\n{buffer_text}Raw unmarked text\n{unmarked_text}");
-        );
+        }
+
         self.assert_selections(expected_selections, marked_text.to_string())
     }