Re: [PATCH v3] range-diff: always pass at least minimal diff options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 4, 2018, 01:35 UTC
- Message-ID
- <xmqqbm62gjl4.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20181203212131.11299-1-sunshine@sunshineco.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 8 quoted lines
> This is a re-roll of Martin's v2[1]. The only difference from v2 is that > it retains coloring when emitting to the terminal (plus an in-code > comment was simplified). > > The regression introduced by d8981c3f88, in which the range-diff only > ever gets emitted to the terminal, and never to the cover letter or > commentary section of a standalone patch, makes the --range-diff option > rather useless, so this fix probably ought to be fast-tracked.
Yup. Thanks. The only thing that makes me wonder is why any of the existing tests (among which I think I saw range-diff driven from format-patch) did not catch this rather obvious glitch.
Show 11 quoted lines
> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh > index e497c1358f..048feaf6dd 100755 > --- a/t/t3206-range-diff.sh > +++ b/t/t3206-range-diff.sh > @@ -248,18 +248,24 @@ test_expect_success 'dual-coloring' ' > for prev in topic master..topic > do > test_expect_success "format-patch --range-diff=$prev" ' > - git format-patch --stdout --cover-letter --range-diff=$prev \ > + git format-patch --cover-letter --range-diff=$prev \ > master..unmodified >actual &&
Ah, of course. Then the "actual" file gets the names of the output files; we expect to see 5 of them.
But now we have lost all the range-diff tests run under "--stdout", so next time some other change regresses only that codepath, we will not notice (which is fine for now but would want to be addressed before the end of the year around which time we certainly will all forget).
Thanks.
Show 25 quoted lines
> - grep "= 1: .* s/5/A" actual && > - grep "= 2: .* s/4/A" actual && > - grep "= 3: .* s/11/B" actual && > - grep "= 4: .* s/12/B" actual > + test_when_finished "rm 000?-*" && > + test_line_count = 5 actual && > + test_i18ngrep "^Range-diff:$" 0000-* && > + grep "= 1: .* s/5/A" 0000-* && > + grep "= 2: .* s/4/A" 0000-* && > + grep "= 3: .* s/11/B" 0000-* && > + grep "= 4: .* s/12/B" 0000-* > ' > done > > test_expect_success 'format-patch --range-diff as commentary' ' > - git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual && > - test_i18ngrep "^Range-diff:$" actual > + git format-patch --range-diff=HEAD~1 HEAD~1 >actual && > + test_when_finished "rm 0001-*" && > + test_line_count = 1 actual && > + test_i18ngrep "^Range-diff:$" 0001-* && > + grep "> 1: .* new message" 0001-* > ' > > test_done