Provide abstraction for generic default root dir

[tpope]

Language server

No response

Requested feature

TL;DR: I am looking to improve this:

Most of the time, this is a .git folder, but each server defines the root config in the lua file.

This is a good 80% solution. In the current SCM monoculture, it may even be a 95% solution. But it has several limitations:

• It assumes you're using Git as your SCM.
• It assumes you're using an SCM at all.
• It assumes the workspace root is the same as the repository root.

The problem is that if you bump into one of these situations, you're screwed. Since the .git detection is inside each LSP config, you need to assess each LSP you use on a case-by-case basis, and more often than not will need to yank out its internal root_dir and manually tweak it.

My proposed solution would be something like the following:

• Add a new Boolean config key which specifies whether a generic fallback is desired.
• Alter launch() to respect this config key and call find_git_ancestor() if no other root was found. This could later be tweaked (e.g., to add a non-SCM based default) or made configurable by some mechanism.
• Change all existing adapters using either find_git_ancestor() or root_pattern(..., '.git') to instead set this Boolean key.

The bulk of the work is in that last step, but that's easy enough to divide and conquer, and it can be done incrementally. As for the first part, I could certainly take a stab at it if there's interest. I imagine someone more familiar with the project than I could make quick work of it.

Other clients which have this feature

No response

[mjlbach]

The problem is that if you bump into one of these situations, you're screwed. Since the .git detection is inside each LSP config, you need to assess each LSP you use on a case-by-case basis, and more often than not will need to yank out its internal root_dir and manually tweak it.

That's true, you can compose root directory functions with the original default, but it's a bit verbose:

require('lspconfig').pyright.setup {
root_dir = function(fname) 
    return require('lspconfig.server_configurations.pyright').default_config.root_dir(fname) or my_fallback_func(fname)
end
}

My proposed solution would be something like the following:

* Add a new Boolean config key which specifies whether a generic fallback is desired.
* Alter launch() to respect this config key and call find_git_ancestor() if no other root was found. This could later be tweaked (e.g., to add a non-SCM based default) or made configurable by some mechanism.
* Change all existing adapters using either find_git_ancestor() or root_pattern(..., '.git') to instead set this Boolean key.

Alternatively, we could add a generic fallback function that we manually add to the default configuration root directory in place of what is usually find_git_ancestors, which would be user overridable. That way, if users want to override the generic fallback behavior, they can, and if they want to override it for a specific server, they just override that servers root pattern as usual.

I'm happy to make the change when we decide on the implementation, I'm open to your solution but I just want to brainstorm the alternatives first, as I try to keep the number of configurations we have as small as possible.

[tpope]

The problem is that if you bump into one of these situations, you're screwed. Since the .git detection is inside each LSP config, you need to assess each LSP you use on a case-by-case basis, and more often than not will need to yank out its internal root_dir and manually tweak it.

That's true, you can compose root directory functions with the original default, but it's a bit verbose:

require('lspconfig').pyright.setup {
root_dir = function(fname) 
    return require('lspconfig.server_configurations.pyright').default_config.root_dir(fname) or my_fallback_func(fname)
end
}

Covers the most common cases, but isn't a 100% solution for things like "I don't want .git detection ever because it always latches onto a submodule root rather than the real project root".

Alternatively, we could add a generic fallback function that we manually add to the default configuration root directory in place of what is usually find_git_ancestors, which would be user overridable. That way, if users want to override the generic fallback behavior, they can, and if they want to override it for a specific server, they just override that servers root pattern as usual.

I'm happy to make the change when we decide on the implementation, I'm open to your solution but I just want to brainstorm the alternatives first, as I try to keep the number of configurations we have as small as possible.

My tastes lean declarative. In fact I almost took it a bit further and proposed root_markers = {'package.json', 'jsconfig.json'} which would be a big win for composability, but that's a bigger change. And on those grounds, I think yours wins because it's the smallest of all. We can shelve my idea for now and bring it back if that extra level of abstraction ends up being desirable.

[mjlbach]

Covers the most common cases, but isn't a 100% solution for things like "I don't want .git detection ever because it always latches onto a submodule root rather than the real project root".

Oh yeah totally, nested submodules/crates/whatever language specific thing are a huge pain. For rust we end up having to shell out to cargo to get it to return the correct root.

My tastes lean declarative. In fact I almost took it a bit further and proposed root_markers = {'package.json', 'jsconfig.json'} which would be a big win for composability, but that's a bigger change. And on those grounds, I think yours wins because it's the smallest of all.

I definitely agree when we've converged to an option that is common/requested, the functional apis accommodate a wide variety of user preferences. I think the issue historically with root directories there are some issues where the fallback needs to have additional logic for submodules/such that have made that difficult, but I'm happy to implement your suggestion and see which fits best.

Also, if you ever want to chat about any of these design things more synchronously we have less high volume matrix rooms like https://matrix.to/#/#neovim-dev:matrix.org, really glad to be getting feedback from an expert :)

[tpope]

I definitely agree when we've converged to an option that is common/requested, the functional apis accommodate a wide variety of user preferences. I think the issue historically with root directories there are some issues where the fallback needs to have additional logic for submodules/such that have made that difficult, but I'm happy to implement your suggestion and see which fits best.

Oh I have no doubt you'll always need a Turing-complete configuration option as an escape hatch. There would still be benefits to saving it for the exceptions rather than the norm. Heck, you could probably use it to generate that docs.default_config.root_dir declaration rather than repeating yourself; that alone would make it worth it.

But all of this is an aside. To reiterate, I think your proposed "generic fallback function" is a safe step in the right direction and wholeheartedly endorse it.

Also, if you ever want to chat about any of these design things more synchronously we have less high volume matrix rooms like https://matrix.to/#/#neovim-dev:matrix.org, really glad to be getting feedback from an expert :)

May take you up on that, thanks.

[mjlbach]

But all of this is an aside. To reiterate, I think your proposed "generic fallback function" is a safe step in the right direction and wholeheartedly endorse it.

Ok! Sounds good, we will start with that. I haven't had as much neovim time lately but I should be able to implement it this week.

Heck, you could probably use it to generate that docs.default_config.root_dir declaration rather than repeating yourself; that alone would make it worth it.

Hah, it's funny you say that because I did have a solution that used the lua debug module to pull the text and insert that in docgen, but there were some unanticipated quirks. I want to revisit out docs pipeline at some point.