zsh-workers
 help / color / mirror / code / Atom feed
From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Daniel Hahler <genml+zsh-workers@thequod.de>
Cc: Zsh Hackers' List <zsh-workers@zsh.org>
Subject: Re: completion: git: mutual exclusive handling of patch diff options
Date: Sun, 29 Mar 2015 08:23:40 +0000	[thread overview]
Message-ID: <20150329082340.GA17926@tarsus.local2> (raw)
In-Reply-To: <550F74E9.40909@thequod.de>

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.


      reply	other threads:[~2015-03-29  8:23 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-23  2:05 Daniel Hahler
2015-03-29  8:23 ` Daniel Shahaf [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150329082340.GA17926@tarsus.local2 \
    --to=d.s@daniel.shahaf.name \
    --cc=genml+zsh-workers@thequod.de \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).