feat(pack): add built-in plugin manager vim.pack - #34009
Conversation
echasnovski
left a comment
There was a problem hiding this comment.
Left some comments in places that I know worth discussing.
This comment was marked as duplicate.
This comment was marked as duplicate.
The actual flexibility that is welcomed here will be a topic of discussion. Right now the most established pattern among plugin managers is to only allow "user/plugin" as a stand-in for "https://github.com/user/plugin". If even this kind of "magic" is welcome needs discussion with core team. There is this comment that brings up this and similar issues. I'd also like to take an opportunity and kindly ask for people outside of Neovim team to deeply consider if adding a comment will help the discussion. There will probably be a lot of comments as is and navigating a PR with lots of comments becomes problematic very quickly. Thanks for understanding! |
|
Can I ask about the choice never to use the |
Then if you want to disable plugins, you have to manually move them to I remember @justinmk once said the Nvim team generally think |
No, that doesn't follow. The plugin manager can move plugins between
I think it's poor UX for neovim's built-in manager to work directly against the underlying vim/neovim package system. It's true that If the core team is against it, I doubt I will convince them, but here's my final argument. It's better to support both |
It does make the code more complicated than just -- pseudo code
if not pack.opt then
vim.cmd('packadd! ' .. pack.name)
endNot to say checking if a plugin is in
It seems you misunderstood what "UX" means.
I already answered that, and Chasnovski has also answered that right in the description of this PR.
Why would people care if the plugin manager use |
Yes, it is possible. Call
The fact that there is choice to make doesn't mean every choice fits every situation. Nothing here "undermines the system".
Both "put all in 'opt/' and Putting a plugin in 'start/' is (in aspects that usually matter, not like 'rtp' order, etc.) the same as putting a Putting a plugin in 'opt/' and load when needed is the same as executing |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
No, In fact, there is currently no functionality of removing plugins from disk at all. I do want to have an "interactive way" to do that inside confirmation buffer, but not sure if the whole design is still acceptable. And the "scripting way" is also planned (see "Planned future work" in first comment).
The plugin not being updated during |
But "are not used" is now ambiguous. Some plugins "are not used (at startup)"—meaning plugins that are wanted but which users will only sometimes call on with a manual I agree with micampe that "a way to separately indicate 'this is a plugin I want installed' and 'this is a plugin I want loaded"' is useful." As he says, one way to do this is with |
If anything, this is a point towards not having automated "cleaning" in favor of manually selected one (which I'd suggest having in
Having 'start/' and 'opt/' causes more troubles than brings good. Yes, having an extra You (collective) were heard on this topic. Please, let's not continue this discussion here and wait for reviews and (hopefully) merge. After that, these separate subjects can be discussed in a more structured fashion inside separate issues. It is clearly stated as yet work in progress. |
|
I realized that original demo didn't showcase the "in-process LSP" part of the PR. So I decided to add 'textDocument/hover' support (as it is pretty straightforward) and here is the result: vim-pack-demo-lsp.mp4This is how a clean install looks, but in combination with "pretty LSP" plugins it can be customized without any burden on |
|
Just to be clear, I'd still like to add at least basic end-to-end tests in initial PR. This will make future changes/discussions much easier. But I'll wait for @justinmk review first. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Beautiful. Thank you @echasnovski for the careful design choices.
After resolving comments, I'm in favor of merging this without tests, since we plan to iterate on it anyway and tests can be part of that.
|
Is there already an option to disable confirmation? I would like to replace lazy.nvim with it while preserving the modular config structure it encourages. This would mean having multiple files with around 40 |
On one hand, it is a problem only for the first install, which is not a huge deal. On the other hand, I've added this to the follow up work as part of "Consider making |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
And I just said "thanks, duly noted, but the final decision is the maintainers'." Input is welcome; arguing is not. (This PR already has over 200 comments, and every single one adds friction.) |
|
Pushed the changes based on the recent round of review:
Here is the demo of the current state: vim-pack-demo-4.mp4 |
This is Phase 1. We will think about next steps after this. Please, leave the off-topic remarks out of this PR. |
|
Added basic mention of external plugins and |
Problem: no easy built-in way to do async Solution: add `vim._async`
Problem: No built-in plugin manager Solution: Add built-in plugin manager Co-authored-by: Lewis Russell <lewis6991@gmail.com>
|
Discuss followup work and feedback in the sub-issues of #34763 |
This PR adds a minimal built-in plugin manager
vim.pack. This is a first step towards resolving #20893. A general design is a minimized (496 LOC for core functionality when PR is opened) and reworked 'mini.deps' plugin. There is a first-pass documentation mostly aimed for reviewers and no tests yet (as it is an open question about how to do that reasonably).Here is a demo of common workflows at the latest PR state:
vim-pack-demo-4.mp4
Demo of initial PR state (for history)
vim-pack-demo.mp4
Top right window comes from 'mini.notify' plugin; that is not part of
vim.pack:Timestamps:
vim.pack.update(), review, confirm).vim.pack.update()with{ offline = true }to skip downloading new changes, review, confirm).General notes about design decisions:
Design decisions notes
Plugin spec is designed to be minimal yet versatile with an eye for future automated packspec support, i.e. plugins themselves containing a special 'pkg.json' file which should contain at least the following information:
vim.pack.add().vim.pack.Spec.Showing "interactive" confirmation buffer might be considered too much, but being able to review updates before approving them is a huge QoL improvement. Right now it also includes in-process LSP server to show structure via
vim.lsp.buf.document_symbol()(or with defaultgO). The whole LSP approach might be an overkill, but:vim.pack. Most useful mappings are already built-in.vim.lsp.completionsources and code actions).There are more LSP-based interactive features planned (see next section).
Left out from this PR but planned after discussions and executive decisions:
Planned future work
"Scripting way" to remove installed plugins from disk. I do have plans to support "delete plugin" code action from inside confirmation buffer, but it needs green light. The "scripting way" can be designed in several ways:Avim.pack.remove()- simple wrapper aroundvim.fn.delete(). Might also trigger dedicated events.Avim.pack.clean()- delete plugins that are not added to current session. Without interactive confirmation, this can become tricky for configs which will lazy load plugins on events/ModeChanged/etc.Do nothing and suggest usingvim.fn.delete()directly with (possibly exported) path to a special plugin's directory. This is meant as minimal plugin manager, after all.After relevant discussions (this and this), there is a
vim.pack.del()present in this PR.User commands. Like
:Pack addor:PackAdd. The latter is more natural when it comes to!and easier to implement completion.Allowing "local plugins" (as "unmanaged plugins" in 'vim-plug'). This can be skipped entirely in favor of suggesting putting those plugins in custom 'pack/mine/opt' package and use
:packadd. But initial Matrix discussions showed interest in having this supported.As far as I can tell, they don't quite fit into the current design: they usually don't require automated installation or update. They require manual handling during
add(explicit source of 'plugin/' and 'after/plugin/') and book keeping to ignore it duringupdate(). Doable, but requires executive decision and extra effort/code.Consider adding
vim.pack.config(). It will configure howvim.packoperates. Possible options:job_threads- how many threads can parallel steps (like "install" and "download updates") use. Current and default value - 80% of all available threads.job_timeout- amount of time to wait until force stopping any job. Current and default value - 30 seconds.default_version- defaultversionto use invim.pack.Spec. It can be used to have "install latest available version" by default withvim.version.range('*')value. Current and default value -nilto infer the default repo branch.Consider moving documentation to a dedicated
pack.txt. Maybe also move the:help packagessection in there.Consider re-introducing dedicated highlight groups for 'nvim-pack' confirmation report. Needs discussing the scope of highlighting groups and the best place to define them.
Consider making
vim.pack.add()more flexible. Either via flags/enums onvim.pack.add()level or as part of per-plugin spec. Some common requested use cases::packadd(even:packadd!) to only ensure that all plugins are installed in favor of manually loading them later. One way to do it is to allowloadas function that accepts a single resolved plugin spec and is responsible for loading into current session.More interactive update features:
Make progress update user-customizable. Either after
vim.ui.progress(progress messages #32537) is a thing or there is a built-in$/progresshandler that actually shows progress ([GSoC] Implementation of AI-Oriented LSP feature & LSP Data Caching module #34254, statusline: reimplement default, include LSP progress #28809)..Discuss and enforce order of event triggering. Right now due to totally async install/update/checkout there is no guaranteed order of
Pack{Install,Update}{Pre,}events across different plugins. This might lead to conflicts when callback of some plugin event relies on features from dependency plugin. As there is no guaranteed order, callback for "main" plugin can trigger before installing/updating "dependency" plugin. The installation order can be enforced by separatevim.pack.add()calls, but update/checkout can not.One approach can be to guarantee processing necessary steps in order that plugins are
:packadded. This is currently done forvim.pack.del()and its events.See more discussions here.
Lockfile support. Basically, store state/commit per source and prefer it only during initial install over resolving
version. Will help with reproducible setups.It (or its "private" alternative) can also contain an information about
versionto be used duringupdate()if the plugin was not (yet) added to current session. In theory, this can also be used to detect if the user has changed plugin'sversionin an attempt to switch version/branch and automatically do so during startup (without having to do an extravim.pack.update()call). One thing why this might not be good is that it breaks the separation of "add()is for installing and loading plugins" and "update()is for updating already installed plugins".More straightforward plugin freeze/pin/lock. Originally this PR proposed a special value of
version('HEAD'string orfalseboolean) to be treated as "freeze the installed plugin from updates". It was decided to postpone the decision about the better "freeze" approach until there is a lockfile support. See this comment for details.More straightforward
versionchange of already installed plugin. Currently it requires extra step ofvim.pack.update()(which needs internet connection) to "sync" plugin on disk with to have properversion. After there is lockfile support, an alternative approach would be to have "add()ensures both presence and correct on disk spec (likesourceandversion)" and "update()is only for downloading and applying changes from source". This simplifies the "change version" step at the cost of not being able to see beforehand and confirm/deny which changes will be applied. They would still be present in the log file for inspection, though. Relevant discussion is here.:checkhealthsupport. It can include suggestions like "the latest tag of this plugin is very far behind default branch; consider setting version = 'master'".Packspec support. It is rather big and needs discussions about the degree of support vs complexity.
Not planned as
vim.packfunctionality:Not planned
vim.pack.add()only installs in 'opt/' directory (as it is all that is needed), it seems unnecessary to also manage 'start/' from the same package path. Plus it is known to cause confusion about how to not load its plugins.now()andlater()functions. They safely execute its input function immediately and "soon" while reporting errors only after all those functions are finished. These are general enough to live outside ofvim.packand should be useful to have invim.func.