From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-36305-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 28528 invoked by alias); 27 Aug 2015 23:11:57 -0000
Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
X-No-Archive: yes
List-Id: Zsh Workers List <zsh-workers.zsh.org>
List-Post: <mailto:zsh-workers@zsh.org>
List-Help: <mailto:zsh-workers-help@zsh.org>
X-Seq: 36305
Received: (qmail 25686 invoked from network); 27 Aug 2015 23:11:57 -0000
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on f.primenet.com.au
X-Spam-Level: 
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU autolearn=ham autolearn_force=no version=3.4.0
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=
	daniel.shahaf.name; h=cc:content-type:date:from:in-reply-to
	:message-id:mime-version:references:subject:to:x-sasl-enc
	:x-sasl-enc; s=mesmtp; bh=ZK3jIxZkNplMruZWrqWAV10J8CI=; b=nmuyvO
	V5Ma25I0sz6HfWnTcdFqq0zxIM9pWEfyas0IQ6B+Bu63gIr9PO/DhIK5UzDmMrtx
	D8DtzOOEAqYyh/9R30Z7WzgLhYLGgd/S6AwfHQxpzDU1fwb34OTACXfTzqzteRtX
	bzU2F+67UZvL76bwYon7TH6O66JTdsbsKGLlY=
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=
	messagingengine.com; h=cc:content-type:date:from:in-reply-to
	:message-id:mime-version:references:subject:to:x-sasl-enc
	:x-sasl-enc; s=smtpout; bh=ZK3jIxZkNplMruZWrqWAV10J8CI=; b=AowQ5
	kPaHs4GjikxnmOLpXjbKIeDC9k/qHhPi+Xf9ZU7gl1aOGmdKGixZCn3oX0rqZL4I
	be3PezpoesNZGzkzh0zsouf3I36kpD+IgLaooRMzki7M5J+jJugCuv4yn/yMUEhb
	eS/nanWt9c2OFsPB2nr6gXMATOzLGwd72Ngyxc=
X-Sasl-enc: G44lyTmTf9YQl07plCo79XSnn/LqTH2jHnEeURvpBDQ5 1440717115
Date: Thu, 27 Aug 2015 23:11:52 +0000
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Mateusz Karbowy <mateusz.karbowy@gmail.com>
Cc: zsh-workers@zsh.org
Subject: Re: PATCH: 3.0.8: git completion update for cherry-pick
Message-ID: <20150827231152.GD21994@tarsus.local2>
References: <CAFiR=Jv1ycq5jWvbyHQX=Csjyv8H1xSUKA45Mj6152Why5qhjg@mail.gmail.com>
 <20150722115307.GC2171@tarsus.local2>
 <CAFiR=JvJefLthQnoQ2YY-nQF69jgAXo76dzMit7sBRBZVgiq_Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAFiR=JvJefLthQnoQ2YY-nQF69jgAXo76dzMit7sBRBZVgiq_Q@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)

Mateusz Karbowy wrote on Tue, Aug 25, 2015 at 23:26:21 +0100:
> My last patch interfered with git-checkout. I've fixed it this time.
> 

Sorry for the late reply.

I have only two comments about this patch: one bugfix and one question.

> @@ -511,7 +513,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 -O expl -C git_commit_opts'

There are other callers of __git_commit_ranges that pass compadd options in
argv.  There is already a compadd option "-C", therefore, choosing this option
letter prevents any caller of __git_commit_ranges from passing the compadd -C
option.

Is this a problem?  Should we find some other way to pass the name
"git_commit_opts"?  If so, what option letters are available?  (Since "-O" is
available, maybe we could pass both array names in it as '-O
"expl:git_commit_opts"'?  I.e., if a colon is present in the argument
value, split on it, else assume the entire argument is a single
parameter name, as in the conventional "-O expl"?)

>  __git_commits () {
> ...
> +  expl=( $@ )

The $@ should be double-quoted to avoid eliding empty arguments.

> 

Apart from these two issues, the patch seems ready for commit to me.

Thanks again.

Cheers,

Daniel