Do not perform OnTypeFormating after pair brace insert (#2672)

Kirill Bulatov created 2 years ago

Closes
https://linear.app/zed-industries/issue/Z-2358/ra-brace-auto-surround-causes-duplicate-end-char-with-selection

Release Notes:

- Fixed a bug when duplicate brace appeared after selected text got
surrounded with braces

Change summary

crates/editor/src/editor.rs       |   6 +
crates/editor/src/editor_tests.rs | 105 +++++++++++++++++++++++++++++++++
2 files changed, 109 insertions(+), 2 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -2026,6 +2026,7 @@ impl Editor {
         }
 
         let selections = self.selections.all_adjusted(cx);
+        let mut brace_inserted = false;
         let mut edits = Vec::new();
         let mut new_selections = Vec::with_capacity(selections.len());
         let mut new_autoclose_regions = Vec::new();
@@ -2084,6 +2085,7 @@ impl Editor {
                                     selection.range(),
                                     format!("{}{}", text, bracket_pair.end).into(),
                                 ));
+                                brace_inserted = true;
                                 continue;
                             }
                         }
@@ -2110,6 +2112,7 @@ impl Editor {
                             selection.end..selection.end,
                             bracket_pair.end.as_str().into(),
                         ));
+                        brace_inserted = true;
                         new_selections.push((
                             Selection {
                                 id: selection.id,
@@ -2177,8 +2180,7 @@ impl Editor {
             let had_active_copilot_suggestion = this.has_active_copilot_suggestion(cx);
             this.change_selections(Some(Autoscroll::fit()), cx, |s| s.select(new_selections));
 
-            // When buffer contents is updated and caret is moved, try triggering on type formatting.
-            if settings::get::<EditorSettings>(cx).use_on_type_format {
+            if !brace_inserted && settings::get::<EditorSettings>(cx).use_on_type_format {
                 if let Some(on_type_format_task) =
                     this.trigger_on_type_formatting(text.to_string(), cx)
                 {

crates/editor/src/editor_tests.rs

@@ -6979,6 +6979,111 @@ async fn test_copilot_disabled_globs(
     assert!(copilot_requests.try_next().is_ok());
 }
 
+#[gpui::test]
+async fn test_on_type_formatting_not_triggered(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+
+    let mut language = Language::new(
+        LanguageConfig {
+            name: "Rust".into(),
+            path_suffixes: vec!["rs".to_string()],
+            brackets: BracketPairConfig {
+                pairs: vec![BracketPair {
+                    start: "{".to_string(),
+                    end: "}".to_string(),
+                    close: true,
+                    newline: true,
+                }],
+                disabled_scopes_by_bracket_ix: Vec::new(),
+            },
+            ..Default::default()
+        },
+        Some(tree_sitter_rust::language()),
+    );
+    let mut fake_servers = language
+        .set_fake_lsp_adapter(Arc::new(FakeLspAdapter {
+            capabilities: lsp::ServerCapabilities {
+                document_on_type_formatting_provider: Some(lsp::DocumentOnTypeFormattingOptions {
+                    first_trigger_character: "{".to_string(),
+                    more_trigger_character: None,
+                }),
+                ..Default::default()
+            },
+            ..Default::default()
+        }))
+        .await;
+
+    let fs = FakeFs::new(cx.background());
+    fs.insert_tree(
+        "/a",
+        json!({
+            "main.rs": "fn main() { let a = 5; }",
+            "other.rs": "// Test file",
+        }),
+    )
+    .await;
+    let project = Project::test(fs, ["/a".as_ref()], cx).await;
+    project.update(cx, |project, _| project.languages().add(Arc::new(language)));
+    let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+    let worktree_id = workspace.update(cx, |workspace, cx| {
+        workspace.project().read_with(cx, |project, cx| {
+            project.worktrees(cx).next().unwrap().read(cx).id()
+        })
+    });
+
+    let buffer = project
+        .update(cx, |project, cx| {
+            project.open_local_buffer("/a/main.rs", cx)
+        })
+        .await
+        .unwrap();
+    cx.foreground().run_until_parked();
+    cx.foreground().start_waiting();
+    let fake_server = fake_servers.next().await.unwrap();
+    let editor_handle = workspace
+        .update(cx, |workspace, cx| {
+            workspace.open_path((worktree_id, "main.rs"), None, true, cx)
+        })
+        .await
+        .unwrap()
+        .downcast::<Editor>()
+        .unwrap();
+
+    fake_server.handle_request::<lsp::request::OnTypeFormatting, _, _>(|params, _| async move {
+        assert_eq!(
+            params.text_document_position.text_document.uri,
+            lsp::Url::from_file_path("/a/main.rs").unwrap(),
+        );
+        assert_eq!(
+            params.text_document_position.position,
+            lsp::Position::new(0, 21),
+        );
+
+        Ok(Some(vec![lsp::TextEdit {
+            new_text: "]".to_string(),
+            range: lsp::Range::new(lsp::Position::new(0, 22), lsp::Position::new(0, 22)),
+        }]))
+    });
+
+    editor_handle.update(cx, |editor, cx| {
+        cx.focus(&editor_handle);
+        editor.change_selections(None, cx, |s| {
+            s.select_ranges([Point::new(0, 21)..Point::new(0, 20)])
+        });
+        editor.handle_input("{", cx);
+    });
+
+    cx.foreground().run_until_parked();
+
+    buffer.read_with(cx, |buffer, _| {
+        assert_eq!(
+            buffer.text(),
+            "fn main() { let a = {5}; }",
+            "No extra braces from on type formatting should appear in the buffer"
+        )
+    });
+}
+
 fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
     let point = DisplayPoint::new(row as u32, column as u32);
     point..point