diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 5770d0cf1a..c9353a8e45 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8437,6 +8437,247 @@ async fn test_completion(cx: &mut gpui::TestAppContext) { apply_additional_edits.await.unwrap(); } +#[gpui::test] +async fn test_multiline_completion(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/a", + json!({ + "main.ts": "a", + }), + ) + .await; + + let project = Project::test(fs, ["/a".as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + let typescript_language = Arc::new(Language::new( + LanguageConfig { + name: "TypeScript".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["ts".to_string()], + ..LanguageMatcher::default() + }, + line_comments: vec!["// ".into()], + ..LanguageConfig::default() + }, + Some(tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into()), + )); + language_registry.add(typescript_language.clone()); + let mut fake_servers = language_registry.register_fake_lsp( + "TypeScript", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string(), ":".to_string()]), + ..lsp::CompletionOptions::default() + }), + signature_help_provider: Some(lsp::SignatureHelpOptions::default()), + ..lsp::ServerCapabilities::default() + }, + // Emulate vtsls label generation + label_for_completion: Some(Box::new(|item, _| { + let text = if let Some(description) = item + .label_details + .as_ref() + .and_then(|label_details| label_details.description.as_ref()) + { + format!("{} {}", item.label, description) + } else if let Some(detail) = &item.detail { + format!("{} {}", item.label, detail) + } else { + item.label.clone() + }; + let len = text.len(); + Some(language::CodeLabel { + text, + runs: Vec::new(), + filter_range: 0..len, + }) + })), + ..FakeLspAdapter::default() + }, + ); + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let worktree_id = workspace + .update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }) + }) + .unwrap(); + let _buffer = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp("/a/main.ts", cx) + }) + .await + .unwrap(); + let editor = workspace + .update(cx, |workspace, cx| { + workspace.open_path((worktree_id, "main.ts"), None, true, cx) + }) + .unwrap() + .await + .unwrap() + .downcast::() + .unwrap(); + let fake_server = fake_servers.next().await.unwrap(); + + let multiline_label = "StickyHeaderExcerpt {\n excerpt,\n next_excerpt_controls_present,\n next_buffer_row,\n }: StickyHeaderExcerpt<'_>,"; + let multiline_label_2 = "a\nb\nc\n"; + let multiline_detail = "[]struct {\n\tSignerId\tstruct {\n\t\tIssuer\t\t\tstring\t`json:\"issuer\"`\n\t\tSubjectSerialNumber\"`\n}}"; + let multiline_description = "d\ne\nf\n"; + let multiline_detail_2 = "g\nh\ni\n"; + + let mut completion_handle = + fake_server.handle_request::(move |params, _| async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + lsp::CompletionItem { + label: multiline_label.to_string(), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + end: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + }, + new_text: "new_text_1".to_string(), + })), + ..lsp::CompletionItem::default() + }, + lsp::CompletionItem { + label: "single line label 1".to_string(), + detail: Some(multiline_detail.to_string()), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + end: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + }, + new_text: "new_text_2".to_string(), + })), + ..lsp::CompletionItem::default() + }, + lsp::CompletionItem { + label: "single line label 2".to_string(), + label_details: Some(lsp::CompletionItemLabelDetails { + description: Some(multiline_description.to_string()), + detail: None, + }), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + end: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + }, + new_text: "new_text_2".to_string(), + })), + ..lsp::CompletionItem::default() + }, + lsp::CompletionItem { + label: multiline_label_2.to_string(), + detail: Some(multiline_detail_2.to_string()), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + end: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + }, + new_text: "new_text_3".to_string(), + })), + ..lsp::CompletionItem::default() + }, + lsp::CompletionItem { + label: "Label with many spaces and \t but without newlines".to_string(), + detail: Some( + "Details with many spaces and \t but without newlines".to_string(), + ), + text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit { + range: lsp::Range { + start: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + end: lsp::Position { + line: params.text_document_position.position.line, + character: params.text_document_position.position.character, + }, + }, + new_text: "new_text_4".to_string(), + })), + ..lsp::CompletionItem::default() + }, + ]))) + }); + + editor.update(cx, |editor, cx| { + editor.focus(cx); + editor.move_to_end(&MoveToEnd, cx); + editor.handle_input(".", cx); + }); + cx.run_until_parked(); + completion_handle.next().await.unwrap(); + + editor.update(cx, |editor, _| { + assert!(editor.context_menu_visible()); + if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() + { + let completion_labels = menu + .completions + .borrow() + .iter() + .map(|c| c.label.text.clone()) + .collect::>(); + assert_eq!( + completion_labels, + &[ + "StickyHeaderExcerpt { excerpt, next_excerpt_controls_present, next_buffer_row, }: StickyHeaderExcerpt<'_>,", + "single line label 1 []struct { SignerId struct { Issuer string `json:\"issuer\"` SubjectSerialNumber\"` }}", + "single line label 2 d e f ", + "a b c g h i ", + "Label with many spaces and \t but without newlines Details with many spaces and \t but without newlines", + ], + "Completion items should have their labels without newlines, also replacing excessive whitespaces. Completion items without newlines should not be altered.", + ); + + for completion in menu + .completions + .borrow() + .iter() { + assert_eq!( + completion.label.filter_range, + 0..completion.label.text.len(), + "Adjusted completion items should still keep their filter ranges for the entire label. Item: {completion:?}" + ); + } + + } else { + panic!("expected completion menu to be open"); + } + }); +} + #[gpui::test] async fn test_completion_page_up_down_keys(cx: &mut gpui::TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 9732939d50..8061c7efa9 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -763,6 +763,14 @@ pub struct FakeLspAdapter { pub capabilities: lsp::ServerCapabilities, pub initializer: Option>, + pub label_for_completion: Option< + Box< + dyn 'static + + Send + + Sync + + Fn(&lsp::CompletionItem, &Arc) -> Option, + >, + >, } /// Configuration of handling bracket pairs for a given language. @@ -1778,6 +1786,7 @@ impl Default for FakeLspAdapter { arguments: vec![], env: Default::default(), }, + label_for_completion: None, } } } @@ -1849,6 +1858,15 @@ impl LspAdapter for FakeLspAdapter { ) -> Result> { Ok(self.initialization_options.clone()) } + + async fn label_for_completion( + &self, + item: &lsp::CompletionItem, + language: &Arc, + ) -> Option { + let label_for_completion = self.label_for_completion.as_ref()?; + label_for_completion(item, language) + } } fn get_capture_indices(query: &Query, captures: &mut [(&str, &mut Option)]) { diff --git a/crates/languages/src/typescript.rs b/crates/languages/src/typescript.rs index 6ceb6f5898..edfdbf5f55 100644 --- a/crates/languages/src/typescript.rs +++ b/crates/languages/src/typescript.rs @@ -212,16 +212,14 @@ impl LspAdapter for TypeScriptLspAdapter { _ => None, }?; - let one_line = |s: &str| s.replace(" ", "").replace('\n', " "); - let text = if let Some(description) = item .label_details .as_ref() .and_then(|label_details| label_details.description.as_ref()) { - format!("{} {}", item.label, one_line(description)) + format!("{} {}", item.label, description) } else if let Some(detail) = &item.detail { - format!("{} {}", item.label, one_line(detail)) + format!("{} {}", item.label, detail) } else { item.label.clone() }; diff --git a/crates/languages/src/vtsls.rs b/crates/languages/src/vtsls.rs index e44e4e295f..22718e410c 100644 --- a/crates/languages/src/vtsls.rs +++ b/crates/languages/src/vtsls.rs @@ -175,16 +175,14 @@ impl LspAdapter for VtslsLspAdapter { _ => None, }?; - let one_line = |s: &str| s.replace(" ", "").replace('\n', " "); - let text = if let Some(description) = item .label_details .as_ref() .and_then(|label_details| label_details.description.as_ref()) { - format!("{} {}", item.label, one_line(description)) + format!("{} {}", item.label, description) } else if let Some(detail) = &item.detail { - format!("{} {}", item.label, one_line(detail)) + format!("{} {}", item.label, detail) } else { item.label.clone() }; diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index c8f4ec52e5..cbd93ad12a 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -4357,7 +4357,7 @@ impl LspStore { // NB: Zed does not have `details` inside the completion resolve capabilities, but certain language servers violate the spec and do not return `details` immediately, e.g. https://github.com/yioneko/vtsls/issues/213 // So we have to update the label here anyway... - let new_label = match snapshot.language() { + let mut new_label = match snapshot.language() { Some(language) => { adapter .labels_for_completions(&[completion_item.clone()], language) @@ -4373,6 +4373,7 @@ impl LspStore { completion_item.filter_text.as_deref(), ) }); + ensure_uniform_list_compatible_label(&mut new_label); let mut completions = completions.borrow_mut(); let completion = &mut completions[completion_index]; @@ -8014,15 +8015,18 @@ async fn populate_labels_for_completions( None }; + let mut label = label.unwrap_or_else(|| { + CodeLabel::plain( + lsp_completion.label.clone(), + lsp_completion.filter_text.as_deref(), + ) + }); + ensure_uniform_list_compatible_label(&mut label); + completions.push(Completion { old_range: completion.old_range, new_text: completion.new_text, - label: label.unwrap_or_else(|| { - CodeLabel::plain( - lsp_completion.label.clone(), - lsp_completion.filter_text.as_deref(), - ) - }), + label, server_id: completion.server_id, documentation, lsp_completion, @@ -8733,6 +8737,70 @@ fn include_text(server: &lsp::LanguageServer) -> Option { } } +/// Completion items are displayed in a `UniformList`. +/// Usually, those items are single-line strings, but in LSP responses, +/// completion items `label`, `detail` and `label_details.description` may contain newlines or long spaces. +/// Many language plugins construct these items by joining these parts together, and we may fall back to `CodeLabel::plain` that uses `label`. +/// All that may lead to a newline being inserted into resulting `CodeLabel.text`, which will force `UniformList` to bloat each entry to occupy more space, +/// breaking the completions menu presentation. +/// +/// Sanitize the text to ensure there are no newlines, or, if there are some, remove them and also remove long space sequences if there were newlines. +fn ensure_uniform_list_compatible_label(label: &mut CodeLabel) { + let mut new_text = String::with_capacity(label.text.len()); + let mut offset_map = vec![0; label.text.len() + 1]; + let mut last_char_was_space = false; + let mut new_idx = 0; + let mut chars = label.text.char_indices().fuse(); + let mut newlines_removed = false; + + while let Some((idx, c)) = chars.next() { + offset_map[idx] = new_idx; + + match c { + '\n' if last_char_was_space => { + newlines_removed = true; + } + '\t' | ' ' if last_char_was_space => {} + '\n' if !last_char_was_space => { + new_text.push(' '); + new_idx += 1; + last_char_was_space = true; + newlines_removed = true; + } + ' ' | '\t' => { + new_text.push(' '); + new_idx += 1; + last_char_was_space = true; + } + _ => { + new_text.push(c); + new_idx += 1; + last_char_was_space = false; + } + } + } + offset_map[label.text.len()] = new_idx; + + // Only modify the label if newlines were removed. + if !newlines_removed { + return; + } + + for (range, _) in &mut label.runs { + range.start = offset_map[range.start]; + range.end = offset_map[range.end]; + } + + if label.filter_range == (0..label.text.len()) { + label.filter_range = 0..new_text.len(); + } else { + label.filter_range.start = offset_map[label.filter_range.start]; + label.filter_range.end = offset_map[label.filter_range.end]; + } + + label.text = new_text; +} + #[cfg(test)] #[test] fn test_glob_literal_prefix() {