My last patch interfered with git-checkout. I've fixed it this time.

On 22 July 2015 at 12:53, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Mateusz Karbowy wrote on Sat, Jul 18, 2015 at 23:19:05 +0100:
>> Currently completion for cherry-pick displays recent commits from the
>> current branch.
>> This patch changes this to show recent commits from all branches except HEAD.
>>
>
> Good morning again Mateusz, thanks for the patch.  Review below.
>
>> Obszczymucha
>>
>> diff --git a/_git b/_git
>> index b8edc10..8b9eb2b 100644
>> --- a/_git
>> +++ b/_git
>> @@ -511,7 +511,7 @@ _git-cherry-pick () {
>>      '*'{-s,--strategy=}'[use given merge strategy]:merge
>> strategy:__git_merge_strategies' \
>>      '*'{-X,--strategy-option=}'[pass merge-strategy-specific option
>> to merge strategy]' \
>>      '(-e --edit -x -n --no-commit -s --signoff)--ff[fast forward, if
>> possible]' \
>> -    ': :__git_commit_ranges'
>> +    ': :__git_commit_ranges --all --not HEAD'
>
> Two things about this.
>
> One, how about passing the rev-list flags in a single word, as in:
>        ': :__git_commit_ranges "--all --not HEAD"'
> This both simplifies the callees (they don't need to reimplement git's
> option parsing) and is more extensible (it'd be trivial to make another
> caller use some other set of rev-list options if we need to).
>
> Two, since there is no space after the second colon, _arguments will
> call __git_commit_ranges with compadd flags in argv.  (If you print argv
> in the callee, you'll see it has both a -J option for compadd and
> a --all --not option for git.)  I don't think we should pass both
> rev-list flags and compadd flags in argv.  So, we have two options:
> either add a space after the second colon (which inhibits passing
> compadd flags — does this have undesired side effects?), or keep the
> compadd flags passed in argv and pass the rev-list flags via some other
> channel, such as a well-known parameter name (the caller defines
> a variable with a specific declared-in-the-(internal)-API name and the
> callee checks whether the variable by that name is defined).
>
> What do you think?
>
>> -  commits=("${(f)"$(_call_program commits git --no-pager log -20
>> --format='%h%n%d%n%s\ \(%cr\)')"}")
>> +  commits=("${(f)"$(_call_program commits git --no-pager log $opts
>> -20 --format='%h%n%d%n%s\ \(%cr\)')"}")
>
> Your mailer inserted a hard line break (here) and munged trailing
> whitespace (elsewhere).  Next time you might want to attach your patch
> (to avoid it being munged) with a *.txt extension (to make it easier to
> open).
>
> Cheers,
>
> Daniel
>