jorgebucaran / fisher

A plugin manager for Fish
https://git.io/fisher
MIT License
7.53k stars 257 forks source link

Get selected version in plugin event handlers #788

Open Realiserad opened 2 months ago

Realiserad commented 2 months ago

When designing a plugin which can be installed with fisher, you can add event handlers in conf.d which get invoked by fisher when the user installs, updates and removes your plugin.

This is great, and I want to leverage these event handlers to install dependencies for my plugin. However, the version of these dependencies may vary depending on which version of the plugin is being installed, but I cannot see any mechanism in fisher for getting the version of the plugin being installed. For example, if the user runs fisher install some/plugin@v1 I would like to get v1 as an argument somewhere.

Is this possible?

jorgebucaran commented 2 months ago

There isn't an automatic way to grab that info right now, but we could make it happen. We can pass the plugin name and tag in the $plugin variable to the install/update event handler as an argument when we're emitting the event here:

https://github.com/jorgebucaran/fisher/blob/2efd33ccd0777ece3f58895a093f32932bd377b6/functions/fisher.fish#L190-L192

This way the event handler receives the full plugin name and version tag, as its first argument. For example, if you installed jorgebucaran/nvm.fish@2.2.13, you'd receive exactly that string. You could then string split -- @ to get the version number.

# Defined in flipper/conf.d/flipper.fish

function _flipper_install --on-event flipper_install --argument-names plugin
    switch (string split -- @ $plugin)[2]
        case v1
            ...
    end
end

We can set it up the same way for when you're uninstalling a plugin here:

https://github.com/jorgebucaran/fisher/blob/2efd33ccd0777ece3f58895a093f32932bd377b6/functions/fisher.fish#L134-L136

What you could do is modify Fisher as I've outlined and see if that works for you. I'm open to adding this in. It's a minimal change, and I've been waiting for a good reason to use event parameters more effectively. :+1:

jorgebucaran commented 2 months ago

You can try it via jorgebucaran/fisher@event-args now.

Realiserad commented 2 months ago

Thanks for the quick and detailed response! 💛

TL;DR It works, but now when I'm thinking about it, would it be possible to invoke fisher list instead?

I patched my local fisher using the diff from your commit and took it for a spin, and it works well! Being used to statically typed languages where method signatures cannot be easily changed, I was worried that it would break backwards compatibility somehow, but that does not seem to be the case (when dereferencing the the $plugin variable on an old fisher installation you just get an empty string).

Considering that plugin developers would have to wait for users to upgrade their version of fisher before they can start using this new feature, maybe a better way would be to simply run fisher list my-plugin in the event handler?

I believe this would achieve the same thing for installs, but for upgrades it looks like the fish_plugins file is updated after the event is emitted, which means that if I run fisher list in the upgrade event handler, I would get the old version of the plugin, not the new version being installed?

jorgebucaran commented 2 months ago

Developers would still need to wait, since there is no fisher list plugin either. We actually had something similar in the early days, but it provided limited information mainly because plugins don't carry metadata with them.

Your initial idea seems to align more naturally with automating things, which I think suits our needs better for this context. But, like you said, ensuring all users update to the latest Fisher version takes time.

In the meantime, you might consider traversing the internal $_fisher_plugins array to identify your plugin (maybe you can use contains) and extract the part after the "@" with string split. I'd need to verify that $_fisher_plugins indeed includes the plugin being installed, updated, or uninstalled, but I believe it should.

Realiserad commented 2 months ago

Developers would still need to wait, since there is no fisher list plugin either.

I'm not sure what you mean by this, since I can run fisher list on my machine and I'm using the latest fisher. This command is also documented in the README.md. Here is an example of how I used fisher list to get the name and version of the plugin being installed.

Anyway, getting the plugin name as an argument would indeed be better, and potentially iron out some weird edge cases (e.g. my code would fail if someone would have two different versions of the plugin installed).

So /lgtm 🙈

jorgebucaran commented 2 months ago

I was referring to fisher list my-plugin, which isn't a valid Fisher command. If fisher list alone works for you, then that's fine. 😄