From 7e46d436fce8cea83338d07b88d374aa94519607 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Thu, 17 Apr 2025 14:26:13 -0700 Subject: [PATCH 1/9] Delete snippets that I don't plan to handle in ark --- crates/ark/resources/snippets/r.code-snippets | 134 ------------------ .../completions/sources/composite/snippets.rs | 5 - 2 files changed, 139 deletions(-) diff --git a/crates/ark/resources/snippets/r.code-snippets b/crates/ark/resources/snippets/r.code-snippets index 375bdfd70..6ebae7754 100644 --- a/crates/ark/resources/snippets/r.code-snippets +++ b/crates/ark/resources/snippets/r.code-snippets @@ -1,47 +1,4 @@ { - "lib": { - "prefix": "lib", - "body": "library(${1:package})", - "description": "Attach an R package" - }, - "src": { - "prefix": "src", - "body": "source(\"${1:file.R}\")", - "description": "Source an R file" - }, - "ret": { - "prefix": "ret", - "body": "return(${1:code})", - "description": "Return a value from a function" - }, - "mat": { - "prefix": "mat", - "body": "matrix(${1:data}, nrow = ${2:rows}, ncol = ${3:cols})", - "description": "Define a matrix" - }, - "sg": { - "prefix": "sg", - "body": [ - "setGeneric(\"${1:generic}\", function(${2:x, ...}) {", - "\tstandardGeneric(\"${1:generic}\")", - "})" - ], - "description": "Define a generic" - }, - "sm": { - "prefix": "sm", - "body": [ - "setMethod(\"${1:generic}\", ${2:class}, function(${2:x, ...}) {", - "\t${0}", - "})" - ], - "description": "Define a method for a generic function" - }, - "sc": { - "prefix": "sc", - "body": "setClass(\"${1:Class}\", slots = c(${2:name = \"type\"}))", - "description": "Define a class definition" - }, "if": { "prefix": "if", "body": [ @@ -60,15 +17,6 @@ ], "description": "Conditional expression" }, - "ei": { - "prefix": "ei", - "body": [ - "else if (${1:condition}) {", - "\t${0}", - "}" - ], - "description": "Conditional expression" - }, "fun": { "prefix": "fun", "body": [ @@ -95,87 +43,5 @@ "}" ], "description": "Define a loop" - }, - "switch": { - "prefix": "switch", - "body": [ - "switch (${1:object},", - "\t${2:case} = ${3:action}", - ")" - ], - "description": "Define a switch statement" - }, - "apply": { - "prefix": "apply", - "body": "apply(${1:array}, ${2:margin}, ${3:...})", - "description": "Use the apply family" - }, - "lapply": { - "prefix": "lapply", - "body": "lapply(${1:list}, ${2:function})", - "description": "Use the apply family" - }, - "sapply": { - "prefix": "sapply", - "body": "sapply(${1:list}, ${2:function})", - "description": "Use the apply family" - }, - "mapply": { - "prefix": "mapply", - "body": "mapply(${1:function}, ${2:...})", - "description": "Use the apply family" - }, - "tapply": { - "prefix": "tapply", - "body": "tapply(${1:vector}, ${2:index}, ${3:function})", - "description": "Use the apply family" - }, - "vapply": { - "prefix": "vapply", - "body": "vapply(${1:list}, ${2:function}, FUN.VALUE = ${3:type}, ${4:...})", - "description": "Use the apply family" - }, - "rapply": { - "prefix": "rapply", - "body": "rapply(${1:list}, ${2:function})", - "description": "Use the apply family" - }, - "ts": { - "prefix": "ts", - "body": "`r paste(\"#\", date(), \"------------------------------\\n\")`", - "description": "Insert a datetime" - }, - "shinyapp": { - "prefix": "shinyapp", - "body": [ - "library(shiny)", - "", - "ui <- fluidPage(", - " ${0}", - ")", - "", - "server <- function(input, output, session) {", - " ", - "}", - "", - "shinyApp(ui, server)" - ], - "description": "Define a Shiny app" - }, - "shinymod": { - "prefix": "shinymod", - "body": [ - "${1:name}_UI <- function(id) {", - " ns <- NS(id)", - " tagList(", - "\t${0}", - " )", - "}", - "", - "${1:name} <- function(input, output, session) {", - " ", - "}" - ], - "description": "Define a Shiny module" } } diff --git a/crates/ark/src/lsp/completions/sources/composite/snippets.rs b/crates/ark/src/lsp/completions/sources/composite/snippets.rs index 455323c7c..1f69e2b54 100644 --- a/crates/ark/src/lsp/completions/sources/composite/snippets.rs +++ b/crates/ark/src/lsp/completions/sources/composite/snippets.rs @@ -116,11 +116,6 @@ mod tests { fn test_snippets() { let snippets = completions_from_snippets().unwrap().unwrap(); - // Hash map isn't stable with regards to ordering - let item = snippets.iter().find(|item| item.label == "lib").unwrap(); - assert_eq!(item.detail, Some("Attach an R package".to_string())); - assert_eq!(item.insert_text, Some("library(${1:package})".to_string())); - // Multiline body let item = snippets.iter().find(|item| item.label == "if").unwrap(); assert_eq!( From 6c203f72ea85ca37dd11212a9780a921707fdec3 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Thu, 17 Apr 2025 15:25:22 -0700 Subject: [PATCH 2/9] Handle reserved word snippets in keyword source --- crates/ark/resources/snippets/r.code-snippets | 45 --------- .../completions/sources/composite/keyword.rs | 93 +++++++++++++++++-- .../completions/sources/composite/snippets.rs | 8 +- 3 files changed, 89 insertions(+), 57 deletions(-) diff --git a/crates/ark/resources/snippets/r.code-snippets b/crates/ark/resources/snippets/r.code-snippets index 6ebae7754..2c63c0851 100644 --- a/crates/ark/resources/snippets/r.code-snippets +++ b/crates/ark/resources/snippets/r.code-snippets @@ -1,47 +1,2 @@ { - "if": { - "prefix": "if", - "body": [ - "if (${1:condition}) {", - "\t${0}", - "}" - ], - "description": "Conditional expression" - }, - "el": { - "prefix": "el", - "body": [ - "else {", - "\t${0}", - "}" - ], - "description": "Conditional expression" - }, - "fun": { - "prefix": "fun", - "body": [ - "${1:name} <- function(${2:variables}) {", - "\t${0}", - "}" - ], - "description": "Function skeleton" - }, - "for": { - "prefix": "for", - "body": [ - "for (${1:variable} in ${2:vector}) {", - "\t${0}", - "}" - ], - "description": "Define a loop" - }, - "while": { - "prefix": "while", - "body": [ - "while (${1:condition}) {", - "\t${0}", - "}" - ], - "description": "Define a loop" - } } diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index d26ad53e6..1cfc59dd7 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -8,6 +8,10 @@ use stdext::unwrap; use tower_lsp::lsp_types::CompletionItem; use tower_lsp::lsp_types::CompletionItemKind; +use tower_lsp::lsp_types::Documentation; +use tower_lsp::lsp_types::InsertTextFormat; +use tower_lsp::lsp_types::MarkupContent; +use tower_lsp::lsp_types::MarkupKind; use crate::lsp::completions::completion_context::CompletionContext; use crate::lsp::completions::completion_item::completion_item; @@ -32,10 +36,13 @@ impl CompletionSource for KeywordSource { pub fn completions_from_keywords() -> anyhow::Result>> { let mut completions = vec![]; - // provide keyword completion results - // NOTE: Some R keywords have definitions provided in the R - // base namespace, so we don't need to provide duplicate - // definitions for these here. + add_bare_keywords(&mut completions); + add_keyword_snippets(&mut completions); + + Ok(Some(completions)) +} + +fn add_bare_keywords(completions: &mut Vec) { let keywords = vec![ "NULL", "NA", @@ -48,9 +55,9 @@ pub fn completions_from_keywords() -> anyhow::Result> "NA_character_", "NA_complex_", "in", - "else", "next", "break", + "repeat", ]; for keyword in keywords { @@ -68,6 +75,80 @@ pub fn completions_from_keywords() -> anyhow::Result> completions.push(item); } +} - Ok(Some(completions)) +fn add_keyword_snippets(completions: &mut Vec) { + // Transplanting previous snippet treatment of certain reserved words from + // the snippet source to this keyword source + let snippet_data = vec![ + // Snippet data is a tuple of: + // - keyword: The reserved word + // - label: The label for the completion item + // - snippet: The snippet to be inserted + // - detail: The detail displayed in the completion UI + + // The only case where `keyword != label` is `fun` for `function`. + // But in the name of preserving original behaviour, this is my opening + // move. + ( + "for", + "for", + "for (${1:variable} in ${2:vector}) {\n\t${0}\n}", + "Define a loop", + ), + ( + "if", + "if", + "if (${1:condition}) {\n\t${0}\n}", + "Conditional expression", + ), + ( + "while", + "while", + "while (${1:condition}) {\n\t${0}\n}", + "Define a loop", + ), + ( + "else", + "else", + "else {\n\t${0}\n}", + "Conditional expression", + ), + ( + "function", + "fun", + "${1:name} <- function(${2:variables}) {\n\t${0}\n}", + "Function skeleton", + ), + ]; + + for (keyword, label, snippet, detail) in snippet_data { + let item = completion_item(label.to_string(), CompletionData::Snippet { + text: snippet.to_string(), + }); + + let mut item = match item { + Ok(item) => item, + Err(err) => { + log::trace!("Failed to construct completion item for reserved keyword '{keyword}' due to {err:?}"); + continue; + }, + }; + + // Markup shows up in the quick suggestion documentation window, + // so you can see what the snippet expands to + let markup = vec!["```r", snippet, "```"].join("\n"); + let markup = MarkupContent { + kind: MarkupKind::Markdown, + value: markup, + }; + + item.detail = Some(detail.to_string()); + item.documentation = Some(Documentation::MarkupContent(markup)); + item.kind = Some(CompletionItemKind::KEYWORD); + item.insert_text = Some(snippet.to_string()); + item.insert_text_format = Some(InsertTextFormat::SNIPPET); + + completions.push(item); + } } diff --git a/crates/ark/src/lsp/completions/sources/composite/snippets.rs b/crates/ark/src/lsp/completions/sources/composite/snippets.rs index 1f69e2b54..77d83bdc1 100644 --- a/crates/ark/src/lsp/completions/sources/composite/snippets.rs +++ b/crates/ark/src/lsp/completions/sources/composite/snippets.rs @@ -116,11 +116,7 @@ mod tests { fn test_snippets() { let snippets = completions_from_snippets().unwrap().unwrap(); - // Multiline body - let item = snippets.iter().find(|item| item.label == "if").unwrap(); - assert_eq!( - item.insert_text, - Some("if (${1:condition}) {\n\t${0}\n}".to_string()) - ); + // We should have an empty list since snippets have been moved to keyword source + assert!(snippets.is_empty()); } } From 43874bc09cc41aecb79da06d93a64e6937418bcc Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Mon, 21 Apr 2025 14:38:43 -0700 Subject: [PATCH 3/9] Add tests for reserved word completions --- .../completions/sources/composite/keyword.rs | 173 +++++++++++------- 1 file changed, 110 insertions(+), 63 deletions(-) diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index 1cfc59dd7..0baaea077 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -42,25 +42,68 @@ pub fn completions_from_keywords() -> anyhow::Result> Ok(Some(completions)) } +const BARE_KEYWORDS: &[&str] = &[ + "NULL", + "NA", + "TRUE", + "FALSE", + "Inf", + "NaN", + "NA_integer_", + "NA_real_", + "NA_character_", + "NA_complex_", + "in", + "next", + "break", + "repeat", +]; + +// Snippet data is a tuple of: +// - keyword: The reserved word +// - label: The label for the completion item +// - snippet: The snippet to be inserted +// - detail: The detail displayed in the completion UI + +// The only case where `keyword != label` is `fun` for `function`. +// But in the name of preserving original behaviour, this is my opening +// move. +const KEYWORD_SNIPPETS: &[(&str, &str, &str, &str)] = &[ + // (keyword, label, snippet, detail) + ( + "for", + "for", + "for (${1:variable} in ${2:vector}) {\n\t${0}\n}", + "Define a loop", + ), + ( + "if", + "if", + "if (${1:condition}) {\n\t${0}\n}", + "Conditional expression", + ), + ( + "while", + "while", + "while (${1:condition}) {\n\t${0}\n}", + "Define a loop", + ), + ( + "else", + "else", + "else {\n\t${0}\n}", + "Conditional expression", + ), + ( + "function", + "fun", + "${1:name} <- function(${2:variables}) {\n\t${0}\n}", + "Function skeleton", + ), +]; + fn add_bare_keywords(completions: &mut Vec) { - let keywords = vec![ - "NULL", - "NA", - "TRUE", - "FALSE", - "Inf", - "NaN", - "NA_integer_", - "NA_real_", - "NA_character_", - "NA_complex_", - "in", - "next", - "break", - "repeat", - ]; - - for keyword in keywords { + for keyword in BARE_KEYWORDS { let item = completion_item(keyword.to_string(), CompletionData::Keyword { name: keyword.to_string(), }); @@ -78,51 +121,8 @@ fn add_bare_keywords(completions: &mut Vec) { } fn add_keyword_snippets(completions: &mut Vec) { - // Transplanting previous snippet treatment of certain reserved words from - // the snippet source to this keyword source - let snippet_data = vec![ - // Snippet data is a tuple of: - // - keyword: The reserved word - // - label: The label for the completion item - // - snippet: The snippet to be inserted - // - detail: The detail displayed in the completion UI - - // The only case where `keyword != label` is `fun` for `function`. - // But in the name of preserving original behaviour, this is my opening - // move. - ( - "for", - "for", - "for (${1:variable} in ${2:vector}) {\n\t${0}\n}", - "Define a loop", - ), - ( - "if", - "if", - "if (${1:condition}) {\n\t${0}\n}", - "Conditional expression", - ), - ( - "while", - "while", - "while (${1:condition}) {\n\t${0}\n}", - "Define a loop", - ), - ( - "else", - "else", - "else {\n\t${0}\n}", - "Conditional expression", - ), - ( - "function", - "fun", - "${1:name} <- function(${2:variables}) {\n\t${0}\n}", - "Function skeleton", - ), - ]; - - for (keyword, label, snippet, detail) in snippet_data { + // Use the externalized constant + for (keyword, label, snippet, detail) in KEYWORD_SNIPPETS { let item = completion_item(label.to_string(), CompletionData::Snippet { text: snippet.to_string(), }); @@ -152,3 +152,50 @@ fn add_keyword_snippets(completions: &mut Vec) { completions.push(item); } } + +#[cfg(test)] +mod tests { + #[test] + fn test_presence_bare_keywords() { + let completions = super::completions_from_keywords().unwrap().unwrap(); + + for keyword in super::BARE_KEYWORDS { + let item = completions.iter().find(|item| item.label == *keyword); + assert!( + item.is_some(), + "Expected keyword '{}' not found in completions", + keyword + ); + let item = item.unwrap(); + assert_eq!(item.detail, Some("[keyword]".to_string())); + assert_eq!( + item.kind, + Some(tower_lsp::lsp_types::CompletionItemKind::KEYWORD) + ); + } + } + + #[test] + fn test_keyword_snippets_present() { + let completions = super::completions_from_keywords().unwrap().unwrap(); + + let snippet_labels: Vec<&str> = super::KEYWORD_SNIPPETS + .iter() + .map(|(_, label, _, _)| *label) + .collect(); + + for label in snippet_labels { + let item = completions.iter().find(|item| item.label == label); + assert!( + item.is_some(), + "Expected snippet '{}' not found in completions", + label + ); + let item = item.unwrap(); + assert_eq!( + item.kind, + Some(tower_lsp::lsp_types::CompletionItemKind::KEYWORD) + ); + } + } +} From b8674dd1a221ccabcb623418c24a3a6601f3dc6d Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Thu, 24 Apr 2025 18:34:37 -0700 Subject: [PATCH 4/9] Provide completion for `function` (as a reserved word) from keyword source --- .../ark/src/lsp/completions/sources/composite/keyword.rs | 9 +++++---- .../src/lsp/completions/sources/composite/search_path.rs | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index 0baaea077..bc4f52496 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -57,6 +57,7 @@ const BARE_KEYWORDS: &[&str] = &[ "next", "break", "repeat", + "function", ]; // Snippet data is a tuple of: @@ -121,7 +122,7 @@ fn add_bare_keywords(completions: &mut Vec) { } fn add_keyword_snippets(completions: &mut Vec) { - // Use the externalized constant + for (keyword, label, snippet, label_details_description) in KEYWORD_SNIPPETS { for (keyword, label, snippet, detail) in KEYWORD_SNIPPETS { let item = completion_item(label.to_string(), CompletionData::Snippet { text: snippet.to_string(), @@ -145,7 +146,7 @@ fn add_keyword_snippets(completions: &mut Vec) { item.detail = Some(detail.to_string()); item.documentation = Some(Documentation::MarkupContent(markup)); - item.kind = Some(CompletionItemKind::KEYWORD); + item.kind = Some(CompletionItemKind::SNIPPET); item.insert_text = Some(snippet.to_string()); item.insert_text_format = Some(InsertTextFormat::SNIPPET); @@ -176,7 +177,7 @@ mod tests { } #[test] - fn test_keyword_snippets_present() { + fn test_presence_keyword_snippets() { let completions = super::completions_from_keywords().unwrap().unwrap(); let snippet_labels: Vec<&str> = super::KEYWORD_SNIPPETS @@ -194,7 +195,7 @@ mod tests { let item = item.unwrap(); assert_eq!( item.kind, - Some(tower_lsp::lsp_types::CompletionItemKind::KEYWORD) + Some(tower_lsp::lsp_types::CompletionItemKind::SNIPPET) ); } } diff --git a/crates/ark/src/lsp/completions/sources/composite/search_path.rs b/crates/ark/src/lsp/completions/sources/composite/search_path.rs index 24ca2aed9..59e4068f0 100644 --- a/crates/ark/src/lsp/completions/sources/composite/search_path.rs +++ b/crates/ark/src/lsp/completions/sources/composite/search_path.rs @@ -52,8 +52,8 @@ fn completions_from_search_path( ) -> anyhow::Result>> { let mut completions = vec![]; - const R_CONTROL_FLOW_KEYWORDS: &[&str] = &[ - "if", "else", "for", "in", "while", "repeat", "break", "next", + const KEYWORD_SOURCE: &[&str] = &[ + "if", "else", "for", "in", "while", "repeat", "break", "next", "function", ]; unsafe { @@ -94,9 +94,9 @@ fn completions_from_search_path( continue; }; - // Skip control flow keywords. + // Skip anything that is covered by the keyword source. let symbol = symbol.as_str(); - if R_CONTROL_FLOW_KEYWORDS.contains(&symbol) { + if KEYWORD_SOURCE.contains(&symbol) { continue; } From c618f01a3fdb9a5115c5bebb92600bb6fa034862 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Thu, 24 Apr 2025 18:44:10 -0700 Subject: [PATCH 5/9] Use CompletionItem fields in the same way as other sources --- .../completions/sources/composite/keyword.rs | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index bc4f52496..270b68e1f 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -8,6 +8,7 @@ use stdext::unwrap; use tower_lsp::lsp_types::CompletionItem; use tower_lsp::lsp_types::CompletionItemKind; +use tower_lsp::lsp_types::CompletionItemLabelDetails; use tower_lsp::lsp_types::Documentation; use tower_lsp::lsp_types::InsertTextFormat; use tower_lsp::lsp_types::MarkupContent; @@ -70,7 +71,7 @@ const BARE_KEYWORDS: &[&str] = &[ // But in the name of preserving original behaviour, this is my opening // move. const KEYWORD_SNIPPETS: &[(&str, &str, &str, &str)] = &[ - // (keyword, label, snippet, detail) + // (keyword, label, snippet, label_detail.description) ( "for", "for", @@ -114,8 +115,11 @@ fn add_bare_keywords(completions: &mut Vec) { continue; }); - item.detail = Some("[keyword]".to_string()); item.kind = Some(CompletionItemKind::KEYWORD); + item.label_details = Some(CompletionItemLabelDetails { + detail: None, + description: Some("[keyword]".to_string()), + }); completions.push(item); } @@ -123,7 +127,6 @@ fn add_bare_keywords(completions: &mut Vec) { fn add_keyword_snippets(completions: &mut Vec) { for (keyword, label, snippet, label_details_description) in KEYWORD_SNIPPETS { - for (keyword, label, snippet, detail) in KEYWORD_SNIPPETS { let item = completion_item(label.to_string(), CompletionData::Snippet { text: snippet.to_string(), }); @@ -144,11 +147,14 @@ fn add_keyword_snippets(completions: &mut Vec) { value: markup, }; - item.detail = Some(detail.to_string()); item.documentation = Some(Documentation::MarkupContent(markup)); item.kind = Some(CompletionItemKind::SNIPPET); item.insert_text = Some(snippet.to_string()); item.insert_text_format = Some(InsertTextFormat::SNIPPET); + item.label_details = Some(CompletionItemLabelDetails { + detail: None, + description: Some(label_details_description.to_string()), + }); completions.push(item); } @@ -156,6 +162,8 @@ fn add_keyword_snippets(completions: &mut Vec) { #[cfg(test)] mod tests { + use tower_lsp::lsp_types::CompletionItemLabelDetails; + #[test] fn test_presence_bare_keywords() { let completions = super::completions_from_keywords().unwrap().unwrap(); @@ -164,11 +172,16 @@ mod tests { let item = completions.iter().find(|item| item.label == *keyword); assert!( item.is_some(), - "Expected keyword '{}' not found in completions", - keyword + "Expected keyword '{keyword}' not found in completions" ); let item = item.unwrap(); - assert_eq!(item.detail, Some("[keyword]".to_string())); + assert_eq!( + item.label_details, + Some(CompletionItemLabelDetails { + detail: None, + description: Some("[keyword]".to_string()), + }) + ); assert_eq!( item.kind, Some(tower_lsp::lsp_types::CompletionItemKind::KEYWORD) @@ -189,8 +202,7 @@ mod tests { let item = completions.iter().find(|item| item.label == label); assert!( item.is_some(), - "Expected snippet '{}' not found in completions", - label + "Expected snippet '{label}' not found in completions" ); let item = item.unwrap(); assert_eq!( From 3a8f91ce6271d9d1def78010768159f3b5cefd25 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Thu, 24 Apr 2025 19:08:38 -0700 Subject: [PATCH 6/9] Remove the snippet completion source --- .../src/lsp/completions/sources/composite.rs | 12 +- .../completions/sources/composite/keyword.rs | 5 +- .../completions/sources/composite/snippets.rs | 122 ------------------ 3 files changed, 5 insertions(+), 134 deletions(-) delete mode 100644 crates/ark/src/lsp/completions/sources/composite/snippets.rs diff --git a/crates/ark/src/lsp/completions/sources/composite.rs b/crates/ark/src/lsp/completions/sources/composite.rs index 085d371d9..645e99eb4 100644 --- a/crates/ark/src/lsp/completions/sources/composite.rs +++ b/crates/ark/src/lsp/completions/sources/composite.rs @@ -10,7 +10,6 @@ mod document; mod keyword; pub(crate) mod pipe; mod search_path; -mod snippets; mod subset; mod workspace; @@ -60,12 +59,6 @@ pub(crate) fn get_completions( if is_identifier_like(completion_context.document_context.node) { push_completions(keyword::KeywordSource, completion_context, &mut completions)?; - push_completions( - snippets::SnippetSource, - completion_context, - &mut completions, - )?; - push_completions( search_path::SearchPathSource, completion_context, @@ -183,8 +176,7 @@ fn is_identifier_like(x: Node) -> bool { // non-`identifier` kinds. However, we do still want to provide completions // here, especially in two cases: // - `for` should provide completions for things like `forcats` - // - `for` should provide snippet completions for the `for` snippet - // The keywords here come from matching snippets in `r.code-snippets`. + // - completions of certain reserved words from the keyword source if matches!(x.node_type(), NodeType::Anonymous(kind) if matches!(kind.as_str(), "if" | "for" | "while")) { return true; @@ -208,7 +200,7 @@ mod tests { fn test_completions_on_anonymous_node_keywords() { r_task(|| { // `if`, `for`, and `while` in particular are both tree-sitter - // anonymous nodes and snippet keywords, so they need to look like + // anonymous nodes and keywords, so they need to look like // identifiers that we provide completions for for keyword in ["if", "for", "while"] { let point = Point { row: 0, column: 0 }; diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index 270b68e1f..a091f72b1 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -63,9 +63,10 @@ const BARE_KEYWORDS: &[&str] = &[ // Snippet data is a tuple of: // - keyword: The reserved word -// - label: The label for the completion item +// - label: The label for the completion item, displayed on the left in the UI // - snippet: The snippet to be inserted -// - detail: The detail displayed in the completion UI +// - label_detail.description: The description displayed on the right in the +// completion UI // The only case where `keyword != label` is `fun` for `function`. // But in the name of preserving original behaviour, this is my opening diff --git a/crates/ark/src/lsp/completions/sources/composite/snippets.rs b/crates/ark/src/lsp/completions/sources/composite/snippets.rs deleted file mode 100644 index 77d83bdc1..000000000 --- a/crates/ark/src/lsp/completions/sources/composite/snippets.rs +++ /dev/null @@ -1,122 +0,0 @@ -// -// snippets.rs -// -// Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved. -// -// - -use std::collections::HashMap; -use std::sync::LazyLock; - -use rust_embed::RustEmbed; -use serde::Deserialize; -use tower_lsp::lsp_types::CompletionItem; -use tower_lsp::lsp_types::CompletionItemKind; -use tower_lsp::lsp_types::Documentation; -use tower_lsp::lsp_types::InsertTextFormat; -use tower_lsp::lsp_types::MarkupContent; -use tower_lsp::lsp_types::MarkupKind; - -use crate::lsp::completions::completion_context::CompletionContext; -use crate::lsp::completions::completion_item::completion_item; -use crate::lsp::completions::sources::CompletionSource; -use crate::lsp::completions::types::CompletionData; - -#[derive(RustEmbed)] -#[folder = "resources/snippets/"] -struct Asset; - -#[derive(Deserialize)] -struct Snippet { - prefix: String, - body: SnippetBody, - description: String, -} - -#[derive(Deserialize)] -#[serde(untagged)] -enum SnippetBody { - Scalar(String), - Vector(Vec), -} - -pub(super) struct SnippetSource; - -impl CompletionSource for SnippetSource { - fn name(&self) -> &'static str { - "snippet" - } - - fn provide_completions( - &self, - _completion_context: &CompletionContext, - ) -> anyhow::Result>> { - completions_from_snippets() - } -} - -pub(crate) fn completions_from_snippets() -> anyhow::Result>> { - // Return clone of cached snippet completion items - let completions = get_completions_from_snippets().clone(); - - Ok(Some(completions)) -} - -fn get_completions_from_snippets() -> &'static Vec { - static SNIPPETS: LazyLock> = - LazyLock::new(|| init_completions_from_snippets()); - &SNIPPETS -} - -fn init_completions_from_snippets() -> Vec { - // Load snippets JSON from embedded file - let file = Asset::get("r.code-snippets").unwrap(); - let snippets: HashMap = serde_json::from_slice(&file.data).unwrap(); - - let mut completions = vec![]; - - for snippet in snippets.values() { - let label = snippet.prefix.clone(); - let details = snippet.description.clone(); - - let body = match &snippet.body { - SnippetBody::Scalar(body) => body.clone(), - SnippetBody::Vector(body) => body.join("\n"), - }; - - // Markup shows up in the quick suggestion documentation window, - // so you can see what the snippet expands to - let markup = vec!["```r", body.as_str(), "```"].join("\n"); - let markup = MarkupContent { - kind: MarkupKind::Markdown, - value: markup, - }; - - let mut item = - completion_item(label, CompletionData::Snippet { text: body.clone() }).unwrap(); - - item.detail = Some(details); - item.documentation = Some(Documentation::MarkupContent(markup)); - item.kind = Some(CompletionItemKind::SNIPPET); - - item.insert_text = Some(body); - item.insert_text_format = Some(InsertTextFormat::SNIPPET); - - completions.push(item); - } - - completions -} - -#[cfg(test)] -mod tests { - use crate::lsp::completions::sources::composite::snippets::completions_from_snippets; - - #[test] - fn test_snippets() { - let snippets = completions_from_snippets().unwrap().unwrap(); - - // We should have an empty list since snippets have been moved to keyword source - assert!(snippets.is_empty()); - } -} From 09e069ac7178173c8f4a8b07ee2ad775adb2b9c7 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 29 Apr 2025 09:21:36 -0700 Subject: [PATCH 7/9] Delete the snippet file --- crates/ark/resources/snippets/r.code-snippets | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 crates/ark/resources/snippets/r.code-snippets diff --git a/crates/ark/resources/snippets/r.code-snippets b/crates/ark/resources/snippets/r.code-snippets deleted file mode 100644 index 2c63c0851..000000000 --- a/crates/ark/resources/snippets/r.code-snippets +++ /dev/null @@ -1,2 +0,0 @@ -{ -} From e6e328ba3275ba109f389c4a0d92b01cf19be5d2 Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 29 Apr 2025 10:00:50 -0700 Subject: [PATCH 8/9] Expand the completion item key Soon, several control flow elements will have both a bare and a snippet representation and we'll use the same `label` (but obviously differentiate elsewhere) --- .../src/lsp/completions/sources/composite.rs | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/ark/src/lsp/completions/sources/composite.rs b/crates/ark/src/lsp/completions/sources/composite.rs index 645e99eb4..83f444692 100644 --- a/crates/ark/src/lsp/completions/sources/composite.rs +++ b/crates/ark/src/lsp/completions/sources/composite.rs @@ -26,6 +26,23 @@ use crate::lsp::completions::sources::CompletionSource; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; +#[derive(Clone, Hash, PartialEq, Eq)] +struct CompletionItemKey { + label: String, + kind_str: String, +} + +impl CompletionItemKey { + fn new(item: &CompletionItem) -> Self { + Self { + label: item.label.clone(), + kind_str: item + .kind + .map_or_else(|| "Text".to_string(), |k| format!("{:?}", k)), + } + } +} + // Locally useful data structure for tracking completions and their source #[derive(Clone, Default)] struct CompletionItemWithSource { @@ -87,7 +104,7 @@ pub(crate) fn get_completions( fn push_completions( source: S, completion_context: &CompletionContext, - completions: &mut HashMap, + completions: &mut HashMap, ) -> anyhow::Result<()> where S: CompletionSource, @@ -96,15 +113,17 @@ where if let Some(source_completions) = collect_completions(source, completion_context)? { for item in source_completions { - if let Some(existing) = completions.get(&item.label) { + let key = CompletionItemKey::new(&item); + if let Some(existing) = completions.get(&key) { log::trace!( - "Completion with label '{}' already exists (first contributed by source: {}, now also from: {})", - item.label, + "Completion with label '{}' and kind '{:?}' already exists (first contributed by source: {}, now also from: {})", + key.label, + key.kind_str, existing.source, source_name ); } else { - completions.insert(item.label.clone(), CompletionItemWithSource { + completions.insert(key, CompletionItemWithSource { item, source: source_name.to_string(), }); @@ -117,7 +136,7 @@ where /// Produce plain old CompletionItems and sort them fn finalize_completions( - completions: HashMap, + completions: HashMap, ) -> Vec { let mut items: Vec = completions .into_values() From 0104a3bec7140d9573318f4bc0ecab8adcc5c7cc Mon Sep 17 00:00:00 2001 From: Jenny Bryan Date: Tue, 29 Apr 2025 14:23:47 -0700 Subject: [PATCH 9/9] Represent certain keywords in both bare and snippet form Deal with reserved words in the order in which they appear in their help topic and be consistent everywhere --- .../completions/sources/composite/keyword.rs | 135 ++++++++++-------- .../sources/composite/search_path.rs | 2 +- 2 files changed, 75 insertions(+), 62 deletions(-) diff --git a/crates/ark/src/lsp/completions/sources/composite/keyword.rs b/crates/ark/src/lsp/completions/sources/composite/keyword.rs index a091f72b1..df9e8a7d5 100644 --- a/crates/ark/src/lsp/completions/sources/composite/keyword.rs +++ b/crates/ark/src/lsp/completions/sources/composite/keyword.rs @@ -44,65 +44,71 @@ pub fn completions_from_keywords() -> anyhow::Result> } const BARE_KEYWORDS: &[&str] = &[ - "NULL", - "NA", "TRUE", "FALSE", + "NULL", "Inf", "NaN", + "NA", "NA_integer_", "NA_real_", - "NA_character_", "NA_complex_", + "NA_character_", + "if", + "else", + "repeat", + "while", + "function", + "for", "in", "next", "break", - "repeat", - "function", ]; -// Snippet data is a tuple of: -// - keyword: The reserved word -// - label: The label for the completion item, displayed on the left in the UI -// - snippet: The snippet to be inserted -// - label_detail.description: The description displayed on the right in the -// completion UI - -// The only case where `keyword != label` is `fun` for `function`. -// But in the name of preserving original behaviour, this is my opening -// move. -const KEYWORD_SNIPPETS: &[(&str, &str, &str, &str)] = &[ - // (keyword, label, snippet, label_detail.description) - ( - "for", - "for", - "for (${1:variable} in ${2:vector}) {\n\t${0}\n}", - "Define a loop", - ), - ( - "if", - "if", - "if (${1:condition}) {\n\t${0}\n}", - "Conditional expression", - ), - ( - "while", - "while", - "while (${1:condition}) {\n\t${0}\n}", - "Define a loop", - ), - ( - "else", - "else", - "else {\n\t${0}\n}", - "Conditional expression", - ), - ( - "function", - "fun", - "${1:name} <- function(${2:variables}) {\n\t${0}\n}", - "Function skeleton", - ), +struct KeywordSnippet { + keyword: &'static str, + label: &'static str, + snippet: &'static str, + label_details_description: &'static str, +} + +const KEYWORD_SNIPPETS: &[KeywordSnippet] = &[ + KeywordSnippet { + keyword: "if", + label: "if", + snippet: "if (${1:condition}) {\n\t${0}\n}", + label_details_description: "An if statement", + }, + KeywordSnippet { + keyword: "else", + label: "else", + snippet: "else {\n\t${0}\n}", + label_details_description: "An else statement", + }, + KeywordSnippet { + keyword: "repeat", + label: "repeat", + snippet: "repeat {\n\t${0}\n}", + label_details_description: "A repeat loop", + }, + KeywordSnippet { + keyword: "while", + label: "while", + snippet: "while (${1:condition}) {\n\t${0}\n}", + label_details_description: "A while loop", + }, + KeywordSnippet { + keyword: "function", + label: "fun", + snippet: "${1:name} <- function(${2:variables}) {\n\t${0}\n}", + label_details_description: "Define a function", + }, + KeywordSnippet { + keyword: "for", + label: "for", + snippet: "for (${1:variable} in ${2:vector}) {\n\t${0}\n}", + label_details_description: "A for loop", + }, ]; fn add_bare_keywords(completions: &mut Vec) { @@ -127,7 +133,13 @@ fn add_bare_keywords(completions: &mut Vec) { } fn add_keyword_snippets(completions: &mut Vec) { - for (keyword, label, snippet, label_details_description) in KEYWORD_SNIPPETS { + for KeywordSnippet { + keyword, + label, + snippet, + label_details_description, + } in KEYWORD_SNIPPETS + { let item = completion_item(label.to_string(), CompletionData::Snippet { text: snippet.to_string(), }); @@ -168,9 +180,15 @@ mod tests { #[test] fn test_presence_bare_keywords() { let completions = super::completions_from_keywords().unwrap().unwrap(); + let keyword_completions: Vec<_> = completions + .iter() + .filter(|item| item.kind == Some(tower_lsp::lsp_types::CompletionItemKind::KEYWORD)) + .collect(); for keyword in super::BARE_KEYWORDS { - let item = completions.iter().find(|item| item.label == *keyword); + let item = keyword_completions + .iter() + .find(|item| item.label == *keyword); assert!( item.is_some(), "Expected keyword '{keyword}' not found in completions" @@ -183,32 +201,27 @@ mod tests { description: Some("[keyword]".to_string()), }) ); - assert_eq!( - item.kind, - Some(tower_lsp::lsp_types::CompletionItemKind::KEYWORD) - ); } } #[test] fn test_presence_keyword_snippets() { let completions = super::completions_from_keywords().unwrap().unwrap(); + let snippet_completions: Vec<_> = completions + .iter() + .filter(|item| item.kind == Some(tower_lsp::lsp_types::CompletionItemKind::SNIPPET)) + .collect(); let snippet_labels: Vec<&str> = super::KEYWORD_SNIPPETS .iter() - .map(|(_, label, _, _)| *label) + .map(|snippet| snippet.label) .collect(); for label in snippet_labels { - let item = completions.iter().find(|item| item.label == label); + let item = snippet_completions.iter().find(|item| item.label == label); assert!( item.is_some(), - "Expected snippet '{label}' not found in completions" - ); - let item = item.unwrap(); - assert_eq!( - item.kind, - Some(tower_lsp::lsp_types::CompletionItemKind::SNIPPET) + "Expected snippet '{label}' with SNIPPET kind not found in completions" ); } } diff --git a/crates/ark/src/lsp/completions/sources/composite/search_path.rs b/crates/ark/src/lsp/completions/sources/composite/search_path.rs index 59e4068f0..5601cec23 100644 --- a/crates/ark/src/lsp/completions/sources/composite/search_path.rs +++ b/crates/ark/src/lsp/completions/sources/composite/search_path.rs @@ -53,7 +53,7 @@ fn completions_from_search_path( let mut completions = vec![]; const KEYWORD_SOURCE: &[&str] = &[ - "if", "else", "for", "in", "while", "repeat", "break", "next", "function", + "if", "else", "repeat", "while", "function", "for", "in", "next", "break", ]; unsafe {