Re: [PATCH] diff,difftool: Don't use the {0,2} notation in usage strings

From: Jeff King <peff@peff.net>
Date: Nov 4, 2010, 18:55 UTC
Message-ID: <20101104185545.GB31016@sigill.intra.peff.net>
In-Reply-To: <20101104183851.GA16865@burratino>

On Thu, Nov 04, 2010 at 01:38:51PM -0500, Jonathan Nieder wrote:

Show 14 quoted lines

> Makes sense.  There is just one particularity of
> 
> 	git diff --cached [<commit>]
> 
> I am worried about.  Namely: according to a recent patch,
> 
> 	git diff --cached
> 
> should not be considered as
> 
> 	git diff --cached HEAD
> 
> with the "HEAD" implicit, but a distinct operation meaning
> "show me what changes git commit would store".

Yeah, I'm not sure I agree with that change, for the reason that it makes "git diff" more conceptually complex. Generally we think of "no commit" as "behave as if HEAD was given". But now it's not, but only in this one particular place.

I guess I should go complain in that thread, though...

Show 19 quoted lines

> >> I would rather treat --cached as one of the options ("instead of
> >> comparing the worktree, compare its cached content in the index to the
> >> specified commit"),
> >
> > Except it is not quite that. For the first two that I listed above,
> > --cached makes that distinction. But --cached doesn't make sense at all
> > in the third or fourth ones. So I think in practice it ends up defining
> > a mode of operation more than simply an option.
> 
> Not sure I understand your logic.  Is your point that --cached in
> those cases does not print
> 
> 	fatal: --cached does not make sense in this operation mode
> 
> but
> 
> 	usage: git diff <options> <rev>{0,2} -- <path>*
> 
> that implies the operation mode is not known?

No, I am fine with what it actually prints[1]. But my point is that "--cached" is not simply an option that should go in [options] in each synopsis line. Even though it _looks_ like an option (because it starts with --, and can go anywhere in the options list) it changes the syntax of the rest of the command line (in particular, you can provide 0 or 1 commits, not 2, and you cannot use --no-index)[2].

Removing it from the synopsis and just listing it as an option does not capture that aspect.

And yes, it's obviously a gray area. There are other mutually exclusive options that are really just normal options. I just happen to think that the action of "--cached" changes the operation significantly enough to be considered a separate mode. Just as we do with "-m" and "-d" for git-branch.

-Peff

[1] If we are on a quest to remove <rev>{0,2}, this is one other spot to
    do it.

[2] This ties in with an idea mentioned in years past (but never
    actually implemented) to have some symbolic name for referring to
    the working tree and index, like:

       git diff HEAD INDEX

    which makes it quite obvious what is going on, and that diff really
    only has one syntactic mode: diff thing A and thing B. The details
    of that depend on what thing A and thing B actually resolve to. But
    in theory that is identical to "git diff --cached HEAD".