Merge branch 'es/format-patch-interdiff'
authorJunio C Hamano <gitster@pobox.com>
Mon, 17 Sep 2018 20:53:55 +0000 (13:53 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 17 Sep 2018 20:53:55 +0000 (13:53 -0700)
"git format-patch" learned a new "--interdiff" option to explain
the difference between this version and the previous atttempt in
the cover letter (or after the tree-dashes as a comment).

* es/format-patch-interdiff:
format-patch: allow --interdiff to apply to a lone-patch
log-tree: show_log: make commentary block delimiting reusable
interdiff: teach show_interdiff() to indent interdiff
format-patch: teach --interdiff to respect -v/--reroll-count
format-patch: add --interdiff option to embed diff in cover letter
format-patch: allow additional generated content in make_cover_letter()

Documentation/git-format-patch.txt
Makefile
builtin/log.c
interdiff.c [new file with mode: 0644]
interdiff.h [new file with mode: 0644]
log-tree.c
revision.h
t/t4014-format-patch.sh
index b41e1329a7d8439762790e663ac52ec5f487bc8b..f8a061794dcabc1b54a2e17dbfc178b5ce25caf9 100644 (file)
@@ -23,6 +23,7 @@ SYNOPSIS
                   [(--reroll-count|-v) <n>]
                   [--to=<email>] [--cc=<email>]
                   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
+                  [--interdiff=<previous>]
                   [--progress]
                   [<common diff options>]
                   [ <since> | <revision range> ]
@@ -228,6 +229,15 @@ feeding the result to `git send-email`.
        containing the branch description, shortlog and the overall diffstat.  You can
        fill in a description in the file before sending it out.
 
+--interdiff=<previous>::
+       As a reviewer aid, insert an interdiff into the cover letter,
+       or as commentary of the lone patch of a 1-patch series, showing
+       the differences between the previous version of the patch series and
+       the series currently being formatted. `previous` is a single revision
+       naming the tip of the previous series which shares a common base with
+       the series being formatted (for example `git format-patch
+       --cover-letter --interdiff=feature/v1 -3 feature/v2`).
+
 --notes[=<ref>]::
        Append the notes (see linkgit:git-notes[1]) for the commit
        after the three-dash line.
index 7cf682001364a55a1bf68cd834fc5d0e1ad1658d..b567ccca45883c15bc9b8dbd60abe6a88186620f 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -884,6 +884,7 @@ LIB_OBJS += linear-assignment.o
 LIB_OBJS += help.o
 LIB_OBJS += hex.o
 LIB_OBJS += ident.o
+LIB_OBJS += interdiff.o
 LIB_OBJS += json-writer.o
 LIB_OBJS += kwset.o
 LIB_OBJS += levenshtein.o
index 517c5e67890d421fd1f15fb454221eb1fa0634f4..df4201131284317948de938242c22d9862c0ffbb 100644 (file)
@@ -32,6 +32,7 @@
 #include "commit-slab.h"
 #include "repository.h"
 #include "commit-reach.h"
+#include "interdiff.h"
 
 #define MAIL_DEFAULT_WRAP 72
 
@@ -999,6 +1000,26 @@ static char *find_branch_name(struct rev_info *rev)
        return branch;
 }
 
+static void show_diffstat(struct rev_info *rev,
+                         struct commit *origin, struct commit *head)
+{
+       struct diff_options opts;
+
+       memcpy(&opts, &rev->diffopt, sizeof(opts));
+       opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+       opts.stat_width = MAIL_DEFAULT_WRAP;
+
+       diff_setup_done(&opts);
+
+       diff_tree_oid(get_commit_tree_oid(origin),
+                     get_commit_tree_oid(head),
+                     "", &opts);
+       diffcore_std(&opts);
+       diff_flush(&opts);
+
+       fprintf(rev->diffopt.file, "\n");
+}
+
 static void make_cover_letter(struct rev_info *rev, int use_stdout,
                              struct commit *origin,
                              int nr, struct commit **list,
@@ -1012,7 +1033,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
        struct strbuf sb = STRBUF_INIT;
        int i;
        const char *encoding = "UTF-8";
-       struct diff_options opts;
        int need_8bit_cte = 0;
        struct pretty_print_context pp = {0};
        struct commit *head = list[0];
@@ -1062,25 +1082,14 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
        shortlog_output(&log);
 
-       /*
-        * We can only do diffstat with a unique reference point
-        */
-       if (!origin)
-               return;
+       /* We can only do diffstat with a unique reference point */
+       if (origin)
+               show_diffstat(rev, origin, head);
 
-       memcpy(&opts, &rev->diffopt, sizeof(opts));
-       opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
-       opts.stat_width = MAIL_DEFAULT_WRAP;
-
-       diff_setup_done(&opts);
-
-       diff_tree_oid(get_commit_tree_oid(origin),
-                     get_commit_tree_oid(head),
-                     "", &opts);
-       diffcore_std(&opts);
-       diff_flush(&opts);
-
-       fprintf(rev->diffopt.file, "\n");
+       if (rev->idiff_oid1) {
+               fprintf_ln(rev->diffopt.file, "%s", rev->idiff_title);
+               show_interdiff(rev, 0);
+       }
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -1420,6 +1429,16 @@ static void print_bases(struct base_tree_info *bases, FILE *file)
        oidclr(&bases->base_commit);
 }
 
+static const char *diff_title(struct strbuf *sb, int reroll_count,
+                      const char *generic, const char *rerolled)
+{
+       if (reroll_count <= 0)
+               strbuf_addstr(sb, generic);
+       else /* RFC may be v0, so allow -v1 to diff against v0 */
+               strbuf_addf(sb, rerolled, reroll_count - 1);
+       return sb->buf;
+}
+
 int cmd_format_patch(int argc, const char **argv, const char *prefix)
 {
        struct commit *commit;
@@ -1447,6 +1466,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        struct base_tree_info bases;
        int show_progress = 0;
        struct progress *progress = NULL;
+       struct oid_array idiff_prev = OID_ARRAY_INIT;
+       struct strbuf idiff_title = STRBUF_INIT;
 
        const struct option builtin_format_patch_options[] = {
                { OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
@@ -1520,6 +1541,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                OPT__QUIET(&quiet, N_("don't print the patch filenames")),
                OPT_BOOL(0, "progress", &show_progress,
                         N_("show progress while generating patches")),
+               OPT_CALLBACK(0, "interdiff", &idiff_prev, N_("rev"),
+                            N_("show changes against <rev> in cover letter or single patch"),
+                            parse_opt_object_name),
                OPT_END()
        };
 
@@ -1705,7 +1729,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                if (rev.pending.nr == 2) {
                        struct object_array_entry *o = rev.pending.objects;
                        if (oidcmp(&o[0].item->oid, &o[1].item->oid) == 0)
-                               return 0;
+                               goto done;
                }
                get_patch_ids(&rev, &ids);
        }
@@ -1729,7 +1753,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        }
        if (nr == 0)
                /* nothing to do */
-               return 0;
+               goto done;
        total = nr;
        if (cover_letter == -1) {
                if (config_cover_letter == COVER_AUTO)
@@ -1742,6 +1766,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        if (numbered)
                rev.total = total + start_number - 1;
 
+       if (idiff_prev.nr) {
+               if (!cover_letter && total != 1)
+                       die(_("--interdiff requires --cover-letter or single patch"));
+               rev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];
+               rev.idiff_oid2 = get_commit_tree_oid(list[0]);
+               rev.idiff_title = diff_title(&idiff_title, reroll_count,
+                                            _("Interdiff:"),
+                                            _("Interdiff against v%d:"));
+       }
+
        if (!signature) {
                ; /* --no-signature inhibits all signatures */
        } else if (signature && signature != git_version_string) {
@@ -1779,6 +1813,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                print_signature(rev.diffopt.file);
                total++;
                start_number--;
+               /* interdiff in cover-letter; omit from patches */
+               rev.idiff_oid1 = NULL;
        }
        rev.add_signoff = do_signoff;
 
@@ -1859,6 +1895,10 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        string_list_clear(&extra_hdr, 0);
        if (ignore_if_in_upstream)
                free_patch_ids(&ids);
+
+done:
+       oid_array_clear(&idiff_prev);
+       strbuf_release(&idiff_title);
        return 0;
 }
 
diff --git a/interdiff.c b/interdiff.c
new file mode 100644 (file)
index 0000000..c81d680
--- /dev/null
@@ -0,0 +1,28 @@
+#include "cache.h"
+#include "commit.h"
+#include "revision.h"
+#include "interdiff.h"
+
+static struct strbuf *idiff_prefix_cb(struct diff_options *opt, void *data)
+{
+       return data;
+}
+
+void show_interdiff(struct rev_info *rev, int indent)
+{
+       struct diff_options opts;
+       struct strbuf prefix = STRBUF_INIT;
+
+       memcpy(&opts, &rev->diffopt, sizeof(opts));
+       opts.output_format = DIFF_FORMAT_PATCH;
+       opts.output_prefix = idiff_prefix_cb;
+       strbuf_addchars(&prefix, ' ', indent);
+       opts.output_prefix_data = &prefix;
+       diff_setup_done(&opts);
+
+       diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, "", &opts);
+       diffcore_std(&opts);
+       diff_flush(&opts);
+
+       strbuf_release(&prefix);
+}
diff --git a/interdiff.h b/interdiff.h
new file mode 100644 (file)
index 0000000..01c730a
--- /dev/null
@@ -0,0 +1,8 @@
+#ifndef INTERDIFF_H
+#define INTERDIFF_H
+
+struct rev_info;
+
+void show_interdiff(struct rev_info *, int indent);
+
+#endif
index 7443e5fcc74b7964c478327c6557d53eb4f6b425..f482f47b83dc3e2a999e00c93ac089fc6de5a6b9 100644 (file)
@@ -15,6 +15,7 @@
 #include "sequencer.h"
 #include "line-log.h"
 #include "help.h"
+#include "interdiff.h"
 
 static struct decoration name_decoration = { "object names" };
 static int decoration_loaded;
@@ -542,6 +543,16 @@ static int show_mergetag(struct rev_info *opt, struct commit *commit)
        return for_each_mergetag(show_one_mergetag, commit, opt);
 }
 
+static void next_commentary_block(struct rev_info *opt, struct strbuf *sb)
+{
+       const char *x = opt->shown_dashes ? "\n" : "---\n";
+       if (sb)
+               strbuf_addstr(sb, x);
+       else
+               fputs(x, opt->diffopt.file);
+       opt->shown_dashes = 1;
+}
+
 void show_log(struct rev_info *opt)
 {
        struct strbuf msgbuf = STRBUF_INIT;
@@ -699,10 +710,8 @@ void show_log(struct rev_info *opt)
 
        if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
            ctx.notes_message && *ctx.notes_message) {
-               if (cmit_fmt_is_mail(ctx.fmt)) {
-                       strbuf_addstr(&msgbuf, "---\n");
-                       opt->shown_dashes = 1;
-               }
+               if (cmit_fmt_is_mail(ctx.fmt))
+                       next_commentary_block(opt, &msgbuf);
                strbuf_addstr(&msgbuf, ctx.notes_message);
        }
 
@@ -729,6 +738,19 @@ void show_log(struct rev_info *opt)
 
        strbuf_release(&msgbuf);
        free(ctx.notes_message);
+
+       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, 2);
+
+               memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));
+       }
 }
 
 int log_tree_diff_flush(struct rev_info *opt)
@@ -766,9 +788,10 @@ int log_tree_diff_flush(struct rev_info *opt)
 
                        /*
                         * We may have shown three-dashes line early
-                        * between notes and the log message, in which
-                        * case we only want a blank line after the
-                        * notes without (an extra) three-dashes line.
+                        * between generated commentary (notes, etc.)
+                        * and the log message, in which case we only
+                        * want a blank line after the commentary
+                        * without (an extra) three-dashes line.
                         * Otherwise, we show the three-dashes line if
                         * we are showing the patch with diffstat, but
                         * in that case, there is no extra blank line
index 12259579271d9c51652f671b29598f09de967f0a..91bc180e37dc7d7810c0ab3e883e26cbe851385a 100644 (file)
@@ -219,6 +219,11 @@ struct rev_info {
        /* notes-specific options: which refs to show */
        struct display_notes_opt notes_opt;
 
+       /* interdiff */
+       const struct object_id *idiff_oid1;
+       const struct object_id *idiff_oid2;
+       const char *idiff_title;
+
        /* commit counts */
        int count_left;
        int count_right;
index 53880da7bbe85521112ced3ff94ead05587c7e81..909c743c134c5e18e0a80db3eae2dfb6e8210e6a 100755 (executable)
@@ -1717,4 +1717,38 @@ test_expect_success 'format-patch --pretty=mboxrd' '
        test_cmp expect actual
 '
 
+test_expect_success 'interdiff: setup' '
+       git checkout -b boop master &&
+       test_commit fnorp blorp &&
+       test_commit fleep blorp
+'
+
+test_expect_success 'interdiff: cover-letter' '
+       sed "y/q/ /" >expect <<-\EOF &&
+       +fleep
+       --q
+       EOF
+       git format-patch --cover-letter --interdiff=boop~2 -1 boop &&
+       test_i18ngrep "^Interdiff:$" 0000-cover-letter.patch &&
+       test_i18ngrep ! "^Interdiff:$" 0001-fleep.patch &&
+       sed "1,/^@@ /d; /^-- $/q" <0000-cover-letter.patch >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'interdiff: reroll-count' '
+       git format-patch --cover-letter --interdiff=boop~2 -v2 -1 boop &&
+       test_i18ngrep "^Interdiff ..* v1:$" v2-0000-cover-letter.patch
+'
+
+test_expect_success 'interdiff: solo-patch' '
+       cat >expect <<-\EOF &&
+         +fleep
+
+       EOF
+       git format-patch --interdiff=boop~2 -1 boop &&
+       test_i18ngrep "^Interdiff:$" 0001-fleep.patch &&
+       sed "1,/^  @@ /d; /^$/q" <0001-fleep.patch >actual &&
+       test_cmp expect actual
+'
+
 test_done