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

plugins/lazy.nvim: new plugin manager #1175

Merged
merged 4 commits into from
Mar 8, 2024
Merged

plugins/lazy.nvim: new plugin manager #1175

merged 4 commits into from
Mar 8, 2024

Conversation

ck3mp3r
Copy link
Contributor

@ck3mp3r ck3mp3r commented Mar 1, 2024

This PR addresses #797

@ck3mp3r ck3mp3r mentioned this pull request Mar 1, 2024
@ck3mp3r ck3mp3r changed the title lazy.nvim plugin manager plugins/lazy.nvim plugin manager Mar 1, 2024
@alisonjenkins
Copy link
Contributor

Looks good, I will have a play with this on the weekend.

I have thoughts on the 'build' part of Lazy though. Ideally the build would be unnecessary in Nixvim as it should have already been built and put into Nixpkgs as part of the packaging of dependencies. Is this not the case for plugins you have tested?

If it isn't I wonder if we can create a derivation in some sane manner to ensure that the dependencies are built in a reproducible way that can be fed to Lazy instead of doing the build inside of Lazy.

Copy link
Member

@traxys traxys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice!

I think we could improve a bit in the future the opts (or config, not sure) field by providing a function of the form helpers.initLuaFromModule ({...}: {#nixvim module}, but this is larger than the scope of simply lazy.nvim.

plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
@jla2000
Copy link

jla2000 commented Mar 1, 2024

Would it somehow be possible to configure a plugin that already comes with nixvim (like eg telescope) using the options provided in nixvim, but then use lazy to load this plugin?

@GaetanLepage
Copy link
Member

Would it somehow be possible to configure a plugin that already comes with nixvim (like eg telescope) using the options provided in nixvim, but then use lazy to load this plugin?

This is the goal in the long term but we are not there yet.
Discussion is happening in #421.

@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 2, 2024

@alisonjenkins

I have thoughts on the 'build' part of Lazy though. Ideally the build would be unnecessary in Nixvim as it should have already been built and put into Nixpkgs as part of the packaging of dependencies. Is this not the case for plugins you have tested?

Totally, I should have come to that conclusion myself 😆

@ck3mp3r ck3mp3r requested a review from traxys March 2, 2024 15:43
@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 2, 2024

@traxys is there a slightly more elegant way than doing this?

helpers.mkNullOrOption (either (maybeRaw str) (maybeRaw (listOf str)))

It does the trick but it has a certain "smell" :)

@traxys
Copy link
Member

traxys commented Mar 2, 2024

@traxys is there a slightly more elegant way than doing this?

helpers.mkNullOrOption (either (maybeRaw str) (maybeRaw (listOf str)))

It does the trick but it has a certain "smell" :)

helpers.mkNullOrOption (maybeRaw (either str (listOf str))) ?

@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 2, 2024

@traxys Ohh, just seen your comment in the thread above, let me try the parenthesis the other way!

Update:

Yup, that was it, forgot the parenthesis the first time... funny I remembered to apply them in the smelly version...

@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 2, 2024

Just slightly meta: the revisions to this PR were done in a nixvim instance that uses this plugin manager.
I think, unless there are any other remarks/suggestions, I'll mark this PR as ready to merge...

@ck3mp3r ck3mp3r marked this pull request as ready for review March 2, 2024 20:25
@ck3mp3r ck3mp3r changed the title plugins/lazy.nvim plugin manager plugins/lazy.nvim: new plugin manager Mar 2, 2024
@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 4, 2024

@traxys can we land this soonish?

@jla2000
Copy link

jla2000 commented Mar 4, 2024

@ck3mp3r Was the "import" lazy.nvim option forgotten or intentionally left out?
I tried doing something like this, but there seems to be no nix option for that:

plugins.lazy = {
  enable = true;
  plugins = with pkgs.vimPlugins; [
    {
      pkg = LazyVim;
      import = "lazyvim.plugins";
    }
  ];
};

error: The option `plugins.lazy.plugins."[definition 1-entry 1]".import' does not exist.

Copy link
Member

@traxys traxys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are quite a lot of really long lines, mainly due to descriptions. Could you use '' quotes, and split them on multiple lines?

plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Show resolved Hide resolved
plugins/pluginmanagers/lazy.nix Outdated Show resolved Hide resolved
@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 4, 2024

I'll see when I have time to address these issues. For the time being I need to get back to my day job.
Alternatively we can merge this, it does the trick, and we can then refactor and improve, as I don't see any of the interfaces change the way this plugin is going to be used.

On a different note: I am not sure the abstraction being added by nixvim isn't doing more harm than good. The more I use it the more I want to keep things simple in the nix realm and allow configuration that isn't related to packaging and distribution to be just handled by the neovim ecosystem...

@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 4, 2024

@ck3mp3r Was the "import" lazy.nvim option forgotten or intentionally left out? I tried doing something like this, but there seems to be no nix option for that:

plugins.lazy = {
  enable = true;
  plugins = with pkgs.vimPlugins; [
    {
      pkg = LazyVim;
      import = "lazyvim.plugins";
    }
  ];
};

error: The option `plugins.lazy.plugins."[definition 1-entry 1]".import' does not exist.

@jla2000 this is not currently in scope. It would have to be translated to nix as it is lazy.nvim specific functionality for bundling plugin dependencies.

@jla2000
Copy link

jla2000 commented Mar 4, 2024

Just slightly meta: the revisions to this PR were done in a nixvim instance that uses this plugin manager.

@ck3mp3r Just curious, how did you install treesitter grammars with the lazy.nvim setup? Does it even work at the moment?

@ck3mp3r
Copy link
Contributor Author

ck3mp3r commented Mar 4, 2024

Just slightly meta: the revisions to this PR were done in a nixvim instance that uses this plugin manager.

@ck3mp3r Just curious, how did you install treesitter grammars with the lazy.nvim setup? Does it even work at the moment?

@jla2000 have a look here, this is my current testing branch. It works fine actually :)
https://github.com/ck3mp3r/xvim/blob/lazy/config/treesitter.nix

@jla2000
Copy link

jla2000 commented Mar 4, 2024

Ah I see, you have to add it to the runtime path again since lazy clears it :) Thanks!

@traxys
Copy link
Member

traxys commented Mar 4, 2024

I'll see when I have time to address these issues. For the time being I need to get back to my day job. Alternatively we can merge this, it does the trick, and we can then refactor and improve, as I don't see any of the interfaces change the way this plugin is going to be used.

On a different note: I am not sure the abstraction being added by nixvim isn't doing more harm than good. The more I use it the more I want to keep things simple in the nix realm and allow configuration that isn't related to packaging and distribution to be just handled by the neovim ecosystem...

There are two main ways to write nixvim modules, the way this PR does it (the "old" way) where we type every option strictly, but there is another way we are starting to look into, inspired by the NixOS RFC 42, that uses freeform options and a lot less magic

@ck3mp3r ck3mp3r requested a review from traxys March 7, 2024 10:08
@traxys traxys merged commit f6e7960 into nix-community:main Mar 8, 2024
51 checks passed
@ck3mp3r ck3mp3r deleted the lazy-plugin-manager branch March 9, 2024 14:23
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.

5 participants