completion: git: mutual exclusive handling of patch diff options

completion: git: mutual exclusive handling of patch diff options

[Daniel Hahler]

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

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.


Regards,
Daniel.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iD8DBQFVD3TpfAK/hT/mPgARAhyaAJ9N5WNsYn5WBxHZ0TSSpjtGXxPdcgCg+5ff
Z2jKc9ic4emhOjEHtEDPEEs=
=jg4S
-----END PGP SIGNATURE-----

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

[Daniel Shahaf]

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.