From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <zsh-workers-return-36154-mason-zsh=primenet.com.au@zsh.org>
Received: (qmail 3346 invoked by alias); 13 Aug 2015 23:23:16 -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: 36154
Received: (qmail 27594 invoked from network); 13 Aug 2015 23:23:14 -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 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=KBj7fOAlMAIRbVvOflj9SRCBIUQ=; b=TVF9/5
	z5PNQALGqZKP/j2M4Pb9dl5nr2TsUZq06OqjeVVPnoklo5fFNnNwctJ9YJhYk06M
	thBL0G9IvhbZgi3QJb7LKVE7vzjRgCnDWmW04jXL7nMAeDId/IG58L0TljdS16l+
	kqF/XgppVLwN19BNoEOXTDo40Oegu/eZ/8oVk=
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=KBj7fOAlMAIRbVvOflj9SRCBIUQ=; b=A/DM9
	3wEP58/Q6BiflPSP/PJKxEnsn9wzWhnVuOUQG0kiGFZJc38rAZgMgwwKdAEFG4oX
	LvGvo7Ordz0Jyfcf2owB/xdcvZIUTn7gLcTuGUsNKr7xRCEkgdrFBUkX9KVVczmG
	8T6JHUdsGvF8ml5XW+txMHekvPhcsfsCc/zQ40=
X-Sasl-enc: S5Gp6CX9Oq9OZRyZTjRc/gCgzET59LVKqfZAhcRXyka+ 1439507640
Date: Thu, 13 Aug 2015 23:13:50 +0000
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Oliver Kiddle <okiddle@yahoo.co.uk>
Cc: Zsh Hackers' List <zsh-workers@zsh.org>
Subject: Re: git log -1 --...
Message-ID: <20150813231350.GA1998@tarsus.local2>
References: <20150813120745.6ffc97af@pwslap01u.europe.root.pri>
 <12769.1439466043@thecus.kiddle.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <12769.1439466043@thecus.kiddle.eu>
User-Agent: Mutt/1.5.21 (2010-09-15)

Oliver Kiddle wrote on Thu, Aug 13, 2015 at 13:40:43 +0200:
> Peter wrote:
> > I haven't looked, but this could well be because _arguments hasn't been
> > told -<num> is a valid completion because there's no point actually
> > completing it, so it assumes it's a regular argument.
> 
> Same problem for git log HEAD --... so it is not quite that.
> It's an effect of the *:: form with _arguments.
> 
> diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
> index a5e4464..fb8ba6a 100644
> --- a/Completion/Unix/Command/_git
> +++ b/Completion/Unix/Command/_git
> @@ -1087,33 +1087,31 @@ _git-log () {
>      $revision_options \
>      '-L+[trace the evolution of a line range or regex within a file]:range' \
>      '(-)--[start file arguments]' \
> +    '1: :->first-commit-ranges-or-files' \
> +    '*: :->commit-ranges-or-files' && ret=0
>  
>    case $state in
> +    (first-commit-ranges-or-files)
> +      if [[ -n ${opt_args[(I)--]} ]]; then
> +	__git_cached_files && ret=0
> +      else
> +	_alternative \
> +	  'commit-ranges::__git_commit_ranges' \
> +	  'cached-files::__git_cached_files' && ret=0
> +      fi
> +    ;;
> +    (commit-ranges-or-files)
> +      # TODO: Write a wrapper function that checks whether we have a
> +      # committish range or comittish and calls __git_tree_files
> +      # appropriately.
> +      if __git_is_committish_range $line[1]; then
> +	__git_tree_files ${PREFIX:-.} $(__git_committish_range_last $line[1]) && ret=0
> +      elif __git_is_committish $line[1]; then
> +	__git_tree_files ${PREFIX:-.} $line[1] && ret=0
> +      else
> +	__git_cached_files && ret=0
> +      fi
> +    ;;
>    esac
>  

There are two additional problems in this code, besides the one you are
fixing:

1. The second-word-or-later should complete commit ranges or revspecs
too (as in 'git log origin/master origin/foobar').

2. Using __git_cached_files is wrong since it wouldn't complete files
that are in HEAD but not in the index (i.e., any uncommitted 'git rm').

These are preexisting issues: they weren't caused by your patch and
can be fixed independently of it.

Cheers,

Daniel

P.S. There is an asymmetry between __git_tree_files
/ __git_cached_files: the former does 'ls-tree' one directory-tree-level
at a time, the latter gets the 'ls -R' output and lets _multi_parts
handle the one-directory-tree-level-at-a-time part.  I'm not sure
which approach is better.

>    return ret