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

fix: rust dap installation #1196

Closed

Conversation

appelgriebsch
Copy link
Contributor

on a clean system codelldb was not installed automatically when importing the rust.lua extension. this PR provides a fix

@dpetka2001
Copy link
Contributor

Strange, with the current rust extra it got installed just fine for me also on a clean installation (that I keep aside for testing purposes). How does making mason a dependency of nvim-dap solve this?

@appelgriebsch
Copy link
Contributor Author

Not sure how the sequence of loading is build, but I found a similar setup in the go.lua (

"mfussenegger/nvim-dap",
) and tried it. For me it was more reliably installing codelldb. Also when configured this way I expect it to not install the dap adapter if the whole dap-module is not installed…

@dpetka2001
Copy link
Contributor

I can understand the latter, you wanting it to be installed only when nvim-dap is configured. I'm just baffled with the automatic installation. Cuz I tried it a 2nd time on my clean installation and it still installs without problems. Just wondering what might be causing the different behavior.

@appelgriebsch
Copy link
Contributor Author

appelgriebsch commented Jul 23, 2023

One important differences between both versions is this line

opts.ensure_installed = opts.ensure_installed or {}

So maybe depending on the startup sequence opts.ensure_installed is not properly initialized and the codelldb is not added to the table...

@folke
Copy link
Collaborator

folke commented Jul 25, 2023

@appelgriebsch probably because you don't import specs in the correct order?

Order should always be:

  1. LazyVim
  2. all extras
  3. personal plugins

@appelgriebsch
Copy link
Contributor Author

Hmm actually have it in the same order, and can really say that on a clean machine codelldb is not installed automatically: https://github.com/appelgriebsch/Nv/blob/main/lua/config/lazy.lua#L54

@abeldekat
Copy link
Contributor

@dpetka2001

... can understand the latter, you wanting it to be installed only when nvim-dap is configure ....

This is not how lazy.nvim operates currently. Even without nvim-dap, its dependencies (ie mason) will still apply, and
codelldb will be installed.
See discussion: Given a disabled plugin, don't consider it's dependencies

@appelgriebsch

Given the original code, your opts.ensure-installed is not a table when entering the function. That's really strange because lsp.lua in stock LazyVim does create a table. I had a peek at your dotfiles and noticed a "deep-extend" in your config/lazy.lua. Would it be possible that during the extend the list of your dependencies changes order?

@dpetka2001
Copy link
Contributor

dpetka2001 commented Jul 26, 2023

@abeldekat

This is not how lazy.nvim operates currently. Even without nvim-dap, its dependencies (ie mason) will still apply, and
codelldb will be installed.
See discussion: folke/lazy.nvim#949

Thank you for clarifying this. I was under the impression that since nvim-dap is configured as optional the whole spec would only apply if nvim-dap was already enabled and configured somewhere. But I just tried enabling rust with the changes applied from @appelgriebsch (mason as a dependency of nvim-dap) and indeed it did install codelldb without me enabling nvim-dap extras. So, yes the dependencies did get apply. I guess optional is only for the plugin's own spec that is configured as optional and not its dependencies included?

Edit: I saw the most recent version of lazy.nvim that includes a change that discards plugin's dependencies conditionally. I guess it disables conditionally if they are not already configured somewhere else (in the example you mentioned nvim-dap-python in your PR). Since mason is already configured, I guess this is not the case for conditionally discarding it as a dependency of nvim-dap in this case? Is that the reason why codelldb gets installed no matter what? Is there a way to only install codelldb when nvim-dap extras is enabled at the moment or not yet?

@abeldekat
Copy link
Contributor

@dpetka2001

I guess optional is only for the plugin's own spec that is configured as optional and not its dependencies included?

Interpreting lazy.nvim's documentation, I think it does not state what should happen to a plugin's dependencies in case the plugin is marked as optional.
Currently, the deps of a completely optional plugin are checked for usage in other active plugins. In this case, mason is an active plugin elsewhere. If mason only existed as a dep of nvim-dap, mason would not even have been eligible for installation.

@abeldekat
Copy link
Contributor

Hello @appelgriebsch,

After loading all extras, your top-level mason-config overwrites ensure-installed.
The problem is caused by this fragment in your repository, lua/plugins/lang.lua:

{
    "williamboman/mason.nvim",
    opts = {
      ensure_installed = {
        "lua-language-server",
        "marksman"
      },
      ui = {
        icons = {
          package_installed = "✓",
          package_pending = "",
          package_uninstalled = "✗"
        }
      }
    },
  }

This could be the fix:

  {
    "williamboman/mason.nvim",
    opts = function(_, opts)
      vim.list_extend(opts.ensure_installed, { "lua-language-server", "marksman" })
      opts.ui = {
        icons = {
          package_installed = "✓",
          package_pending = "",
          package_uninstalled = "✗",
        },
      }
    end,
  },

LazyVim's examples mention this situation in the treesitter configuration:

-- since vim.tbl_deep_extend, can only merge tables and not lists, the code above
-- would overwrite ensure_installed with the new value.
-- If you'd rather extend the default config, use the code below instead:

@appelgriebsch appelgriebsch deleted the fix/rust-dap-installation branch August 2, 2023 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants