On Mon, Sep 22, 2025 at 08:45:36AM -0700, Junio C Hamano wrote:

"git stash show" gives a "git diff --stat stash~ stash" (i.e. the
worktree relative to then-current-HEAD in diffstat format), and "git
stash show --" (no other arguments) gives us "git diff -p" for the
same, it seems; this is with or without your patch.

Oof, that is certainly unexpected. Ironically I had done all of the
manual testing with "-p --", because the point of this topic was looking
at how we passed arguments to setup_revisions(). And then I simplified
it when I added the test, because it seemed that the "-p" would just be
noise there.

So it is surprising that the test passes, but you explained that clearly
below.

We may care "--" by itself does not change the output, but it has
already been giving different output without your patch.

I think the difference is probably a bug, but one that is out of scope
for what this patch is trying to fix. So even if we wanted to fix it,
I'd prefer not to deal with it here.

I do not think we want to drop this test (we do want the "handles
without leaking" part of the test), but we should not expect the
output from these commands match.

Yeah, I had originally written just:

  test_expect_success 'stash show -- does not leak' '
	git stash show --
  '

but it felt funny, since the test is doing nothing in a build without
SANITIZE=leak. If we are OK with that funniness, I can switch back to
that.

I only discovered this while merging this and another topic that
happen to touch the same t3903 into 'seen'.

I'm glad you did. I would rather find out about it now while it is fresh
in my mind, then 3 years from now when we all wonder what the heck is
going on.

* I personally find the traditional behaviour nonsense and it may be
coming from the crappy command line parsing we have had forever, but
I am sure people who wrote

    git stash show --
    git stash show --end-of-options

out of "principle" in their scripts, and assumed that the patch
output is the norm for the command even though it should have been
giving diffstat, would be unhappy if we suddenly made them behave
exactly like "git stash show" (nothing else on the command line).

Yeah, I agree with both points (that the current behavior is nonsense,
but people may accidentally have been relying on it). I wouldn't feel
_too_ bad about saying "you were relying on nonsense, and we have fixed
the bug" in this case. But I think it should be a separate topic (and
possibly one that makes "git stash show -- foo" do something sensible).

-Peff