From 4b4ab0934cbf68443218ca4df5df84158f2a20d4 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 4 Nov 2024 15:27:45 +0100 Subject: [PATCH] Add package information in completions (#616) And: * Remove parameters from completion items entirely * Add `::` details to namespace completions --- CHANGELOG.md | 5 +++ .../src/lsp/completions/completion_item.rs | 37 +++++++++++++------ .../sources/composite/workspace.rs | 4 +- crates/ark/src/lsp/state_handlers.rs | 4 ++ 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31e7d3797..159b0b341 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ ## 2024-10 +- Results from completions have been improved with extra details. + Package functions now display the package name (posit-dev/positron#5225) + and namespace completions now display `::` to hint at what is being + completed. + - The document symbol kind for assigned variables is now `VARIABLE` (@kv9898, posit-dev/positron#5071). This produces a clearer icon in the outline. - Added support for outline headers in comments (@kv9898, posit-dev/positron#3822). diff --git a/crates/ark/src/lsp/completions/completion_item.rs b/crates/ark/src/lsp/completions/completion_item.rs index cb789a5a9..c0ee1aab4 100644 --- a/crates/ark/src/lsp/completions/completion_item.rs +++ b/crates/ark/src/lsp/completions/completion_item.rs @@ -13,7 +13,6 @@ use harp::r_symbol; use harp::utils::is_symbol_valid; use harp::utils::r_env_binding_is_active; use harp::utils::r_envir_name; -use harp::utils::r_formals; use harp::utils::r_promise_force_with_rollback; use harp::utils::r_promise_is_forced; use harp::utils::r_promise_is_lazy_load_binding; @@ -31,6 +30,7 @@ use stdext::*; use tower_lsp::lsp_types::Command; use tower_lsp::lsp_types::CompletionItem; use tower_lsp::lsp_types::CompletionItemKind; +use tower_lsp::lsp_types::CompletionItemLabelDetails; use tower_lsp::lsp_types::CompletionTextEdit; use tower_lsp::lsp_types::Documentation; use tower_lsp::lsp_types::InsertTextFormat; @@ -148,6 +148,10 @@ pub(super) unsafe fn completion_item_from_package( })?; item.kind = Some(CompletionItemKind::MODULE); + item.label_details = Some(CompletionItemLabelDetails { + detail: Some(String::from("::")), + description: None, + }); if append_colons { item.insert_text_format = Some(InsertTextFormat::SNIPPET); @@ -162,10 +166,9 @@ pub(super) unsafe fn completion_item_from_package( return Ok(item); } -pub(super) fn completion_item_from_function>( +pub(super) fn completion_item_from_function( name: &str, package: Option<&str>, - parameters: &[T], ) -> Result { let label = format!("{}", name); let mut item = completion_item(label, CompletionData::Function { @@ -175,8 +178,8 @@ pub(super) fn completion_item_from_function>( item.kind = Some(CompletionItemKind::FUNCTION); - let detail = format!("{}({})", name, parameters.joined(", ")); - item.detail = Some(detail); + let label_details = item_details(package); + item.label_details = Some(label_details); let insert_text = sym_quote_invalid(name); item.insert_text_format = Some(InsertTextFormat::SNIPPET); @@ -192,6 +195,21 @@ pub(super) fn completion_item_from_function>( return Ok(item); } +fn item_details(package: Option<&str>) -> CompletionItemLabelDetails { + let description = package.map(|p| { + // Environments from the search path often have a "package:" prefix. + // Remove it from display. This creates some rare ambiguities but + // improves the display generally. + let p = p.strip_prefix("package:").unwrap_or(p); + format!("{{{p}}}") + }); + + CompletionItemLabelDetails { + detail: None, + description, + } +} + // TODO pub(super) unsafe fn completion_item_from_dataset(name: &str) -> Result { let mut item = completion_item(name.to_string(), CompletionData::Unknown)?; @@ -238,19 +256,14 @@ pub(super) unsafe fn completion_item_from_object( // In other words, when creating a completion item for these functions, // we should also figure out where we can receive the help from. if Rf_isFunction(object) != 0 { - let formals = r_formals(object)?; - let arguments = formals - .iter() - .map(|formal| formal.name.as_str()) - .collect::>(); - return completion_item_from_function(name, package, &arguments); + return completion_item_from_function(name, package); } let mut item = completion_item(name, CompletionData::Object { name: name.to_string(), })?; - item.detail = Some("(Object)".to_string()); + item.label_details = Some(item_details(package)); item.kind = Some(CompletionItemKind::STRUCT); if !is_symbol_valid(name) { diff --git a/crates/ark/src/lsp/completions/sources/composite/workspace.rs b/crates/ark/src/lsp/completions/sources/composite/workspace.rs index 2903174a7..29abfec75 100644 --- a/crates/ark/src/lsp/completions/sources/composite/workspace.rs +++ b/crates/ark/src/lsp/completions/sources/composite/workspace.rs @@ -63,8 +63,8 @@ pub(super) fn completions_from_workspace( } match &entry.data { - indexer::IndexEntryData::Function { name, arguments } => { - let mut completion = unwrap!(completion_item_from_function(name, None, arguments), Err(error) => { + indexer::IndexEntryData::Function { name, .. } => { + let mut completion = unwrap!(completion_item_from_function(name, None), Err(error) => { error!("{:?}", error); return; }); diff --git a/crates/ark/src/lsp/state_handlers.rs b/crates/ark/src/lsp/state_handlers.rs index 6970b40bd..2c3d8b246 100644 --- a/crates/ark/src/lsp/state_handlers.rs +++ b/crates/ark/src/lsp/state_handlers.rs @@ -11,6 +11,7 @@ use anyhow::anyhow; use serde_json::Value; use struct_field_names_as_array::FieldNamesAsArray; use tower_lsp::lsp_types::CompletionOptions; +use tower_lsp::lsp_types::CompletionOptionsCompletionItem; use tower_lsp::lsp_types::ConfigurationItem; use tower_lsp::lsp_types::DidChangeConfigurationParams; use tower_lsp::lsp_types::DidChangeTextDocumentParams; @@ -120,6 +121,9 @@ pub(crate) fn initialize( trigger_characters: Some(vec!["$".to_string(), "@".to_string(), ":".to_string()]), work_done_progress_options: Default::default(), all_commit_characters: None, + completion_item: Some(CompletionOptionsCompletionItem { + label_details_support: Some(true), + }), ..Default::default() }), signature_help_provider: Some(SignatureHelpOptions {