fix(elixirls): root_dir priority: mix.exs > .git

[giddie]

If we're in a subdirectory of a git repo, and that subdirectory contains a mix.exs file, it makes sense to assume the subdirectory is the project root, rather than the git repo root.

[georgeguimaraes]

Hi @giddie, that breaks the reason I explained in #2866 in order to have elixir-ls work in Elixir umbrella apps.

I'd suggest reverting this commit because it breaks the ability for elixir-ls to see references throughout the entire umbrella app.

[glepnir]

Oops the or check should remove now .

[giddie]

If we revert, the server will fail to start at all for Mix projects in subdirectories of a git repo. That's the problem I wanted to solve.

So how about this: look for a mix.exs file in the .git root. If it doesn't exist, look for a mix.exs file in the closest ancestor directory?

Even better would be to look for the closest mix.exs file to the .git root, to cover the case of an umbrella app in a subdirectory of the repo, but I guess that might be a little trickier?

Taking this further - why stop at a git repo boundary? Could it be sensible to simply look for the highest ancestor directory containing a mix.exs?

[georgeguimaraes]

If we revert, the server will fail to start at all for Mix projects in subdirectories of a git repo. That's the problem I wanted to solve.

Could you let me know if you are sure? I tested this scenario with the config prioritizing .git folder.
I tried again, and it works. I created a new dir with this content:

/tmp/test
├── .elixir_ls/
│  └── .gitignore
├── .git/
│  ├── config
│  ├── description
│  └── HEAD
└── subdir/
   └── simple_blog/
      ├── _build/
      ├── assets/
      ├── config/
      ├── deps/
      ├── lib/
      ├── priv/
      ├── test/
      ├── .formatter.exs
      ├── .gitignore
      ├── mix.exs
      ├── mix.lock
      └── README.md

Does that match your expectation on the directory structure?

For this dir, I could load Neovim and the elixirls LSP too:

You can see that the root for the LSP is set to /tmp/test, which is where the .git dir is. There were no errors from LSP.

Maybe your config is wonky somehow?

[georgeguimaraes]

Even better would be to look for the closest mix.exs file to the .git root, to cover the case of an umbrella app in a subdirectory of the repo, but I guess that might be a little trickier?

The best way to detect this scenario would be something like: given an fname, we could traverse upwards to detect the first mix.exs and then check if there's another mix.exs 2 dirs above the first mix.exs.

Not sure if this is a good approach for lspconfig, tough.

Checking for the .git dir seems to me like a "good enough" approximation.

[georgeguimaraes]

So how about this: look for a mix.exs file in the .git root. If it doesn't exist, look for a mix.exs file in the closest ancestor directory?

Hmmm, I understood it now! It seems like a good compromise. That can work, however, I still think that the previous config was already working in all those scenarios we discussed.

[giddie]

Could you let me know if you are sure? I tested this scenario with the config prioritizing .git folder.

Here is my test directory structure:

/tmp/test
├── /tmp/test/.git
└── /tmp/test/subdir
    ├── /tmp/test/subdir/_build
    ├── /tmp/test/subdir/lib
    ├── /tmp/test/subdir/test
    ├── /tmp/test/subdir/.formatter.exs
    ├── /tmp/test/subdir/.gitignore
    ├── /tmp/test/subdir/mix.exs
    └── /tmp/test/subdir/README.md

Here is the LSP output:

[WARN][2023-11-22 11:51:09] ...lsp/handlers.lua:537     'No mixfile found in project. To use a subdirectory, set `elixirLS.projectDir` in your settings. Looked for mixfile at "/tmp/test/mix.exs"'

Maybe your config is wonky somehow?

Maybe you define elixirLS.projectDir? Personally I'd like to avoid that. And I think we can solve the problem without needing to resort to that.

[georgeguimaraes]

Maybe you define elixirLS.projectDir?

Nope. My config is a pretty standard LazyVim config (https://github.com/georgeguimaraes/nvim-config) and as far as I know LazyVim doesn't change that.

It seems that this particular warning you got was removed:
elixir-lsp/elixir-ls#307

Maybe you're running an old elixirls?

[giddie]

It seems that this particular warning you got was removed: elixir-lsp/elixir-ls#307

I don't think so: https://github.com/elixir-lsp/elixir-ls/blob/master/apps/language_server/lib/language_server/server.ex#L1414

I'm on v0.17.10 which is also the current HEAD.

[georgeguimaraes]

Hmm, then I can only guess that your debug level is different from mine. But regardless, I've tested again my config with the subdir. Elixirls here is running without a mix.exs, and that doesn't seem reasonable.

So I think your way of detecting a mix.exs in the same dir as .git and then fallbacking to find mix.exs in the ancestors dirs is now the best compromise between umbrella, single app and subdir single app.

[georgeguimaraes]

elixir-tools takes an interesting approach:

https://github.com/elixir-tools/elixir-tools.nvim/blob/8f573b0b089567aa9c544b029000c97e715e5f58/lua/elixir/utils.lua#L18-L29

@giddie what do you think?

@glepnir do you think we could use something like that in lspconfig? Would that be too much?

[giddie]

That looks interesting. It certainly feels better to me to not assume anything about version control (git repo or not). I'm not sure about limit = 2. That seems arbitrary to me. (I guess the minimum needed to find the umbrella root, so long as you're launching nvim in a child project root?) This isn't a particular expensive operation. I'd set that much higher, like 100. Just as a sanity limit in case of some weird bind-mount loop. That way we can detect the root even if nvim is launched in a deep subdirectory for some reason.

[georgeguimaraes]

limit = 2 means that when the first 2 mix.exs are found, the search halts. So I guess that's already a sanity limit builtin, even for a deeply nested fname.

and since it's using upwards, it will never detect a mix.exs from "another" directory like another subdir app or a child umbrella app that is not the one we're in.

[giddie]

Ah! Sorry, I had rushed reading the function docs, and thought it was a limit on the search depth. But as a limit for number of results it makes total sense. And is actually a really nice solution 👍

[georgeguimaraes]

@giddie not sure the best way to get that into the nice root_dir option from lspconfig. I won't have the time today to hack it. Do you see a way? Can you try it?

[giddie]

PR is up: #2911