Re: PATCH: 3.0.8: git completion update for cherry-pick

[Daniel Shahaf <d.s@daniel.shahaf.name>]

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

Mateusz Karbowy wrote on Sat, Aug 29, 2015 at 17:53:57 +0100:
> I've fixed the no-quotes issue with $@ and now I'm passing both
> compadd and git commit options using one -O parameter (the array names
> are separated with colon, as you suggested).

Looks good.

I've made a few changes (see attached series):

1. Some style tweaks

2. Avoid printing wrong [HEAD~$n] descriptions in __git_recent_commits

3. Extra safety check in case somebody passed just the traditional '-O
expl' with no colon

I'll commit the patch with those changes (as soon as I get an X-Seq
number for this email).  Many thanks!

Daniel


[-- Attachment #2: 0001-minor-Comment-and-style-fixes.-No-functional-change.patch --]
[-- Type: text/x-patch, Size: 2791 bytes --]

>From 496068c764074a5816b09d456b34e0e6096d988a Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sun, 30 Aug 2015 11:26:39 +0000
Subject: [PATCH 1/3] minor: Comment and style fixes.  No functional change.

---
 Completion/Unix/Command/_git | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 18f9e7c..69d7719 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5568,7 +5568,10 @@ __git_commit_objects_prefer_recent () {
 __git_commits () {
   local -a argument_array_names
   zparseopts -D -E O:=argument_array_names
-  (( $#argument_array_names )) && argument_array_names=( "${(@s/:/)argument_array_names[2]}" ) && set -- "${(@P)argument_array_names[1]}"
+  # Turn (-O foo:bar) to (foo bar)
+  (( $#argument_array_names )) && argument_array_names=( "${(@s/:/)argument_array_names[2]}" )
+  set -- "${(@P)argument_array_names[1]}"
+  local commit_opts__argument_name=$argument_array_names[2]
 
   # TODO: deal with things that __git_heads and __git_tags has in common (i.e.,
   # if both exists, they need to be completed to heads/x and tags/x.
@@ -5579,7 +5582,7 @@ __git_commits () {
   _alternative \
     "heads::__git_heads $sopts" \
     "commit-tags::__git_commit_tags $sopts" \
-    'commit-objects:: __git_commit_objects_prefer_recent -O expl:$argument_array_names[2]'
+    'commit-objects:: __git_commit_objects_prefer_recent -O expl:$commit_opts__argument_name'
 }
 
 (( $+functions[__git_heads] )) ||
@@ -5639,6 +5642,7 @@ __git_recent_commits () {
   integer distance_from_head
 
   zparseopts -D -E O:=argument_array_names
+  # Turn (-O foo:bar) to (foo bar)
   (( $#argument_array_names )) && argument_array_names=( "${(@s/:/)argument_array_names[2]}" )
   (( $#argument_array_names > 1 )) && commit_opts=( "${(@P)argument_array_names[2]}" )
 
@@ -5729,9 +5733,13 @@ __git_commits2 () {
 __git_commit_ranges () {
   local -a argument_array_names
   zparseopts -D -E O:=argument_array_names
-  (( $#argument_array_names )) && argument_array_names=( "${(@s/:/)argument_array_names[2]}" ) && set -- "${(@P)argument_array_names[1]}"
+  # Turn (-O foo:bar) to (foo bar)
+  (( $#argument_array_names )) && argument_array_names=( "${(@s/:/)argument_array_names[2]}" )
+  set -- "${(@P)argument_array_names[1]}"
+  local commit_opts__argument_name=$argument_array_names[2]
 
-  local -a expl suf
+  local -a suf
+  local -a expl
   if compset -P '*..(.|)'; then
     expl=( $* )
   else
@@ -5739,7 +5747,7 @@ __git_commit_ranges () {
     expl=( $* $suf )
   fi
 
-  __git_commits -O expl:$argument_array_names[2]
+  __git_commits -O expl:$commit_opts__argument_name
 }
 
 (( $+functions[__git_commit_ranges2] )) ||
-- 
2.1.4


[-- Attachment #3: 0002-Don-t-show-wrong-labels.patch --]
[-- Type: text/x-patch, Size: 2124 bytes --]

>From a7936c3d11dae2cecfb185cba2db5310c9e9c2b4 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sun, 30 Aug 2015 11:41:32 +0000
Subject: [PATCH 2/3] Don't show wrong labels

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

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 69d7719..dd2d771 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5640,6 +5640,7 @@ __git_recent_commits () {
   declare -a descr tags heads commits argument_array_names commit_opts
   local i j k ret
   integer distance_from_head
+  local label
 
   zparseopts -D -E O:=argument_array_names
   # Turn (-O foo:bar) to (foo bar)
@@ -5654,17 +5655,25 @@ __git_recent_commits () {
   for i j k in "$commits[@]" ; do
     # Note: the after-the-colon part must be unique across the entire array;
     # see workers/34768
-    if (( distance_from_head == 0 )); then
-      descr+=($i:"[HEAD]    $k")
+    if (( $#commit_opts )); then
+      # $commit_opts is set, so the commits we receive might not be in order,
+      # or might not be ancestors of HEAD.  However, we must make the
+      # description unique (due to workers/34768), which we do by including the
+      # hash.  Git always prints enough hash digits to make the output unique.)
+      label="[$i]"
+    elif (( distance_from_head == 0 )); then
+      label="[HEAD]   "
     elif (( distance_from_head == 1 )); then
-      descr+=($i:"[HEAD^]   $k")
+      label="[HEAD^]  "
     elif (( distance_from_head == 2 )); then
-      descr+=($i:"[HEAD^^]  $k")
+      label="[HEAD^^] "
     elif (( distance_from_head < 10 )); then
-      descr+=($i:"[HEAD~$distance_from_head]  $k")
+      label="[HEAD~$distance_from_head] "
     else
-      descr+=($i:"[HEAD~$distance_from_head] $k")
+      label="[HEAD~$distance_from_head]"
     fi
+    # label is now 9 bytes, so the descriptions ($k) will be aligned.
+    descr+=($i:"${label} $k")
     (( ++distance_from_head ))
 
     j=${${j# \(}%\)} # strip leading ' (' and trailing ')'
-- 
2.1.4


[-- Attachment #4: 0003-belt-and-braces-for-the-O-expl-empty-string-case.patch --]
[-- Type: text/x-patch, Size: 1105 bytes --]

>From 7fe49eca0d39d3cc566fb6480b87fb16fc1caf13 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sun, 30 Aug 2015 13:01:29 +0000
Subject: [PATCH 3/3] belt and braces for the -O 'expl:<empty string>' case

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

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index dd2d771..2c79ed0 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5645,7 +5645,8 @@ __git_recent_commits () {
   zparseopts -D -E O:=argument_array_names
   # Turn (-O foo:bar) to (foo bar)
   (( $#argument_array_names )) && argument_array_names=( "${(@s/:/)argument_array_names[2]}" )
-  (( $#argument_array_names > 1 )) && commit_opts=( "${(@P)argument_array_names[2]}" )
+  (( $#argument_array_names > 1 )) && && ${(P)+argument_array_names[2]} &&
+    commit_opts=( "${(@P)argument_array_names[2]}" )
 
   # Careful: most %d will expand to the empty string.  Quote properly!
   # NOTE: we could use %D directly, but it's not available in git 1.9.1 at least.
-- 
2.1.4