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

Options completion #389

Merged
merged 3 commits into from
Mar 17, 2021
Merged

Conversation

renkun-ken
Copy link
Member

@renkun-ken renkun-ken commented Mar 13, 2021

This PR adds arg completion for options() calls so that it is easier to complete options.

Note that if user code calls e.g. library(data.table), and the new options defined in data.table will also appear in the options arg completion since the package is also loaded by languageserver.

@ManuelHentschel
Copy link
Member

Works nicely, thanks!

Would you consider listing the options used in base R that are not set initially? (Might be useful, but would also require keeping the list up to date and hence introduce a source of inconsistencies)

Also I saw that in the help for options() that using .Options is apparently faster than calling options(), in case this (probably rather small) performance difference is of any concern here.

@renkun-ken
Copy link
Member Author

Would you consider listing the options used in base R that are not set initially? (Might be useful, but would also require keeping the list up to date and hence introduce a source of inconsistencies)

Do we have a programmatic way to do this like parsing R source files, find all getOption() calls and extract those names? Maybe it is worth another PR to do this?

Also I saw that in the help for options() that using .Options is apparently faster than calling options(), in case this (probably rather small) performance difference is of any concern here.

Yes, it is definitely faster. Thanks!

@ManuelHentschel
Copy link
Member

Do we have a programmatic way to do this like parsing R source files, find all getOption() calls and extract those names? Maybe it is worth another PR to do this?

That sounds possible, but would probably warrant a separate PR as you suggested

@renkun-ken renkun-ken merged commit 4e4de49 into REditorSupport:master Mar 17, 2021
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