Re: git log -1 --...

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

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