@matheusbsbs / matheus-bagliano / commits / ac0edf1f46

range-diff: always pass at least minimal diff options

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 the fields we absolutely require, such as output file and color. 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. [es: retain diff coloring when going to stdout] Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Dec 3, 2018 at 16:21 UTC ac0edf1f46fcf9b9f6f1156e555bdf740cd56c5f

3 files changed +33 -9

builtin/log.c

+10 -1

@@ -1094,9 +1094,18 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,

1094 }

1095

1096 if (rev->rdiff1) {

1097 + /*

1098 + * Pass minimum required diff-options to range-diff; others

1099 + * can be added later if deemed desirable.

1100 + */

1101 + struct diff_options opts;

1102 + diff_setup(&opts);

1103 + opts.file = rev->diffopt.file;

1104 + opts.use_color = rev->diffopt.use_color;

1105 + diff_setup_done(&opts);

1106 fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);

1107 show_range_diff(rev->rdiff1, rev->rdiff2,

1099 - rev->creation_factor, 1, NULL);

1108 + rev->creation_factor, 1, &opts);

1109 }

1110 }

1111

log-tree.c

+10 -1

@@ -755,14 +755,23 @@ void show_log(struct rev_info *opt)

755

756 if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {

757 struct diff_queue_struct dq;

758 + struct diff_options opts;

759

760 memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));

761 DIFF_QUEUE_CLEAR(&diff_queued_diff);

762

763 next_commentary_block(opt, NULL);

764 fprintf_ln(opt->diffopt.file, "%s", opt->rdiff_title);

765 + /*

766 + * Pass minimum required diff-options to range-diff; others

767 + * can be added later if deemed desirable.

768 + */

769 + diff_setup(&opts);

770 + opts.file = opt->diffopt.file;

771 + opts.use_color = opt->diffopt.use_color;

772 + diff_setup_done(&opts);

773 show_range_diff(opt->rdiff1, opt->rdiff2,

765 - opt->creation_factor, 1, NULL);

774 + opt->creation_factor, 1, &opts);

775

776 memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));

777 }

t/t3206-range-diff.sh

+13 -7

@@ -248,18 +248,24 @@ test_expect_success 'dual-coloring' '

248 for prev in topic master..topic

249 do

250 test_expect_success "format-patch --range-diff=$prev" '

251 - git format-patch --stdout --cover-letter --range-diff=$prev \

251 + git format-patch --cover-letter --range-diff=$prev \

252 master..unmodified >actual &&

253 - grep "= 1: .* s/5/A" actual &&

254 - grep "= 2: .* s/4/A" actual &&

255 - grep "= 3: .* s/11/B" actual &&

256 - grep "= 4: .* s/12/B" actual

253 + test_when_finished "rm 000?-*" &&

254 + test_line_count = 5 actual &&

255 + test_i18ngrep "^Range-diff:$" 0000-* &&

256 + grep "= 1: .* s/5/A" 0000-* &&

257 + grep "= 2: .* s/4/A" 0000-* &&

258 + grep "= 3: .* s/11/B" 0000-* &&

259 + grep "= 4: .* s/12/B" 0000-*

260 '

261 done

262

263 test_expect_success 'format-patch --range-diff as commentary' '

261 - git format-patch --stdout --range-diff=HEAD~1 HEAD~1 >actual &&

262 - test_i18ngrep "^Range-diff:$" actual

264 + git format-patch --range-diff=HEAD~1 HEAD~1 >actual &&

265 + test_when_finished "rm 0001-*" &&

266 + test_line_count = 1 actual &&

267 + test_i18ngrep "^Range-diff:$" 0001-* &&

268 + grep "> 1: .* new message" 0001-*

269 '

270

271 test_done