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

feat: add extraConfig option #17

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Anomalocaridid
Copy link

Currently, there is no way to for users to add extra code to programs.yazi.initLua if they are using any of the plugins that depend on using require() and running a setup function in init.lua, namely full-border, relative-motions, and starship.

In addition, using more than one of these plugins causes a conflict when nix tries to evaluate it.

This PR remedies both of these problems.

I added an extraConfig option for any extra Lua code that a user wishes to include after the setup functions, similar to various other programs that have home manager modules.

I also added an option, requiredPlugins, for plugins that need to be required and run a setup function. It takes a list of attrsets that have two attributes, name, the plugin's name, and setup, for any options that need to be passed to setup(). I was not sure what to name this, and I am more than happy to change it if you think a different name would suit it better.

@haennes
Copy link
Contributor

haennes commented Feb 15, 2025

This look great :)
I am just a bit unsure about the type of requiredPlugins.
Is there an instance where require gets called twice with the same name (from within the same plugin)?
Maybe we can get away with: requiredPlugins.${requireCall} = null # or {} which looks way cleaner imho.
In that case this could also be solved by setting it twice in extraConfig. 🤔
Imho requiredPlugins sound like they are dependencies. Naming it simply "require" conveys the meaning more effectively.

plugins being dependent on each other could be a whole other discussion on its own. Did someone ever stumble on such a situation?

@Anomalocaridid Anomalocaridid force-pushed the extraconfig-option branch 4 times, most recently from 01c1e95 to a5ff27a Compare February 16, 2025 01:53
@Anomalocaridid
Copy link
Author

This look great :)

Thanks :)

I am just a bit unsure about the type of requiredPlugins.
Is there an instance where require gets called twice with the same name (from within the same plugin)?

As far as I am aware, there are no yazi plugins that do this. I think it is probably safe to assume we will not need to worry about it because I am not sure why a plugin would need require to be called on the same file multiple times.

Maybe we can get away with: requiredPlugins.${requireCall} = null # or {} which looks way cleaner imho.
In that case this could also be solved by setting it twice in extraConfig. 🤔
Imho requiredPlugins sound like they are dependencies. Naming it simply "require" conveys the meaning more effectively.

I have renamed requiredPlugins to require and it is now an attrset of attrsets, where each plugin's name is the name of the attribute and the inner attrsets are the arguments to setup().

plugins being dependent on each other could be a whole other discussion on its own. Did someone ever stumble on such a situation?

As far as I am aware, there are no yazi plugins that depend on other yazi plugins.

@haennes
Copy link
Contributor

haennes commented Feb 16, 2025

One more thing, mainly as a reminder for myself:
Not exposing / documenting the require(dPlugins) or extraConfig options on the per plugin module is a good thing to suggest that a user should not normally need to set it.
Thanks for quickly changing the name and type.
I will test this PR soon TM

@Anomalocaridid
Copy link
Author

Not exposing / documenting the require(dPlugins) or extraConfig options on the per plugin module is a good thing to suggest that a user should not normally need to set it.

This is true for require, but the whole point of extraConfig is so the user has a way to add code to init.lua when using a plugin from nix-yazi-plugins that requires require().

@haennes
Copy link
Contributor

haennes commented Feb 16, 2025

The user can set yaziPlugins.extraConfig as well.

However having the extraConfig available to the user as well seems like a good way to allow users to separate out each plugin more cleanly.

Could you maybe add a description and option for the dynamically generated extraConfig as well :). So that if we eventually decide to build docs for this it is already documented.

@Anomalocaridid Anomalocaridid force-pushed the extraconfig-option branch 2 times, most recently from 42912e3 to 8396494 Compare February 17, 2025 02:52
@Anomalocaridid
Copy link
Author

The user can set yaziPlugins.extraConfig as well.

However having the extraConfig available to the user as well seems like a good way to allow users to separate out each plugin more cleanly.

My mistake, I was not really aware there was a distinction between yaziPlugins.extraConfig and extraConfig. Although in retrospect it makes sense there is considering how the flake is set up, at least as I understand it.

Could you maybe add a description and option for the dynamically generated extraConfig as well :). So that if we eventually decide to build docs for this it is already documented.

Sure, but where should I put this? I cannot figure out where it goes. I have tried putting it in a few places but I get errors every time, usually errors about it already being defined. The most recent commit to this PR has what I tried last.

@haennes
Copy link
Contributor

haennes commented Feb 17, 2025

Oops. already answered this, but then didn´t send my message.
Here you go:
Just add it below this line
These are basically options that get generated for each plugin, hence the ${v.name} which is the plugin name
After this, I think it should be ready to merge.

@Anomalocaridid Anomalocaridid force-pushed the extraconfig-option branch 5 times, most recently from 9a5c3e4 to 3b832b5 Compare February 18, 2025 00:15
@Anomalocaridid
Copy link
Author

Oh I see, so you want each plugin to have its own extraConfig option, in addition to the global one, right? Sorry, I somehow missed that part.

I have added an extraConfig option to the options that gets generated for each plugin.

@haennes
Copy link
Contributor

haennes commented Feb 18, 2025

Just checked it out locally. Apart from generating a newline per plugin (which I think is fine)
could be fixed like this
Just rebase the commit if you want this

@Anomalocaridid
Copy link
Author

I have rebased with your fix

@haennes
Copy link
Contributor

haennes commented Feb 18, 2025

Should be ready to be merged @lordkekz

This was referenced Feb 18, 2025
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.

2 participants