MichaelAquilina / zsh-you-should-use

📎 ZSH plugin that reminds you to use existing aliases for commands you just typed
GNU General Public License v3.0
1.42k stars 44 forks source link

Match full git aliases with any order of arguments #67

Closed crater2150 closed 5 years ago

crater2150 commented 5 years ago

This fixes git alias matching for aliases with parameters (#66).

I made two variants of this. In this one, the order of arguments is irellevant, as long as all arguments in the alias occur in the actual command. The other one in branch git-alias-exact-match only matches if the command starts with the exact string from the arguments.

So in this variant, for alias.recommit commit --amend --reuse-message=HEAD:

git commit --amend --reuse-message=HEAD
git commit --amend --reuse-message=HEAD --some-further-param
git commit --reuse-message=HEAD --amend
git commit --reuse-message=HEAD --amend --some-further-param
git commit --prepended-param --reuse-message=HEAD --amend

will all match. The other variant would only match the first two. I think this variant works well, but may possibly have edge cases when there are parameters that could mean different things depending on position (e.g. branch names)

MichaelAquilina commented 5 years ago

In terms of the variants, I think I would prefer sticking to last one you described (i.e. the one that only matches the first 2).

The reason is that I can forsee situations where the order of the parameters make a difference.

e.g.

git commit --amend --reuse-message HEAD

is also a valid way to run that command

MichaelAquilina commented 5 years ago

Once we are happy with the approach I would also ask that you add some tests :) They should be relatively easy to write if you copy a similar test and adjust it for the behaviour you are trying to add

crater2150 commented 5 years ago

Ok, I'll work on a test for the exact match branch, should get around to it tomorrow.

MichaelAquilina commented 5 years ago

@crater2150 any progress on this? :)

crater2150 commented 5 years ago

Sorry, I didn't have time this weekend, but it should now be finished. I force-pushed the exact match branch to here, the code for this is simpler than for matching in any order. I also added tests as requested.

MichaelAquilina commented 5 years ago

Thanks for the PR and well done @crater2150 :tada: