feature: for all config values that override commands, canonicalize the provided path

[davidbarsky]

(The way that I came across this issue was a confluence of weird BigCo constraints, the use of a virtualized file system, and me acting as a de-facto vendor for Rust Analyzer. I don't expect most people to encounter this in the first place.)

I've noticed a small inconsistency when working with config values that override commands: namely, the config value provided for rust-analyzer.server.path will trim the leading ~ from the provided path, but the same will not occur for rust-analyzer.rustfmt.overrideCommand (I assume the holds for every other overridable command in Rust Analyzer, but I haven't tested those other commands yet) . This means that I can't point every instance of Rust Analyzer used at my employer at the appropriate rustfmt (or any other binary) inside of the monorepo, as providing the home directory-relative path to rustfmt results in a textDocument/formatting failed. Message: Failed to spawn error being shown.

If it's okay with y'all, I'd be happy to write a bit of code in the Rust Analyzer server itself to bring the existing behavior for rust-analyzer.server.path to all other config values that accept paths. I don't think it'll cause any issues for people who don't have weird setups like my employer does, but it'll certainly reduce my support burden. If nothing else, it might remove an inconsistency.

Alternatives Considered

• Using "${userHome} instead of ~/. Unfortunately, variable interpolation is not supported inside of VS Code's settings (Support variables when resolving values in settings microsoft/vscode#2809).
• Adding rustfmt to the $PATH by default. This is feels a bit hacky and can mess with people's rustup-managed toolchain, so I'd rather avoid this if I can.

[Veykril]

I am planning on adding proper server sided interpolation to our configs (or at least all overrideCommand configs) which would help out here if we provide something like ${userHome}. See #13128 and #13297
The main blocker right now is that I am unsure how the interpolation should look like (as changing it once implemented won't be an option).

[davidbarsky]

I am planning on adding proper server sided interpolation to our configs (or at least all overrideCommand configs) which would help out here if we provide something like ${userHome}. See #13128 and #13297.

Gotcha! I like that idea; it seems like it'll work for my use-case.

The main blocker right now is that I am unsure how the interpolation should look like (as changing it once implemented won't be an option).

For my curiosity: is it because you'd like to minimize churn in the configs or is it because of a new stability policy? In either case, I'd be happy to test your implementation, provide feedback from hundreds of people and migrate them if/when breaking changes are made. Naively, it seems like an unstable flag that's passed to rust-analyzer at process startup (not via the initialize request) that enumerates which unstable flags are enabled seems like an approach that can work and reduce down on the blast radius to people who have explicitly opted in while you're figuring out the implementation details. Again, happy to help out with implementing portions of this (e.g., the unstable feature flags).

To step back to the original question: would y'all accept a PR that checks if the first argument provided to overrideCommand starts with ~/, and if so, removes it and replaces it with the full home directory (provided by home, which seems to be the recommended replacement)? I know that my solution is a bit of stopgap for your correct solution, but the stopgap does resolve a non-trivial pain-point for me and my users and it doesn't block variable interpretation down the line. If you don't want to accept it, I can try working on the unstable feature flags, patch this into the vendored rust-analyzer binary used at my employer, or direct any contributions to rust-analyzer wherever you might consider to be most impactful.

[Veykril]

No our config is not stable, but we'll try to minimize breakages as much as possible (I already pushed for a major breakage to make it more uniform once and other client implementation maintainers were anything but happy, understandably so), and breaking the syntax for interpolation sounds even worse than renaming config keys.

I don't like the ~/ replacement as overrideCommands are more than just a path (they specify a list). There is one thing that we can do on the client side (if I see this right you all use VSCode). We have the following issue #9626, that is as youve pointed out, extensions dont have the variable substitution that VSCode has for its native configs, but most of them can be re-implemented by us in the client. So to solve your problem without the server sided interpolation, I'd say lets implements the client side substitution for ${userHome} in the VSCode r-a client (no need to create a general system for this, just doing a simple replacement for that specific string works fine for the time being).

[davidbarsky]

No our config is not stable, but we'll try to minimize breakages as much as possible (I already pushed for a major breakage to make it more uniform once and other client implementation maintainers were anything but happy, understandably so), and breaking the syntax for interpolation sounds even worse than renaming config keys.

Gotcha, that makes sense. If you'd like, I'd be happy to make an issue for the unstable flags proposal.

I don't like the ~/ replacement as overrideCommands are more than just a path (they specify a list). There is one thing that we can do on the client side (if I see this right you all use VSCode). We have the following issue #9626, that is as youve pointed out, extensions dont have the variable substitution that VSCode has for its native configs, but most of them can be re-implemented by us in the client. So to solve your problem without the server sided interpolation, I'd say lets implements the client side substitution for ${userHome} in the VSCode r-a client (no need to create a general system for this, just doing a simple replacement for that specific string works fine for the time being).

Yeah, I think this will resolve the immediate issue. Folks who are using other editors are largely on their own, so making them do config changes isn't an unreasonable demand.

[Veykril]

#13423 should allow you to set ${userHome} (note today's stable won't ship it, as I want this to appear in some nightlies first starting with tomorrow's one)

[Veykril]

I believe we can close this issue since the vscode side does substitute userhome now