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

[Thomas Gummerer <t.gummerer@gmail.com>]

On 03/20, Jeff King wrote:
> 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.

Thanks for the explanation!

> 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)

Yep, that should make the test a bit easier to understand.  Will do
that in v2.

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

Makes sense, I don't have a strong opinion on which way we should go
here, but I'll leave that separately from the bug fix either way.