Skip to content
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

draft: Add snippets #158

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

draft: Add snippets #158

wants to merge 2 commits into from

Conversation

gadenbuie
Copy link
Contributor

@georgestagg @wch In returning to #153, I wondered if the feature I'm actually wanting is snippets.

I took a stab at defining language-specific shiny snippets for R and Python, using the same snippets we use in the Shiny VS Code extension...

...and it almost works. I have the sense I'm fairly close (drawing mainly from this example), but the snippets aren't appearing in the editor. I'm hoping one of you might be able to quickly spot the issue.

@georgestagg
Copy link
Collaborator

I've made a commit that gets things working for me. With this change, typing shiny in the editor offers the snippets as autocomplete options.

We could think about populating other properties like displayLabel⁠ alongside the label, so as to make the snippets clearer in the autocomplete list. Providing a type would give the entries an icon, for example. We could also think about setting boost to some number so that snippets float to the top.

@@ -60,7 +60,6 @@ export function languageServerExtensions(
}
}
}),
autocompletion(lspClient, filename),
Copy link
Collaborator

@georgestagg georgestagg Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, but removing this definitely was required to get snippets working. I guess it's replacing the autocompletion extension setup by the file above.

In a quick test, removing this did not seem to do any harm and context aware completion still seemed to work. Still, we should try and work out why this was originally here and if it's still strictly necessary - though it doesn't seem to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants