Re: [REGRESSION ps/stash-in-c] git stash show -v

[Jeff King <peff@peff.net>]

On Tue, Mar 19, 2019 at 11:18:26PM +0000, Thomas Gummerer wrote:

> From a quick search I couldn't find where 'git diff' actually parses
> the '-v' argument, but I wonder if we should actually disallow it, in
> case we want to use it for something else in the future?  It's not
> documented anywhere in the docs either.

It's a bit interesting, actually. git-diff uses setup_revisions() to
parse its arguments, which picks up any diff options, as well as parsing
the revs and pathspecs.

But it also means we accept any random revision options. So nonsense
like:

  git diff --ancestry-path HEAD^ HEAD

is accepted, even though nobody ever looks at the flags set by parsing
that option.

And "-v" is mostly in the same boat. It works more or less like --pretty
(try rev-list with and without it), and does nothing for an endpoint
diff. What's a little interesting, though, is that it was originally
added as a diff-tree option in the very early days, via cee99d2257
(diff-tree: add "verbose header" mode, 2005-05-06). And the reason there
is that "diff-tree --stdin" filled a "log"-like role; it didn't traverse
the commits itself, but it was responsible for showing them.

Most of that is historical curiosity, but I think the takeaways are:

  - we probably should use a less bizarre option to demonstrate this bug
    (Junio suggested --patience, which makes perfect sense to me)

  - we may want to teach the "diff" porcelain not to accept useless
    revision options. I suspect it may be a bit tricky, just because of
    the way the code takes advantage of setup_revisions.  It would also
    be nice if "diff-tree" in non-stdin mode could do the same, but I
    suspect that is even trickier (we do not even know whether we are in
    --stdin mode or not until we've fed the options to setup_revisions).
    So I'd guess this is not really worth the effort it would take.

  - "-v" is a real thing; we should consider either documenting it or
    deprecating it.

-Peff