From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18904 invoked by alias); 22 Jul 2015 12:04:38 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 35853 Received: (qmail 14690 invoked from network); 22 Jul 2015 12:04:34 -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.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL 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-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=tAfT3z4b956xkp0f 0B8N8CFCNsg=; b=ZbtEOSHsMefCvF27MywhSu/ILwgDA9edjeygIRIZfS/5lfYE 7rpGuBM1qXr7/QR/cnEOwUWbKNuV0u0zgP/djq34Y9RhEss4ov56tAroLujmVCh4 9jadcmQiWhqTSnM0wCrBQ7LYav4V/6lNDD+7G76piTjZ0ay1RMlw9NfV++s= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=smtpout; bh=tAfT3z4b956xkp0 f0B8N8CFCNsg=; b=FKYsbWs2/J79x0uobzGu1t5JnjqhsIyd/ZvIoyzGwekJsIh v5KSjgj4hj3gWy5B6/Bb5xLqTH1S39ixwrVss3PzE00G4obLRFwc3OpQNq4TaApr vDxRsTIBsMNcmfrltulvKtz2J4pp4hhkfeJz6SDN3CzgmGXKvyp2zTYp//KE= X-Sasl-enc: e+++iSZseFepytNHnDlqSc5maZX1J7aj2BZQU7yqXLV+ 1437565990 Date: Wed, 22 Jul 2015 11:53:07 +0000 From: Daniel Shahaf To: Mateusz Karbowy Cc: zsh-workers@zsh.org Subject: Re: PATCH: 3.0.8: git completion update for cherry-pick Message-ID: <20150722115307.GC2171@tarsus.local2> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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