From 84ed5055153ccfe5c38501e68ac58634001bbd9d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 May 2024 15:50:06 -0700 Subject: show_log: factor out interdiff/range-diff generation The integration of "git range-diff" with "git format-patch" for a single patch (i.e., not generating "range-diff" into the cover letter) hooks into log-tree.c:show_log(), which is responsible for writing the log message out and other stuff. Essentially, everything you see before the diffstat and the patch is generated there. Split out the code that spits out the interdiff/range-diff into a separate helper function show_diff_of_diff(). Hopefully this will make it easier to move things around in the output stream in the future patches. This is supposed to be a no-op refactoring. Signed-off-by: Junio C Hamano --- log-tree.c | 88 ++++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 48 insertions(+), 40 deletions(-) diff --git a/log-tree.c b/log-tree.c index 41416de4e3..e7cd2c491f 100644 --- a/log-tree.c +++ b/log-tree.c @@ -673,6 +673,53 @@ static void next_commentary_block(struct rev_info *opt, struct strbuf *sb) opt->shown_dashes = 1; } +static void show_diff_of_diff(struct rev_info *opt) +{ + if (!cmit_fmt_is_mail(opt->commit_format)) + return; + + if (opt->idiff_oid1) { + struct diff_queue_struct dq; + + 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->idiff_title); + show_interdiff(opt->idiff_oid1, opt->idiff_oid2, 2, + &opt->diffopt); + + memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); + } + + if (opt->rdiff1) { + struct diff_queue_struct dq; + struct diff_options opts; + struct range_diff_options range_diff_opts = { + .creation_factor = opt->creation_factor, + .dual_color = 1, + .diffopt = &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. + */ + repo_diff_setup(the_repository, &opts); + opts.file = opt->diffopt.file; + opts.use_color = opt->diffopt.use_color; + diff_setup_done(&opts); + show_range_diff(opt->rdiff1, opt->rdiff2, &range_diff_opts); + + memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); + } +} + void show_log(struct rev_info *opt) { struct strbuf msgbuf = STRBUF_INIT; @@ -857,46 +904,7 @@ void show_log(struct rev_info *opt) free(ctx.notes_message); free(ctx.after_subject); - if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) { - struct diff_queue_struct dq; - - 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->idiff_title); - show_interdiff(opt->idiff_oid1, opt->idiff_oid2, 2, - &opt->diffopt); - - memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); - } - - if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) { - struct diff_queue_struct dq; - struct diff_options opts; - struct range_diff_options range_diff_opts = { - .creation_factor = opt->creation_factor, - .dual_color = 1, - .diffopt = &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. - */ - repo_diff_setup(the_repository, &opts); - opts.file = opt->diffopt.file; - opts.use_color = opt->diffopt.use_color; - diff_setup_done(&opts); - show_range_diff(opt->rdiff1, opt->rdiff2, &range_diff_opts); - - memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff)); - } + show_diff_of_diff(opt); } int log_tree_diff_flush(struct rev_info *opt) -- cgit v1.3 From 2fa04cebfbe57be54b9bc3371a4b70f701d56da9 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 24 May 2024 16:02:55 -0700 Subject: format-patch: move range/inter diff at the end of a single patch output When running "format-patch" on a multiple patch series, the output coming from "--interdiff" and "--range-diff" options is inserted after the "shortlog" list of commits and the overall diffstat. The idea is that shortlog/diffstat are shorter and with denser information content, which gives a better overview before the readers dive into more details of range/inter diff. When working on a single patch, however, we stuff the inter/range diff output before the actual patch, next to the diffstat. This pushes down the patch text way down with inter/range diff output, distracting readers. Move the inter/range diff output to the very end of the output, after all the patch text is shown. As the inter/range diff is no longer part of the commentary block (i.e., what comes after the log message and "---", but before the patch text), stop producing "---" in the function that generates them. But to separate it out visually (note: this is not needed to help tools like "git apply" that pay attention to the hunk headers to figure out the length of the hunks), add an extra blank line between the end of the patch text and the inter/range diff. Signed-off-by: Junio C Hamano --- log-tree.c | 11 +++++------ t/t4014-format-patch.sh | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/log-tree.c b/log-tree.c index e7cd2c491f..7de744911e 100644 --- a/log-tree.c +++ b/log-tree.c @@ -684,8 +684,7 @@ static void show_diff_of_diff(struct rev_info *opt) 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->idiff_title); + fprintf_ln(opt->diffopt.file, "\n%s", opt->idiff_title); show_interdiff(opt->idiff_oid1, opt->idiff_oid2, 2, &opt->diffopt); @@ -704,8 +703,7 @@ static void show_diff_of_diff(struct rev_info *opt) 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); + fprintf_ln(opt->diffopt.file, "\n%s", opt->rdiff_title); /* * Pass minimum required diff-options to range-diff; others * can be added later if deemed desirable. @@ -903,8 +901,6 @@ void show_log(struct rev_info *opt) strbuf_release(&msgbuf); free(ctx.notes_message); free(ctx.after_subject); - - show_diff_of_diff(opt); } int log_tree_diff_flush(struct rev_info *opt) @@ -1173,9 +1169,12 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) } if (opt->track_linear && !opt->linear && opt->reverse_output_stage) fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar); + if (shown) + show_diff_of_diff(opt); opt->loginfo = NULL; maybe_flush_or_die(opt->diffopt.file, "stdout"); opt->diffopt.no_free = no_free; + diff_free(&opt->diffopt); return shown; } diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index ba85b582c5..de039825a9 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -2482,13 +2482,37 @@ test_expect_success 'interdiff: reroll-count with a integer' ' ' test_expect_success 'interdiff: solo-patch' ' - cat >expect <<-\EOF && - +fleep - - EOF git format-patch --interdiff=boop~2 -1 boop && - test_grep "^Interdiff:$" 0001-fleep.patch && - sed "1,/^ @@ /d; /^$/q" 0001-fleep.patch >actual && + + # remove up to the last "patch" output line, + # and remove everything below the signature mark. + sed -e "1,/^+fleep\$/d" -e "/^-- /,\$d" 0001-fleep.patch >actual && + + # fabricate Interdiff output. + git diff boop~2 boop >inter && + { + echo && + echo "Interdiff:" && + sed -e "s/^/ /" inter + } >expect && + test_cmp expect actual +' + +test_expect_success 'range-diff: solo-patch' ' + git format-patch --creation-factor=999 \ + --range-diff=boop~2..boop~1 -1 boop && + + # remove up to the last "patch" output line, + # and remove everything below the signature mark. + sed -e "1,/^+fleep\$/d" -e "/^-- /,\$d" 0001-fleep.patch >actual && + + # fabricate range-diff output. + { + echo && + echo "Range-diff:" && + git range-diff --creation-factor=999 \ + boop~2..boop~1 boop~1..boop + } >expect && test_cmp expect actual ' -- cgit v1.3