In a previous commit we added proper support for separate configurations
per workspace folder, but that effectively broke support for processing out-of-workspace
files. Given how useful this is (e.g. when iterating on a test case in /tmp), this
commit refactors server creation to support this again. We support this case using
a "fallback" server that specifically handles files not within the workspace. This uses
the configuration settings for the current workspace itself (not the specific folder).
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/utils/vscode/src/mlirContext.ts | ||
---|---|---|
132 | And all of this is required as we can have different dialects linked into different LSPs? (so PDLL doesn't need this?). And so to avoid requiering custom concatted LSP servers with deps, one can have one per workspace? (well probably that doesn't completetly avoid it as a user could still be using the union of the dialects in some client). It made sense when you added this, but now I'm wondering benefit vs implementation cost. |
mlir/utils/vscode/src/mlirContext.ts | ||
---|---|---|
132 | It's not just about concatted server deps, it also applies to the the version of LLVM that is checked out. The way that my workspace is structured (and I assume I'm not alone) is that I have a workspace folder for the different projects I develop (e.g. one for LLVM, one for Modular AI, etc.). Not all of these have an overlap of deps (Modular doesn't register the MLIR test dialect for example), and not all have the same version (i.e. git checkout) of every dialect. I've also been bitten by PDLL the same way, where the checkout of LLVM has a different version of PDLL than the one I have in other projects. If we don't allow multiple servers, workspaces like mine generally just can't work. This is especially true given that the downstream projects are often further behind HEAD than my LLVM checkout (meaning that even if I do concat all the deps, the server doesn't understand LLVM HEAD). |
Thanks for fixing this River :)
mlir/utils/vscode/src/mlirContext.ts | ||
---|---|---|
143 | maybe my JS/TS is out of date, but this vscode.EnvironmentVariableMutatorType looks like an no-op statement to me. |
mlir/utils/vscode/src/mlirContext.ts | ||
---|---|---|
143 | Nice catch (not sure how that slipped in there). |
And all of this is required as we can have different dialects linked into different LSPs? (so PDLL doesn't need this?). And so to avoid requiering custom concatted LSP servers with deps, one can have one per workspace? (well probably that doesn't completetly avoid it as a user could still be using the union of the dialects in some client). It made sense when you added this, but now I'm wondering benefit vs implementation cost.