Incorrect path passing for LSP server

[to266]

Reproduction steps

• Create (in my case rust) project with the name appearing somewhere higher in the path tree. e.g. /home/to266/projects/funny/some/folder/workspace/funny. If workspace literally corresponds to rust workspace, and funny inside is a rust package withing the workspace.
• Start hx inside funny the package
• Observe that LSP is not initialized. The logs show

2021-06-14T09:40:14.641 helix_lsp::transport [ERROR] err <- [ERROR rust_analyzer] failed to find any projects in [AbsPathBuf("/home/to266/projects/funny")]
2021-06-14T09:40:14.642 helix_lsp::transport [INFO] <- {"jsonrpc":"2.0","method":"window/showMessage","params":{"type":1,"message":"rust-analyzer failed to discover workspace"}}

Environment

• Platform: Linux
• Helix version: 0.2.0

[archseer]

I think this is a rust-analyzer issue: rust-lang/rust-analyzer#1798

We try to detect a .git repository root in any of the parent directories, if that fails we fall back to passing None to the LSP and letting it figure out the correct setting.

[nrdxp]

It's not just rust-analyzer, as rls failed with the same issue. I have roots = ["Cargo.toml"] specified in languages.toml as well. (see my duplicate issue above for more details).

[archseer]

Yeah currently we don't consume those roots yet. I think we need to change find_roots to accept a markers list, then modify this:

helix/helix-core/src/lib.rs

Lines 42 to 46 in 1bda454

	for ancestor in root.ancestors() {
	// TODO: also use defined roots if git isn't found
	if ancestor.join(".git").is_dir() {
	return Some(ancestor.to_path_buf());
	}

The reason I haven't done this yet is because I don't know what the behavior should be here. If we stop at the first parent dir with a Cargo.toml we might miss the fact that we're in a workspace. On the other hand, if this is a multi-language repository, searching from the root -> cwd and finding .git might not be what we want (instead we want the subfolder with Cargo.toml)

[pickfire]

I think if we look at .git recursively we might hit $HOME as our root which may not be what the user want.

[archseer]

$HOME doesn't contain .git though, it contains a .gitconfig/.gitignore. I think likely the correct way to do this is to scan for a git repo first, then scan for defined roots next.

[pickfire]

$HOME doesn't contain .git though, it contains a .gitconfig/.gitignore. I think likely the correct way to do this is to scan for a git repo first, then scan for defined roots next.

I used to have it until it broke some applications so I use an alias home git --work-tree=/home/ivan --git-dir=/home/ivan/.home, so we may want to beware of it. Technically it is possible to have .git/ it home for say dotfiles so we may want to avoid it.

[nrdxp]

Perhaps, at least for rust, we could search for a directory containing both a Cargo.toml and a .git directory. Seems like a fairly clear indication of the top level rust crate.

[to266]

Perhaps, at least for rust, we could search for a directory containing both a Cargo.toml and a .git directory. Seems like a fairly clear indication of the top level rust crate.

Perhaps for rust-only projects. In my original example it's not the case

/home/
    to266/
        projects/
            funny/                       # git root
                some/                # just folder
                    folder/          # just folder
                        workspace/       # rust workspace
                            funny        # Crate

To me it seems the proper solution is actually reading the Cargo.toml files. If it contains the keyword [workspace], then it's a workspace, and no other (at least related) will be above that (because nested workspaces are not a thing rust-lang/cargo#5042).

if additional verification is wanted, then of course it's possible to check that the Cargo.toml in workspace contains the member crate, which is presumably where we started in the first place.

[archseer]

nvim-lsp uses cargo metadata --no-deps --format-version 1 to dump data then search for workspace_root. Wish it was more straightforward.

[nrdxp]

Since both are rust projects, perhaps we could just steal some of the code

[kirawi]

Is this still an issue since the assert seems to have been changed on master?

[matoous]

I think this is still an issue, and not just for Julia but any mono-repo. Our structure looks like this

repo/
    .git
    apps/
        ui/
            package.json
        backend/
            go.mod

gopls in the repo/apps/backend doesn't work either currently.

I think (as mentioned above) the issue is with the find_root function

helix/helix-core/src/lib.rs

Line 41 in 3156577

pub fn find_root(root: Option<&str>) -> Option<std::path::PathBuf> {

. The question is what should be the optimal behavior:

• still look for root where the .git is but initialize lsp in directory that matches roots.
• look for roots or .git
• always keep the current folder as the root and only initialize the LSP in the first parent directory that matches roots or stop at .git
• something else?

If I understand it correctly, ideally this call to find_root https://github.com/helix-editor/helix/blob/master/helix-lsp/src/client.rs#L228 would take the roots configuration into consideration.

[archseer]

I think the procedure should be:

1. look for the highest directory with the marker file (we have these in languages l.toml, they're just not used currently)
2. else look for a .git folder
3. fallback to using current directory

[amousset]

I opened #1370 with a slightly different logic (stop looking for markers when reaching git repo boundary, but not sure it makes more sense)

[archseer]

Resolved by #1370