jhillyerd / plugin-git

Git aliases plugin for the Fish shell (similar to oh-my-zsh git)
MIT License
607 stars 85 forks source link

Fix 9313 universal abbr #88

Closed hgibs closed 1 year ago

hgibs commented 1 year ago

Fixes two issues:

  1. Upstream issue 9313 changes how abbreviations work. They are no longer universal and need to get loaded on every interactive prompt (but also they are much faster so it doesn't matter as much). This fix loads in these abbreviations on every call to __git.init

  2. fisher installations without oh-my-fish (like mine) don't call anything in the hooks directory. So these abbreviations aren't loaded on subsequent new shells. Triggering _git_init on interactive prompts fixes this.

I am running this current config on my installation and seems to work as intended.

jhillyerd commented 1 year ago

Thanks for sending this, but I am not in favor of this particular approach to supporting fish 3.6.0. Even without universal abbreviations, we shouldn't need to set them all on each prompt, once per fish process should be enough. I expect we can check __git_plugin_initialized from each prompt instead.

Edit: __git_plugin_initialized must no longer be a universal variable for this to work

faho commented 1 year ago

Outsider question:

Why wouldn't it be enough to just not check __git_plugin_initialized?

Note that abbr in fish 3.6.0 is a builtin, and as such is much faster.

So you could probably just do

not builtin -q abbr; and set -q __git_plugin_initialized; and return 0

To support older fish as well (where abbr is a function).

Edit: You would then also want to stop setting __git_plugin_initialized, or at least not set it with a date (which can take on the order of a few milliseconds on a slow system - not a ton, but since you never read it anywhere why take that at all?).

Edit 2: Just as a datapoint, running the 162 abbreviations in this with just abbr -a key value takes ~64ms in fish 3.5.1, and ~4ms in fish 3.6.0.

hgibs commented 1 year ago

@faho yes something like that would probably be enough. Issuing a return caused me issues on my prompt crashing, but I'm new to fish - I'll check this out and make a better suggested PR.

jhillyerd commented 1 year ago

Obsoleted by #92; thanks for bringing this to my attention.