Vscode-r: Add r.rpath.xxx setting

Created on 24 Nov 2020  路  17Comments  路  Source: Ikuyadeu/vscode-R

Currently the extensions vscode-r-lsp, vscode-r-debugger and the help panel from this extension each have a configuration entry where the user needs to specify the path of an R binary.

These are all "backend" usecases where alternatives like radian offer no advantages (as far as I'm aware) over the standard R executable. To make vscode+R easier to configure, I'd suggest introducing a setting r.rpath.xxx, similar to r.rterm.xxx which is supposed to be a "vanilla" R binary that can be used by the mentioned functions/packages.

Advantages include:

  • Users don't need to make the same configuration in three different places
  • For developers it's easier to implement new features that need an R process as backend (e.g. an R CMD INSTALL . task)

Disadvantages include:

  • Users cannot configure different R executables for the different extensions/functions.
  • Changing the settings might invalidate existing settings

As a solution to these two issues, I'd suggest introducing a new unified r.rpath.xxx setting, while still honoring the old settings (i.e. to get the R path in the lsp check r.lsp.path -> r.rterm.xxx -> read from path etc.). Even if the package/function specific settings are not specified in the package.json anymore, it is still possible to read their value from existing setting.json files.

feature-request

Most helpful comment

How about making these values from each OS path to like one r.path and r.term?

That would be nicer, but might cause problems for people using WSL or syncing settings between different OSs. A discussion about scoping settings by platform has been ongoing for a while now but as far as I'm aware, there is no clean solution for this yet

All 17 comments

The three PRs linked above contain a first implementation of the unified r.rpath.xxx setting.

As mentioned before, there is a tradeoff here, but I think for the majority of users the comfort of setting the rpath only once (if at all) outweighs the possible disadvantage of not being able to configure individual paths for each extension.

I am not sure to what extent this configuration sharing is intended by the authors of vscode, but it seems to work nicely. (In the latest version the hyperlinks in deprecation messages sometimes don't work correctly. In the current insiders version, shared configurations are sometimes displayed twice, but I think that's not intended).

As mentioned before, there is a tradeoff here, but I think for the majority of users the comfort of setting the rpath only once (if at all) outweighs the possible disadvantage of not being able to configure individual paths for each extension.

Agree. There's already too much confusion in these settings and configuration for new users.

I am not sure to what extent this configuration sharing is intended by the authors of vscode, but it seems to work nicely.

Ideally, I support "share what could be shared", which I believe is for the benefit of most users.

There is a pending discussion of moving vscode-R to REditorSupport so we could merge this repo with vscode-r-lsp: https://github.com/Ikuyadeu/vscode-R/issues/98

What about the possibility of defaulting some of these settings and picking R from PATH?

I suppose there are a couple ways to do this:

  1. attempt to find R on the path if not set - needs to be looked up every time extension is run.

  2. offer a 'configure' command that attempts to detect R and set the settings. The VSpaceCode extension does this: https://github.com/VSpaceCode/VSpaceCode/blob/master/src/configuration/configuration.ts (not for R, but for settings it cares about)

Could even prefer path to radian if detected for the interactive case.

What about the possibility of defaulting some of these settings and picking R from PATH?

We already have these. If the settings are left blank, then we find R in registry (only on Windows) and then in PATH.

Oh sorry, although I don't think this is the case for the LSP right?

Oh sorry, although I don't think this is the case for the LSP right?

https://github.com/REditorSupport/vscode-r-lsp/blob/master/src/util.ts is the lsp version of getting R path. It does not explicitly find R in PATH after searching the Windows registry but I guess it should work by spawning R directly?

Regarding this setting in the lsp: If the user specifies an invalid path in their setting (detected here), the lsp-client silently moves on to reading the R path from registry/PATH. To avoid confusion it might be better if the lsp-client fails loudly and tells the user about their invalid config.

It does not explicitly find R in PATH after searching the Windows registry but I guess it should work by spawning R directly?

I don't know if there are any scenarios where using R fails but finding R on the path works. To avoid weird behaviour in edge cases we could consider using the same functions in vscode-r-lsp as in vscode-r, but ideally this shouldn't make much of a difference

Sharing values is a good idea!

Currently, we have setting items for each OS like r.path.windows, r.path.linux, but they have been separated only for the default values.
However, the default values are not so working especially for windows.
How about making these values from each OS path to like one r.path and r.term?

How about making these values from each OS path to like one r.path and r.term?

That would be nicer, but might cause problems for people using WSL or syncing settings between different OSs. A discussion about scoping settings by platform has been ongoing for a while now but as far as I'm aware, there is no clean solution for this yet

Yes, the issue looks important, and we implemented r.path.windows by that background.
However, I am concerned that expanding configuration will complicate the introduction of light users.

A similar issue has been discussed in vscode-python, and it's closed as a VSCode issue. https://github.com/microsoft/vscode-python/issues/10822

In the VSCode team comment, they have two future solutions, https://github.com/microsoft/vscode/issues/5595#issuecomment-565381016
As he says, our current solution 1. is already covered by separated user/workspace/folder settings.

  1. Use language specific settings style. This is thought about and after thinking further we decided not to go with this approach. Reason being language overrides has strong precedence and they can always win against user/workspace/folder settings. This is not intended for platform specific settings. See #86737 for more information.
  2. Separate platform specific files as mentioned in comment #5595 (comment). This way sync will also sync settings appropriately per platform.

Regarding the platform dependent settings, I'd suppose we stick to our current approach and wait for updates from vscode to solve this cleanly.

Are there more issues with the suggested configuration or should we go ahead and merge the changes?

I think this can be done independenly form the discussed merge of vscode-R and vscode-R-lsp, since the shared configuration can just be replaced by a single configuartion entry once they are combined.

@ManuelHentschel I understand, it can be merge once.
We can combine configuration after the issue is solved.

Closed by #467.

Was this page helpful?
0 / 5 - 0 ratings