LSP workspaces

[pascalkuthe]

Supersedes #4439 and (mostly) #3207
Fixes: #3993
Fixes: #4436
fixes #5852
Closes #5919

The aim of this PR is primarily to address the problem from #4439 as described in #4439 (comment). As the solution described there uses a workspace config I ended up implementing support for that as well. The syntax I proposed in #4439 (comment) didn't quite work out as adding a new top-level header (that can be accessed in helix-view) like [workspace] would have required more changes that didn't seem worth the effort here. Furthermore, the language specialization I proposed in my comment is incompatible with the toml standard (sadly). I ended up with the following solution:

To force LSP to stop searching for roots in the fuzz folder you can
add the following .helix/config.toml file:

[editor]
workspace-lsp-roots = ["fuzz"]

To only apply this setting only to rust (but not other languages where this might cause problems) you can add the following .helix/languages.toml file instead:

[[language]]
name = "rust"
workspace-lsp-roots = ["fuzz"]

To handle local config I originally wanted to pull in #3207, but when I started reviewing the changes there I quickly found many things that required changes so that I decided to reimplement the functionality here instead. Compared to #3207 this PR does not implement the security setting for workspace config. It's disabled by default anyway so that feature can be added as a separate followup PR (or #3207 could be rebased on this)

Furthermore, this PR correctly handles:

• Respect local config during :config-reload and :set (would require larger refactor with feat: load local config #3207)
• Full keymap merging of arbitrary nested keybindings (just like merging with the default keymap works on master)
• Unify config loading in helix-term/src/main.rs and during config-reload/set

Finally, this PR contains on breaking change: How .helix is treated. Currently, we simply merge all .helix/langauges.toml from the CWD to the workspace root. This PR changes the workspace lookup so .helix acts just like .git. It's not intended for marking LSP roots (that's what workspace-roots is for) but rather for cases where:

• You are working without git
• Monorepos where you want to really only work on part of the repo as a workspace (I am talking HUGE monorepos like the windows monorepo) for all operations (including the file picker)

As the workspace root search always stops at the first .helix directory, there is only a single .helix/config.toml and .helix/languages.toml, so the merging behavior is removed (breaking change I was talking about). In addition to the use-cases listed above this also ensure that it's always clear what the workspace-roots setting is referring to: A path relative to the parent of .helix. When multiple configs are merged it can be quite confusing what these directories refer to otherwise. Finally, this behavior is also more inline with other editors (like .vscode for VSCode).

[pascalkuthe]

One thing I would like feedback on: Right now the :change-directory command does not automatically load the local configuration from that folder.

The reason is that the global and the local configuration are not saved seperatly and therefore can only be reloaded together (so essentially calling :config-reload). It would have been yet another (subtle) breaking change to automatically call :reload-config when changing directories so I held off. To me this feels like a better solution so different workspace-roots are picked up automatically. Ofcourse calling :config-reload manually works but it's easy to forget and will likely cause confusion in the future

[pascalkuthe]

rebased on master to resolve conflicts, the changes from #5420 actually make this PR a bit smaller/more elegant as the config can be accessed trough the document now.

[LeoniePhiline]

5083fd1 works perfectly for my situation described at #5852, without having to define any local .helix config.

Edit: Also 8428f73 works for me.

[pascalkuthe]

I expanded this PR to also fix #5852.

The fundamental issue there was that the workspace root and the lsp root lookup were unified when these are really two separate functions. The LSP lookup always starts at the document while the workspace lookup should always start at the CWD. The LSP lookup should not look for the workspace root (.git or .helix) itself because then netsted .git repos don't work as the LSP lookup stops too early (which was the cause of #5852).

With the changes here the workspace lookup and the LSP root lookup are now separate. The workspace lookup just scans up the directory tree from CWD till .git or .helix is found and fallsback to CWD if neither exists.

The LSP root lookup searches for the topmost root_marker from the document.
If the document is inside the workspace folder the search stops if one of the configured workspace-roots are discovered or if the workspace root is reached.
Outside of the workspace the search stops at the FS root (still fallsback to the workspace at that case as that seems like the better fallback to me).

The file picker (and helix-loader) now use find_workspace. The find_root function is now lsp specific and has been moved to helix-lsp

[pascalkuthe]

While testing this further I noticed that the new solution wasn't working well because we just used the first root we encountered as the LSP workspace/root.

The right solution here is to use one LSP workspace for each separate resolved root. This not just makes my new solution here viable but also fixes existing issues. Consider the following directory structure with two separate Cargo workspaces in the same git repo (not two crates, two seperate workspaces, there is no Cargo.toml in the repo root) :

| - .git
| - workspace1
|   | - Cargo.toml
|   | - src/bar.rs
| - workspace2
|   | - Cargo.toml
|   | - src/foo.rs

If you open workspace1/src/bar.rs with helix master, RA will be initalized with a single workspace: workspace1.
If you open workspace2/src/foo.rs afterwards RA will show the file as not being part of any cargo project¹.
If you open those two files in reverse order the same problem will occur in reverse (so bar.rs would have no LSP).

With this PR the workspaces are both added as an LSP workspace and therefore everything works as intended.
Note that for servers that don't support multiple workspaces we simply spawn one server per LSP workspace (VSCode does the same thing).
This also makes the original use-case of this PR work correctly, where a subfolder is not part of the root workspace. For example:

| - .git
|  - Cargo.toml
|  - src/bar.rs
| - example
|   | - Cargo.toml
|   | - src/foo.rs

Here the same problem as above would occur if the newly introduced config option was used. Whatever workspace gets opened first gets used and the other one doesn't work. With the latest commit the problem is fixed.

While further testing this PR I noticed that RA was indexing a new workspace whenever I opened up a crates.io dependency with gd and even showed errors in their testsuite (which RA usually ignores for dependencies). The reason is that with these new changes helix now correctly detected those crates Cargo.toml and added them as a new workspace.

This is not desirable as dependencies are obviously not supposed to be part of the workspace.
Therefore I have changed the LSP root detection to only create LSP workspaces for files in the current workspace.
All files outside of the current workspace are:

• If there is not LSP yet: Spawn an LSP with workspace_dirs set to an empty list, root_uri set to None and root_dir set to CWD (see Bogus workspaceFolders when initializing an LSP server #4436 (comment), root_dir must be set for backwards compatibility)
• If there is already an LSP and it supports multiple workspaces: Files are just added to the LSP without creating a new workspace
• If there is already an LSP and it doesn't support multiple workspaces: Use the LSP with root_uri set to None (or create it if necessary)

While working on this I ran across #4436. Fixing that issue was easy by creating a special case if the current workspace resolved to the CWD because there is no .helix or .git:
If no root marker matches for the given file, treat the file as tough like it lives outside of the current workspace (as described above).

Footnotes

1. Note that this is only true if these two workspaces don't depend on each other. However that is RA specific behavior and not relevant here. RA is able to handle a file belonging to multiple workspaces just fine. ↩

[LeoniePhiline]

fcc445a crashes on me with:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/leonie/Projekte/rust/helix-install/helix-term/src/application.rs:1013:42
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::panicking::panic
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:48:5
   3: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
   4: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
   5: helix_term::application::Application::handle_editor_event::{{closure}}
   6: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   7: tokio::runtime::park::CachedParkThread::block_on
   8: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
   9: hx::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Steps:

1. Open file
2. Press j to move cursor down
3. Panic.

[jknoetzke]

Hi. Silly question. These commits are in the main branch yes ? To test, I simply have to pull ?

[LeoniePhiline]

@jknoetzke

git fetch origin pull/5748/head
git checkout FETCH_HEAD
cargo install --locked --path helix-term

You will be in 'detached head' mode.

You can also pull into a new branch with:

git fetch origin pull/5748/head:branch-name

[pascalkuthe]

fcc445a crashes on me with:

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/leonie/Projekte/rust/helix-install/helix-term/src/application.rs:1013:42
stack backtrace:
   0: rust_begin_unwind
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
   2: core::panicking::panic
             at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:48:5
   3: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
   4: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
   5: helix_term::application::Application::handle_editor_event::{{closure}}
   6: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
   7: tokio::runtime::park::CachedParkThread::block_on
   8: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
   9: hx::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Steps:

1. Open file

2. Press `j` to move cursor down

3. Panic.

that's odd it works well with RA and shouldn't really be happening. Which LS are you using?

[LeoniePhiline]

Intelephense. I tried to verify by reproducing #5852

Edit: The situation you described at #5852 (comment)

Update:
Pressing j has nothing to do with it. Just open the file and wait half a second for the crash.

Happens if I open a file within the nested .git, but also if I open a file within the root .git, but within its own composer.json (which is configured as PHP root marker).

I had previously changed php root markers to only ["composer.lock"]. Maybe the revert causes the sudden panic. I'll see what happens if I add that again to my languages.toml

Update:

Still crashes with the following addition to ~/.config/helix/languages.toml

[[language]]
name = "php"
roots = ["composer.lock"]

I'll try to bisect (3b5bdbc)

Update:

Still crashing when reverted to 3b5bdbc (this time full backtrace):

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/leonie/Projekte/rust/helix-install/helix-term/src/application.rs:1013:42
stack backtrace:
   0:     0x55bed4ea29d4 - std::backtrace_rs::backtrace::libunwind::trace::h22893a5306c091b4
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55bed4ea29d4 - std::backtrace_rs::backtrace::trace_unsynchronized::h29c3bc6f9e91819d
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55bed4ea29d4 - std::sys_common::backtrace::_print_fmt::he497d8a0ec903793
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x55bed4ea29d4 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h9c2a9d2774d81873
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x55bed45d479c - core::fmt::write::hba4337c43d992f49
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/fmt/mod.rs:1194:17
   5:     0x55bed4e9bd55 - std::io::Write::write_fmt::heb73de6e02cfabed
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/io/mod.rs:1655:15
   6:     0x55bed4ea48ce - std::sys_common::backtrace::_print::h63c8b24acdd8e8ce
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x55bed4ea48ce - std::sys_common::backtrace::print::h426700d6240cdcc2
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x55bed4ea48ce - std::panicking::default_hook::{{closure}}::hc9a76eed0b18f82b
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:295:22
   9:     0x55bed4ea45fd - std::panicking::default_hook::h2e88d02087fae196
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:314:9
  10:     0x55bed4ea4f5b - std::panicking::rust_panic_with_hook::habfdcc2e90f9fd4c
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:702:17
  11:     0x55bed4ea4d5c - std::panicking::begin_panic_handler::{{closure}}::he054b2a83a51d2cd
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:586:13
  12:     0x55bed4ea2f04 - std::sys_common::backtrace::__rust_end_short_backtrace::ha48b94ab49b30915
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/sys_common/backtrace.rs:138:18
  13:     0x55bed4ea4aed - rust_begin_unwind
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:584:5
  14:     0x55bed4541e53 - core::panicking::panic_fmt::h366d3a309ae17c94
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:143:14
  15:     0x55bed4541d1d - core::panicking::panic::h8705e81f284be8a5
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/panicking.rs:48:5
  16:     0x55bed4d1bde9 - <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold::hedec03767c0a3f9b
  17:     0x55bed4d63b50 - <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter::h658725280a12b178
  18:     0x55bed4d313f1 - helix_term::application::Application::handle_editor_event::{{closure}}::h65d2c6ec26a2ea53
  19:     0x55bed4d4d398 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::hbfb464632ff5164b
  20:     0x55bed4cee583 - tokio::runtime::park::CachedParkThread::block_on::h8fb31feee02e3eeb
  21:     0x55bed4d2b5a7 - tokio::runtime::scheduler::multi_thread::MultiThread::block_on::hec5847f053998419
  22:     0x55bed4d52936 - hx::main::h23c91916c4a5df24
  23:     0x55bed4d3d793 - std::sys_common::backtrace::__rust_begin_short_backtrace::hc92a77c5c5557790
  24:     0x55bed4cf6e5d - std::rt::lang_start::{{closure}}::hc397c723102811d8
  25:     0x55bed4e9661b - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::had4f69b3aefb47a8
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/core/src/ops/function.rs:259:13
  26:     0x55bed4e9661b - std::panicking::try::do_call::hf2ad5355fcafe775
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:492:40
  27:     0x55bed4e9661b - std::panicking::try::h0a63ac363423e61e
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:456:19
  28:     0x55bed4e9661b - std::panic::catch_unwind::h18088edcecb8693a
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panic.rs:137:14
  29:     0x55bed4e9661b - std::rt::lang_start_internal::{{closure}}::ha7dad166dc711761
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/rt.rs:128:48
  30:     0x55bed4e9661b - std::panicking::try::do_call::hda0c61bf3a57d6e6
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:492:40
  31:     0x55bed4e9661b - std::panicking::try::hbc940e68560040a9
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panicking.rs:456:19
  32:     0x55bed4e9661b - std::panic::catch_unwind::haed0df2aeb3fa368
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/panic.rs:137:14
  33:     0x55bed4e9661b - std::rt::lang_start_internal::h9c06694362b5b80c
                               at /rustc/fe5b13d681f25ee6474be29d748c65adcd91f69e/library/std/src/rt.rs:128:20
  34:     0x55bed4d52b12 - main
  35:     0x7f31f77e35b0 - __libc_start_call_main
  36:     0x7f31f77e3679 - __libc_start_main@GLIBC_2.2.5
  37:     0x55bed457f555 - _start
                               at /home/abuild/rpmbuild/BUILD/glibc-2.36/csu/../sysdeps/x86_64/start.S:115
  38:                0x0 - <unknown>

[the-mikedavis]

I've been running this for a while and it works great! I have some minor comments

[krobelus]

If there is not LSP yet: Spawn an LSP with workspace_dirs set to an empty list, root_uri set to None and root_dir set to CWD (see #4436 (comment), root_dir must be set for backwards compatibility)

@pascalkuthe is this still the behavior?
I'm trying to fix the same marksman issue in Kakoune.
It looks like Helix always adds the file's root as single workspace folder when starting a new server:
In helix-lsp/src/client.rs

        let workspace_folders = root_uri
            .clone()
            .map(|root| vec![workspace_for_uri(root)])
            .unwrap_or_default();