On 03/19, Denton Liu wrote:
> Hi all,
>
> I've been using jch's branch and I discovered a regression in
> ps/stash-in-c.
>
> Here's a test-case on git.git:
>
> echo '/**/' >>abspath.c
> git stash
> git stash show -v
> # I am expecting the diff to show up here
>
> I am expecting the diff to show up but in reality, I get no output.
> However, if I compile the latest master, the diff does show up.
>
> Note that I can get the patch to show up using "git stash show -p" so
> it's not really a showstopper. However, my understanding is that this
> was supposed to be a one-to-one (bug included!) port.
Yes, I think this is indeed a regression, even if it worked more or
less accidentally. '-v' would just be passed through to 'git diff'
which wouldn't do anything with it. It would show the patch because
that's the default behaviour of 'git diff'.
>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.
Anyway a patch to fix this is below, as we should have some
deprecation plan if we wanted to get rid of the '-v', and not just
silently change the behaviour.
--- >8 ---
Subject: [PATCH] stash: setup default diff output format if necessary
In the scripted 'git stash show' when no arguments are passed, we just
pass '--stat' to 'git diff'. When any argument is passed to 'stash
show', we no longer pass '--stat' to 'git diff', and pass whatever
flags are passed directly through to 'git diff'.
By default 'git diff' shows the patch output. So when we a user uses
'git stash show -v', they would be shown the diff, because that's the
default behaviour of 'git diff', but not actually directly triggered
by passing the '-v'.
In the C version of 'git stash show', we try to emulate that
behaviour using the internal diff API. However we forgot to set up
the default output format, in case it wasn't set by any of the flags
that were passed through. So 'git stash show -v' in the builtin
version of stash would be completely silent, while it would show the
diff before.
Fix this by setting up the default output format for 'git diff'.
Reported-by: Denton Liu <liu.denton@xxxxxxxxx>
Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
---
builtin/stash.c | 4 ++++
t/t3903-stash.sh | 18 ++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/builtin/stash.c b/builtin/stash.c
index 51df092633..012662ce68 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -761,6 +761,10 @@ static int show_stash(int argc, const char **argv, const char *prefix)
free_stash_info(&info);
usage_with_options(git_stash_show_usage, options);
}
+ if (!rev.diffopt.output_format) {
+ rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+ diff_setup_done(&rev.diffopt);
+ }
rev.diffopt.flags.recursive = 1;
setup_diff_pager(&rev.diffopt);
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 97cc71fbaf..e0a50ab267 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -612,6 +612,24 @@ test_expect_success 'stash show -p - no stashes on stack, stash-like argument' '
test_cmp expected actual
'
+test_expect_success 'stash show -v shows diff' '
+ git reset --hard &&
+ echo foo >>file &&
+ STASH_ID=$(git stash create) &&
+ git reset --hard &&
+ cat >expected <<-EOF &&
+ diff --git a/file b/file
+ index 7601807..71b52c4 100644
+ --- a/file
+ +++ b/file
+ @@ -1 +1,2 @@
+ baz
+ +foo
+ EOF
+ git stash show -v ${STASH_ID} >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'drop: fail early if specified stash is not a stash ref' '
git stash clear &&
test_when_finished "git reset --hard HEAD && git stash clear" &&
--
2.21.0.157.g0e94f7aa73.dirty
