Improved completion for git commit objects (__git_commit_objects)

Improved completion for git commit objects (__git_commit_objects)

[Daniel Hahler]

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

Hi,

I am trying to improve completion for Git commit objects.

__git_commit_objects is defined as follows (in Completion/Unix/Command/_git):

    __git_commit_objects () {
      _guard '[[:xdigit:]](#c,40)' 'commit object name'
    }

I think it would be great if it actually (optionally?) could provide
completion for commit objects, maybe only on second invocation / as fallback.

I am not sure what to use for completion here, but having completion of the
(shortened) hashes, with more information (subject of the commit) in the
description would be nice!
E.g., using the output from "git --no-pager log --oneline -20 --format='%h%n%s'"


I've experimented a bit with this:

First, I've also added __git_commits to _git-commit where relevant:

diff --git i/Completion/Unix/Command/_git w/Completion/Unix/Command/_git
index 9552780..0bfa057 100644
--- i/Completion/Unix/Command/_git
+++ w/Completion/Unix/Command/_git
@@ -647,8 +647,8 @@ _git-commit () {
   # TODO: --interactive isn't explicitly listed in the documentation.
   _arguments -w -S -s \
     '(-a --all --interactive -o --only -i --include *)'{-a,--all}'[stage all modified and deleted paths]' \
-    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:' \
-    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:' \
+    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
+    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
     $reset_author_opt \
     '(        --porcelain --dry-run)--short[output dry run in short format]' \
     '(--short             --dry-run)--porcelain[output dry run in porcelain-ready format]' \


And then this (very much WIP):

@@ -5631,7 +5631,23 @@ __git_heads () {
 
 (( $+functions[__git_commit_objects] )) ||
 __git_commit_objects () {
+  local gitdir expl start
+  declare -A commits
+
+  # local -a logs
+  # logs=(${(f)"$(git --no-pager log --oneline -20)"})
+  # __git_command_successful $pipestatus || return 1
+  #
+  # for l in $logs; do
+  #   commits[${l%% *}]=${l#* }
+  # done
+
   _guard '[[:xdigit:]](#c,40)' 'commit object name'
+
+  : ${(AA)commits::=${(f)"$(_call_program commits git --no-pager log --oneline -2 --format='%h%n%s')"}}
+  __git_command_successful $pipestatus || return 1
+
+  _wanted commits expl 'commit object name' compadd "$@" -k - commits && ret=0
 }

I do not fully understand how "_guard" is meant to be used - I need to comment/remove it.
Should this get chained, e.g. with "_wanted", instead?
 

As for using the commit objects in the completion, I would not like to have then for
"git checkout <tab>" by default, but with "git commit --fixup=".
With "git checkout", any partial matching commit objects should be completed though,
and the list would be useful to have on second "<tab>".

What do you think?

Can the completion be improved in this regard?


Thanks,
Daniel.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 173 bytes --]

Re: Improved completion for git commit objects (__git_commit_objects)

[Daniel Shahaf]

Daniel Hahler wrote on Wed, Mar 04, 2015 at 00:46:42 +0100:
> First, I've also added __git_commits to _git-commit where relevant:
> 
> diff --git i/Completion/Unix/Command/_git w/Completion/Unix/Command/_git
> index 9552780..0bfa057 100644
> --- i/Completion/Unix/Command/_git
> +++ w/Completion/Unix/Command/_git
> @@ -647,8 +647,8 @@ _git-commit () {
>    # TODO: --interactive isn't explicitly listed in the documentation.
>    _arguments -w -S -s \
>      '(-a --all --interactive -o --only -i --include *)'{-a,--all}'[stage all modified and deleted paths]' \
> -    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:' \
> -    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:' \
> +    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
> +    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
>      $reset_author_opt \
>      '(        --porcelain --dry-run)--short[output dry run in short format]' \
>      '(--short             --dry-run)--porcelain[output dry run in porcelain-ready format]' \
> 

Looks like we could apply this part immediately, it isn't WIP like the
rest of your change.

> +  : ${(AA)commits::=${(f)"$(_call_program commits git --no-pager log --oneline -2 --format='%h%n%s')"}}

You can remove '--oneline': it has no effect since you use '--format' later.

>    _guard '[[:xdigit:]](#c,40)' 'commit object name'
> +
> +  : ${(AA)commits::=${(f)"$(_call_program commits git --no-pager log --oneline -2 --format='%h%n%s')"}}
> +  __git_command_successful $pipestatus || return 1
> +
> +  _wanted commits expl 'commit object name' compadd "$@" -k - commits && ret=0
>  }
> 
> I do not fully understand how "_guard" is meant to be used - I need to comment/remove it.
> Should this get chained, e.g. with "_wanted", instead?

What's the desired behaviour?  Suppose that ${(k)commits} is [0123abc,
4567def] and the user types '--fixup=fedbca<TAB>'.  In this case, you'd
still want the second argument to _guard to be displayed, right?

In this case, I think you should retain _guard in the "the 'commits' tag
is wanted, but none of ${(k)commits} matches the input so far" codepath.
Makes sense?

> As for using the commit objects in the completion, I would not like to have then for
> "git checkout <tab>" by default, but with "git commit --fixup=".
> With "git checkout", any partial matching commit objects should be completed though,
> and the list would be useful to have on second "<tab>".
> 
> What do you think?

Haven't tried the patch, but I like the idea of offering recent commits
as completions for --fixup.  I use --fixup regularly, and that's exactly
the completion offers I'd want.

Cheers,

Daniel

Re: Improved completion for git commit objects (__git_commit_objects)

[Daniel Shahaf]

Daniel Shahaf wrote on Wed, Mar 04, 2015 at 02:40:00 +0000:
> Daniel Hahler wrote on Wed, Mar 04, 2015 at 00:46:42 +0100:
> >    _guard '[[:xdigit:]](#c,40)' 'commit object name'
> > +
> > +  : ${(AA)commits::=${(f)"$(_call_program commits git --no-pager log --oneline -2 --format='%h%n%s')"}}
> > +  __git_command_successful $pipestatus || return 1
> > +
> > +  _wanted commits expl 'commit object name' compadd "$@" -k - commits && ret=0
> >  }
> > 
> > I do not fully understand how "_guard" is meant to be used - I need to comment/remove it.
> > Should this get chained, e.g. with "_wanted", instead?
> 
> What's the desired behaviour?  Suppose that ${(k)commits} is [0123abc,
> 4567def] and the user types '--fixup=fedbca<TAB>'.  In this case, you'd
> still want the second argument to _guard to be displayed, right?
> 
> In this case, I think you should retain _guard in the "the 'commits' tag
> is wanted, but none of ${(k)commits} matches the input so far" codepath.
> Makes sense?

Never mind that — I think you can just remove the _guard call.  The
function of _guard is to display a message when the current word matches
a pattern (sort of conditionalizing the second colon-separated part of
an _arguments option definition); _wanted will print that message
instead of _guard.

That confused me too the first time I encountered that function.
Perhaps a clarification:

diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 5890f17..89cd051 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -4278,12 +4278,12 @@ option `tt(-)tt(-help)'.
 )
 findex(_guard)
 item(tt(_guard) [ var(options) ] var(pattern descr))(
-This function is intended to be used in the var(action) for
-the specifications passed to tt(_arguments) and similar functions.  It
-returns immediately with a non-zero return status if
-the string to be completed does not match the var(pattern).  If the
-pattern matches, the var(descr) is displayed; the function then returns
-status zero if the word to complete is not empty, non-zero otherwise.
+This function displays var(descr) if var(pattern) matches the string to
+be completed.  It is intended to be used in the var(action) for the
+specifications passed to tt(_arguments) and similar functions.
+
+The return status is zero if the message was displayed and the word to
+complete is not empty, and non-zero otherwise.
 
 The var(pattern) may be preceded by any of the options understood by
 tt(compadd) that are passed down from tt(_description), namely tt(-M),

Daniel

Re: Improved completion for git commit objects (__git_commit_objects)

[Daniel Shahaf]

[ Re-adding the list to CC ]

Good morning Daniel,

Daniel Hahler wrote on Thu, Mar 05, 2015 at 01:29:59 +0100:
> On 04.03.2015 03:40, Daniel Shahaf wrote:
> > Daniel Hahler wrote on Wed, Mar 04, 2015 at 00:46:42 +0100:
> >> +  : ${(AA)commits::=${(f)"$(_call_program commits git --no-pager log --oneline -2 --format='%h%n%s')"}}
> > 
> > You can remove '--oneline': it has no effect since you use '--format' later.
> 
> Ok.
> 
> It uses "-2" here (for testing purposes), but should be wider in general.
> Should this be configurable?  What would be a good default?
> 
> I think it depends on if it's displayed on the first "<tab>", or
> later.  But a basic default is probably 20?!

Yes, 20 sounds like a good default.

Re configurable, I had the same thought.  Configuring the history length
by a style would be straightforward, but I'd suggest waiting for
somebody to step up and say he'd use that knob before adding it.
(Knobs have a maintainability cost.)

I'm not sure how to go about making the second <TAB> behave differently.
Perhaps others could weigh in about that.

Cheers,

Daniel

Re: Improved completion for git commit objects (__git_commit_objects)

[Daniel Shahaf]

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

[ tl;dr: applied patch, attaching further patches for _git and _describe ]

Daniel Hahler wrote on Wed, Mar 04, 2015 at 00:46:42 +0100:
> Hi,
> 
> I am trying to improve completion for Git commit objects.
> 
> __git_commit_objects is defined as follows (in Completion/Unix/Command/_git):
> 
>     __git_commit_objects () {
>       _guard '[[:xdigit:]](#c,40)' 'commit object name'
>     }
> 
> I think it would be great if it actually (optionally?) could provide
> completion for commit objects, maybe only on second invocation / as fallback.

Good morning Daniel,

Hope you don't mind, but I went ahead and tweaked your function a bit:

1. I removed the _guard call which generated a spurious message and
prevented <TAB> from inserting a unique completion to the command line
in my configuration.

2. I used _describe instead of 'compadd -k'.

I've committed the patch with these changes.  Thanks for the patch!

I also made two follow-ups.  The first completes only tags/branches that
refer to one of the most recent N commits, rather than all tags and
branches — see attached.  WDYT?

The second follow-up sorts the hashes offered (as completions)
chronologically, rather than alphabetically, by extending _describe.
I'd appreciate some review of this one, to make sure I didn't miss
anything.

With all these patches applied, I get (in the zsh repo):

% git ci --fixup=4<TAB>
> commit object name
4bc554b  -- 34636: replace broken isprint() on Mac OS X
42e5f59  -- 34640: clarify documentation for _guard function
4edcacb  -- users/19934: document %D{...} for WATCHFMT
49776e8  -- 34588: Complete 'usermod -a'

Thanks again!

Daniel



[-- Attachment #2: 0002-git-completion-only-offer-recent-commits-tags-heads-.patch --]
[-- Type: text/x-patch, Size: 2663 bytes --]

>From c786fb8b28c5ea68cd7a87c3e704f0c137d4e7d8 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sat, 7 Mar 2015 02:05:44 +0000
Subject: [PATCH 2/3] git completion: only offer recent commits' tags/heads for
 --fixup

---
 Completion/Unix/Command/_git | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 58d6422..aa11247 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -647,8 +647,8 @@ _git-commit () {
   # TODO: --interactive isn't explicitly listed in the documentation.
   _arguments -w -S -s \
     '(-a --all --interactive -o --only -i --include *)'{-a,--all}'[stage all modified and deleted paths]' \
-    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
-    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_commits' \
+    '--fixup=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_recent_commits' \
+    '--squash=[construct a commit message for use with rebase --autosquash]:commit to be amended:__git_recent_commits' \
     $reset_author_opt \
     '(        --porcelain --dry-run)--short[output dry run in short format]' \
     '(--short             --dry-run)--porcelain[output dry run in porcelain-ready format]' \
@@ -5640,6 +5640,38 @@ __git_commit_objects () {
   _describe -t commits 'commit object name' commits
 }
 
+(( $+functions[__git_recent_commits] )) ||
+__git_recent_commits () {
+  local gitdir expl start
+  declare -a descr tags heads commits
+  local i j k
+
+  # Careful: most %d will expand to the empty string.  Quote properly!
+  : "${(A)commits::=${(@f)"$(_call_program commits git --no-pager log -20 --format='%h%n%d%n%s')"}}"
+  __git_command_successful $pipestatus || return 1
+
+  for i j k in "$commits[@]" ; do
+    descr+=($i:$k)
+    j=${${j# \(}%\)} # strip leading ' (' and trailing ')'
+    for j in ${(s:, :)j}; do
+      if [[ $j == 'tag: '* ]] ; then
+        tags+=( ${j#tag: } )
+      else
+        heads+=( $j )
+      fi
+    done
+  done
+
+  ret=1
+  # Resetting expl to avoid it 'leaking' from one line to the next.
+  expl=()
+  _wanted commit-tags expl 'commit tag' compadd "$@" -a - tags && ret=0
+  expl=()
+  _wanted heads expl 'head' compadd "$@" -a - heads && ret=0
+  expl=()
+  _describe -t commits 'commit object name' descr && ret=0
+}
+
 (( $+functions[__git_blob_objects] )) ||
 __git_blob_objects () {
   _guard '[[:xdigit:]](#c,40)' 'blob object name'
-- 
1.9.1


[-- Attachment #3: 0003-Add-1-2-J-V-x-to-_describe-use-them-to-sort-git-fixu.patch --]
[-- Type: text/x-patch, Size: 3375 bytes --]

>From 99c9b5507a8725132ee9332bc889b3b3ff03a94d Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sat, 7 Mar 2015 03:05:16 +0000
Subject: [PATCH 3/3] Add -1 -2 -J -V -x to _describe, use them to sort 'git
 --fixup' hash completions

---
 Completion/Base/Utility/_describe | 35 ++++++++++++++++++++---------------
 Completion/Unix/Command/_git      |  2 +-
 Doc/Zsh/compsys.yo                |  5 ++++-
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/Completion/Base/Utility/_describe b/Completion/Base/Utility/_describe
index 1a9f52f..90dcac5 100644
--- a/Completion/Base/Utility/_describe
+++ b/Completion/Base/Utility/_describe
@@ -6,23 +6,28 @@ local _opt _expl _tmpm _tmpd _mlen _noprefix
 local _type=values _descr _ret=1 _showd _nm _hide _args _grp _sep
 local csl="$compstate[list]" csl2
 local _oargv _argv _new _strs _mats _opts _i _try=0
+local OPTIND OPTARG
+local -a _jvx12
 
 # Get the option.
 
-if [[ "$1" = -o ]]; then
-  _type=options
-  shift
-elif [[ "$1" = -O ]]; then
-  _type=options
-  _noprefix=1
-  shift
-elif [[ "$1" = -t ]]; then
-  _type="$2"
-  shift 2
-elif [[ "$1" = -t* ]]; then
-  _type="${1[3,-1]}"
-  shift
-fi
+while getopts "oOt:12JVx" _opt; do
+  case $_opt in
+    (o)
+      _type=options;;
+    (o)
+      _type=options
+      _noprefix=1
+      ;;
+    (t)
+      _type="$OPTARG"
+      ;;
+    (1|2|J|V|x)
+      _jvx12+=(-$_opt)
+  esac
+done
+shift $(( OPTIND - 1 ))
+unset _opt
 
 [[ "$_type$_noprefix" = options && ! -prefix [-+]* ]] && \
     zstyle -T ":completion:${curcontext}:options" prefix-needed &&
@@ -53,7 +58,7 @@ fi
 
 _tags "$_type"
 while _tags; do
-  while _next_label "$_type" _expl "$_descr"; do
+  while _next_label $_jvx12 "$_type" _expl "$_descr"; do
 
     if (( $#_grp )); then
   
diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index aa11247..ac5f70b 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5669,7 +5669,7 @@ __git_recent_commits () {
   expl=()
   _wanted heads expl 'head' compadd "$@" -a - heads && ret=0
   expl=()
-  _describe -t commits 'commit object name' descr && ret=0
+  _describe -2V -t commits 'commit object name' descr && ret=0
 }
 
 (( $+functions[__git_blob_objects] )) ||
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 89cd051..47e96af 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -4140,7 +4140,7 @@ tt(compadd) when generating matches from the style value, or to
 the functions for the fields if they are called.
 )
 findex(_describe)
-item(tt(_describe) [ tt(-oO) | tt(-t) var(tag) ] var(descr) var(name1) [ var(name2) ] var(opts) ... tt(-)tt(-) ...)(
+item(tt(_describe) [-12JVx] [ tt(-oO) | tt(-t) var(tag) ] var(descr) var(name1) [ var(name2) ] var(opts) ... tt(-)tt(-) ...)(
 This function associates completions with descriptions.
 Multiple groups separated by tt(-)tt(-) can be supplied, potentially with
 different completion options var(opts).
@@ -4171,6 +4171,9 @@ tt(prefix-needed) style.
 With the tt(-t) option a var(tag) can be specified.  The default is
 `tt(values)' or, if the tt(-o) option is given, `tt(options)'.
 
+The options tt(-1), tt(-2), tt(-J), tt(-V), tt(-x) are passed to
+tt(_next_labels).
+
 If selected by the tt(list-grouped) style, strings with the same
 description will appear together in the list.
 
-- 
1.9.1

Re: Improved completion for git commit objects (__git_commit_objects)

[Mikael Magnusson]

On Sat, Mar 7, 2015 at 5:01 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> [ tl;dr: applied patch, attaching further patches for _git and _describe ]
>
> Daniel Hahler wrote on Wed, Mar 04, 2015 at 00:46:42 +0100:
>> Hi,
>>
>> I am trying to improve completion for Git commit objects.
>>
>> __git_commit_objects is defined as follows (in Completion/Unix/Command/_git):
>>
>>     __git_commit_objects () {
>>       _guard '[[:xdigit:]](#c,40)' 'commit object name'
>>     }
>>
>> I think it would be great if it actually (optionally?) could provide
>> completion for commit objects, maybe only on second invocation / as fallback.
>
> Good morning Daniel,
>
> Hope you don't mind, but I went ahead and tweaked your function a bit:
>
> 1. I removed the _guard call which generated a spurious message and
> prevented <TAB> from inserting a unique completion to the command line
> in my configuration.
>
> 2. I used _describe instead of 'compadd -k'.
>
> I've committed the patch with these changes.  Thanks for the patch!
>
> I also made two follow-ups.  The first completes only tags/branches that
> refer to one of the most recent N commits, rather than all tags and
> branches — see attached.  WDYT?
>
> The second follow-up sorts the hashes offered (as completions)
> chronologically, rather than alphabetically, by extending _describe.
> I'd appreciate some review of this one, to make sure I didn't miss
> anything.
>
> With all these patches applied, I get (in the zsh repo):
>
> % git ci --fixup=4<TAB>
>> commit object name
> 4bc554b  -- 34636: replace broken isprint() on Mac OS X
> 42e5f59  -- 34640: clarify documentation for _guard function
> 4edcacb  -- users/19934: document %D{...} for WATCHFMT
> 49776e8  -- 34588: Complete 'usermod -a'

When I tried adding sha1 completion a few years ago, the big
showstopper was that any completion of a commit-ish or any -ish for
that matter, would take several minutes in the linux kernel, since it
has millions of blobs. Is this faster?

-- 
Mikael Magnusson

Re: Improved completion for git commit objects (__git_commit_objects)

[Daniel Shahaf]

Mikael Magnusson wrote on Sat, Mar 07, 2015 at 13:59:39 +0100:
> On Sat, Mar 7, 2015 at 5:01 AM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > % git ci --fixup=4<TAB>
> >> commit object name
> > 4bc554b  -- 34636: replace broken isprint() on Mac OS X
> > 42e5f59  -- 34640: clarify documentation for _guard function
> > 4edcacb  -- users/19934: document %D{...} for WATCHFMT
> > 49776e8  -- 34588: Complete 'usermod -a'
> 
> When I tried adding sha1 completion a few years ago, the big
> showstopper was that any completion of a commit-ish or any -ish for
> that matter, would take several minutes in the linux kernel, since it
> has millions of blobs. Is this faster?

It only completes hashes from 'git log -20 --pretty=%h', which should do
O(1) work regardless of repo size.

FWIW, on my datasets the completion of "--fix<TAB>" to "--fixup=" takes
longer than generating and displaying the completion suggestions upon
hitting <TAB> after the equal sign.  Even with cold caches on a 250MB
repo it takes just a few seconds.

Daniel