fix(engine): only start one engine for umbrella apps#462
fix(engine): only start one engine for umbrella apps#462katafrakt wants to merge 3 commits intoelixir-lang:mainfrom
Conversation
|
I'm kind of confused why so much specific code is required for this. Shouldn't the way to determine the project is by comparing to the root_uri/workspace folder? In Next LS that is how I dispatched requests/notifications to each engine node/document store |
There might be no mix project at this. This is quite common for monorepos actually. |
That is not what an umbrella app is tho. An umbrella app will have a project. And to clarify, i meant at the root of the workspace folder. If someone opens a mono repo with 1 umbrella and 1 normal, you still just need to treat the umbrella as just an app So for any file you check # pseudo code, there would be more around absolute paths/etc
for project_path <- ["monorepo/umbrella", "monorepo/normal"] do
String.starts_with?(file_uri, project_path)
end |
|
Basically, the editor tells us the root directories of each workspace (folder) and we need to route requests to them based on the file uri. I don't think there is much else to it. |
|
I'm opening my work monorepo now and it just sends one workspace folder: The Elixir project is at |
You have to tell the editor the workspace folders, it doesn't automatically guess them. |
|
The thing is that I don't have to do it anymore. With automatic subprojects detection I just open one workspace folder and Expert finds a mix project in a subfolder. |
I think there may be confusion as to what we are trying to fix here. Are you implying we already have that? I am not under the impression that we do. That is what workspace folders are for. |
|
What happens today:
We're using |
|
yes, we have it I think since #18 |
|
Yes, and that is what this PR is trying to fix. If we know you're at an umbrella, we'd need to start an engine for the umbrella root only. |
|
In my opinion, we need to assume the root of the workspace folder is the root of the app with the mix.exs file and start the engine in there. |
Yeah, great catch. One engine for the whole umbrella means shared indexer, so actually B gets completions from A. It actually works as you described (A gets completions from B, but B does not from A) currently, without the changes from this PR - and with starting a separate engine for every subapp. I think ElixirSense fallback simply resolves B as a dependency of A and that's why the completion works. |
This adds check for being in an umbrella while determining the project the file belongs to. The detection mechanism is finding
mix.exswithapps_pathdefined inproject, which is the best I was able to come up with without actually evaluatingmix.exs.When going up in search of umbrella
mix.exs, we put a boundary on the root of the currently open project, so opening individual apps separately is still possible (and we don't go spelunking through the filesystem).fixes #460