git log -1 --...

git log -1 --...

[Peter Stephenson]

Minor completion oddity: "git log -1 -<TAB>" doesn't complete options.
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.

pws

Re: git log -1 --...

[Oliver Kiddle]

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]' \
-    '*:: :->commit-range-or-file' && ret=0
+    '1: :->first-commit-ranges-or-files' \
+    '*: :->commit-ranges-or-files' && ret=0
 
   case $state in
-    (commit-range-or-file)
-      case $CURRENT in
-        (1)
-          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
-          ;;
-        (*)
-          # 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
+    (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
 
   return ret

Re: git log -1 --...

[Daniel Shahaf]

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

Re: git log -1 --...

[Daniel Shahaf]

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

Daniel Shahaf wrote on Thu, Aug 13, 2015 at 23:13:50 +0000:
> 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').
> 

Patches for these issues two attached.  They are to be applied on top of
Oliver's patch from this thread.  (They conflict textually but should be
independent semantically.)

> These are preexisting issues: they weren't caused by your patch and
> can be fixed independently of it.
...
> 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

[-- Attachment #2: 0002-_git-log-complete-git-rm-d-files-too.patch --]
[-- Type: text/x-patch, Size: 1182 bytes --]

>From 3af6437ee181fbcb4d1d764d6a7b2e5e098d1dd8 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Tue, 18 Aug 2015 04:19:27 +0000
Subject: [PATCH 2/3] _git-log: complete 'git rm'd files

---
 Completion/Unix/Command/_git | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 25af4fc..90969bd 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -1093,11 +1093,11 @@ _git-log () {
   case $state in
     (first-commit-ranges-or-files)
       if [[ -n ${opt_args[(I)--]} ]]; then
-	__git_cached_files && ret=0
+	__git_tree_files ${PREFIX:-.} HEAD && ret=0
       else
 	_alternative \
 	  'commit-ranges::__git_commit_ranges' \
-	  'cached-files::__git_cached_files' && ret=0
+	  'cached-files::__git_tree_files ${PREFIX:-.} HEAD' && ret=0
       fi
     ;;
     (commit-ranges-or-files)
@@ -1109,7 +1109,7 @@ _git-log () {
       elif __git_is_committish $line[1]; then
 	__git_tree_files ${PREFIX:-.} $line[1] && ret=0
       else
-	__git_cached_files && ret=0
+	__git_tree_files ${PREFIX:-.} HEAD && ret=0
       fi
     ;;
   esac
-- 
2.1.4


[-- Attachment #3: 0003-_git-log-Complete-multiple-revspecs.patch --]
[-- Type: text/x-patch, Size: 912 bytes --]

>From ff8b9df282efbec19319719f67b99cf7d259087a Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Tue, 18 Aug 2015 04:26:48 +0000
Subject: [PATCH 3/3] _git-log: Complete multiple revspecs

e.g., git log origin/master origin/foo origin/bar
---
 Completion/Unix/Command/_git | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 90969bd..6922393 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -1101,6 +1101,11 @@ _git-log () {
       fi
     ;;
     (commit-ranges-or-files)
+      # Multiple revspecs are permitted.
+      if [[ -z ${opt_args[(I)--]} ]]; then
+        __git_commit_ranges "$@" && ret=0
+      fi
+
       # TODO: Write a wrapper function that checks whether we have a
       # committish range or comittish and calls __git_tree_files
       # appropriately.
-- 
2.1.4