From f9c60b98c040a49aa5c528e405187bd0c1dc3c3f Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Wed, 19 Apr 2023 19:57:23 +0300 Subject: [PATCH] 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 | 1 + assets/keymaps/default.json | 1 + crates/collab/Cargo.toml | 1 + crates/collab/src/tests/integration_tests.rs | 104 +++++++++++++++++- crates/editor/src/editor.rs | 93 ++++++++++++++-- crates/editor/src/editor_tests.rs | 49 +++++++++ crates/editor/src/test/editor_test_context.rs | 32 ++++-- 7 files changed, 264 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 365f383b48..6f05512b76 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1213,6 +1213,7 @@ dependencies = [ "git", "gpui", "hyper", + "indoc", "language", "lazy_static", "lipsum", diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 14e3056e6a..f570f20bc5 100644 --- a/assets/keymaps/default.json +++ b/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": [ diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 2891fe3010..58fc602c94 100644 --- a/crates/collab/Cargo.toml +++ b/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"] } diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index eb10640d59..092fdddb96 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/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, - Rename, ToOffset, ToggleCodeActions, Undo, + test::editor_test_context::EditorTestContext, ConfirmCodeAction, ConfirmCompletion, + 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, + 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, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b3f3723bc1..48940c3075 100644 --- a/crates/editor/src/editor.rs +++ b/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) { + pub fn newline_above(&mut self, _: &NewlineAbove, cx: &mut ViewContext) { 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 - .clip_point(Point::new(row, snapshot.line_len(row)), Bias::Left) - .to_point(&snapshot); + let start_of_line = snapshot.clip_point(Point::new(row, 0), Bias::Left); 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) { + 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); }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 9cf3c4a81e..ce293ed064 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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.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); diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 9e66fea8df..ccd0e936a4 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/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, - "Unmarked text doesn't match buffer text" - ); + + if buffer_text != unmarked_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()) } @@ -254,10 +270,10 @@ impl<'a> EditorTestContext<'a> { panic!( indoc! {" {}Editor has unexpected selections. - + Expected selections: {} - + Actual selections: {} "},