mrcjkb / rustaceanvim

Supercharge your Rust experience in Neovim! A heavily modified fork of rust-tools.nvim
GNU General Public License v2.0
1.3k stars 47 forks source link

feat(config): Allow overriding the root_dir #402

Closed bgw closed 1 month ago

bgw commented 2 months ago

I'm working in a repository with git submodules. Each submodule has a Cargo.toml with a workspace definition. The parent repository's workspace is a carefully-configured superset of all of the the child repository workspaces.

When working in the top-level repository, I always want rust-analyzer to use the top-level repository as the root, so that I don't end up with multiple overlapping instances of rust-analyzer.

Cargo doesn't really support nested workspaces, and its default behavior is to find the nearest/innermost workspace, which is not what I want.

This is a niche use-case, but it would be nice to have a way to override the default behavior.

This is loosely modeled after nvim-lspconfig's root_dir option: https://github.com/neovim/nvim-lspconfig/blob/94513a5b246cf32a8f87ca714af50911df63351c/CONTRIBUTING.md#adding-a-server-to-lspconfig

Testing

Run tests:

nix build .#checks.x86_64-linux.nvim-stable-tests --print-build-logs

Configure a get_root_dir function to return a hardcoded value, and inspect the results with :LspInfo.

 {
   "mrcjkb/rustaceanvim",
   dir = "/home/bgw/rustaceanvim",
   opts = {
     server = {
       get_root_dir = function()
         return "/home/bgw/..."  -- hardcoded value
       end,
     },
   },
 },

Configure neotest, and use the neotest summary to see the appropriate tests listed for my hardcoded root.

github-actions[bot] commented 2 months ago

Review Checklist

Does this PR follow the Contribution Guidelines? Following is a partial checklist:

Proper conventional commit scoping:

If applicable:

mrcjkb commented 2 months ago

Hey 👋

Thanks for the PR 🙏

Why get_root_dir and not root_dir? RustaceanLspClientConfig is a subtype of vim.lsp.ClientConfig. The vim.lsp.ClientConfig type has a conflicting root_dir field that's typed as a string (not a function).

🤔 Is it possible to override the root_dir field with LuaCATS annotation, so that it's a string | fun(config: RustaceanLspClientConfig, filename: string):string|nil?

If the lua-ls linter complains, it ought to be possible to disable the warning for the next line (I do that in some places).

bgw commented 1 month ago

Thanks for taking a look. I've updated the PR.

Renaming get_root_dir to root_dir "just works". I'm guessing vim.tbl_deep_extend('force', {}, client_config) is not properly/strongly typed. My attempt to avoid it was less because I technically couldn't, and more because I felt like it would be less ugly to pick a name than override the type. If you're okay with it, I am.

Type checks pass with

nix build .#checks.x86_64-linux.type-check-stable --print-build-logs

so that it's a string | fun(config: RustaceanLspClientConfig, filename: string):string|nil?

I don't think we should accept a simple string for this value (instead of a function), as (outside of contrived tests like I'm running) you'd never want a fixed value for the root_dir.

bgw commented 1 month ago

Thanks for the thorough and thoughtful review. I'll revise this PR with the suggested changes soon.

bgw commented 1 month ago

I've updated the PR with the suggested changes! Let me know if there's any further changes I should make.

mrcjkb commented 1 month ago

Thanks again 🙏