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

Suggestion – Temporary Workaround for OMF Compatibility #630

Closed
nickeb96 opened this issue Nov 26, 2020 · 6 comments
Closed

Suggestion – Temporary Workaround for OMF Compatibility #630

nickeb96 opened this issue Nov 26, 2020 · 6 comments

Comments

@nickeb96
Copy link

I think I have a solution to the OMF compatibility problem. Currently Fisher puts all *.fish files from the root of the repository to .config/functions. From what you've talked about here, it sounds like this behavior is legacy and will eventually be removed.

If Fisher removed this legacy feature and started ignoring files outside of plugin/functions/, plugin/conf.d/, and plugin/completions/ we wouldn't have to worry about this.

It seems like you can't remove this feature quite yet, but in the mean time you could make fisher just ignore OMF files like uninstall.fish when they are in the root. This would give plugin managers the ability to support both OMF and Fisher. It would also solve the problem we are having with this pull request.

It also wouldn't have any negative effects when the legacy feature is removed.

@jorgebucaran
Copy link
Owner

@nickeb96 From what you've talked about here, it sounds like this behavior is legacy and will eventually be removed.

This will break all plugins that place files outside functions/, e.g. bobthefish.

It's going to take a lot of patience and time before we can even migrate fisher.fish to functions/fisher.fish, as we don't know how many users are still using 3.x.

That's because of the lamentable decision I made in 3.x to hardcode the (now obsolete) self-update mechanism to fetch Fisher from a raw URL that points directly to the fisher.fish file:

fisher/fisher.fish

Lines 154 to 157 in c84294b

function _fisher_self_update -a file
set -l url "https://raw.githubusercontent.com/jorgebucaran/fisher/main/fisher.fish"
echo "fetching $url" >&2
command curl -s "$url?nocache" >$file.

We now use Fisher to bootstrap Fisher (thanks to @IlanCosman for the innovation), so this is fixed for good, but we're kind of stuck for a while. The only immediate solution I can think of would be renaming Fisher to Fissure and using Fisher to upgrade to that. Not my worst idea ever, but then again, see above. 😄

We can think about removing support for *.fish files outside Fish prescribed directories again then. As for bringing back OMF-specific support, nope, we've done that for years. How long is long enough?

@nickeb96
Copy link
Author

@jorgebucaran the suggestion isn't to fast track ignoring *.fish files outside of functions/. Instead I'm suggesting that you keep the legacy support that puts *.fish files in the plugin root to .config/fish/functions, except if the file is uninstall.fish (and maybe key_bindings.fish).

So the only difference would be that plugin/uninstall.fish would not be moved to .config/fish/functions, but other plugin/*.fish files in the root would continue to be moved (for now). This way we wouldn't break functionality with fisher.fish, or any other plugin still relying on this behavior including bobthefish.

If we did this, uninstall.fish could still be in the plugin root and have uninstall logic triggered by OMF. It just wouldn't be moved to .config/fish/functions by Fisher. Fisher would just skip over it.

I'm with you on not wanting to deal with OMF support and the obsolete install logic, but I think that this is the best compromise. The uninstall.fish file would just need a single line emitting the uninstall event and OMF would execute it when it uninstalls. Fisher would completely ignore this file instead of treating it as a function.

I think this would also be very simple to implement on Fisher's side, since there's no additional install logic or compatibility needed.

@jorgebucaran
Copy link
Owner

Just playing the devil's advocate here. What if there was a plugin named foo/uninstall, foo/key_bindings or foo/init that doesn't use a functions directory? I guess they're out of luck!

Seriously, though, which init/uninstall-based OMF plugins are in fact "popular", or actively maintained? I'm really curious.

@nickeb96
Copy link
Author

The first plugin that comes to mind that still uses uninstall/init is Pisces, and it’s a pretty popular plugin, but I’m sure there are others.

It’s true that an uninstall function in the root wouldn’t be loaded as a function, but it wouldn’t be loaded as a function in OMF either, so I don’t think we have anything to worry about.

If a plugin really needed to create a function called uninstall, they could just put it in the functions directory, which they should be doing anyway as you mentioned. If they are not doing this, we can safely assume it’s an older plugin and they wouldn’t be using foo/uninstall.fish for a regular function anyway.

I really don’t think that this change would introduce any problems. Any plugin with uninstall logic in foo/uninstall.fish is already not being installed properly by fisher. This change just makes it easier for plugin authors to support both fisher and omf without needing separate branches or repos.

@jorgebucaran
Copy link
Owner

I didn't find any other plugins that met my criteria, and it seems that you found an excellent workaround for Pisces that I hadn't even noticed.

It looks like there's no need to follow through with the second part of this proposal then.

@jorgebucaran
Copy link
Owner

To whoever is watching. I've since created Autopair, partly as a response to this issue and to see if I could do better. Read my comparison with Pisces here.

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

No branches or pull requests

2 participants