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

R executable selection #1143

Closed
wants to merge 23 commits into from
Closed

Conversation

ElianHugh
Copy link
Collaborator

@ElianHugh ElianHugh commented Jul 4, 2022

WIP R executable selection

Major contribution: extended R executable selection. Related issues #696 #1130 #1148 #747

This PR may need to be split into two parts: (1) R executable selection and (2) improved multi-root support

Screenshots

image

image

image

TODO

R Executable

  • Language status item
  • Executable quick pick
  • Unix support for standard R installations
  • Unix support for conda, mamba
  • Windows support for standard R installations
  • Windows support for conda, mamba
  • Look into better supporting renv

Multi-root

Multi-root executable support potentially requires modification of many vscode-R features

  • R terminal
  • Language service
  • Help provider
  • Tasks

Other features should probably be deferred to another PR

@ElianHugh
Copy link
Collaborator Author

I'll have access to windows later today, so I should be able to work on the windows side to get to feature parity

@ElianHugh
Copy link
Collaborator Author

An issue I've found that on windows side, if an R executable isn't openable it causes an annoying popup when loading the extension. It doesn't look like fs.access or fs.stat checks for this on windows

@ElianHugh
Copy link
Collaborator Author

This is currently how the picker is looking:

image

Recommendation is due to the renv lock file denoting a matching R version

ElianHugh and others added 12 commits July 19, 2022 21:16
- Use R --version for details
- Only create executables when calling refreshPaths()
Prevent outputting malformed tooltips when executable arch or version is undefined
Executables access is now handled purely through the RExecutableManager, rather than any member classes
- Revert language service changes
    - need to look at how python does it
- Recommend binaries that are in the workspace or are specified by a renv lockfile
- Fix possibly undefined issues
- Normalise path for windows
- Update vscode types and engine to 1.65
- Async init method for the manager
Use an enum to restrict the input for the setting: r.rmarkdown.codeLensCommands
Enum change was pushed to package.json for some reason
QP items now grouped into
- recommended
- virtual
- global

- Changed various functions into arrow functions to pass `'this'
- rpath is machine-overridable
- task rpath dependent on active workspace executable
- quickpick shows arrow for active path
Error is triggered when selecting an R installation that cannot be opened
- Create R terminal now injects  conda env values where appropriate
- Conda env values are saved in /tmp/ for injection into processes
Minor stylistic changes, no behaviour change
@ElianHugh
Copy link
Collaborator Author

Coming back to this PR, I think it's probably okay if support for "all" virtual environments is scaled back to just conda and renv?

@eitsupi
Copy link
Contributor

eitsupi commented Sep 7, 2022

if support for "all" virtual environments is scaled back to just conda and renv?

I think it's almost the same as conda, but also mamba?

ElianHugh and others added 7 commits February 20, 2023 16:38
Add tests for pathStorage,
make rPath === activeExecPath again,
linter
terminals once again respect activeExecutable
- Add documentation for various classes
- Ensure that superfluous `await`s are removed
- Rename some types for clarity
@ElianHugh
Copy link
Collaborator Author

I'm reasonably happy with the executable UI & the executable locator services. main issue for this PR right now are conda activation (seems a little fragile in terms of how I'm handling it) and also feature parity on windows devices

@floriandeboissieu
Copy link

I'm waiting eagerly for the R executable selection with conda/mamba support. Any chance to have this PR finalized & merged in the near future?

@ElianHugh
Copy link
Collaborator Author

Hi @floriandeboissieu, thank you for your interest in the PR. Unfortunately this year has been pretty chock-a-block and I haven't had much time to do open source projects unless they are directly related to work. Having said that, I will have plenty of free time over the Christmas break, so I'm hoping to jump back into things then.

@ElianHugh
Copy link
Collaborator Author

Thank you all for the feedback. I am closing this PR in favour of work on a new branch (#1473).

@ElianHugh ElianHugh closed this Jan 5, 2024
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.

4 participants