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

lazy-simple #2577

Closed
wants to merge 1 commit into from
Closed

lazy-simple #2577

wants to merge 1 commit into from

Conversation

khaneliman
Copy link
Contributor

No description provided.

@traxys
Copy link
Member

traxys commented Nov 27, 2024

I think we should try to do the plumbing to lz.n ourselves (though it's a larger change)
We could introduce a luaConfig.requireMethod that's an enum of init (the current way, added to init.lua), lz.n (added to lz.n), <another lazy provider> (added to the other lazy provider) and none, which would result in what you proposed, with the user able to choose as he wants.

Maybe a good first step would be supporting only init and none, doing a second step for the lazy loading provider

@khaneliman khaneliman mentioned this pull request Dec 1, 2024
@MattSturgeon
Copy link
Member

Maybe a good first step would be supporting only init and none, doing a second step for the lazy loading provider

I'm wondering about maybe allowing either bool submodule?

For now it could be just bool, where if true we add the config to init.lua and if false we do not.

In the future we can add either submodule or either (coercedTo str _ submodule) to allow users to define a lazy spec specifying which lazy loader to use and what events will trigger loading the plugin.

I also wonder whether which lazy loading implementation to use should be a global option or a per-plugin one? Surely it wouldn't make sense for multiple lazy loaders to be used simultaneously?

Perhaps it would be enough to just figure out which lazy loading plugin is enabled? And assert that multiple are not enabled at the same time?

@traxys
Copy link
Member

traxys commented Dec 1, 2024

The issue I could forsee is that not all lazy providers could present the same API, meaning that we would not easily provide a RFC-42 style submodule, though this may not be a large issue if all lazy loading providers use a common interface

@Eveeifyeve
Copy link

When enabling lz-n, it should automatic in each plugin enable lazyloading then you have a choice to disable lazyloading if need to (for issues,) but I feel like it would save like between 1-50 lines of code plus having a benefit of cleaner code in a nixvim config.

@khaneliman khaneliman force-pushed the lazy branch 2 times, most recently from 88cfd06 to ff6910c Compare December 2, 2024 16:28
@khaneliman khaneliman changed the title Lazy lazy-simple Dec 2, 2024
Comment on lines +118 to +120
default = {
enable = false;
};
Copy link
Member

Choose a reason for hiding this comment

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

If enable has its own default, this top-level default can simply be {}

'';
type = lib.types.submodule {
options = {
enable = lib.mkOption {
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering, if it is likely we will have a different "enable" in the future, should this be named something else?

Having it named something like dontCallSetup may be more explicit and would also allow us to more easily deprecate this option if we need to in the future?


If you enable this, the plugin's lua configuration will need to be manually loaded by other means.

A usage would be passing the plugin's luaConfig to the `plugins.lz-n.plugins` configuration.
Copy link
Member

Choose a reason for hiding this comment

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

Q: I like this initial implementation, but do you think it is too much of a stretch to go straight to implementing this ourselves in this PR?

That way we never have the interim stage of end-users having to enable a "dont setup" option and passing their plugin's settings to another plugin which lazy-loads them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have another branch i'm playing around with right now for setting up lz-n for a user following the work done in #1875

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... I thought about that when I started going through and implementing my own lazy loading. It felt tedious enough, let alone if I had to redo it again.

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.

4 participants