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>

diff --git a/builtin/log.c b/builtin/log.c
index 5ac18e28486b61b91239d859c445bc620c51c5ad..e8e51068bd903c01e12207637d59d80ea940bf2d 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1094,9 +1094,18 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
        }
 
        if (rev->rdiff1) {
+               /*
+                * Pass minimum required diff-options to range-diff; others
+                * can be added later if deemed desirable.
+                */
+               struct diff_options opts;
+               diff_setup(&opts);
+               opts.file = rev->diffopt.file;
+               opts.use_color = rev->diffopt.use_color;
+               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 b243779a0b7dc15200689febe68549a85ce5c6a7..10680c139eeb530bc0e70e0c1713767b6ca5be21 100644 (file)
--- a/log-tree.c
+++ b/log-tree.c
@@ -755,14 +755,23 @@ 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);
+               /*
+                * Pass minimum required diff-options to range-diff; others
+                * can be added later if deemed desirable.
+                */
+               diff_setup(&opts);
+               opts.file = opt->diffopt.file;
+               opts.use_color = opt->diffopt.use_color;
+               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));
        }

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index e497c1358f8b2da6960d3d73db3cc75f9f7df85c..048feaf6ddf845279b30b7f1ebb7056c60e01c6d 100755 (executable)
--- 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