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

Is serverExecutablePath actually being deprecated? #529

Closed
chiroptical opened this issue Feb 7, 2022 · 9 comments · Fixed by #742 or #773
Closed

Is serverExecutablePath actually being deprecated? #529

chiroptical opened this issue Feb 7, 2022 · 9 comments · Fixed by #742 or #773

Comments

@chiroptical
Copy link

"markdownDescription": "Manually set a language server executable. Can be something on the $PATH or the full path to the executable itself. Works with `~,` `${HOME}` and `${workspaceFolder}`. **Deprecated scope**: This option will be set to `machine` scope in a future release, so it can be changed only globally, not per workspace."

A colleague of mine recently noticed this "deprecated scope" message. At Mercury we have quite a bit of trouble running HLS on our codebase. A coworker wrote an alternative which provides only type on hover and go to definition and we use this variable to point to the alternate provider. It makes perfect sense to default to hls, but it would be really nice to leave room for an alternate server. Thanks!

@hasufell
Copy link
Member

The next release will still have it, but you have to make sure to not enable manageHLS config option. The extension will ask you if you want that on first start.

@fendor
Copy link
Collaborator

fendor commented May 1, 2022

The deprecation warning is only regarding the scope of the option afaict.

But I don't see why it should be machine scope rather than per-project scope?

@friedbrice
Copy link

friedbrice commented Jul 6, 2022

Please reconsider removing the ability to set haskell.serverExecutablePath on a per-workspace scope.

Removing the ability to set this option on a per-workspace scope breaks a common and reasonable use case. I'll elaborate.

On most Haskell projects, HLS works just fine and I'd like to be able to use it. However, on one particularly-gnarly project HLS just doesn't work at all. This project happens to be the one I most-frequently work in. On that project's workspace I use haskell.serverExecutablePath to use an alternative light-weight, low-feature language server.

Again, this "bad" project happens to be the one I most-frequently work on, and so I definitely need to be using this alternative backend, at least for this one workspace. If haskell.serverExecutablePath is changed to global-only scope, then I will be forced to set haskell.serverExecutablePath to the alternative backend globally and forgo the use of HLS on all my other "good" projects. Needless to say, that would make me very sad.

@fendor
Copy link
Collaborator

fendor commented Jul 6, 2022

I agree, that seems like a totally legit use-case.

I have no idea why this deprecation notice was added, I will try to dig in the git logs, but if I can't find any compelling reasoning, this deprecation notice will be deprecated soon. (I.e. it will not be deprecated any longer)

EDIT:
Reasoning is given here: https://github.com/haskell/vscode-haskell#security-warning and the PR that added the deprecation notice: bc49326, main issue being: #387

@friedbrice
Copy link

friedbrice commented Jul 6, 2022

@fendor This is why we can't have nice things 😞

(Also, thank you for checking.)

@friedbrice
Copy link

friedbrice commented Jul 6, 2022

As an alternative, a strictly-global scope setting that says allowWorkspaceExecutable, that defaults to false, and that users can change at their own peril. I would totally set it to true at my own peril! (This is because I disable vscode-haskell globally and only enable it on a per-workspace basis. I do this for most of my IDE-like extensions. In that vein, though, I guess the "right" solution is to make a vscode extension specifically for this alternative backend I use 😞)

@fendor
Copy link
Collaborator

fendor commented Jul 7, 2022

vscode-clangd uses machine-overridable: clangd/vscode-clangd#64
rust-analyzer also uses machine-overridable: https://github.com/rust-lang/rust-analyzer/blob/master/editors/code/package.json#L303
Same for python (which contains many path settings): https://github.com/microsoft/vscode-python/blob/main/package.json#L397

Documentation for machine-overridable: https://code.visualstudio.com/api/references/contribution-points

I can't really tell straight away the difference between resource and machine-overridable, but seems like we have some reason to switch to machine-overridable.

@mikelpr
Copy link

mikelpr commented Oct 17, 2022

+1 on please DON'T

I get my HLS and GHC per project via nix expressions. I set haskell.serverExecutablePath to nix-shell --pure hls.nix --run "haskell-language-server $@" per workspace, this gets written to .vscode/settings.json. This has worked perfectly for me to keep projects on a specific GHC version until I want to update them and have no clashes. Now there's this ominous warning about all this work going to the trash and I don't understand why is it a problem at all and why was it thought the solution would be to get rid of this functionality.

machine-overrideable seems like it fixes your concerns and still allows for setting it on individual workspaces. I really hope the setting stays

I'm sorry for coming here to +1 but the warning is still there and scary and nobody has mentioned the nix usecase :3

fendor added a commit to fendor/vscode-haskell that referenced this issue Oct 18, 2022
Removes deprecation warning.
In accordance with haskell#529 (comment)
don't deprecate the serverExecutablePath's scope, and set it to the
standard 'machine-overridable'
fendor added a commit that referenced this issue Oct 18, 2022
Removes deprecation warning.
In accordance with #529 (comment)
don't deprecate the serverExecutablePath's scope, and set it to the
standard 'machine-overridable'
@fendor
Copy link
Collaborator

fendor commented Oct 18, 2022

serverExecutablePath is no longer deprecated and the scope is set to machine-overridable.

Please shout, if that doesn't work for someone.

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