Re: [REGRESSION ps/stash-in-c] git stash show -v
- From
Thomas Gummerer <t.gummerer@gmail.com>
- Date
- Mar 19, 2019, 23:18 UTC
- Message-ID
- <20190319231826.GB32487@hank.intra.tgummerer.com>
- In-Reply-To
- <20190319190503.GA10066@dev-l>
On 03/19, Denton Liu wrote:
Show 18 quoted lines
> 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@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> --- 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