Re: [PATCH DONOTAPPLY 5/4] revision: let --stdin set rev_input_given

[Jeff King <peff@peff.net>]

On Thu, Aug 03, 2017 at 08:59:33AM -0700, Junio C Hamano wrote:

> >   (echo --; echo t) | git log --stdin
> >
> > no longer defaults to HEAD. Which maybe people would see as a
> > regression. I could see arguments either way.
> 
> Yeah, thanks for thinking this through.  I do think this would be a
> regression.  On the other hand,
> 
>     (printf "%s\n" --tags=no-such -- t) | git log --stdin
> 
> should not default to HEAD and show nothing, I would think.

That was the same conclusion I got to, but it actually doesn't matter,
because we don't allow it anyway:

  $ echo --tags=no-such | git log --stdin
  fatal: options not supported in --stdin mode

If we were to make that work later, it should automatically do the right
thing with respect to rev_input_given, since the stdin code would just
call into handle_revision_arg().

> So if we wanted to do the "--stdin" thing properly, we probably need
> to keep the "--stdin" option itself neutral wrt "did we get rev
> input?"; instead, each input item that comes in from the standard
> input stream would decide if the user wants us to fall back to the
> default, perhaps?

Yes, exactly. It is a shame that scripts can't rely on an empty input to
"rev-list --stdin" to produce a empty output, though. That seems like it
would be handy. On the other hand, nobody has complained about it in the
past 10 years, so perhaps it just doesn't come up.

> Hmph.  Do you mean the former should traverse from HEAD while the
> latter should give us empty in the following two, because unlike
> "log", "rev-list" does not do the "default to HEAD" thing if it is
> not told to do so?
> 
>     git rev-list --default HEAD --stdin </dev/null
>     git rev-list --stdin </dev/null
> 
> If so, I think the reasoning makes sense.

TBH, I think both would make sense with an empty output. But it looks
like filter-branch is relying on the former to show HEAD. But like I
said, I didn't dig into whether its expectation is sane. If anybody is
interested in digging further you can apply the --stdin patch in this
thread and run t7003.

> > I didn't dig enough to see if it's actually sane or
> > not. The failing tests seem to be weird noop filters that our test
> > script uses. But I'm worried it would break some real case, too.
> 
> Thanks.  Let's not rush things.  
> 
> The ones you sent for application 1-4/4 all are improvements.

Thanks. I agree that is probably an OK stopping point, as the --stdin
behavior is largely orthogonal (it just happens to be in the same
general area).

-Peff