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

Fisher should stop installing into $__fish_config_dir #612

Closed
lilyball opened this issue Nov 10, 2020 · 1 comment
Closed

Fisher should stop installing into $__fish_config_dir #612

lilyball opened this issue Nov 10, 2020 · 1 comment

Comments

@lilyball
Copy link
Contributor

Fisher writes its files to $__fish_config_dir by default. This means it doesn't need to touch any other shell configuration, but it has two downsides:

  1. If a plugin has a file named the same as a file I already have, fisher will simply overwrite it.
  2. Fisher can orphan installed files due to its reliance on universal variables (see Fisher should stop using universal variables for plugin state #611).

Fisher does support a variable $fisher_path that makes it install into a different location, though it doesn't properly document what setup I need to do to ensure that works (it just tells me to put it in my function path, but that won't source the conf.d files, which is non-trivial to do as they're sourced prior to the user's config.fish, and trying to source them manually will introduce behavioral differences around ordering or overriding). But most people are presumably using the default configuration.

I think Fisher should really just default installation into a separate directory (such as ~/.config/fisher/plugins, or possibly ~/.config/fish/fisher/ if we want to try and get picked up automatically with setups that sync the fish config), and in fact it should refuse to install into ~/.config/fish. It can then modify $fish_function_path and $fish_complete_path as appropriate. For conf.d, this is more complicated due to the fact that config files are sourced before fisher can load anything. Normally with conf.d it loads in order "User > Admin > Vendor" and favors earlier files in the event of conflicts (which is to say, a file foo.fish in the User conf dir prevents a file foo.fish in Admin or Vendor from being sourced). I can think of three reasonable approaches for Fisher:

  1. Simulate "User > Plugins > Admin > Vendor" as best as we can. We could do this by adding a file conf.d/~~~fisher.fish (the use of the ~~~ prefix is to try and get it to be the last file loaded as part of the user config set, as ~ is the highest printable ASCII char; or maybe pick some relatively high non-alphabetic unicode char, but I feel like sticking with ASCII is probably best). This file would then be responsible for sourcing all of the installed plugin config files, and it can skip any files whose names already exist in the User config, to simulate the User overriding it. The downside here is we can't properly override any Admin or Vendor config files.

  2. Symlink the config files into place. If there are conflicts, just skip the symlink. This also simulates "User > Plugins > Admin > Vendor", but the downside is if I then delete my overriding user config file, the plugin one won't get sourced until I reinstall that plugin.

  3. Symlink the config files into place, but if there are conflicts then rename the symlink. This way the plugins behave like User files rather than a separate tier. The downside here is if I delete a conflicting user config file, the plugin symlink won't get renamed until I reinstall the plugin, meaning it won't block any overridden admin or vendor config file from loading.

Regardless of the approach chosen, by installing into a separate directory, we can also track precise install state in that directory to avoid any orphaned files. With the symlink approaches, in the worst case where we lose the separate dir (or where it's simply not synced along with the rest of the fish config), we can still detect these symlinks by looking for any conf.d symlinks that point into our fisher install path and clean them up as part of an install/update/remove process.

@jorgebucaran
Copy link
Owner

jorgebucaran commented Nov 11, 2020

@lilyball If a plugin has a file named the same as a file I already have, fisher will simply overwrite it.

It's very unlikely for this to happen in practice, because, first, there aren't many plugins to begin with (and even if there were, people don't use hundreds of plugins), and second, plugin authors can effectively eliminate conflicts by using a strict file naming convention, (every file and function from piyopoyo should be prefixed by _piyopoyo_). And even if we could prevent this from happening at all, functions in fish are global, so it's ultimately up to the user to make sure they are not trying to install two plugins with the same name.

@lilyball Fisher does support a variable $fisher_path that makes it install into a different location, though it doesn't properly document what setup I need to do to ensure that works (it just tells me to put it in my function path, but that won't source the conf.d files, which is non-trivial to do as they're sourced prior to the user's config.fish, and trying to source them manually will introduce behavioral differences around ordering or overriding)

Exactly. I'm not a fan of even moderately complicated shell configurations, specially in fish where no configuration can go a long way. Installing files in $__fisher_config_dir is simple and works out of the box. That's part of what makes Fisher simple and satisfying to use for many people.

@lilyball Fisher can orphan installed files due to its reliance on universal variables (see #611).

Not if we can sync your fish config between two or more machines the way you want, right? unless the user corrupts Fisher's state which we save into $_fisher_plugins and $_fisher_PLUGIN_NAME_files.


Related: #601 #602 #603

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