Skip to content

Rationalize completion treatment of reserved words #779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jennybc opened this issue Apr 16, 2025 · 0 comments · Fixed by #782
Closed

Rationalize completion treatment of reserved words #779

jennybc opened this issue Apr 16, 2025 · 0 comments · Fixed by #782
Assignees

Comments

@jennybc
Copy link
Member

jennybc commented Apr 16, 2025

Let's analyze the current treatment of R's reserved words in ark's completion system.

Three completion sources are relevant:

Keywords

Explicit inclusion of a subset of R's reserved words. See keyword.rs. The main point here is (I think) setting the completion item kind in such a way that the UI gives the item a special icon and annotates it with [keyword] as opposed to {base}.

Image

Search path

Generates completions by recursively walking the search path, which includes the base package, by definition. But certain reserved words are explicitly excluded. I think the original intent here is to exclude words that are being handled by another source. See search_path.rs.

The exclusion list is stored in the variable R_CONTROL_FLOW_KEYWORDS, which suggests the main targets are words related to control flow. If not excluded, these words would bubble up from the base namespace. Side observation: function and return() (not a reserved word) were recently removed from this exclusion list (#768). Therefore function and return() now appear in search path completions and are marked with the {base} namespace.

Snippets

This source is generated from a file of built-in snippets that lives in ark at r.code-snippets. These generally insert a whole template. E.g., for if or else or for, the snippet adds any necessary parentheses/brackets and controls placeholder traversal and position of cursor at exit.

It's important to distinguish between the snippet completion source and just the general idea of snippets. The snippet completion source does not have a monopoly on snippet completions. Any completion item can insert plain text, a snippet, or a text edit.

The file of built-in snippets originally lived in positron-r, which is more in line with VS Code conventions around snippets. The file was moved into ark to confer the ability to not display snippets in obviously inappropriate contexts, such as completing data frame column names.

Reserved words and coverage by different sources

Reserved word keyword search_path r.code-snippets
if ✅ (prefix: "if")
else ✅ (prefix: "el")
repeat
while ✅ (prefix: "while")
function ✅ (prefix: "fun")
for ✅ (prefix: "for")
in
next
break
TRUE
FALSE
NULL
Inf
NaN
NA and all type-specific variants

I have a few proposals on rationalizing the completion treatment of reserved words:

  • Every reserved word should have a completion item contributed by either the search path source or the keyword source. Probably by the keyword source. No reserved word should be covered only (or even at all?) by a snippet.
    • The biggest violation of this currently is repeat, which is not contributed as a completion item at all! I feel like there's an implicit contract where we promise to expose the entire "base language" in completions. In practice, I believe a typical user assumes that the completion list enumerates "everything", up to any expected filtering based on context.
    • if, while, and for are currently only covered by the snippet source, which also feels odd. I propose we move these into the keyword source (else is already handled there). We can still get the snippet-y behaviour of inserting a larger construct and leaving the cursor in a specific location.
  • I'm now convinced there should be no built-in snippets. See R: approach snippets in a more conventional VS Code way positron#7234 and Eliminate built-in snippets #780 for more.
  • We should move function to the keyword source (as opposed to letting it bubble up in the search path source).
  • The else snippet has prefix (label) el and is preventing the creation of a completion item for el() in the methods package. If we got rid of built-in snippets and handled else in the keyword source, this problem goes away.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant