Detailed changes
@@ -6,8 +6,8 @@ use call::ActiveCall;
use collections::HashMap;
use editor::{
actions::{
- ConfirmCodeAction, ConfirmCompletion, ConfirmRename, Redo, Rename, RevertSelectedHunks,
- ToggleCodeActions, Undo,
+ ConfirmCodeAction, ConfirmCompletion, ConfirmRename, ContextMenuFirst, Redo, Rename,
+ RevertSelectedHunks, ToggleCodeActions, Undo,
},
test::{
editor_hunks,
@@ -444,6 +444,93 @@ async fn test_collaborating_with_completion(cx_a: &mut TestAppContext, cx_b: &mu
"use d::SomeTrait;\nfn main() { a.first_method() }"
);
});
+
+ // Now we do a second completion, this time to ensure that documentation/snippets are
+ // resolved
+ editor_b.update(cx_b, |editor, cx| {
+ editor.change_selections(None, cx, |s| s.select_ranges([46..46]));
+ editor.handle_input("; a", cx);
+ editor.handle_input(".", cx);
+ });
+
+ buffer_b.read_with(cx_b, |buffer, _| {
+ assert_eq!(
+ buffer.text(),
+ "use d::SomeTrait;\nfn main() { a.first_method(); a. }"
+ );
+ });
+
+ let mut completion_response = fake_language_server
+ .handle_request::<lsp::request::Completion, _, _>(|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(1, 32),
+ );
+
+ Ok(Some(lsp::CompletionResponse::Array(vec![
+ lsp::CompletionItem {
+ label: "third_method(…)".into(),
+ detail: Some("fn(&mut self, B, C, D) -> E".into()),
+ text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+ // no snippet placehodlers
+ new_text: "third_method".to_string(),
+ range: lsp::Range::new(
+ lsp::Position::new(1, 32),
+ lsp::Position::new(1, 32),
+ ),
+ })),
+ insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+ documentation: None,
+ ..Default::default()
+ },
+ ])))
+ });
+
+ // The completion now gets a new `text_edit.new_text` when resolving the completion item
+ let mut resolve_completion_response = fake_language_server
+ .handle_request::<lsp::request::ResolveCompletionItem, _, _>(|params, _| async move {
+ assert_eq!(params.label, "third_method(…)");
+ Ok(lsp::CompletionItem {
+ label: "third_method(…)".into(),
+ detail: Some("fn(&mut self, B, C, D) -> E".into()),
+ text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+ // Now it's a snippet
+ new_text: "third_method($1, $2, $3)".to_string(),
+ range: lsp::Range::new(lsp::Position::new(1, 32), lsp::Position::new(1, 32)),
+ })),
+ insert_text_format: Some(lsp::InsertTextFormat::SNIPPET),
+ documentation: Some(lsp::Documentation::String(
+ "this is the documentation".into(),
+ )),
+ ..Default::default()
+ })
+ });
+
+ cx_b.executor().run_until_parked();
+
+ completion_response.next().await.unwrap();
+
+ editor_b.update(cx_b, |editor, cx| {
+ assert!(editor.context_menu_visible());
+ editor.context_menu_first(&ContextMenuFirst {}, cx);
+ });
+
+ resolve_completion_response.next().await.unwrap();
+ cx_b.executor().run_until_parked();
+
+ // When accepting the completion, the snippet is insert.
+ editor_b.update(cx_b, |editor, cx| {
+ assert!(editor.context_menu_visible());
+ editor.confirm_completion(&ConfirmCompletion { item_ix: Some(0) }, cx);
+ assert_eq!(
+ editor.text(cx),
+ "use d::SomeTrait;\nfn main() { a.first_method(); a.third_method(, , ) }"
+ );
+ });
}
#[gpui::test(iterations = 10)]
@@ -59,6 +59,7 @@ impl CompletionProvider for MessageEditorCompletionProvider {
fn resolve_completions(
&self,
+ _buffer: Model<Buffer>,
_completion_indices: Vec<usize>,
_completions: Arc<RwLock<Box<[Completion]>>>,
_cx: &mut ViewContext<Editor>,
@@ -837,6 +837,7 @@ impl CompletionsMenu {
}
fn pre_resolve_completion_documentation(
+ buffer: Model<Buffer>,
completions: Arc<RwLock<Box<[Completion]>>>,
matches: Arc<[StringMatch]>,
editor: &Editor,
@@ -852,6 +853,7 @@ impl CompletionsMenu {
};
let resolve_task = provider.resolve_completions(
+ buffer,
matches.iter().map(|m| m.candidate_id).collect(),
completions.clone(),
cx,
@@ -880,7 +882,12 @@ impl CompletionsMenu {
};
let resolve_task = project.update(cx, |project, cx| {
- project.resolve_completions(vec![completion_index], self.completions.clone(), cx)
+ project.resolve_completions(
+ self.buffer.clone(),
+ vec![completion_index],
+ self.completions.clone(),
+ cx,
+ )
});
let delay_ms =
@@ -3463,7 +3470,7 @@ impl Editor {
)
})
.collect(),
- buffer,
+ buffer: buffer.clone(),
completions: Arc::new(RwLock::new(completions.into())),
matches: Vec::new().into(),
selected_item: 0,
@@ -3490,6 +3497,7 @@ impl Editor {
.completion_documentation_pre_resolve_debounce
.fire_new(delay, cx, |editor, cx| {
CompletionsMenu::pre_resolve_completion_documentation(
+ buffer,
completions,
matches,
editor,
@@ -10213,6 +10221,7 @@ pub trait CompletionProvider {
fn resolve_completions(
&self,
+ buffer: Model<Buffer>,
completion_indices: Vec<usize>,
completions: Arc<RwLock<Box<[Completion]>>>,
cx: &mut ViewContext<Editor>,
@@ -10241,12 +10250,13 @@ impl CompletionProvider for Model<Project> {
fn resolve_completions(
&self,
+ buffer: Model<Buffer>,
completion_indices: Vec<usize>,
completions: Arc<RwLock<Box<[Completion]>>>,
cx: &mut ViewContext<Editor>,
) -> Task<Result<bool>> {
self.update(cx, |project, cx| {
- project.resolve_completions(completion_indices, completions, cx)
+ project.resolve_completions(buffer, completion_indices, completions, cx)
})
}
@@ -186,6 +186,18 @@ impl LspAdapter for TypeScriptLspAdapter {
})))
}
+ async fn workspace_configuration(
+ self: Arc<Self>,
+ _: &Arc<dyn LspAdapterDelegate>,
+ _cx: &mut AsyncAppContext,
+ ) -> Result<Value> {
+ Ok(json!({
+ "completions": {
+ "completeFunctionCalls": true
+ }
+ }))
+ }
+
fn language_ids(&self) -> HashMap<String, String> {
HashMap::from_iter([
("TypeScript".into(), "typescript".into()),
@@ -1505,18 +1505,11 @@ impl LspCommand for GetCompletions {
let edit = match lsp_completion.text_edit.as_ref() {
// If the language server provides a range to overwrite, then
// check that the range is valid.
- Some(lsp::CompletionTextEdit::Edit(edit)) => {
- let range = range_from_lsp(edit.range);
- let start = snapshot.clip_point_utf16(range.start, Bias::Left);
- let end = snapshot.clip_point_utf16(range.end, Bias::Left);
- if start != range.start.0 || end != range.end.0 {
- log::info!("completion out of expected range");
- return false;
+ Some(completion_text_edit) => {
+ match parse_completion_text_edit(completion_text_edit, &snapshot) {
+ Some(edit) => edit,
+ None => return false,
}
- (
- snapshot.anchor_before(start)..snapshot.anchor_after(end),
- edit.new_text.clone(),
- )
}
// If the language server does not provide a range, then infer
@@ -1570,21 +1563,6 @@ impl LspCommand for GetCompletions {
.clone();
(range, text)
}
-
- Some(lsp::CompletionTextEdit::InsertAndReplace(edit)) => {
- let range = range_from_lsp(edit.insert);
-
- let start = snapshot.clip_point_utf16(range.start, Bias::Left);
- let end = snapshot.clip_point_utf16(range.end, Bias::Left);
- if start != range.start.0 || end != range.end.0 {
- log::info!("completion out of expected range");
- return false;
- }
- (
- snapshot.anchor_before(start)..snapshot.anchor_after(end),
- edit.new_text.clone(),
- )
- }
};
completion_edits.push(edit);
@@ -1684,6 +1662,44 @@ impl LspCommand for GetCompletions {
}
}
+pub(crate) fn parse_completion_text_edit(
+ edit: &lsp::CompletionTextEdit,
+ snapshot: &BufferSnapshot,
+) -> Option<(Range<Anchor>, String)> {
+ match edit {
+ lsp::CompletionTextEdit::Edit(edit) => {
+ let range = range_from_lsp(edit.range);
+ let start = snapshot.clip_point_utf16(range.start, Bias::Left);
+ let end = snapshot.clip_point_utf16(range.end, Bias::Left);
+ if start != range.start.0 || end != range.end.0 {
+ log::info!("completion out of expected range");
+ None
+ } else {
+ Some((
+ snapshot.anchor_before(start)..snapshot.anchor_after(end),
+ edit.new_text.clone(),
+ ))
+ }
+ }
+
+ lsp::CompletionTextEdit::InsertAndReplace(edit) => {
+ let range = range_from_lsp(edit.insert);
+
+ let start = snapshot.clip_point_utf16(range.start, Bias::Left);
+ let end = snapshot.clip_point_utf16(range.end, Bias::Left);
+ if start != range.start.0 || end != range.end.0 {
+ log::info!("completion out of expected range");
+ None
+ } else {
+ Some((
+ snapshot.anchor_before(start)..snapshot.anchor_after(end),
+ edit.new_text.clone(),
+ ))
+ }
+ }
+ }
+}
+
#[async_trait(?Send)]
impl LspCommand for GetCodeActions {
type Response = Vec<CodeAction>;
@@ -98,7 +98,7 @@ use std::{
};
use task::static_source::{StaticSource, TrackedFile};
use terminals::Terminals;
-use text::{Anchor, BufferId};
+use text::{Anchor, BufferId, LineEnding};
use util::{
debug_panic, defer,
http::{HttpClient, Url},
@@ -5597,6 +5597,7 @@ impl Project {
pub fn resolve_completions(
&self,
+ buffer: Model<Buffer>,
completion_indices: Vec<usize>,
completions: Arc<RwLock<Box<[Completion]>>>,
cx: &mut ModelContext<Self>,
@@ -5607,6 +5608,9 @@ impl Project {
let is_remote = self.is_remote();
let project_id = self.remote_id();
+ let buffer_id = buffer.read(cx).remote_id();
+ let buffer_snapshot = buffer.read(cx).snapshot();
+
cx.spawn(move |this, mut cx| async move {
let mut did_resolve = false;
if is_remote {
@@ -5628,9 +5632,10 @@ impl Project {
(server_id, completion)
};
- Self::resolve_completion_documentation_remote(
+ Self::resolve_completion_remote(
project_id,
server_id,
+ buffer_id,
completions.clone(),
completion_index,
completion,
@@ -5665,8 +5670,9 @@ impl Project {
};
did_resolve = true;
- Self::resolve_completion_documentation_local(
+ Self::resolve_completion_local(
server,
+ &buffer_snapshot,
completions.clone(),
completion_index,
completion,
@@ -5680,8 +5686,9 @@ impl Project {
})
}
- async fn resolve_completion_documentation_local(
+ async fn resolve_completion_local(
server: Arc<lsp::LanguageServer>,
+ snapshot: &BufferSnapshot,
completions: Arc<RwLock<Box<[Completion]>>>,
completion_index: usize,
completion: lsp::CompletionItem,
@@ -5702,9 +5709,9 @@ impl Project {
return;
};
- if let Some(lsp_documentation) = completion_item.documentation {
+ if let Some(lsp_documentation) = completion_item.documentation.as_ref() {
let documentation = language::prepare_completion_documentation(
- &lsp_documentation,
+ lsp_documentation,
&language_registry,
None, // TODO: Try to reasonably work out which language the completion is for
)
@@ -5718,11 +5725,31 @@ impl Project {
let completion = &mut completions[completion_index];
completion.documentation = Some(Documentation::Undocumented);
}
+
+ if let Some(text_edit) = completion_item.text_edit.as_ref() {
+ // Technically we don't have to parse the whole `text_edit`, since the only
+ // language server we currently use that does update `text_edit` in `completionItem/resolve`
+ // is `typescript-language-server` and they only update `text_edit.new_text`.
+ // But we should not rely on that.
+ let edit = parse_completion_text_edit(text_edit, snapshot);
+
+ if let Some((old_range, mut new_text)) = edit {
+ LineEnding::normalize(&mut new_text);
+
+ let mut completions = completions.write();
+ let completion = &mut completions[completion_index];
+
+ completion.new_text = new_text;
+ completion.old_range = old_range;
+ }
+ }
}
- async fn resolve_completion_documentation_remote(
+ #[allow(clippy::too_many_arguments)]
+ async fn resolve_completion_remote(
project_id: u64,
server_id: LanguageServerId,
+ buffer_id: BufferId,
completions: Arc<RwLock<Box<[Completion]>>>,
completion_index: usize,
completion: lsp::CompletionItem,
@@ -5733,6 +5760,7 @@ impl Project {
project_id,
language_server_id: server_id.0 as u64,
lsp_completion: serde_json::to_string(&completion).unwrap().into_bytes(),
+ buffer_id: buffer_id.into(),
};
let Some(response) = client
@@ -5744,21 +5772,32 @@ impl Project {
return;
};
- let documentation = if response.text.is_empty() {
+ let documentation = if response.documentation.is_empty() {
Documentation::Undocumented
- } else if response.is_markdown {
+ } else if response.documentation_is_markdown {
Documentation::MultiLineMarkdown(
- markdown::parse_markdown(&response.text, &language_registry, None).await,
+ markdown::parse_markdown(&response.documentation, &language_registry, None).await,
)
- } else if response.text.lines().count() <= 1 {
- Documentation::SingleLine(response.text)
+ } else if response.documentation.lines().count() <= 1 {
+ Documentation::SingleLine(response.documentation)
} else {
- Documentation::MultiLinePlainText(response.text)
+ Documentation::MultiLinePlainText(response.documentation)
};
let mut completions = completions.write();
let completion = &mut completions[completion_index];
completion.documentation = Some(documentation);
+
+ let old_range = response
+ .old_start
+ .and_then(deserialize_anchor)
+ .zip(response.old_end.and_then(deserialize_anchor));
+ if let Some((old_start, old_end)) = old_range {
+ if !response.new_text.is_empty() {
+ completion.new_text = response.new_text;
+ completion.old_range = old_start..old_end;
+ }
+ }
}
pub fn apply_additional_edits_for_completion(
@@ -8962,19 +9001,53 @@ impl Project {
})??
.await?;
- let mut is_markdown = false;
- let text = match completion.documentation {
+ let mut documentation_is_markdown = false;
+ let documentation = match completion.documentation {
Some(lsp::Documentation::String(text)) => text,
Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { kind, value })) => {
- is_markdown = kind == lsp::MarkupKind::Markdown;
+ documentation_is_markdown = kind == lsp::MarkupKind::Markdown;
value
}
_ => String::new(),
};
- Ok(proto::ResolveCompletionDocumentationResponse { text, is_markdown })
+ // If we have a new buffer_id, that means we're talking to a new client
+ // and want to check for new text_edits in the completion too.
+ let mut old_start = None;
+ let mut old_end = None;
+ let mut new_text = String::default();
+ if let Ok(buffer_id) = BufferId::new(envelope.payload.buffer_id) {
+ let buffer_snapshot = this.update(&mut cx, |this, cx| {
+ let buffer = this
+ .opened_buffers
+ .get(&buffer_id)
+ .and_then(|buffer| buffer.upgrade())
+ .ok_or_else(|| anyhow!("unknown buffer id {}", buffer_id))?;
+ anyhow::Ok(buffer.read(cx).snapshot())
+ })??;
+
+ if let Some(text_edit) = completion.text_edit.as_ref() {
+ let edit = parse_completion_text_edit(text_edit, &buffer_snapshot);
+
+ if let Some((old_range, mut text_edit_new_text)) = edit {
+ LineEnding::normalize(&mut text_edit_new_text);
+
+ new_text = text_edit_new_text;
+ old_start = Some(serialize_anchor(&old_range.start));
+ old_end = Some(serialize_anchor(&old_range.end));
+ }
+ }
+ }
+
+ Ok(proto::ResolveCompletionDocumentationResponse {
+ documentation,
+ documentation_is_markdown,
+ old_start,
+ old_end,
+ new_text,
+ })
}
async fn handle_apply_code_action(
@@ -1024,15 +1024,22 @@ message ResolveState {
}
}
+// This type is used to resolve more than just
+// the documentation, but for backwards-compatibility
+// reasons we can't rename the type.
message ResolveCompletionDocumentation {
uint64 project_id = 1;
uint64 language_server_id = 2;
bytes lsp_completion = 3;
+ uint64 buffer_id = 4;
}
message ResolveCompletionDocumentationResponse {
- string text = 1;
- bool is_markdown = 2;
+ string documentation = 1;
+ bool documentation_is_markdown = 2;
+ Anchor old_start = 3;
+ Anchor old_end = 4;
+ string new_text = 5;
}
message ResolveInlayHint {