[PATCH v2] range-diff: always pass at least minimal diff options
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Dec 3, 2018, 20:07 UTC
- Message-ID
- <20181203200734.527341-1-martin.agren@gmail.com>
- In-Reply-To
- <CAN0heSrfH39-37KDU3XDhxiYs1_3eUMdjbdAm37cPAmnOYUZMA@mail.gmail.com>
Commit d8981c3f88 ("format-patch: do not let its diff-options affect --range-diff", 2018-11-30) taught `show_range_diff()` to accept a NULL-pointer as an indication that it should use its own "reasonable default". That fixed a regression from a5170794 ("Merge branch 'ab/range-diff-no-patch'", 2018-11-18), but unfortunately it introduced a regression of its own.
In particular, it means we forget the `file` member of the diff options, so rather than placing a range-diff in the cover-letter, we write it to stdout. In order to fix this, rewrite the two callers adjusted by d8981c3f88 to instead create a "dummy" set of diff options where they only fill in which file to use.
Plus, turn off coloring to make sure we don't write any color codes. Maybe we could do `opts.use_color = opts.file != stdout`, but for now, I'd much rather always write uncolored output than write color codes where there shouldn't be any.
Modify and extend the existing tests to try and verify that the right contents end up in the right place.
Don't revert `show_range_diff()`, i.e., let it keep accepting NULL. Rather than removing what is dead code and figuring out it isn't actually dead and we've broken 2.20, just leave it for now.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- Here's another attempt at fixing this recent regression.
t/t3206-range-diff.sh | 20 +++++++++++++------- builtin/log.c | 13 ++++++++++++- log-tree.c | 13 ++++++++++++- 3 files changed, 37 insertions(+), 9 deletions(-)
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 && - 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 diff --git a/builtin/log.c b/builtin/log.c index 5ac18e2848..e42487b46d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1094,9 +1094,20 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, } if (rev->rdiff1) { + /* + * (At least for now) we only want to pass down + * the file handle where we want the range-diff + * to appear. Avoid any other diff options until + * we know how we want to handle them. + */ + struct diff_options opts; + diff_setup(&opts); + opts.file = rev->diffopt.file; + opts.use_color = 0; + diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); show_range_diff(rev->rdiff1, rev->rdiff2, - rev->creation_factor, 1, NULL); + rev->creation_factor, 1, &opts); } } diff --git a/log-tree.c b/log-tree.c index b243779a0b..fd79a3ec37 100644 --- a/log-tree.c +++ b/log-tree.c @@ -755,14 +755,25 @@ void show_log(struct rev_info *opt) if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) { struct diff_queue_struct dq; + struct diff_options opts; memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff)); DIFF_QUEUE_CLEAR(&diff_queued_diff); next_commentary_block(opt, NULL); fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title); + /* + * (At least for now) we only want to pass down + * the file handle where we want the range-diff + * to appear. Avoid any other diff options until + * we know how we want to handle them. + */ + diff_setup(&opts); + opts.file = opt->diffopt.file; + opts.use_color = 0; + diff_setup_done(&opts); show_range_diff(opt->rdiff1, opt->rdiff2, - opt->creation_factor, 1, NULL); + opt->creation_factor, 1, &opts); memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); }
-- 2.20.0.rc2.1.gfcc5f94f1e