Jeff King [off-list ref] writes:
This patch makes "rev-list --stdin </dev/null" return an empty set.
Which makes sense to me. But a side effect is that:
git log --stdin </dev/null
now shows nothing (rather than HEAD). I think that's probably the right
thing. But:
(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.
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?
But this also breaks filter-branch (or at least a few of its tests),
which really wants to do:
git rev-list --default HEAD --stdin <maybe-empty
and traverse HEAD.
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.
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.
On Thu, Aug 03, 2017 at 08:59:33AM -0700, Junio C Hamano wrote:
quoted
(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.
quoted
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