mkNeovimPlugin: add lazyLoad with lz-n support #2602

khaneliman · 2024-12-02T19:32:30Z

lib/options.nix

lib/neovim-plugin.nix

tests/test-sources/plugins/lazy-load.nix

traxys · 2024-12-04T16:02:48Z

lib/options.nix

+            # Spec loading:
+            enabled = mkStrLuaFnOr types.bool (lazyLoadDefaults.enabledInSpec or null) ''
+              When false, or if the function returns false, then ${originalName} will not be included in the spec.
+
+              Equivalence: lz.n => enabled; lazy.nvim => enabled
+            '';
+
+            priority = mkNullable types.number (lazyLoadDefaults.priority or null) ''
+              Only useful for start plugins (not lazy-loaded) to force loading certain plugins first.
+
+              Equivalence: lz.n => priority; lazy.nvim => priority
+            '';
+
+            # Spec setup
+            # Actions
+            beforeAll = mkLuaFn (lazyLoadDefaults.beforeAll or null) ''
+              Always executed before any plugins are loaded.
+
+              Equivalence: lz.n => beforeAll; lazy.nvim => init
+            '';
+
+            before = mkLuaFn (lazyLoadDefaults.before or null) ''
+              Executed before ${originalName} is loaded.
+
+              Equivalence: lz.n => before; lazy.nvim => None
+            '';
+
+            after = mkLuaFn (lazyLoadDefaults.after or null) ''
+              Executed after ${originalName} is loaded.
+
+              Equivalence: lz.n => after; lazy.nvim => config
+            '';
+
+            # Triggers
+            event = mkNullable triggerType (lazyLoadDefaults.event or null) ''
+              Lazy-load on event. Events can be specified as `BufEnter` or with a pattern like `BufEnter *.lua`
+
+              Equivalence: lz.n => event; lazy.nvim => event
+            '';
+
+            cmd = mkNullable triggerType (lazyLoadDefaults.cmd or null) ''
+              Lazy-load on command.
+
+              Equivalence: lz.n => cmd; lazy.nvim => cmd
+            '';
+
+            ft = mkNullable triggerType (lazyLoadDefaults.ft or null) ''
+              Lazy-load on filetype.
+
+              Equivalence: lz.n => ft; lazy.nvim => ft
+            '';
+
+            keys = mkListOf (types.attrsOf types.anything) (lazyLoadDefaults.keys or null) ''
+              Lazy-load on key mapping.
+
+              Equivalence: lz.n => keys; lazy.nvim => keys
+            '';
+
+            colorscheme = mkNullable triggerType (lazyLoadDefaults.colorscheme or null) ''
+              Lazy-load on colorscheme.
+
+              Equivalence: lz.n => colorscheme; lazy.nvim => None
+            '';
+
+            extraSettings = mkSettingsOption {
+              description = ''
+                Extra settings to pass to the lazy loader backend.
+              '';
+              example = {
+                dependencies = {
+                  __unkeyed-1 = "nvim-lua/plenary.nvim";
+                  lazy = true;
+                };
+              };


It would maybe be better to only support a backendSettings option of type attrsOf anything, that will be passed to the code that implements the lazy loading provider.
I'm not confident that we can find a common API for all possible lazy loading schemes

That makes sense. I was just changing the options nesting path and this would fit nicely. We can wrap all the backendSettings in a mkSettingsOption style option with the known options specified for documentation.

I'm not confident that we can find a common API for all possible lazy loading schemes

Perhaps. But I think we will need some level of abstraction even if it is just knowing where to put our setup call.

That could look like a setupOptionLoc option with a default based on which backend is enabled?

E.g.

setupOptionLoc = mkOption { type = listOf str; # maybe a non-merging list instead? default = if config.pligins.lz-n.enable then [ "after" ] else if config.lazy.enable then [ "config" ] else throw "setupOptionLoc cannot be used when no lazyloader is enabled"; defaultText = literalMD "TODO"; }

I was thinking of a function that takes as an input all the required information.... But that will end up in an infinite recursion.

We do need an enumeration of supported backends, but we may not need more than that

lib/options.nix

lib/neovim-plugin.nix

MattSturgeon · 2024-12-05T20:05:50Z

lib/neovim-plugin.nix

-                ++ (lib.optionals (!hasConfigAttrs) [
+                ++ lib.optionals (!hasConfigAttrs) [
                  (lib.optionalAttrs callSetup (setLuaConfig setupCode))
-                ])
-                ++ (lib.optionals hasConfigAttrs [
+                ]


How should the !hasConfigAttrs case be handled when lazyloading?

Looks like we only have one instance of no config attrs, in rustaceanvim, because they use global options for configuration. Seems like something you don't get to really lazy load through a manager and is handled by the plugin itself?

Certainly for lz-n, there's nothing to lazyload if there's no setup function. lazy.nvim is a bit fancier though.

If we did go down that route, maybe the plugins.*.lazyload option should only exist when hasSetup is true? Although I'm happy for that to be tackled in future work if it is too much for this PR.

lib/neovim-plugin.nix

MattSturgeon

Looks good. Just last minute discussions

lib/neovim-plugin.nix

lib/options.nix

tests/test-sources/plugins/lazyloading/lz-n.nix

traxys · 2024-12-10T16:07:18Z

lib/neovim-plugin.nix

+                    warnings = lib.optional (!config.plugins.lz-n.enable) ''
+                      You have enabled lazy loading for ${originalName} but have not enabled any lazy loading plugins.
+                      We will not insert the lua for this plugin and you must handle loading it yourself.  
+
+                      Currently supported lazy providers:
+                        - lz-n
+                    '';


This means that if someone wants to use an unsupported lazy loader they will have warnings for each of their plugins right?

In another PR we should consider adding a global option that will allow to silence the warnings, something like useUnsupportedLazyLoader

Oh duh, yeah.. I'll move it out

We will support more, might as well properly organize them.

Instead of trying to manage upstream configuration options, just keep using our freeform options so we can do less finicky logic and workarounds.

We don't need to run nvim and see what happens, these are just used for making sure we're generating the config appropriately.

GaetanLepage

LGTM !!!

MattSturgeon

Thanks for working on this!

GaetanLepage · 2024-12-10T16:20:43Z

@Mergifyio queue

mergify · 2024-12-10T16:20:51Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b752606

mergify · 2024-12-10T16:21:01Z

This pull request, with head sha b752606681ded3f69e99ed568c7075b3578dce48, has been successfully merged with fast-forward by Mergify.

This pull request will be automatically closed by GitHub.

As soon as GitHub detects that the sha b752606681ded3f69e99ed568c7075b3578dce48 is part of the main branch, it will mark this pull request as merged.

It is possible for this pull request to remain open if this detection does not happen, this usually happens when a force-push is done on this branch lazy-complex, this means GitHub will fail to detect the merge.

khaneliman force-pushed the lazy-complex branch 4 times, most recently from d73d2b6 to 6635837 Compare December 2, 2024 20:27

khaneliman commented Dec 2, 2024

View reviewed changes

lib/options.nix Outdated Show resolved Hide resolved

khaneliman commented Dec 2, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

Eveeifyeve reviewed Dec 2, 2024

View reviewed changes

tests/test-sources/plugins/lazy-load.nix Outdated Show resolved Hide resolved

Eveeifyeve reviewed Dec 2, 2024

View reviewed changes

tests/test-sources/plugins/lazy-load.nix Outdated Show resolved Hide resolved

khaneliman mentioned this pull request Dec 4, 2024

plugins/trouble: fix originalName #2607

Merged

traxys reviewed Dec 4, 2024

View reviewed changes

khaneliman force-pushed the lazy-complex branch 5 times, most recently from 7bd6017 to 3d27ea0 Compare December 5, 2024 15:19

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/options.nix Outdated Show resolved Hide resolved

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/options.nix Outdated Show resolved Hide resolved

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

khaneliman force-pushed the lazy-complex branch 2 times, most recently from adeebd3 to 9dd8bd4 Compare December 5, 2024 18:56

khaneliman changed the title ~~lazy-complex~~ mkNeovimPlugin: add lazyLoad Dec 5, 2024

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

MattSturgeon reviewed Dec 5, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

khaneliman force-pushed the lazy-complex branch 4 times, most recently from f9b74d2 to b1ba2ac Compare December 5, 2024 21:21

khaneliman force-pushed the lazy-complex branch from 2f8a47a to 2882658 Compare December 10, 2024 14:28

MattSturgeon reviewed Dec 10, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

khaneliman force-pushed the lazy-complex branch from 2882658 to 7b581b9 Compare December 10, 2024 15:03

traxys reviewed Dec 10, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

traxys reviewed Dec 10, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

khaneliman force-pushed the lazy-complex branch 4 times, most recently from 25efd2e to a7c13e1 Compare December 10, 2024 15:48

MattSturgeon reviewed Dec 10, 2024

View reviewed changes

lib/neovim-plugin.nix Outdated Show resolved Hide resolved

lib/options.nix Outdated Show resolved Hide resolved

tests/test-sources/plugins/lazyloading/lz-n.nix Show resolved Hide resolved

traxys reviewed Dec 10, 2024

View reviewed changes

khaneliman added 4 commits December 10, 2024 10:07

lib/neovim-plugin: refactor mkLazyLoadOption

c3f9cb7

plugins/typescript-tools: todo about lazy loading

277b265

tests/lazyloading: lazy-load -> lz-n

d24dd31

We will support more, might as well properly organize them.

lib/neovim-plugin: freeform lazy settings

0997b37

Instead of trying to manage upstream configuration options, just keep using our freeform options so we can do less finicky logic and workarounds.

khaneliman force-pushed the lazy-complex branch from a7c13e1 to ebfa0c9 Compare December 10, 2024 16:07

khaneliman added 3 commits December 10, 2024 10:15

modules/lazyload: warn enabling a lazy loading provider

76b567b

tests/lazyloading/lz-n: reduce repetition

26f1daa

tests/lazyloading/lz-n: runNvim false

b752606

We don't need to run nvim and see what happens, these are just used for making sure we're generating the config appropriately.

khaneliman force-pushed the lazy-complex branch from ebfa0c9 to b752606 Compare December 10, 2024 16:15

GaetanLepage approved these changes Dec 10, 2024

View reviewed changes

MattSturgeon approved these changes Dec 10, 2024

View reviewed changes

mergify bot merged commit b752606 into nix-community:main Dec 10, 2024
4 checks passed

mergify bot temporarily deployed to github-pages December 10, 2024 16:21 Inactive

khaneliman deleted the lazy-complex branch December 10, 2024 16:21

This was referenced Dec 10, 2024

[Feature] Lazy loading #421

Closed

[TRACKING] Lazy loading roadmap #2632

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mkNeovimPlugin: add lazyLoad with lz-n support #2602

mkNeovimPlugin: add lazyLoad with lz-n support #2602

khaneliman commented Dec 2, 2024

traxys Dec 4, 2024

khaneliman Dec 4, 2024 •

edited

Loading

MattSturgeon Dec 4, 2024

traxys Dec 4, 2024

MattSturgeon Dec 5, 2024

khaneliman Dec 5, 2024

MattSturgeon Dec 5, 2024

MattSturgeon left a comment

traxys Dec 10, 2024

khaneliman Dec 10, 2024

GaetanLepage left a comment

MattSturgeon left a comment

GaetanLepage commented Dec 10, 2024

mergify bot commented Dec 10, 2024 •

edited

Loading

mergify bot commented Dec 10, 2024

mkNeovimPlugin: add lazyLoad with lz-n support #2602

mkNeovimPlugin: add lazyLoad with lz-n support #2602

Conversation

khaneliman commented Dec 2, 2024

Choose a reason for hiding this comment

khaneliman Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MattSturgeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaetanLepage left a comment

Choose a reason for hiding this comment

MattSturgeon left a comment

Choose a reason for hiding this comment

GaetanLepage commented Dec 10, 2024

mergify bot commented Dec 10, 2024 • edited Loading

✅ The pull request has been merged automatically

mergify bot commented Dec 10, 2024

khaneliman Dec 4, 2024 •

edited

Loading

mergify bot commented Dec 10, 2024 •

edited

Loading