Replies: 14 comments 33 replies
-
Hello @mehalter, I read your comments on the original discussion. I can follow your line of thinking and would like to add my own thoughts. Do note that they are merely thoughts, based on my experience in adding a tiny piece of implementation to the code base. The user has a lot of flexibility provided by lazy.nvim in the manner his setup is configured. Consider a normal plugin, at the top level. It can be defined and enhanced in multiple places, providing a powerful base for a setup like for example LazyVim. Reading the code, plugins defined in "dependencies" can be configured as a full spec also. Following these thoughts, the plugin manager does not need to care about the semantics of the configuration of plugin A. The options provided are treated technically, as data to be merged. The "user" knows that "bar=true" is relevant for the definition placed as a dependency of B. The package manager, given the dependencies field, needs to make one thing sure: The relevant plugins need to be setup before the parent plugin (ie B) activates. It does provide an "extra" service: When plugin A can never be active, offer to uninstall. In the case of plugin A being defined in an optional context only: Do not install at all. Best regards! |
Beta Was this translation helpful? Give feedback.
-
Hi @mehalter, I have been thinking about your idea the last couple of days. The explanation in my previous response outlines a possible reasoning given the current implementation, where plugin definitions are first "processed" into plugins. Inspecting them for disabled/optional happens thereafter. As a consequence, all individual definitions are already merged. Removing a dependency is only possible when not in active use elsewhere. As a challenge I am trying my hand at reversing the process. Gathering raw plugin definitions, only entering the "merging" stage with active ones. |
Beta Was this translation helpful? Give feedback.
-
Linking to a relevant discussion in a pull request for LazyVim |
Beta Was this translation helpful? Give feedback.
-
Hi @mehalter, The code is ready to test. I think the approach would be an improvement, but there are breaking changes, described as a top-level comment in "lazy/core/plugin.lua" I tested it locally, with the code checked out in ~/projects/lazy.nvim
In lazy.lua, the "dev" option needs to be activated. Thanks! |
Beta Was this translation helpful? Give feedback.
-
I am not very knowledgeable regarding AstroNvim, so I had a try:
Add the following line below linenr 21:
Add the following line below the line containing spec=spec:
Run:
AstroNvim now runs with lazy.nvim, found in ~/projects/lazy.nvim Best regards! |
Beta Was this translation helpful? Give feedback.
-
Just to make sure, a quick response: |
Beta Was this translation helpful? Give feedback.
-
@abeldekat I have gone ahead and done some more testing now that I now how to get around the previous bug and I can't verify that it's actually not considering dependency specs of disabled plugins at all. I have made a new branch on this local root = vim.fn.fnamemodify("./.repro", ":p")
-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end
-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
vim.fn.system({ "git", "clone", "--filter=blob:none", "https://github.com/folke/lazy.nvim.git", lazypath })
end
vim.opt.rtp:prepend(vim.env.LAZY or lazypath)
-- install plugins
local plugins = {
{ "mehalter/test_lazy", branch = "dependency_check", import = "test_lazy.plugins" },
-- { "telescope.nvim", enabled = false }, -- COMMENT TO DISABLE TELESCOPE
}
require("lazy").setup(plugins, {
dev = { patterns = jit.os:find("Windows") and {} or { "abeldekat", "lazy.nvim" } },
root = root .. "/plugins",
}) When you run this with Here is a link to the repo+branch for that plugin test: https://github.com/mehalter/test_lazy/tree/dependency_check |
Beta Was this translation helpful? Give feedback.
-
Fixed! I handled the reference in top-level plugins (ie "AstroNvim" vs "AstroNvim/AstroNvim) incorrectly. Thanks! I appreciate your effort to provide me with reproducable configs. |
Beta Was this translation helpful? Give feedback.
-
Hi @mehalter, I committed a new version with some refactoring and one change that will affect the "astrocommunity" plugin.
When using this version, that should be changed into:
|
Beta Was this translation helpful? Give feedback.
-
I think my approach limits lazy.nvim too much. The functions a user can write using keywords "cond, enabled" used to be called when lazy.nvim has a complete picture of its plugins. This should not be broken. Today, I realized how to "repair" active plugins after lazy.nvim has done its normal parsing. The changes to the code are much simpler. I expect no breaking changes, except for the effect your idea has anyhow. Users might already have expected this new behaviour to exist as the idea is very intuitive in my opinion. This is the commit message:
I created a new branch I am hoping you can find the time to test the new code! I tested your scenarios. |
Beta Was this translation helpful? Give feedback.
-
Just for reference: I searched the issues and found one expressing the idea. |
Beta Was this translation helpful? Give feedback.
-
Hi @mehalter Your responses and the feedback from the review in this PR increase my confidence. If so, the following questions come to mind:
|
Beta Was this translation helpful? Give feedback.
-
Hi @mehalter, I created the PR. I would appreciate your feedback! |
Beta Was this translation helpful? Give feedback.
-
I've been working on a refactor of how plugin deps are calculated in lazy. You can find the PR here #1058 The biggest change is similar to @abeldekat's PR where plugin spec fragments from optional and disabled plugins will no longer be included in the final spec. This actually simplifies dealing with disabled plugin deps quite a bit. As far as I can see eveything works as expected, but if you have any feedback, let me know :) |
Beta Was this translation helpful? Give feedback.
-
EDIT: This has been implementd in Lazy v10.4.0 with #1058
Some plugins might be configuring options in other plugins on a need to have basis through it's dependencies. If this plugin is then disabled it seems that it would make sense to not consider the plugins defined in it's dependencies as they aren't needed anymore. With the current implementation this is sorta how it works. Here are a few examples:
Case 1 (works as intended)
In this example both plugins
A
andB
are installed andA
has the final options of{ foo = true, bar = true }
This makes sense and the result is showing thatA
should always havefoo = true
as an option andbar = true
is required for pluginB
to work.Case 2 (works as intended)
Here in this example we are only defining
A
as a dependency ofB
andB
is currently disabled. This means thatA
is not installed at all and is not available because it is no longer needed anywhere else and the plugin that depends on it isn't availableCase 3 (potentially incorrect behavior)
This is similar to Case 1 but the difference here is
B
is disabled. The final result of this case isB
is not installed andA
is installed and setup with the options of{ foo = true, bar = true }
. It seems like this might not be the intended behavior. The plugin specification is saying thatbar = true
is needed as a dependency ofB
and is no longer necessary becauseB
is not needed.Proposed behavior: I think it would be better to not consider the dependency plugin specifications at all where
A
would only be setup in this case with the options of{ foo = true }
It is a bit strange how committed/not committed the current approach is to caring/not caring about dependencies. where dependencies are fully considered and processed, options and all unless the dependencies are not available anywhere else in which case they are fully not considered and they are uninstalled. I think it makes sense to unifying this "caring" of dependencies and just not processing them at all. If the user wants
bar = true
in this case without pluginB
added then it should not be specified as adependencies
options.Let me know what you think! 😄 I am a huge fan of the semantics that lazy.nvim implements and adheres to. This is just one place I found that seems to have a bit of a disconnect. Thanks for your time and all of the effort you have put into this project and the Neovim community as a whole! ❤️
Beta Was this translation helpful? Give feedback.
All reactions