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

From: Junio C Hamano <gitster@pobox.com>
Date: May 18, 2011, 05:29 UTC
Message-ID: <7v8vu4efvj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DD30F87.2000807@gmx.de>

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

Show 5 quoted lines

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

Show 8 quoted lines

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