vscode: Offer an option to set the Slint-lsp binary #8964
Conversation
|
Upside: More flexibility for users |
editors/vscode/package.json
Outdated
| @@ -163,6 +163,11 @@ | |||
| } | |||
| }, | |||
| "description": "Map of paths in which the `import` statement for `@mylibrary` imports are looked up. This is an object such as `{\"mylibrary\": \"/path/to/library\"}`. Relative paths are resolved against the workspace root." | |||
| }, | |||
| "slint.searchForLspInPath": { | |||
There was a problem hiding this comment.
Can we also have an option to set the full path to the LSP ?
There was a problem hiding this comment.
Same here, I think an option to specify the full path instead of a boolean is better, because it's 100% deterministic (unlike PATH) and it makes more sense in the context of VS code's command line launch semantics. (running code /path isn't guaranteed to pick up PATH).
There was a problem hiding this comment.
The original reporter was using nixos: They will have the slint-lsp binary stored in the nix store. A part of the file path will be a hash value that will change on each update. Nix-shell (or whatever that thing is called) will typically set up PATH to contain a long list of entries to bring everything the user had configured into scope.
For that use-case I think the boolean I implemented makes more sense.
We can add another setting with a full file path. What should be the semantics when that is set? Only try that file and error out when it is not found? Or append it as the first entry in the list of file paths to check?
What should we do with the boolean when we add the full file path? Remove that again?
There was a problem hiding this comment.
Not all the nixos users will have slint-lsp in their path or using devshells.
That's the beauty of Nixos which is amazing when it comes to containerization.
For example, if we set the configuration option for the slint-lsp path to something like ${lib.getExe pkgs.slint-lsp}, the LSP will only be available there (in vscode) and not in the user path. Which makes sense, it's very rare when users have to deal themselves with a LSP, it's usually their IDE that does the job.
Here's an example with Tinymist, a LSP for Typst: https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/editors/vscode/extensions/myriad-dreamin.tinymist/default.nix (line 26).
Other examples: rust-lang.rust-analyzer, betterthantomorrow.calva, azdavis.Millet, tekumara.typos-vscode...
Users won't have access to Tinymist but the vscode will have access successfully.
The same pattern is applied for linters.
There was a problem hiding this comment.
Indeed, I was reading #3903 I thought a path is exactly what’s wanted:
I'm not sure if this should be considered a bug, but as I am using NixOS and use locally built packages for 99% of the system. I would like to specify the path of the slint-slp executable in VSCode settings.
But from the issue you quoted that doesn’t help indeed.
Suggestion: provide one setting that’s a path to the lsp. If it’s an absolute path: all good. If it’s a relative path, it’s searched for in PATH. That works with slint lsp being in PATH as well as folks who may even rename the binary.
There was a problem hiding this comment.
The newest version has a string setting that is either empty (what we did before), an absolute path (use exactly that binary) or a relative path (look for this in $PATH).
hunger
force-pushed
the
tobias/push-omsmutxowonx
branch
from
00f03a8 to
b43f8ce
Compare
hunger
force-pushed
the
tobias/push-omsmutxowonx
branch
from
b43f8ce to
05553eb
Compare
|
Latest version records the fact that a custom binary was used jy appending " [CUSTOM BINARY]" the the version information, so we do not run of chasing wild geese when crash reports of some random slint-lsp get uploaded. |
|
Nice :) |
|
I don't want this. I want an option to provide a path to the |
That is what this patch provides, doesn't it? The title of the PR may not be adjusted, but to me the diff reads that a setting is added to permit specifying a path to the slint-lsp binary. What am I missing? |
|
@spikespaz: You can set an absolute path to the slint-lsp with this PR applied. |
Ah, sorry, I assumed the title described the changes. |
hunger
changed the title
|
@spikespaz Oh, right: I forgot to update it after the first attempt got rejected. I did now. Sorry for causing confusion. |
|
@hunger Thank you for doing this! |
Add a string configuration option to configure whith LSP to use. Possible values are: * Empty string: Use the packaged LSP (or one found in /target) * Absolute path: Use exactly that slint-lsp * Relative PATH: Try to look up that name in PATH
hunger
force-pushed
the
tobias/push-omsmutxowonx
branch
from
05553eb to
3ca264a
Compare
|
@spikespaz: You are welcome |
... which is disabled by default.
This might help with issues similar to #2577