Re: completion: git: mutual exclusive handling of patch diff options

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Daniel Hahler wrote on Mon, Mar 23, 2015 at 03:05:29 +0100:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Hi,
> 
> __git_setup_diff_options makes the assumption that different options
> cannot be used together, based on:
> 
>   diff_types='(-p -u --patch -U --unified --raw --patch-with-raw --stat
>   --numstat --shortstat --dirstat --dirstat-by-file --summary
>   --patch-with-stat --name-only --name-status --cumulative -s
>   --no-patch)'
> 
> But "git diff --stat -p" is a useful combination, and Zsh will currently
> only complete "changed in working tree file" after this.
> 
> I've noticed this because I've changed my "gd" alias to: "git diff
> - --submodule --stat -p"
> 
> Options like "--name-only" can only be used alone, but there's also no
> error when using "-p" or "--stat" with it.
> 
> For my alias I can use "--patch-with-stat" (instead of "-p --stat"), but
> then the completion would still be broken for "gd --no-patch" (in case
> you would not want a diff-stat once).
> 
> I think the options shouldn't be mutual exclusive here in general, but
> only where it makes sense, e.g. "--name-only".
> 

Agreed — '-p --stat' and '-p --stat --minimal' are valid combinations,
and so _git should support them.

I don't think we have to nail down the exact set of permitted
combinations, either; it would be better to have a false positive (offer
an option that is preempted by an earlier option on the command line)
than to have a false negative (not offer a valid option).

> It seems like the reason for only using "changed in working tree files" is
> that the previous word is not recognized as an option nor as a
> commit/blob/reference.  Maybe that restriction could be also lifted to
> skip/ignore any arguments starting with a dash ("-").  Otherwise any new
> options to git-diff, which are not handled by the completion would cause
> the same problem when being used.

That's a general problem with completion, e.g., 'rsync --foo
-<TAB>' does nothing (not even an error).  The completion code doesn't
know what to suggest because it doesn't know whether the --foobar option
takes an argument or not.