From e697cf97475887ac67bf42b3e67395f4a332c8c9 Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Wed, 30 Apr 2025 21:44:20 +0530 Subject: [PATCH] editor: Fix edit range for linked edits on do completion (#29650) Closes #29544 Fixes an issue where accepting an HTML completion would correctly edit the start tag but incorrectly update the end tag due to incorrect linked edit ranges. I want to handle multi cursor case (as it barely works now), but seems like this should go first. As, it might need whole `do_completions` overhaul. Todo: - [x] Tests for completion aceept on linked edits Before: https://github.com/user-attachments/assets/917f8d2a-4a0f-46e8-a004-675fde55fe3d After: https://github.com/user-attachments/assets/84b760b6-a5b9-45c4-85d8-b5dccf97775f Release Notes: - Fixes an issue where accepting an HTML completion would correctly edit the start tag but incorrectly update the end tag. --- crates/editor/src/editor.rs | 6 +- crates/editor/src/editor_tests.rs | 141 ++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d3e89df3fc094866c5c9f92962b04ca9856b6131..3a98e0241900da2bc374ae4bcdd6490bf9db5a89 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5005,11 +5005,11 @@ impl Editor { range }; - ranges.push(range); + ranges.push(range.clone()); if !self.linked_edit_ranges.is_empty() { - let start_anchor = snapshot.anchor_before(selection.head()); - let end_anchor = snapshot.anchor_after(selection.tail()); + let start_anchor = snapshot.anchor_before(range.start); + let end_anchor = snapshot.anchor_after(range.end); if let Some(ranges) = self .linked_editing_ranges_for(start_anchor.text_anchor..end_anchor.text_anchor, cx) { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 739233ea8d6a84fb86377cc754d0f6b43b0b2b7f..1baf7c39cbb8a4d6956210e751af498940fcd6e2 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -1,6 +1,7 @@ use super::*; use crate::{ JoinLines, + linked_editing_ranges::LinkedEditingRanges, scroll::scroll_amount::ScrollAmount, test::{ assert_text_with_selections, build_editor, @@ -19559,6 +19560,146 @@ async fn test_hide_mouse_context_menu_on_modal_opened(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_html_linked_edits_on_completion(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.html"), Default::default()) + .await; + + let project = Project::test(fs, [path!("/").as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + let html_language = Arc::new(Language::new( + LanguageConfig { + name: "HTML".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["html".to_string()], + ..LanguageMatcher::default() + }, + brackets: BracketPairConfig { + pairs: vec![BracketPair { + start: "<".into(), + end: ">".into(), + close: true, + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }, + Some(tree_sitter_html::LANGUAGE.into()), + )); + language_registry.add(html_language); + let mut fake_servers = language_registry.register_fake_lsp( + "HTML", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + resolve_provider: Some(true), + ..Default::default() + }), + ..Default::default() + }, + ..Default::default() + }, + ); + + let workspace = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + + let worktree_id = workspace + .update(cx, |workspace, _window, cx| { + workspace.project().update(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }) + }) + .unwrap(); + project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/file.html"), cx) + }) + .await + .unwrap(); + let editor = workspace + .update(cx, |workspace, window, cx| { + workspace.open_path((worktree_id, "file.html"), None, true, window, cx) + }) + .unwrap() + .await + .unwrap() + .downcast::() + .unwrap(); + + let fake_server = fake_servers.next().await.unwrap(); + editor.update_in(cx, |editor, window, cx| { + editor.set_text("", window, cx); + editor.change_selections(None, window, cx, |selections| { + selections.select_ranges([Point::new(0, 3)..Point::new(0, 3)]); + }); + let Some((buffer, _)) = editor + .buffer + .read(cx) + .text_anchor_for_position(editor.selections.newest_anchor().start, cx) + else { + panic!("Failed to get buffer for selection position"); + }; + let buffer = buffer.read(cx); + let buffer_id = buffer.remote_id(); + let opening_range = + buffer.anchor_before(Point::new(0, 1))..buffer.anchor_after(Point::new(0, 3)); + let closing_range = + buffer.anchor_before(Point::new(0, 6))..buffer.anchor_after(Point::new(0, 8)); + let mut linked_ranges = HashMap::default(); + linked_ranges.insert( + buffer_id, + vec![(opening_range.clone(), vec![closing_range.clone()])], + ); + editor.linked_edit_ranges = LinkedEditingRanges(linked_ranges); + }); + let mut completion_handle = + fake_server.set_request_handler::(move |_, _| async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + lsp::CompletionItem { + label: "head".to_string(), + text_edit: Some(lsp::CompletionTextEdit::InsertAndReplace( + lsp::InsertReplaceEdit { + new_text: "head".to_string(), + insert: lsp::Range::new( + lsp::Position::new(0, 1), + lsp::Position::new(0, 3), + ), + replace: lsp::Range::new( + lsp::Position::new(0, 1), + lsp::Position::new(0, 3), + ), + }, + )), + ..Default::default() + }, + ]))) + }); + editor.update_in(cx, |editor, window, cx| { + editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + }); + cx.run_until_parked(); + completion_handle.next().await.unwrap(); + editor.update(cx, |editor, _| { + assert!( + editor.context_menu_visible(), + "Completion menu should be visible" + ); + }); + editor.update_in(cx, |editor, window, cx| { + editor.confirm_completion(&ConfirmCompletion::default(), window, cx) + }); + cx.executor().run_until_parked(); + editor.update(cx, |editor, cx| { + assert_eq!(editor.text(cx), ""); + }); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(DisplayRow(row as u32), column as u32); point..point