[PATCH/WIP] completion: complete git diff with only changed files.

[PATCH/WIP] completion: complete git diff with only changed files.

[Paul Ebermann]

The bash completion script for git diff completes either for
references or for file names. If there was a -- given, it uses
the default bash filename completion.

The completion for hg diff completes only changed file names
here - this is quite useful if we want to see changes in only
some file (or directory). (This was mentioned on stackoverflow,
which gave me the idea: http://stackoverflow.com/q/6034472/600500 )

I ported this idea to git. Now
   git diff -- <tab>
will complete any changed files. It also works for the other ways
of calling git diff, except the .. and ... notations (as I'm
not sure how to do this).

Signed-off-by: Paŭlo Ebermann <Paul-Ebermann@gmx.de>
---

Hello,
this is my first contribution to git at all, and I only joined the
mailing list some hours ago (right before starting to code this), so I hope
I'm not making any mistakes here. (I read the SubmittingPatches document,
though.)


I only made this work after the --, while the usual file completion already
seems to work if there is no --. I'm not really sure what is wanted here.

I'm checking the non-option arguments on being commits (or tags), and pass
only the matching ones to the nested `git diff` call.
It might be easier to ommit this check and pass everything that does not
start with a `-`. Then it would also easily work for the .. and ... syntax,
I think.
Opinions?

The same completion function or a variation might also be useful for other
commands like git add (completing changed or new files) or git rm
(completing already removed files). Input is welcome here.

This patch is based on the current master branch, I hope this is the right
one.


 contrib/completion/git-completion.bash |   72 +++++++++++++++++++++++++++++++-
 1 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bb8d7d0..c529bdf 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -663,6 +663,69 @@ __git_compute_merge_strategies ()
 	: ${__git_merge_strategies:=$(__git_list_merge_strategies)}
 }
 
+
+# Completion for the file argument for git diff.
+# It completes only files actually changed. This might be useful
+# as completion for other commands as well.
+#
+# The idea comes from the bash completion for Mercurial (hg),
+# which does something similar (but more simple, only difference of
+# working directory to HEAD and/or index, if I understand right).
+# It (the idea) was brought to us by the question
+#      http://stackoverflow.com/q/6034472/600500
+#  from "olt".
+__git_complete_changed_files()
+{
+  #
+  # We use "git diff --name-only --relative" to generate the list,
+  # but this needs the same --cached and <commit> arguments as the
+  # command line being constructed.
+  #
+
+
+    # first grab arguments like --cached and any commit arguments.
+
+    local -a args=()
+    local finish=false
+
+    for (( i=1 ; i < cword ; i++)) do
+    local current_arg=${words[$i]}
+    #  echo checking $current_arg >&2
+       case $current_arg in
+           --cached)
+               args+=( $current_arg )
+               ;;
+           --)
+               # finish parsing arguments, the rest are file names
+               break
+               ;;
+           -*)
+               # other options are ignored
+               ;;
+           *)
+               if git cat-file -e $current_arg 2> /dev/null
+               then
+                   case $( git cat-file -t $current_arg ) in
+                       commit|tag)
+                       # commits and tags are added to the command line.
+                           args+=( $current_arg )
+                           # echo adding $current_arg >&2
+                           ;;
+                       *)
+                   esac
+               fi
+               ;;
+       esac
+    done
+
+    # now we can call `git diff`
+
+    COMPREPLY=( $( compgen \
+        -W "$( git diff --name-only --relative "${args[@]}" -- )" -- $cur ) )
+}
+
+
+
 __git_complete_revlist_file ()
 {
 	local pfx ls ref cur_="$cur"
@@ -1314,10 +1377,14 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
 			--dirstat-by-file= --cumulative
 "
 
+
 _git_diff ()
 {
-	__git_has_doubledash && return
-
+    if __git_has_doubledash
+    then
+        # complete for the file part: only changed files
+        __git_complete_changed_files
+    else
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
@@ -1328,6 +1395,7 @@ _git_diff ()
 		;;
 	esac
 	__git_complete_revlist_file
+    fi
 }
 
 __git_mergetools_common="diffuse ecmerge emerge kdiff3 meld opendiff

Re: [PATCH/WIP] completion: complete git diff with only changed files.

[Junio C Hamano]

Paul Ebermann <Paul-Ebermann@gmx.de> writes:

> I ported this idea to git. Now
>    git diff -- <tab>
> will complete any changed files. It also works for the other ways
> of calling git diff, except the .. and ... notations (as I'm
> not sure how to do this).

Very interesting.

I would be a bit dissapointed if this change makes

	git diff maint master -- builtin/lo<TAB>

not complete for me when builtin/log.c did not change between these two
branches, as running between different maintenance tracks and the master
branch to see how far things have diverged is my first quick sanity check
before deciding where in the history a new patch should be queued.

Spending cycles to keep me waiting while running "diff" before giving me
the control back, and implicitly telling me by not telling me that lo...
is a completion candidate, would surely make me go "Huh? Is it being slow?
Hello? <TAB> <TAAAB> <TAAAAAAAB> ... hmm, did I mistype the directory
name?  B-U-I-L-T-I-N ... that's correct.... what's going on?  ah wait a
minute, we recently applied that stupid change that tells me what I really
want to know by not telling me, which is backwards."

That's already 12 seconds wasted, and worse yet, even after I learn the
quirk of the new behaviour that tells what I want to know by not telling
me, the need to make sure that I didn't mistype the directory name would
not disappear.

I would rather not to be retrained to wire my brain backwards in the first
place.

> +    local -a args=()
> +    local finish=false
> +
> +    for (( i=1 ; i < cword ; i++)) do
> +    local current_arg=${words[$i]}
> +    #  echo checking $current_arg >&2
> +       case $current_arg in
> +           --cached)

case arms align with case and esac in our codebase, i.e.

	case $current_arg in
        --cached)
        	...
                ;;
                ...
	esac

> +           *)
> +               if git cat-file -e $current_arg 2> /dev/null
> +               then
> +                   case $( git cat-file -t $current_arg ) in

I do not see the need for the outer if/then/fi here. Wouldn't this
sufficient?

		case "$(git cat-file -t $current_arg 2>/dev/null)" in
		commit|tag)
                	...

If you are interested in dealing with ../... notation, you could instead
use "git rev-parse --revs-only --no-flags", e.g.

	$ git rev-parse --revs-only --no-flags maint..master
        b602ed7dea968d72c5b3f61ca016de7f285d80ef
	^ea1ab4b280ed3b041da53e161e32e38930569f3e

        $ git rev-parse --revs-only --no-flags jch...pu
        11b715c624d3766546a52cc333bc2ea2e426f631
        14f92e20522bae26faa841374bbbe6c0d08770de
        ^14f92e20522bae26faa841374bbbe6c0d08770de

But I tend to think this change itself is not such a great idea to begin
with, so....

Re: [PATCH/WIP] completion: complete git diff with only changed files.

[Paul Ebermann]

Junio C Hamano skribis:
> Paul Ebermann <Paul-Ebermann@gmx.de> writes:
> 
>> I ported this idea to git. Now
>>    git diff -- <tab>
>> will complete any changed files. It also works for the other ways
>> of calling git diff, except the .. and ... notations (as I'm
>> not sure how to do this).
> 
> Very interesting.
> 
> I would be a bit dissapointed if this change makes
> 
> 	git diff maint master -- builtin/lo<TAB>
> 
> not complete for me when builtin/log.c did not change between these two
> branches, as running between different maintenance tracks and the master
> branch to see how far things have diverged is my first quick sanity check
> before deciding where in the history a new patch should be queued.

In fact, it does complete. (Even if there is no maint branch like in my
clone.) There seems to be a fallback to filename completion if the completion
list is empty.  It takes quite longer than before, though.
And it does not give a list of all files if there are changed files starting
with the prefix typed.

> Spending cycles to keep me waiting while running "diff" before giving me
> the control back, and implicitly telling me by not telling me that lo...
> is a completion candidate, would surely make me go "Huh? Is it being slow?
> Hello? <TAB> <TAAAB> <TAAAAAAAB> ... hmm, did I mistype the directory
> name?  B-U-I-L-T-I-N ... that's correct.... what's going on?  ah wait a
> minute, we recently applied that stupid change that tells me what I really
> want to know by not telling me, which is backwards."
>
> That's already 12 seconds wasted, and worse yet, even after I learn the
> quirk of the new behaviour that tells what I want to know by not telling
> me, the need to make sure that I didn't mistype the directory name would
> not disappear.
>
> I would rather not to be retrained to wire my brain backwards in the first
> place.

Okay, this seems to depend on the usage pattern.

I'm normally using (for differences to head) git status  first, and then
have a look at the files I really want to see. Then completion of only the
changed files seems useful. 

I must confess that I seldomly use
    git diff <commit> <commit> -- <path>
at all, while it seems to be an important tool for maintainers of projects
with lots of contributors (like you).

What about having this completion only apply for the no-commit-given case
(i.e. `git diff -- <path>`), and using the normal file-based completion
in the case of a given <commit>?
(But shouldn't we complete for files existent in one of the comparison ends
 instead of in the working tree?)

I also thought about somehow caching the results to make it faster, but the
problem with this is that this will change after each commit, and also
between the commits with every edit to a file.

>> +    local -a args=()
>> +    local finish=false
>> +
>> +    for (( i=1 ; i < cword ; i++)) do
>> +    local current_arg=${words[$i]}
>> +    #  echo checking $current_arg >&2
>> +       case $current_arg in
>> +           --cached)
> 
> case arms align with case and esac in our codebase, i.e.
> 
>         case $current_arg in
>         --cached)
>         	...
>                 ;;
>                 ...
>         esac

Okay. (I simply used what my Emacs auto-indented.)
I will change this if I submit another patch.

>> +           *)
>> +               if git cat-file -e $current_arg 2> /dev/null
>> +               then
>> +                   case $( git cat-file -t $current_arg ) in
> 
> I do not see the need for the outer if/then/fi here. Wouldn't this
> sufficient?
> 
> 		case "$(git cat-file -t $current_arg 2>/dev/null)" in
> 		commit|tag)
>                 	...

Seems like it.
Looks like I was reading the documentation of -e, which said
"Suppress all output; instead exit with zero status if <object>
exists and is a valid object.", and thus I thought I could avoid
using the redirection at all - I then added it when it showed
that the redirect still was necessary.

Yes, your version works, too.

> If you are interested in dealing with ../... notation, you could instead
> use "git rev-parse --revs-only --no-flags", e.g.
> 
> 	$ git rev-parse --revs-only --no-flags maint..master
>         b602ed7dea968d72c5b3f61ca016de7f285d80ef
> 	^ea1ab4b280ed3b041da53e161e32e38930569f3e
> 
>         $ git rev-parse --revs-only --no-flags jch...pu
>         11b715c624d3766546a52cc333bc2ea2e426f631
>         14f92e20522bae26faa841374bbbe6c0d08770de
>         ^14f92e20522bae26faa841374bbbe6c0d08770de

The reason for using cat-file instead of rev-parse was the ability
to distinguish between commits and blobs (for example). 

	$ git rev-parse --revs-only --no-flags c4d58da4e
	c4d58da4e9050c6330ff145914cc379f0600f703

(This is zlib.c in the current master, and the exit code is still 0.
Using the git 1.7.1 binaries on OpenSUSE, I did not compile the current
master code.)

I'm not sure this is really necessary, as I wrote before:

>> I'm checking the non-option arguments on being commits (or tags), and pass
>> only the matching ones to the nested `git diff` call.
>> It might be easier to ommit this check and pass everything that does not
>> start with a `-`. Then it would also easily work for the .. and ... syntax,
>> I think.
>> Opinions?

Thus I could simply write here

	case $current_arg in
	...
	*)
		args+=( $current_arg )
	esac

instead. If someone supplies something non-commitish, I don't have to care
about it. It would also catch any non-option arguments, but it looks like
there are none of these (apart from the commits).

> But I tend to think this change itself is not such a great idea to begin
> with, so....

Yeah, I understood this.

If this is not accepted, I'll publish it separately for the ones who like it
(not as a patch, but as a separate bash file which you could source after the
main comments.)
Still thanks for your comments.


Paŭlo

Re: [PATCH/WIP] completion: complete git diff with only changed files.

[Junio C Hamano]

Paul Ebermann <Paul-Ebermann@gmx.de> writes:

> I'm normally using (for differences to head) git status first, and then
> have a look at the files I really want to see. Then completion of only
> the changed files seems useful.

By the time completion offers you the choices, you already have spent
enough extra cycles to compute the paths, which is half the cost of
generating the diff itself.

I have this nagging feeling that you are trying to solve a problem that
does not exist.  Perhaps you have too many things going on in your working
tree at once, and if git helped in such a way that your workflow does not
have to touch so many (possibly unrelated) things at once, you do not have
to worry about unconstrained "git diff" output overwhelming you?

Re: [PATCH/WIP] completion: complete git diff with only changed files.

[Paul Ebermann]

Junio C Hamano schrieb:
> Paul Ebermann <Paul-Ebermann@gmx.de> writes:
> 
>> I'm normally using (for differences to head) git status first, and then
>> have a look at the files I really want to see. Then completion of only
>> the changed files seems useful.
> 
> By the time completion offers you the choices, you already have spent
> enough extra cycles to compute the paths, which is half the cost of
> generating the diff itself.
> 
> I have this nagging feeling that you are trying to solve a problem that
> does not exist.

Maybe.

For me, it is not so much about saving CPU cycles (I have enough of
these) but about not seeing things I don't want to see, and helping me
decide what to type. This might be against the Git philosophy, I'm
starting to realize.

>  Perhaps you have too many things going on in your working
> tree at once, and if git helped in such a way that your workflow does not
> have to touch so many (possibly unrelated) things at once, you do not have
> to worry about unconstrained "git diff" output overwhelming you?

If I only want to do "git diff" (without any paths), I obviously don't
need path completion at all.


Here is an example:

Yesterday, I addes a new Java class (193 lines)
  src/de/hub/sam/es/managementclient/ssh/TunnelSocketImpl.java
and at the same time made some changes to
  src/de/hub/sam/es/managementclient/ssh/ConnectionManager.java
to use this new class (adding 29 lines).

I wanted to look only at the changes made to ConnectionManager.java.

(The changes to TunnelSocketImpl.java were obvious: creating the whole
new class, thus I could look at this in my editor if I wanted).

With the usual filename-completion, this goes like

    git diff -- s<tab><tab><tab><tab><tab><tab>s<tab>C<tab>

If I had a broader package tree (like in some other projects),
it takes even more work, as I must remember which package name
starting letters to type between the tabs.

(In some projects I started to choose the package names so that
there never would be two sibling directories starting with the
same letter, to help autocompletion.)

With the new completion scheme, this would be

   git diff -- <tab>C<tab>

It might need doubled time to complete-and-execute, but
my computer is quite faster than I can type (and think),
even if the .git directory is on a NFS.

(You could ask why my shell working directory is not the
 managementclient directory. The reason is that sometimes
 there are files in ./ which get changed, too.)


Paul

Re: [PATCH/WIP] completion: complete git diff with only changed files.

[Junio C Hamano]

Paul Ebermann <Paul-Ebermann@gmx.de> writes:

> For me, it is not so much about saving CPU cycles (I have enough of
> these) but about not seeing things I don't want to see, and helping me
> decide what to type. This might be against the Git philosophy, I'm
> starting to realize.

I would say Git UI philosophy is it is justified to spend CPU cycles in
order to reduce brain cycles (of course it does not justify spending extra
CPU cycles for no gain), but your change cuts both ways. In the use case I
presented, it _wasted_ a dozen or so seconds of my brain cycle before I
get what I wanted to see. In your use case, it will reduce the need to
waste your brain cycle skiping the completion you would not want to see to
get to what you want. So I am not fundamentally opposed to the change, but
the trade-off will largely depend on what your workflow is and what system
you are on.

One thing that I am worried about is the latency before getting the list
of completion. I've heard enough horror stories on a filesystem with slow
lstat(3) even "diff-files --name-only" introduces a noticeable lag, so I
am not sure limiting this new codepath only to the case where you know the
comparison is made between the index and the working tree would save those
folks.

There already are existing knobs in the completion script to tweak how
much extra cycles the user is willing to spend to generate PS1. Perhaps
the new codepath can be made to trigger only to people who want it (or the
other way around, to allow people to disable)?