patchcompletion: complete git diff with only changed files.

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(-)

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

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.

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

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

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....

Junio C Hamano skribis:

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.

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.

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

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.

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:

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).

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

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

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?

Junio C Hamano schrieb:

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.

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

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

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)?