Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Git dependencies get erroneously added as workspaces #373

Closed
6 tasks done
eero-lehtinen opened this issue Apr 26, 2024 · 5 comments · Fixed by #374
Closed
6 tasks done

Git dependencies get erroneously added as workspaces #373

eero-lehtinen opened this issue Apr 26, 2024 · 5 comments · Fixed by #374
Labels
bug Something isn't working

Comments

@eero-lehtinen
Copy link
Contributor

eero-lehtinen commented Apr 26, 2024

Have you read the docs and searched existing issues?

Neovim version (nvim -v)

v0.9.5

Operating system/version

EndeavourOS

Output of :checkhealth rustaceanvim

rustaceanvim: require("rustaceanvim.health").check()

checking for lua dependencies ~
- warning dap not installed. needed for debugging features [mfussenegger/nvim-dap](https://github.com/mfussenegger/nvim-dap)

checking external dependencies ~
- ok rust-analyzer: found rust-analyzer 1.77.2 (25ef9e3 2024-04-09)
- ok cargo: found cargo 1.77.2 (e52e36006 2024-03-26)
- ok rustc: found rustc 1.77.2 (25ef9e3d8 2024-04-09)
- ok lldb: found lldb version 17.0.6

checking config ~
- ok no errors found in config.

checking for conflicting plugins ~
- ok no conflicting plugins detected.

checking for tree-sitter parser ~
- warning no tree-sitter parser for rust detected. required by 'rustc unpretty' command.

How to reproduce the issue

Make Cargo.toml in a project:

[package]
name = "rust-test"
version = "0.1.0"
edition = "2021"

[dependencies]
jittr_git = { package = "jittr", git = "https://github.com/eero-lehtinen/jittr" }
jittr = "*"

Make src/main.rs

use jittr;
use jittr_git;

fn main() {}
export NVIM_DATA_MINIMAL=$(mktemp -d)
export NVIM_APP_NAME="nvim-ht-minimal"
nvim -u minimal.lua src/main.rs

Use go to definition on both jittr and jittr_git. jittr works as it should, but jittr_git's folder gets added to workspaces erroneously.

Expected behaviour

Neither of these crates are added to workspaces.

Actual behaviour

:lua =vim.lsp.buf.list_workspace_folders() shows that ~/.cargo/git/checkouts/jittr-{something} is added to lsp workspaces and then warnings from this dependency are shown as if we were editing it.

The minimal config used to reproduce this issue.

-- Minimal nvim config with lazy
-- Assumes a directory in $NVIM_DATA_MINIMAL
-- Start with
--
-- export NVIM_DATA_MINIMAL=$(mktemp -d)
-- export NVIM_APP_NAME="nvim-ht-minimal"
-- nvim -u minimal.lua
--
-- Then exit out of neovim and start again.

-- Ignore default config
local config_path = vim.fn.stdpath("config")
vim.opt.rtp:remove(config_path)

-- Ignore default plugins
local data_path = vim.fn.stdpath("data")
local pack_path = data_path .. "/site"
vim.opt.packpath:remove(pack_path)

-- bootstrap lazy.nvim
data_path = assert(os.getenv("NVIM_DATA_MINIMAL"), "$NVIM_DATA_MINIMAL environment variable not set!")
local lazypath = data_path .. "/lazy/lazy.nvim"
local uv = vim.uv
	---@diagnostic disable-next-line: deprecated
	or vim.loop
if not uv.fs_stat(lazypath) then
	vim.fn.system({
		"git",
		"clone",
		"--filter=blob:none",
		"[email protected]:folke/lazy.nvim.git",
		"--branch=stable",
		lazypath,
	})
end
im.opt.rtp:prepend(lazypath)

local lazy = require("lazy")

lazy.setup({
	{
		"mrcjkb/rustaceanvim",
		version = "^4",
		init = function()
			vim.g.rustaceanvim = {}

			vim.keymap.set("n", "gd", vim.lsp.buf.definition, { desc = "LSP: Go to definition" })
		end,
		lazy = false,
	},
	-- Add any other plugins needed to reproduce the issue.
	-- see https://github.com/folke/lazy.nvim#-lazynvim for details.
}, { root = data_path, state = data_path .. "/lazy-state.json", lockfile = data_path .. "/lazy-lock.json" })
@eero-lehtinen eero-lehtinen added the bug Something isn't working label Apr 26, 2024
@mrcjkb
Copy link
Owner

mrcjkb commented Apr 26, 2024

Hey 👋

Thanks for the detailed report 🙏
Do you know if the behaviour is different when using rust-analyzer with VSCode?

I'm not sure if there's a good solution to this for rustaceanvim, as you would want to be able to navigate through the dependency with textDocument/definition, etc.

I'd have to check if not attaching an LSP client if a file is in ~/.cargo/git/checkouts (or whatever the path is on macos/windows) would cause problems.

@eero-lehtinen
Copy link
Contributor Author

I tested on VSCode and it works like I want in this situation.

@eero-lehtinen
Copy link
Contributor Author

eero-lehtinen commented Apr 26, 2024

I think just ignoring workspaces in this path would be fine (if that is possible).

@eero-lehtinen
Copy link
Contributor Author

eero-lehtinen commented Apr 26, 2024

I guess root_dir passed into vim.lsp.start is the culprit for adding new workspaces, while I think in VSCode you always manually create your workspaces.

@eero-lehtinen
Copy link
Contributor Author

Looks like you already have an explicit check for the normal dependency registry, so adding the same for git checkouts seems to just work.

diff --git a/lua/rustaceanvim/cargo.lua b/lua/rustaceanvim/cargo.lua
index 9678aa5..f506ba1 100644
--- a/lua/rustaceanvim/cargo.lua
+++ b/lua/rustaceanvim/cargo.lua
@@ -13,11 +13,13 @@ local function get_mb_active_client_root(file_name)
   local cargo_home = compat.uv.os_getenv('CARGO_HOME') or joinpath(vim.env.HOME, '.cargo')
   local registry = joinpath(cargo_home, 'registry', 'src')
 
+  local checkouts = joinpath(cargo_home, 'git', 'checkouts')
+
   ---@diagnostic disable-next-line: missing-parameter
   local rustup_home = compat.uv.os_getenv('RUSTUP_HOME') or joinpath(vim.env.HOME, '.rustup')
   local toolchains = joinpath(rustup_home, 'toolchains')
 
-  for _, item in ipairs { toolchains, registry } do
+  for _, item in ipairs { toolchains, registry, checkouts } do
     item = os.normalize_path_on_windows(item)
     if file_name:sub(1, #item) == item then
       local clients = rust_analyzer.get_active_rustaceanvim_clients()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants