{"thread":{"id":"48936","subject":"[PATCH 00/14] format-patch: add --interdiff and --range-diff options","startedAt":"2018-07-22T09:57:59Z","lastAt":"2018-09-07T09:15:28Z","messageCount":38,"participants":["Eric Sunshine","Duy Nguyen","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":14},"messages":[{"id":"353306","messageId":"20180722095717.17912-1-sunshine@sunshineco.com","threadId":"48936","inReplyTo":null,"subject":"[PATCH 00/14] format-patch: add --interdiff and --range-diff options","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:03Z","receivedAt":"2018-07-22T09:57:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When re-submitting a patch series, it is often helpful (for reviewers)\nto include an interdiff or range-diff against the previous version.\nDoing so requires manually running git-diff or git-range-diff and\ncopy/pasting the result into the cover letter of the new version.\n\nThis series automates the process by introducing git-format-patch\noptions --interdiff and --range-diff which insert such a diff into the\ncover-letter or into the commentary section of the lone patch of a\n1-patch series. In the latter case, the interdiff or range-diff is\nindented to avoid confusing git-am and human readers.\n\nPatches 1-6 add --interdiff and can apply directly on 'master'.\nPatches 7-14 add --range-diff and apply atop js/range-diff v4[1].\n\nAn earlier RFC[2] implemented only --range-diff, and only for the\ncover-letter.\n\nChanges since the RFC:\n\n* add --interdiff option for cover-letter and lone patch\n\n* based on js/range-diff v4[1]\n\n* --range-diff works with lone patch (no longer limited to cover\n letter)\n\n* --range-diff colors output when used with --stdout, just as patches\n themselves are already colored\n\n* --range-diff takes advantage of libified range-diff mechanism in v4\n rather than invoking git-range-diff command\n\nNo interdiff or range-diff is included in this cover-letter since the\nimplementation changed dramatically.\n\n[1]: https://public-inbox.org/git/pull.1.v4.git.gitgitgadget@gmail.com/\n[2]: https://public-inbox.org/git/20180530080325.37520-1-sunshine@sunshineco.com/\n\nEric Sunshine (14):\n format-patch: allow additional generated content in\n make_cover_letter()\n format-patch: add --interdiff option to embed diff in cover letter\n format-patch: teach --interdiff to respect -v/--reroll-count\n interdiff: teach show_interdiff() to indent interdiff\n log-tree: show_log: make commentary block delimiting reusable\n format-patch: allow --interdiff to apply to a lone-patch\n range-diff: respect diff_option.file rather than assuming 'stdout'\n range-diff: publish default creation factor\n range-diff: relieve callers of low-level configuration burden\n format-patch: add --range-diff option to embed diff in cover letter\n format-patch: extend --range-diff to accept revision range\n format-patch: teach --range-diff to respect -v/--reroll-count\n format-patch: add --creation-factor tweak for --range-diff\n format-patch: allow --range-diff to apply to a lone-patch\n\n Documentation/git-format-patch.txt | 29 ++++++\n Makefile | 1 +\n builtin/log.c | 139 ++++++++++++++++++++++++-----\n builtin/range-diff.c | 25 ++----\n interdiff.c | 28 ++++++\n interdiff.h | 8 ++\n log-tree.c | 52 +++++++++--\n range-diff.c | 26 +++++-\n range-diff.h | 5 +-\n revision.h | 11 +++\n t/t3206-range-diff.sh | 12 +++\n t/t4014-format-patch.sh | 34 +++++++\n 12 files changed, 319 insertions(+), 51 deletions(-)\n create mode 100644 interdiff.c\n create mode 100644 interdiff.h\n\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353307","messageId":"20180722095717.17912-2-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 01/14] format-patch: allow additional generated content in make_cover_letter()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:04Z","receivedAt":"2018-07-22T09:58:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"make_cover_letter() returns early when it lacks sufficient state to emit\na diffstat, which makes it difficult to extend the function to reliably\nemit additional generated content. Work around this shortcoming by\nfactoring diffstat-printing logic out to its own function and calling it\nas needed without otherwise inhibiting normal control flow.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/log.c | 43 +++++++++++++++++++++++--------------------\n 1 file changed, 23 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 805f89d7e1..873aabcf40 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -997,6 +997,26 @@ static char *find_branch_name(struct rev_info *rev)\n \treturn branch;\n }\n \n+static void show_diffstat(struct rev_info *rev,\n+\t\t\t struct commit *origin, struct commit *head)\n+{\n+\tstruct diff_options opts;\n+\n+\tmemcpy(&opts, &rev->diffopt, sizeof(opts));\n+\topts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n+\topts.stat_width = MAIL_DEFAULT_WRAP;\n+\n+\tdiff_setup_done(&opts);\n+\n+\tdiff_tree_oid(get_commit_tree_oid(origin),\n+\t\t get_commit_tree_oid(head),\n+\t\t \"\", &opts);\n+\tdiffcore_std(&opts);\n+\tdiff_flush(&opts);\n+\n+\tfprintf(rev->diffopt.file, \"\\n\");\n+}\n+\n static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n@@ -1010,7 +1030,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tstruct strbuf sb = STRBUF_INIT;\n \tint i;\n \tconst char *encoding = \"UTF-8\";\n-\tstruct diff_options opts;\n \tint need_8bit_cte = 0;\n \tstruct pretty_print_context pp = {0};\n \tstruct commit *head = list[0];\n@@ -1060,25 +1079,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \n \tshortlog_output(&log);\n \n-\t/*\n-\t * We can only do diffstat with a unique reference point\n-\t */\n-\tif (!origin)\n-\t\treturn;\n-\n-\tmemcpy(&opts, &rev->diffopt, sizeof(opts));\n-\topts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n-\topts.stat_width = MAIL_DEFAULT_WRAP;\n-\n-\tdiff_setup_done(&opts);\n-\n-\tdiff_tree_oid(get_commit_tree_oid(origin),\n-\t\t get_commit_tree_oid(head),\n-\t\t \"\", &opts);\n-\tdiffcore_std(&opts);\n-\tdiff_flush(&opts);\n-\n-\tfprintf(rev->diffopt.file, \"\\n\");\n+\t/* We can only do diffstat with a unique reference point */\n+\tif (origin)\n+\t\tshow_diffstat(rev, origin, head);\n }\n \n static const char *clean_message_id(const char *msg_id)\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353308","messageId":"20180722095717.17912-3-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:05Z","receivedAt":"2018-07-22T09:58:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When submitting a revised version of a patch series, it can be helpful\n(to reviewers) to include a summary of changes since the previous\nattempt in the form of an interdiff, however, doing so involves manually\ncopy/pasting the diff into the cover letter.\n\nAdd an --interdiff option to automate this process. The argument to\n--interdiff specifies the tip of the previous attempt against which to\ngenerate the interdiff. For example:\n\n git format-patch --cover-letter --interdiff=v1 -3 v2\n\nThe previous attempt and the patch series being formatted must share a\ncommon base.\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/git-format-patch.txt | 9 +++++++++\n Makefile | 1 +\n builtin/log.c | 24 ++++++++++++++++++++++--\n interdiff.c | 17 +++++++++++++++++\n interdiff.h | 8 ++++++++\n revision.h | 4 ++++\n t/t4014-format-patch.sh | 17 +++++++++++++++++\n 7 files changed, 78 insertions(+), 2 deletions(-)\n create mode 100644 interdiff.c\n create mode 100644 interdiff.h\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex b41e1329a7..a1b1bafee7 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -23,6 +23,7 @@ SYNOPSIS\n \t\t [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet] [--notes[=]]\n+\t\t [--interdiff=]\n \t\t [--progress]\n \t\t []\n \t\t [ | ]\n@@ -228,6 +229,14 @@ feeding the result to `git send-email`.\n \tcontaining the branch description, shortlog and the overall diffstat. You can\n \tfill in a description in the file before sending it out.\n \n+--interdiff=::\n+\tAs a reviewer aid, insert an interdiff into the cover letter showing\n+\tthe differences between the previous version of the patch series and\n+\tthe series currently being formatted. `previous` is a single revision\n+\tnaming the tip of the previous series which shares a common base with\n+\tthe series being formatted (for example `git format-patch\n+\t--cover-letter --interdiff=feature/v1 -3 feature/v2`).\n+\n --notes[=]::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\n \tafter the three-dash line.\ndiff --git a/Makefile b/Makefile\nindex 41b93689ad..2af389c0d9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -872,6 +872,7 @@ LIB_OBJS += linear-assignment.o\n LIB_OBJS += help.o\n LIB_OBJS += hex.o\n LIB_OBJS += ident.o\n+LIB_OBJS += interdiff.o\n LIB_OBJS += kwset.o\n LIB_OBJS += levenshtein.o\n LIB_OBJS += line-log.o\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 873aabcf40..1020b78477 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -30,6 +30,7 @@\n #include \"gpg-interface.h\"\n #include \"progress.h\"\n #include \"commit-slab.h\"\n+#include \"interdiff.h\"\n \n #define MAIL_DEFAULT_WRAP 72\n \n@@ -1082,6 +1083,11 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t/* We can only do diffstat with a unique reference point */\n \tif (origin)\n \t\tshow_diffstat(rev, origin, head);\n+\n+\tif (rev->idiff_oid1) {\n+\t\tfprintf_ln(rev->diffopt.file, \"%s\", _(\"Interdiff:\"));\n+\t\tshow_interdiff(rev);\n+\t}\n }\n \n static const char *clean_message_id(const char *msg_id)\n@@ -1448,6 +1454,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct base_tree_info bases;\n \tint show_progress = 0;\n \tstruct progress *progress = NULL;\n+\tstruct oid_array idiff_prev = OID_ARRAY_INIT;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n@@ -1521,6 +1528,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT__QUIET(&quiet, N_(\"don't print the patch filenames\")),\n \t\tOPT_BOOL(0, \"progress\", &show_progress,\n \t\t\t N_(\"show progress while generating patches\")),\n+\t\tOPT_CALLBACK(0, \"interdiff\", &idiff_prev, N_(\"rev\"),\n+\t\t\t N_(\"show changes against in cover letter\"),\n+\t\t\t parse_opt_object_name),\n \t\tOPT_END()\n \t};\n \n@@ -1706,7 +1716,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (rev.pending.nr == 2) {\n \t\t\tstruct object_array_entry *o = rev.pending.objects;\n \t\t\tif (oidcmp(&o[0].item->oid, &o[1].item->oid) == 0)\n-\t\t\t\treturn 0;\n+\t\t\t\tgoto done;\n \t\t}\n \t\tget_patch_ids(&rev, &ids);\n \t}\n@@ -1730,7 +1740,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t}\n \tif (nr == 0)\n \t\t/* nothing to do */\n-\t\treturn 0;\n+\t\tgoto done;\n \ttotal = nr;\n \tif (cover_letter == -1) {\n \t\tif (config_cover_letter == COVER_AUTO)\n@@ -1743,6 +1753,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (numbered)\n \t\trev.total = total + start_number - 1;\n \n+\tif (idiff_prev.nr) {\n+\t\tif (!cover_letter)\n+\t\t\tdie(_(\"--interdiff requires --cover-letter\"));\n+\t\trev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];\n+\t\trev.idiff_oid2 = get_commit_tree_oid(list[0]);\n+\t}\n+\n \tif (!signature) {\n \t\t; /* --no-signature inhibits all signatures */\n \t} else if (signature && signature != git_version_string) {\n@@ -1860,6 +1877,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstring_list_clear(&extra_hdr, 0);\n \tif (ignore_if_in_upstream)\n \t\tfree_patch_ids(&ids);\n+\n+done:\n+\toid_array_clear(&idiff_prev);\n \treturn 0;\n }\n \ndiff --git a/interdiff.c b/interdiff.c\nnew file mode 100644\nindex 0000000000..d0fac10c7c\n--- /dev/null\n+++ b/interdiff.c\n@@ -0,0 +1,17 @@\n+#include \"cache.h\"\n+#include \"commit.h\"\n+#include \"revision.h\"\n+#include \"interdiff.h\"\n+\n+void show_interdiff(struct rev_info *rev)\n+{\n+\tstruct diff_options opts;\n+\n+\tmemcpy(&opts, &rev->diffopt, sizeof(opts));\n+\topts.output_format = DIFF_FORMAT_PATCH;\n+\tdiff_setup_done(&opts);\n+\n+\tdiff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, \"\", &opts);\n+\tdiffcore_std(&opts);\n+\tdiff_flush(&opts);\n+}\ndiff --git a/interdiff.h b/interdiff.h\nnew file mode 100644\nindex 0000000000..793c0144fe\n--- /dev/null\n+++ b/interdiff.h\n@@ -0,0 +1,8 @@\n+#ifndef INTERDIFF_H\n+#define INTERDIFF_H\n+\n+struct rev_info;\n+\n+void show_interdiff(struct rev_info *);\n+\n+#endif\ndiff --git a/revision.h b/revision.h\nindex bf2239f876..61931fbac5 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -212,6 +212,10 @@ struct rev_info {\n \t/* notes-specific options: which refs to show */\n \tstruct display_notes_opt notes_opt;\n \n+\t/* interdiff */\n+\tconst struct object_id *idiff_oid1;\n+\tconst struct object_id *idiff_oid2;\n+\n \t/* commit counts */\n \tint count_left;\n \tint count_right;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 53880da7bb..57b46322aa 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1717,4 +1717,21 @@ test_expect_success 'format-patch --pretty=mboxrd' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'interdiff: setup' '\n+\tgit checkout -b boop master &&\n+\ttest_commit fnorp blorp &&\n+\ttest_commit fleep blorp\n+'\n+\n+test_expect_success 'interdiff: cover-letter' '\n+\tsed \"y/q/ /\" >expect <<-\\EOF &&\n+\t+fleep\n+\t--q\n+\tEOF\n+\tgit format-patch --cover-letter --interdiff=boop~2 -1 boop &&\n+\ttest_i18ngrep \"^Interdiff:$\" 0000-cover-letter.patch &&\n+\tsed \"1,/^@@ /d; /^-- $/q\" <0000-cover-letter.patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353309","messageId":"20180722095717.17912-4-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:06Z","receivedAt":"2018-07-22T09:58:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The --interdiff option introduces the embedded interdiff generically as\n\"Interdiff:\", however, we can do better when --reroll-count is specified\nby emitting \"Interdiff against v{n}:\" instead.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/log.c | 17 ++++++++++++++++-\n revision.h | 1 +\n t/t4014-format-patch.sh | 5 +++++\n 3 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 1020b78477..99ddfe8bb0 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1085,7 +1085,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\tshow_diffstat(rev, origin, head);\n \n \tif (rev->idiff_oid1) {\n-\t\tfprintf_ln(rev->diffopt.file, \"%s\", _(\"Interdiff:\"));\n+\t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->idiff_title);\n \t\tshow_interdiff(rev);\n \t}\n }\n@@ -1427,6 +1427,16 @@ static void print_bases(struct base_tree_info *bases, FILE *file)\n \toidclr(&bases->base_commit);\n }\n \n+static const char *diff_title(struct strbuf *sb, int reroll_count,\n+\t\t const char *generic, const char *rerolled)\n+{\n+\tif (reroll_count <= 0)\n+\t\tstrbuf_addstr(sb, generic);\n+\telse /* RFC may be v0, so allow -v1 to diff against v0 */\n+\t\tstrbuf_addf(sb, rerolled, reroll_count - 1);\n+\treturn sb->buf;\n+}\n+\n int cmd_format_patch(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit *commit;\n@@ -1455,6 +1465,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint show_progress = 0;\n \tstruct progress *progress = NULL;\n \tstruct oid_array idiff_prev = OID_ARRAY_INIT;\n+\tstruct strbuf idiff_title = STRBUF_INIT;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n@@ -1758,6 +1769,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"--interdiff requires --cover-letter\"));\n \t\trev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];\n \t\trev.idiff_oid2 = get_commit_tree_oid(list[0]);\n+\t\trev.idiff_title = diff_title(&idiff_title, reroll_count,\n+\t\t\t\t\t _(\"Interdiff:\"),\n+\t\t\t\t\t _(\"Interdiff against v%d:\"));\n \t}\n \n \tif (!signature) {\n@@ -1880,6 +1894,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \n done:\n \toid_array_clear(&idiff_prev);\n+\tstrbuf_release(&idiff_title);\n \treturn 0;\n }\n \ndiff --git a/revision.h b/revision.h\nindex 61931fbac5..ffeadc261a 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -215,6 +215,7 @@ struct rev_info {\n \t/* interdiff */\n \tconst struct object_id *idiff_oid1;\n \tconst struct object_id *idiff_oid2;\n+\tconst char *idiff_title;\n \n \t/* commit counts */\n \tint count_left;\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 57b46322aa..5950890d30 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1734,4 +1734,9 @@ test_expect_success 'interdiff: cover-letter' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'interdiff: reroll-count' '\n+\tgit format-patch --cover-letter --interdiff=boop~2 -v2 -1 boop &&\n+\ttest_i18ngrep \"^Interdiff ..* v1:$\" v2-0000-cover-letter.patch\n+'\n+\n test_done\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353310","messageId":"20180722095717.17912-5-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 04/14] interdiff: teach show_interdiff() to indent interdiff","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:07Z","receivedAt":"2018-07-22T09:58:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"A future change will allow \"git format-patch --interdiff= -1\" to\ninsert an interdiff into the commentary section of the lone patch of a\n1-patch series. However, to prevent the inserted interdiff from\nconfusing git-am, as well as human readers, it needs to be indented.\nTherefore, teach show_interdiff() how to indent.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/log.c | 2 +-\n interdiff.c | 13 ++++++++++++-\n interdiff.h | 2 +-\n 3 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 99ddfe8bb0..8078a43d14 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1086,7 +1086,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \n \tif (rev->idiff_oid1) {\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->idiff_title);\n-\t\tshow_interdiff(rev);\n+\t\tshow_interdiff(rev, 0);\n \t}\n }\n \ndiff --git a/interdiff.c b/interdiff.c\nindex d0fac10c7c..c81d680a6c 100644\n--- a/interdiff.c\n+++ b/interdiff.c\n@@ -3,15 +3,26 @@\n #include \"revision.h\"\n #include \"interdiff.h\"\n \n-void show_interdiff(struct rev_info *rev)\n+static struct strbuf *idiff_prefix_cb(struct diff_options *opt, void *data)\n+{\n+\treturn data;\n+}\n+\n+void show_interdiff(struct rev_info *rev, int indent)\n {\n \tstruct diff_options opts;\n+\tstruct strbuf prefix = STRBUF_INIT;\n \n \tmemcpy(&opts, &rev->diffopt, sizeof(opts));\n \topts.output_format = DIFF_FORMAT_PATCH;\n+\topts.output_prefix = idiff_prefix_cb;\n+\tstrbuf_addchars(&prefix, ' ', indent);\n+\topts.output_prefix_data = &prefix;\n \tdiff_setup_done(&opts);\n \n \tdiff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, \"\", &opts);\n \tdiffcore_std(&opts);\n \tdiff_flush(&opts);\n+\n+\tstrbuf_release(&prefix);\n }\ndiff --git a/interdiff.h b/interdiff.h\nindex 793c0144fe..01c730a5c9 100644\n--- a/interdiff.h\n+++ b/interdiff.h\n@@ -3,6 +3,6 @@\n \n struct rev_info;\n \n-void show_interdiff(struct rev_info *);\n+void show_interdiff(struct rev_info *, int indent);\n \n #endif\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353311","messageId":"20180722095717.17912-6-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 05/14] log-tree: show_log: make commentary block delimiting reusable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:08Z","receivedAt":"2018-07-22T09:58:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"In patches generated by git-format-patch, the area below the \"---\" line\nfollowing the commit message and before the actual 'diff' can be used\nfor commentary which the patch author wants to convey to readers of the\npatch itself but not include in the commit message proper.\n\nBy default, the commentary area is empty, however, the --notes option\ncauses it to be populated with notes associated with the commit. In the\nfuture, other options may be added which also insert content into the\ncommentary section.\n\nTo accommodate this, factor out the logic which delimits commentary\nblocks from the commit message so that it can be re-used for upcoming\noptional inserted content.\n\nSigned-off-by: Eric Sunshine \n---\n log-tree.c | 23 ++++++++++++++++-------\n 1 file changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 4a3907fea0..9d38f1cf79 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -541,6 +541,16 @@ static int show_mergetag(struct rev_info *opt, struct commit *commit)\n \treturn for_each_mergetag(show_one_mergetag, commit, opt);\n }\n \n+static void next_commentary_block(struct rev_info *opt, struct strbuf *sb)\n+{\n+\tconst char *x = opt->shown_dashes ? \"\\n\" : \"---\\n\";\n+\tif (sb)\n+\t\tstrbuf_addstr(sb, x);\n+\telse\n+\t\tfputs(x, opt->diffopt.file);\n+\topt->shown_dashes = 1;\n+}\n+\n void show_log(struct rev_info *opt)\n {\n \tstruct strbuf msgbuf = STRBUF_INIT;\n@@ -698,10 +708,8 @@ void show_log(struct rev_info *opt)\n \n \tif ((ctx.fmt != CMIT_FMT_USERFORMAT) &&\n \t ctx.notes_message && *ctx.notes_message) {\n-\t\tif (cmit_fmt_is_mail(ctx.fmt)) {\n-\t\t\tstrbuf_addstr(&msgbuf, \"---\\n\");\n-\t\t\topt->shown_dashes = 1;\n-\t\t}\n+\t\tif (cmit_fmt_is_mail(ctx.fmt))\n+\t\t\tnext_commentary_block(opt, &msgbuf);\n \t\tstrbuf_addstr(&msgbuf, ctx.notes_message);\n \t}\n \n@@ -765,9 +773,10 @@ int log_tree_diff_flush(struct rev_info *opt)\n \n \t\t\t/*\n \t\t\t * We may have shown three-dashes line early\n-\t\t\t * between notes and the log message, in which\n-\t\t\t * case we only want a blank line after the\n-\t\t\t * notes without (an extra) three-dashes line.\n+\t\t\t * between generated commentary (notes, etc.)\n+\t\t\t * and the log message, in which case we only\n+\t\t\t * want a blank line after the commentary\n+\t\t\t * without (an extra) three-dashes line.\n \t\t\t * Otherwise, we show the three-dashes line if\n \t\t\t * we are showing the patch with diffstat, but\n \t\t\t * in that case, there is no extra blank line\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353312","messageId":"20180722095717.17912-7-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:09Z","receivedAt":"2018-07-22T09:58:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When submitting a revised version of a patch or series, it can be\nhelpful (to reviewers) to include a summary of changes since the\nprevious attempt in the form of an interdiff, typically in the cover\nletter. However, it is occasionally useful, despite making for a noisy\nread, to insert an interdiff into the commentary section of the lone\npatch of a 1-patch series.\n\nTherefore, extend \"git format-patch --interdiff=\" to insert an\ninterdiff into the commentary section of a lone patch rather than\nrequiring a cover letter. The interdiff is indented to avoid confusing\ngit-am and human readers into considering it part of the patch proper.\n\nImplementation note: Generating an interdiff for insertion into the\ncommentary section of a patch which itself is currently being generated\nrequires invoking the diffing machinery recursively. However, the\nmachinery does not (presently) support this since it uses global state.\nConsequently, we need to take care to stash away the state of the\nin-progress operation while generating the interdiff, and restore it\nafter.\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/git-format-patch.txt | 3 ++-\n builtin/log.c | 8 +++++---\n log-tree.c | 14 ++++++++++++++\n t/t4014-format-patch.sh | 12 ++++++++++++\n 4 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex a1b1bafee7..f8a061794d 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -230,7 +230,8 @@ feeding the result to `git send-email`.\n \tfill in a description in the file before sending it out.\n \n --interdiff=::\n-\tAs a reviewer aid, insert an interdiff into the cover letter showing\n+\tAs a reviewer aid, insert an interdiff into the cover letter,\n+\tor as commentary of the lone patch of a 1-patch series, showing\n \tthe differences between the previous version of the patch series and\n \tthe series currently being formatted. `previous` is a single revision\n \tnaming the tip of the previous series which shares a common base with\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 8078a43d14..e990027c28 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1540,7 +1540,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"progress\", &show_progress,\n \t\t\t N_(\"show progress while generating patches\")),\n \t\tOPT_CALLBACK(0, \"interdiff\", &idiff_prev, N_(\"rev\"),\n-\t\t\t N_(\"show changes against in cover letter\"),\n+\t\t\t N_(\"show changes against in cover letter or single patch\"),\n \t\t\t parse_opt_object_name),\n \t\tOPT_END()\n \t};\n@@ -1765,8 +1765,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\trev.total = total + start_number - 1;\n \n \tif (idiff_prev.nr) {\n-\t\tif (!cover_letter)\n-\t\t\tdie(_(\"--interdiff requires --cover-letter\"));\n+\t\tif (!cover_letter && total != 1)\n+\t\t\tdie(_(\"--interdiff requires --cover-letter or single patch\"));\n \t\trev.idiff_oid1 = &idiff_prev.oid[idiff_prev.nr - 1];\n \t\trev.idiff_oid2 = get_commit_tree_oid(list[0]);\n \t\trev.idiff_title = diff_title(&idiff_title, reroll_count,\n@@ -1811,6 +1811,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\n \t\tstart_number--;\n+\t\t/* interdiff in cover-letter; omit from patches */\n+\t\trev.idiff_oid1 = NULL;\n \t}\n \trev.add_signoff = do_signoff;\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 9d38f1cf79..56513fa83d 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -14,6 +14,7 @@\n #include \"sequencer.h\"\n #include \"line-log.h\"\n #include \"help.h\"\n+#include \"interdiff.h\"\n \n static struct decoration name_decoration = { \"object names\" };\n static int decoration_loaded;\n@@ -736,6 +737,19 @@ void show_log(struct rev_info *opt)\n \n \tstrbuf_release(&msgbuf);\n \tfree(ctx.notes_message);\n+\n+\tif (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {\n+\t\tstruct diff_queue_struct dq;\n+\n+\t\tmemcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n+\t\tDIFF_QUEUE_CLEAR(&diff_queued_diff);\n+\n+\t\tnext_commentary_block(opt, NULL);\n+\t\tfprintf_ln(opt->diffopt.file, \"%s\", opt->idiff_title);\n+\t\tshow_interdiff(opt, 2);\n+\n+\t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n+\t}\n }\n \n int log_tree_diff_flush(struct rev_info *opt)\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 5950890d30..909c743c13 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1730,6 +1730,7 @@ test_expect_success 'interdiff: cover-letter' '\n \tEOF\n \tgit format-patch --cover-letter --interdiff=boop~2 -1 boop &&\n \ttest_i18ngrep \"^Interdiff:$\" 0000-cover-letter.patch &&\n+\ttest_i18ngrep ! \"^Interdiff:$\" 0001-fleep.patch &&\n \tsed \"1,/^@@ /d; /^-- $/q\" <0000-cover-letter.patch >actual &&\n \ttest_cmp expect actual\n '\n@@ -1739,4 +1740,15 @@ test_expect_success 'interdiff: reroll-count' '\n \ttest_i18ngrep \"^Interdiff ..* v1:$\" v2-0000-cover-letter.patch\n '\n \n+test_expect_success 'interdiff: solo-patch' '\n+\tcat >expect <<-\\EOF &&\n+\t +fleep\n+\n+\tEOF\n+\tgit format-patch --interdiff=boop~2 -1 boop &&\n+\ttest_i18ngrep \"^Interdiff:$\" 0001-fleep.patch &&\n+\tsed \"1,/^ @@ /d; /^$/q\" <0001-fleep.patch >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353313","messageId":"20180722095717.17912-9-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 08/14] range-diff: publish default creation factor","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:11Z","receivedAt":"2018-07-22T09:58:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The range-diff back-end allows its heuristic to be tweaked via the\n\"creation factor\". git-range-diff, the only client of the back-end,\ndefaults the factor to 60% (hard-coded in builtin/range-diff.c), but\nallows the user to override it with the --creation-factor option.\n\nPublish the default range factor to allow new callers of the range-diff\nback-end to default to the same value without duplicating the hard-coded\nconstant, and to avoid worrying about various callers becoming\nout-of-sync if the default ever needs to change.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/range-diff.c | 2 +-\n range-diff.h | 2 ++\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 77ac3bff7b..bbe6b05ae9 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -18,7 +18,7 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n \n int cmd_range_diff(int argc, const char **argv, const char *prefix)\n {\n-\tint creation_factor = 60;\n+\tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n \tstruct diff_options diffopt = { NULL };\n \tint simple_color = -1;\n \tstruct option options[] = {\ndiff --git a/range-diff.h b/range-diff.h\nindex aea9d43f34..73d6e232fe 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -3,6 +3,8 @@\n \n #include \"diff.h\"\n \n+#define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n+\n int show_range_diff(const char *range1, const char *range2,\n \t\t int creation_factor, struct diff_options *diffopt);\n \n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353314","messageId":"20180722095717.17912-10-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 09/14] range-diff: relieve callers of low-level configuration burden","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:12Z","receivedAt":"2018-07-22T09:58:07Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"There are a number of very low-level configuration details which need to\nbe managed precisely to generate a proper range-diff. In particular,\n'diff_options' output format, header suppression, indentation, and\ndual-color mode must all be set appropriately to ensure proper behavior.\n\nHandle these details locally in the libified range-diff back-end rather\nthan forcing each caller to have specialized knowledge of these\nimplementation details, and to avoid duplication as new callers are\nadded.\n\nWhile at it, localize these tweaks to be active only while generating\nthe range-diff, so they don't clobber the caller-provided\n'diff_options', which might be used beyond range-diff generation.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/range-diff.c | 23 ++++-------------------\n range-diff.c | 24 ++++++++++++++++++++++--\n range-diff.h | 3 ++-\n 3 files changed, 28 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex bbe6b05ae9..91463a1df6 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -11,11 +11,6 @@ N_(\"git range-diff [] \"),\n NULL\n };\n \n-static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n-{\n-\treturn data;\n-}\n-\n int cmd_range_diff(int argc, const char **argv, const char *prefix)\n {\n \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n@@ -29,17 +24,11 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \tint i, j, res = 0;\n-\tstruct strbuf four_spaces = STRBUF_INIT;\n \tstruct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;\n \n \tgit_config(git_diff_ui_config, NULL);\n \n \tdiff_setup(&diffopt);\n-\tdiffopt.output_format = DIFF_FORMAT_PATCH;\n-\tdiffopt.flags.suppress_diff_headers = 1;\n-\tdiffopt.output_prefix = output_prefix_cb;\n-\tstrbuf_addstr(&four_spaces, \" \");\n-\tdiffopt.output_prefix_data = &four_spaces;\n \n \targc = parse_options(argc, argv, NULL, options,\n \t\t\t builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN);\n@@ -55,12 +44,9 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \targc = j;\n \tdiff_setup_done(&diffopt);\n \n-\tif (simple_color < 1) {\n-\t\tif (!simple_color)\n-\t\t\t/* force color when --dual-color was used */\n-\t\t\tdiffopt.use_color = 1;\n-\t\tdiffopt.flags.dual_color_diffed_diffs = 1;\n-\t}\n+\t/* force color when --dual-color was used */\n+\tif (!simple_color)\n+\t\tdiffopt.use_color = 1;\n \n \tif (argc == 2) {\n \t\tif (!strstr(argv[0], \"..\"))\n@@ -96,11 +82,10 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t}\n \n \tres = show_range_diff(range1.buf, range2.buf, creation_factor,\n-\t\t\t &diffopt);\n+\t\t\t simple_color < 1, &diffopt);\n \n \tstrbuf_release(&range1);\n \tstrbuf_release(&range2);\n-\tstrbuf_release(&four_spaces);\n \n \treturn res;\n }\ndiff --git a/range-diff.c b/range-diff.c\nindex 76e053c2b2..091fc344cd 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -414,8 +414,14 @@ static void output(struct string_list *a, struct string_list *b,\n \tstrbuf_release(&dashes);\n }\n \n+static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n+{\n+\treturn data;\n+}\n+\n int show_range_diff(const char *range1, const char *range2,\n-\t\t int creation_factor, struct diff_options *diffopt)\n+\t\t int creation_factor, int dual_color,\n+\t\t struct diff_options *diffopt)\n {\n \tint res = 0;\n \n@@ -428,9 +434,23 @@ int show_range_diff(const char *range1, const char *range2,\n \t\tres = error(_(\"could not parse log for '%s'\"), range2);\n \n \tif (!res) {\n+\t\tstruct diff_options opts;\n+\t\tstruct strbuf indent = STRBUF_INIT;\n+\n+\t\tmemcpy(&opts, diffopt, sizeof(opts));\n+\t\topts.output_format = DIFF_FORMAT_PATCH;\n+\t\topts.flags.suppress_diff_headers = 1;\n+\t\topts.flags.dual_color_diffed_diffs = dual_color;\n+\t\topts.output_prefix = output_prefix_cb;\n+\t\tstrbuf_addstr(&indent, \" \");\n+\t\topts.output_prefix_data = &indent;\n+\t\tdiff_setup_done(&opts);\n+\n \t\tfind_exact_matches(&branch1, &branch2);\n \t\tget_correspondences(&branch1, &branch2, creation_factor);\n-\t\toutput(&branch1, &branch2, diffopt);\n+\t\toutput(&branch1, &branch2, &opts);\n+\n+\t\tstrbuf_release(&indent);\n \t}\n \n \tstring_list_clear(&branch1, 1);\ndiff --git a/range-diff.h b/range-diff.h\nindex 73d6e232fe..bf265594f3 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -6,6 +6,7 @@\n #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n \n int show_range_diff(const char *range1, const char *range2,\n-\t\t int creation_factor, struct diff_options *diffopt);\n+\t\t int creation_factor, int dual_color,\n+\t\t struct diff_options *diffopt);\n \n #endif\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353315","messageId":"20180722095717.17912-11-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:13Z","receivedAt":"2018-07-22T09:58:09Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When submitting a revised version of a patch series, it can be helpful\n(to reviewers) to include a summary of changes since the previous\nattempt in the form of a range-diff, however, doing so involves manually\ncopy/pasting the diff into the cover letter.\n\nAdd a --range-diff option to automate this process. The argument to\n--range-diff specifies the tip of the previous attempt against which to\ngenerate the range-diff. For example:\n\n git format-patch --cover-letter --range-diff=v1 -3 v2\n\n(At this stage, the previous attempt and the patch series being\nformatted must share a common base, however, a subsequent enhancement\nwill make it possible to specify an explicit revision range for the\nprevious attempt.)\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/git-format-patch.txt | 10 +++++++++\n builtin/log.c | 35 ++++++++++++++++++++++++++++++\n revision.h | 5 +++++\n t/t3206-range-diff.sh | 12 ++++++++++\n 4 files changed, 62 insertions(+)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex f8a061794d..e7f404be3d 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -24,6 +24,7 @@ SYNOPSIS\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet] [--notes[=]]\n \t\t [--interdiff=]\n+\t\t [--range-diff=]\n \t\t [--progress]\n \t\t []\n \t\t [ | ]\n@@ -238,6 +239,15 @@ feeding the result to `git send-email`.\n \tthe series being formatted (for example `git format-patch\n \t--cover-letter --interdiff=feature/v1 -3 feature/v2`).\n \n+--range-diff=::\n+\tAs a reviewer aid, insert a range-diff (see linkgit:git-range-diff[1])\n+\tinto the cover letter showing the differences between the previous\n+\tversion of the patch series and the series currently being formatted.\n+\t`previous` is a single revision naming the tip of the previous\n+\tseries which shares a common base with the series being formatted (for\n+\texample `git format-patch --cover-letter --range-diff=feature/v1 -3\n+\tfeature/v2`).\n+\n --notes[=]::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\n \tafter the three-dash line.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e990027c28..d6e57e8b04 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -31,6 +31,7 @@\n #include \"progress.h\"\n #include \"commit-slab.h\"\n #include \"interdiff.h\"\n+#include \"range-diff.h\"\n \n #define MAIL_DEFAULT_WRAP 72\n \n@@ -1088,6 +1089,12 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->idiff_title);\n \t\tshow_interdiff(rev, 0);\n \t}\n+\n+\tif (rev->rdiff1) {\n+\t\tfprintf_ln(rev->diffopt.file, \"%s\", _(\"Range-diff:\"));\n+\t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n+\t\t\t\trev->creation_factor, 1, &rev->diffopt);\n+\t}\n }\n \n static const char *clean_message_id(const char *msg_id)\n@@ -1437,6 +1444,17 @@ static const char *diff_title(struct strbuf *sb, int reroll_count,\n \treturn sb->buf;\n }\n \n+static void infer_range_diff_ranges(struct strbuf *r1,\n+\t\t\t\t struct strbuf *r2,\n+\t\t\t\t const char *prev,\n+\t\t\t\t struct commit *head)\n+{\n+\tconst char *head_oid = oid_to_hex(&head->object.oid);\n+\n+\tstrbuf_addf(r1, \"%s..%s\", head_oid, prev);\n+\tstrbuf_addf(r2, \"%s..%s\", prev, head_oid);\n+}\n+\n int cmd_format_patch(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit *commit;\n@@ -1466,6 +1484,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct progress *progress = NULL;\n \tstruct oid_array idiff_prev = OID_ARRAY_INIT;\n \tstruct strbuf idiff_title = STRBUF_INIT;\n+\tconst char *rdiff_prev = NULL;\n+\tstruct strbuf rdiff1 = STRBUF_INIT;\n+\tstruct strbuf rdiff2 = STRBUF_INIT;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n@@ -1542,6 +1563,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK(0, \"interdiff\", &idiff_prev, N_(\"rev\"),\n \t\t\t N_(\"show changes against in cover letter or single patch\"),\n \t\t\t parse_opt_object_name),\n+\t\tOPT_STRING(0, \"range-diff\", &rdiff_prev, N_(\"refspec\"),\n+\t\t\t N_(\"show changes against in cover letter\")),\n \t\tOPT_END()\n \t};\n \n@@ -1774,6 +1797,16 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t _(\"Interdiff against v%d:\"));\n \t}\n \n+\tif (rdiff_prev) {\n+\t\tif (!cover_letter)\n+\t\t\tdie(_(\"--range-diff requires --cover-letter\"));\n+\n+\t\tinfer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev, list[0]);\n+\t\trev.rdiff1 = rdiff1.buf;\n+\t\trev.rdiff2 = rdiff2.buf;\n+\t\trev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n+\t}\n+\n \tif (!signature) {\n \t\t; /* --no-signature inhibits all signatures */\n \t} else if (signature && signature != git_version_string) {\n@@ -1897,6 +1930,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n done:\n \toid_array_clear(&idiff_prev);\n \tstrbuf_release(&idiff_title);\n+\tstrbuf_release(&rdiff1);\n+\tstrbuf_release(&rdiff2);\n \treturn 0;\n }\n \ndiff --git a/revision.h b/revision.h\nindex ffeadc261a..11159416dc 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -217,6 +217,11 @@ struct rev_info {\n \tconst struct object_id *idiff_oid2;\n \tconst char *idiff_title;\n \n+\t/* range-diff */\n+\tconst char *rdiff1;\n+\tconst char *rdiff2;\n+\tint creation_factor;\n+\n \t/* commit counts */\n \tint count_left;\n \tint count_right;\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 2237c7f4af..dd854b6ebc 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -142,4 +142,16 @@ test_expect_success 'changed message' '\n \ttest_cmp expected actual\n '\n \n+for prev in topic\n+do\n+\ttest_expect_success \"format-patch --range-diff=$prev\" '\n+\t\tgit format-patch --stdout --cover-letter --range-diff=$prev \\\n+\t\t\tmaster..unmodified >actual &&\n+\t\tgrep \"= 1: .* s/5/A\" actual &&\n+\t\tgrep \"= 2: .* s/4/A\" actual &&\n+\t\tgrep \"= 3: .* s/11/B\" actual &&\n+\t\tgrep \"= 4: .* s/12/B\" actual\n+\t'\n+done\n+\n test_done\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353316","messageId":"20180722095717.17912-8-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:10Z","receivedAt":"2018-07-22T09:58:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The actual diffs output by range-diff respect diff_option.file, which\nrange-diff passes down the call-chain, thus are destination-agnostic.\nHowever, output_pair_header() is hard-coded to emit to 'stdout'. Fix\nthis by making output_pair_header() respect diff_option.file, as well.\n\nSigned-off-by: Eric Sunshine \n---\n range-diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 347b4a79f2..76e053c2b2 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -328,7 +328,7 @@ static void output_pair_header(struct diff_options *diffopt,\n \t}\n \tstrbuf_addf(buf, \"%s\\n\", color_reset);\n \n-\tfwrite(buf->buf, buf->len, 1, stdout);\n+\tfwrite(buf->buf, buf->len, 1, diffopt->file);\n }\n \n static struct userdiff_driver no_func_name = {\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353317","messageId":"20180722095717.17912-13-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 12/14] format-patch: teach --range-diff to respect -v/--reroll-count","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:15Z","receivedAt":"2018-07-22T09:58:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"The --range-diff option announces the embedded range-diff generically\nas \"Range-diff:\", however, we can do better when --reroll-count is\nspecified by emitting \"Range-diff against v{n}:\" instead.\n\nSigned-off-by: Eric Sunshine \n---\n builtin/log.c | 7 ++++++-\n revision.h | 1 +\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 4f937aad15..fdb2180d7e 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1091,7 +1091,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t}\n \n \tif (rev->rdiff1) {\n-\t\tfprintf_ln(rev->diffopt.file, \"%s\", _(\"Range-diff:\"));\n+\t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n \t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n \t\t\t\trev->creation_factor, 1, &rev->diffopt);\n \t}\n@@ -1496,6 +1496,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tconst char *rdiff_prev = NULL;\n \tstruct strbuf rdiff1 = STRBUF_INIT;\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n+\tstruct strbuf rdiff_title = STRBUF_INIT;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n@@ -1815,6 +1816,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\trev.rdiff1 = rdiff1.buf;\n \t\trev.rdiff2 = rdiff2.buf;\n \t\trev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n+\t\trev.rdiff_title = diff_title(&rdiff_title, reroll_count,\n+\t\t\t\t\t _(\"Range-diff:\"),\n+\t\t\t\t\t _(\"Range-diff against v%d:\"));\n \t}\n \n \tif (!signature) {\n@@ -1942,6 +1946,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&idiff_title);\n \tstrbuf_release(&rdiff1);\n \tstrbuf_release(&rdiff2);\n+\tstrbuf_release(&rdiff_title);\n \treturn 0;\n }\n \ndiff --git a/revision.h b/revision.h\nindex 11159416dc..cd0873b575 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -221,6 +221,7 @@ struct rev_info {\n \tconst char *rdiff1;\n \tconst char *rdiff2;\n \tint creation_factor;\n+\tconst char *rdiff_title;\n \n \t/* commit counts */\n \tint count_left;\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353318","messageId":"20180722095717.17912-15-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:17Z","receivedAt":"2018-07-22T09:58:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When submitting a revised version of a patch or series, it can be\nhelpful (to reviewers) to include a summary of changes since the\nprevious attempt in the form of a range-diff, typically in the cover\nletter. However, it is occasionally useful, despite making for a noisy\nread, to insert a range-diff into the commentary section of the lone\npatch of a 1-patch series.\n\nTherefore, extend \"git format-patch --range-diff=\" to insert a\nrange-diff into the commentary section of a lone patch rather than\nrequiring a cover letter.\n\nImplementation note: Generating a range-diff for insertion into the\ncommentary section of a patch which itself is currently being generated\nrequires invoking the diffing machinery recursively. However, the\nmachinery does not (presently) support this since it uses global state.\nConsequently, we need to take care to stash away the state of the\nin-progress operation while generating the range-diff, and restore it\nafter.\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/git-format-patch.txt | 3 ++-\n builtin/log.c | 9 +++++----\n log-tree.c | 15 +++++++++++++++\n 3 files changed, 22 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 9b2e172159..aba4c5febe 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -241,7 +241,8 @@ feeding the result to `git send-email`.\n \n --range-diff=::\n \tAs a reviewer aid, insert a range-diff (see linkgit:git-range-diff[1])\n-\tinto the cover letter showing the differences between the previous\n+\tinto the cover letter, or as commentary of the lone patch of a\n+\t1-patch series, showing the differences between the previous\n \tversion of the patch series and the series currently being formatted.\n \t`previous` can be a single revision naming the tip of the previous\n \tseries if it shares a common base with the series being formatted (for\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 10c3801ceb..f0e99256a0 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1575,7 +1575,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"show changes against in cover letter or single patch\"),\n \t\t\t parse_opt_object_name),\n \t\tOPT_STRING(0, \"range-diff\", &rdiff_prev, N_(\"refspec\"),\n-\t\t\t N_(\"show changes against in cover letter\")),\n+\t\t\t N_(\"show changes against in cover letter or single patch\")),\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n@@ -1816,8 +1816,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--creation-factor requires --range-diff\"));\n \n \tif (rdiff_prev) {\n-\t\tif (!cover_letter)\n-\t\t\tdie(_(\"--range-diff requires --cover-letter\"));\n+\t\tif (!cover_letter && total != 1)\n+\t\t\tdie(_(\"--range-diff requires --cover-letter or single patch\"));\n \n \t\tinfer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev,\n \t\t\t\t\torigin, list[0]);\n@@ -1866,8 +1866,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\n \t\tstart_number--;\n-\t\t/* interdiff in cover-letter; omit from patches */\n+\t\t/* interdiff/range-diff in cover-letter; omit from patches */\n \t\trev.idiff_oid1 = NULL;\n+\t\trev.rdiff1 = NULL;\n \t}\n \trev.add_signoff = do_signoff;\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 56513fa83d..37afc585dc 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -15,6 +15,7 @@\n #include \"line-log.h\"\n #include \"help.h\"\n #include \"interdiff.h\"\n+#include \"range-diff.h\"\n \n static struct decoration name_decoration = { \"object names\" };\n static int decoration_loaded;\n@@ -750,6 +751,20 @@ void show_log(struct rev_info *opt)\n \n \t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n \t}\n+\n+\tif (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {\n+\t\tstruct diff_queue_struct dq;\n+\n+\t\tmemcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n+\t\tDIFF_QUEUE_CLEAR(&diff_queued_diff);\n+\n+\t\tnext_commentary_block(opt, NULL);\n+\t\tfprintf_ln(opt->diffopt.file, \"%s\", opt->rdiff_title);\n+\t\tshow_range_diff(opt->rdiff1, opt->rdiff2,\n+\t\t\t\topt->creation_factor, 1, &opt->diffopt);\n+\n+\t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n+\t}\n }\n \n int log_tree_diff_flush(struct rev_info *opt)\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353319","messageId":"20180722095717.17912-14-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 13/14] format-patch: add --creation-factor tweak for --range-diff","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:16Z","receivedAt":"2018-07-22T09:58:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When generating a range-diff, matching up commits between two version of\na patch series involves heuristics, thus may give unexpected results.\ngit-range-diff allows tweaking the heuristic via --creation-factor.\nFollow suit by accepting --creation-factor in combination with\n--range-diff when generating a range-diff for a cover-letter.\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/git-format-patch.txt | 8 +++++++-\n builtin/log.c | 10 +++++++++-\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 425145f536..9b2e172159 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -24,7 +24,7 @@ SYNOPSIS\n \t\t [--to=] [--cc=]\n \t\t [--[no-]cover-letter] [--quiet] [--notes[=]]\n \t\t [--interdiff=]\n-\t\t [--range-diff=]\n+\t\t [--range-diff= [--creation-factor=]]\n \t\t [--progress]\n \t\t []\n \t\t [ | ]\n@@ -250,6 +250,12 @@ feeding the result to `git send-email`.\n \tdisjoint (for example `git format-patch --cover-letter\n \t--range-diff=feature/v1~3..feature/v1 -3 feature/v2`).\n \n+--creation-factor=::\n+\tUsed with `--range-diff`, tweak the heuristic which matches up commits\n+\tbetween the previous and current series of patches by adjusting the\n+\tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n+\tfor details.\n+\n --notes[=]::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\n \tafter the three-dash line.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex fdb2180d7e..10c3801ceb 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1497,6 +1497,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff1 = STRBUF_INIT;\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n+\tint creation_factor = -1;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\t{ OPTION_CALLBACK, 'n', \"numbered\", &numbered, NULL,\n@@ -1575,6 +1576,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t parse_opt_object_name),\n \t\tOPT_STRING(0, \"range-diff\", &rdiff_prev, N_(\"refspec\"),\n \t\t\t N_(\"show changes against in cover letter\")),\n+\t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n+\t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n \t};\n \n@@ -1807,6 +1810,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t _(\"Interdiff against v%d:\"));\n \t}\n \n+\tif (creation_factor < 0)\n+\t\tcreation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n+\telse if (!rdiff_prev)\n+\t\tdie(_(\"--creation-factor requires --range-diff\"));\n+\n \tif (rdiff_prev) {\n \t\tif (!cover_letter)\n \t\t\tdie(_(\"--range-diff requires --cover-letter\"));\n@@ -1815,7 +1823,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t\t\torigin, list[0]);\n \t\trev.rdiff1 = rdiff1.buf;\n \t\trev.rdiff2 = rdiff2.buf;\n-\t\trev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n+\t\trev.creation_factor = creation_factor;\n \t\trev.rdiff_title = diff_title(&rdiff_title, reroll_count,\n \t\t\t\t\t _(\"Range-diff:\"),\n \t\t\t\t\t _(\"Range-diff against v%d:\"));\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353320","messageId":"20180722095717.17912-12-sunshine@sunshineco.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"[PATCH 11/14] format-patch: extend --range-diff to accept revision range","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T09:57:14Z","receivedAt":"2018-07-22T09:58:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"When submitting a revised a patch series, the --range-diff option embeds\na range-diff in the cover letter showing changes since the previous\nversion of the patch series. The argument to --range-diff is a simple\nrevision naming the tip of the previous series, which works fine if the\nprevious and current versions of the patch series share a common base.\n\nHowever, it fails if the revision ranges of the old and new versions of\nthe series are disjoint. To address this shortcoming, extend\n--range-diff to also accept an explicit revision range for the previous\nseries. For example:\n\n git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2\n\nSigned-off-by: Eric Sunshine \n---\n Documentation/git-format-patch.txt | 8 +++++---\n builtin/log.c | 16 +++++++++++++---\n t/t3206-range-diff.sh | 2 +-\n 3 files changed, 19 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex e7f404be3d..425145f536 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -243,10 +243,12 @@ feeding the result to `git send-email`.\n \tAs a reviewer aid, insert a range-diff (see linkgit:git-range-diff[1])\n \tinto the cover letter showing the differences between the previous\n \tversion of the patch series and the series currently being formatted.\n-\t`previous` is a single revision naming the tip of the previous\n-\tseries which shares a common base with the series being formatted (for\n+\t`previous` can be a single revision naming the tip of the previous\n+\tseries if it shares a common base with the series being formatted (for\n \texample `git format-patch --cover-letter --range-diff=feature/v1 -3\n-\tfeature/v2`).\n+\tfeature/v2`), or a revision range if the two versions of the series are\n+\tdisjoint (for example `git format-patch --cover-letter\n+\t--range-diff=feature/v1~3..feature/v1 -3 feature/v2`).\n \n --notes[=]::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\ndiff --git a/builtin/log.c b/builtin/log.c\nindex d6e57e8b04..4f937aad15 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1447,12 +1447,21 @@ static const char *diff_title(struct strbuf *sb, int reroll_count,\n static void infer_range_diff_ranges(struct strbuf *r1,\n \t\t\t\t struct strbuf *r2,\n \t\t\t\t const char *prev,\n+\t\t\t\t struct commit *origin,\n \t\t\t\t struct commit *head)\n {\n \tconst char *head_oid = oid_to_hex(&head->object.oid);\n \n-\tstrbuf_addf(r1, \"%s..%s\", head_oid, prev);\n-\tstrbuf_addf(r2, \"%s..%s\", prev, head_oid);\n+\tif (!strstr(prev, \"..\")) {\n+\t\tstrbuf_addf(r1, \"%s..%s\", head_oid, prev);\n+\t\tstrbuf_addf(r2, \"%s..%s\", prev, head_oid);\n+\t} else if (!origin) {\n+\t\tdie(_(\"failed to infer range-diff ranges\"));\n+\t} else {\n+\t\tstrbuf_addstr(r1, prev);\n+\t\tstrbuf_addf(r2, \"%s..%s\",\n+\t\t\t oid_to_hex(&origin->object.oid), head_oid);\n+\t}\n }\n \n int cmd_format_patch(int argc, const char **argv, const char *prefix)\n@@ -1801,7 +1810,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (!cover_letter)\n \t\t\tdie(_(\"--range-diff requires --cover-letter\"));\n \n-\t\tinfer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev, list[0]);\n+\t\tinfer_range_diff_ranges(&rdiff1, &rdiff2, rdiff_prev,\n+\t\t\t\t\torigin, list[0]);\n \t\trev.rdiff1 = rdiff1.buf;\n \t\trev.rdiff2 = rdiff2.buf;\n \t\trev.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex dd854b6ebc..3d7a2d8a4d 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -142,7 +142,7 @@ test_expect_success 'changed message' '\n \ttest_cmp expected actual\n '\n \n-for prev in topic\n+for prev in topic master..topic\n do\n \ttest_expect_success \"format-patch --range-diff=$prev\" '\n \t\tgit format-patch --stdout --cover-letter --range-diff=$prev \\\n-- \n2.18.0.345.g5c9ce644c3\n\n"},{"id":"353322","messageId":"CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grM1b8Gw@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-3-sunshine@sunshineco.com","subject":"Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-22T10:31:56Z","receivedAt":"2018-07-22T10:32:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jul 22, 2018 at 5:57 AM Eric Sunshine wrote:\n> Add an --interdiff option to automate this process. The argument to\n> --interdiff specifies the tip of the previous attempt against which to\n> generate the interdiff.\n>\n> Signed-off-by: Eric Sunshine \n> ---\n> diff --git a/interdiff.c b/interdiff.c\n> @@ -0,0 +1,17 @@\n> +void show_interdiff(struct rev_info *rev)\n> +{\n> + struct diff_options opts;\n> +\n> + memcpy(&opts, &rev->diffopt, sizeof(opts));\n> + opts.output_format = DIFF_FORMAT_PATCH;\n> + diff_setup_done(&opts);\n> +\n> + diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, \"\", &opts);\n\nPassing a 'rev_info' to this function is overkill. It actually only\nneeds the two OID's and a 'diff_options'. I'll make the change if I\nneed to reroll for some other reason, otherwise I'll do it as a\nfollow-up patch.\n"},{"id":"353370","messageId":"CACsJy8C8RK6HkfoEYJGZg=sgtJS0WksHD3=7Souw3jYebRo=Sg@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-3-sunshine@sunshineco.com","subject":"Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-23T16:02:55Z","receivedAt":"2018-07-23T16:03:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n\n> diff --git a/interdiff.c b/interdiff.c\n> new file mode 100644\n> index 0000000000..d0fac10c7c\n> --- /dev/null\n> +++ b/interdiff.c\n> @@ -0,0 +1,17 @@\n> +#include \"cache.h\"\n> +#include \"commit.h\"\n> +#include \"revision.h\"\n> +#include \"interdiff.h\"\n> +\n> +void show_interdiff(struct rev_info *rev)\n> +{\n> + struct diff_options opts;\n> +\n> + memcpy(&opts, &rev->diffopt, sizeof(opts));\n> + opts.output_format = DIFF_FORMAT_PATCH;\n> + diff_setup_done(&opts);\n> +\n> + diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, \"\", &opts);\n> + diffcore_std(&opts);\n> + diff_flush(&opts);\n> +}\n\nIs it worth adding a new file just for a single function? I haven't\nread the rest of the series, but the cover letter's diffstat suggests\nthis is it. Is interdiff intended to become a lot more complicated in\nthe future? If not maybe just add this function in diff-lib.c\n-- \nDuy\n"},{"id":"353371","messageId":"CACsJy8AGwbrEFU2bTynXyQi1DmEYtwHrtmefMz+jD13GQs=9HQ@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-4-sunshine@sunshineco.com","subject":"Re: [PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-23T16:12:14Z","receivedAt":"2018-07-23T16:12:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n>\n> The --interdiff option introduces the embedded interdiff generically as\n> \"Interdiff:\", however, we can do better when --reroll-count is specified\n\nOh boy. --reroll-count was added in 2012 and here I am typing\n--subject-prefix='PATCH vX' everyday, thinking that somebody should\nreally do something about it. I've learned --reroll-count today!\n\n> diff --git a/revision.h b/revision.h\n> index 61931fbac5..ffeadc261a 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -215,6 +215,7 @@ struct rev_info {\n> /* interdiff */\n> const struct object_id *idiff_oid1;\n> const struct object_id *idiff_oid2;\n> + const char *idiff_title;\n\nI feel we're abusing struct rev_info a bit for this since this\ninterdiff thing is very builtin/log.c's business and not at all\nrelated to rev walk. Is it possible (and easy) to just pass\nidfff_title from cmd_format_patch to make_cover_letter()? If it's a\nlot of code, then I guess we can just leave it here.\n-- \nDuy\n"},{"id":"353372","messageId":"CACsJy8Aw6R8-3kDfhCqunXziajCg9O_1WrEYc4rfKa+-=m1D5g@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-7-sunshine@sunshineco.com","subject":"Re: [PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-23T16:22:00Z","receivedAt":"2018-07-23T16:22:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine wrote:\n> diff --git a/log-tree.c b/log-tree.c\n> index 9d38f1cf79..56513fa83d 100644\n> --- a/log-tree.c\n> +++ b/log-tree.c\n> @@ -14,6 +14,7 @@\n> #include \"sequencer.h\"\n> #include \"line-log.h\"\n> #include \"help.h\"\n> +#include \"interdiff.h\"\n>\n> static struct decoration name_decoration = { \"object names\" };\n> static int decoration_loaded;\n> @@ -736,6 +737,19 @@ void show_log(struct rev_info *opt)\n>\n> strbuf_release(&msgbuf);\n> free(ctx.notes_message);\n> +\n> + if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {\n\nOK putting idiff stuff in rev_info is probably the right choice. But\nwe all three fields prefixed with idiff_, maybe you could just add a\nnew struct \"idiff_options\" to contain them and add a pointer to that\nstruct in rev_info. Then \"opt->idiff\" is enough to know if idiff is\nrequested instead of relying on idiff_oid1 (seems too random).\n\n> + struct diff_queue_struct dq;\n> +\n> + memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n> + DIFF_QUEUE_CLEAR(&diff_queued_diff);\n> +\n> + next_commentary_block(opt, NULL);\n> + fprintf_ln(opt->diffopt.file, \"%s\", opt->idiff_title);\n> + show_interdiff(opt, 2);\n> +\n> + memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n> + }\n> }\n-- \nDuy\n"},{"id":"353373","messageId":"CACsJy8AOeiKp2JnG0h9mw40TdsNft80vUu573ORtqKMor7B+vw@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-11-sunshine@sunshineco.com","subject":"Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-23T16:28:16Z","receivedAt":"2018-07-23T16:28:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine wrote:\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index f8a061794d..e7f404be3d 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -24,6 +24,7 @@ SYNOPSIS\n> [--to=] [--cc=]\n> [--[no-]cover-letter] [--quiet] [--notes[=]]\n> [--interdiff=]\n> + [--range-diff=]\n\nI wonder if people will use both --interdiff= and\n--range-diff= often enough to justify a shortcut\n\"--all-kinds-of-diff=\" so that we don't have to type \ntwice. But I guess we don't have to worry about this right now.\n-- \nDuy\n"},{"id":"353375","messageId":"CACsJy8BgAMYykkNOJc5NgMj-X6SMyskU3iCzTKnL0CLRTdF2oA@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-1-sunshine@sunshineco.com","subject":"Re: [PATCH 00/14] format-patch: add --interdiff and --range-diff options","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-23T16:32:18Z","receivedAt":"2018-07-23T16:32:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n>\n> When re-submitting a patch series, it is often helpful (for reviewers)\n> to include an interdiff or range-diff against the previous version.\n> Doing so requires manually running git-diff or git-range-diff and\n> copy/pasting the result into the cover letter of the new version.\n>\n> This series automates the process by introducing git-format-patch\n> options --interdiff and --range-diff which insert such a diff into the\n> cover-letter or into the commentary section of the lone patch of a\n> 1-patch series. In the latter case, the interdiff or range-diff is\n> indented to avoid confusing git-am and human readers.\n\nI gave up after 10/14. But what I've seen is nice (yes I have a couple\ncomments here and there but you probably won't need to update\nanything).\n-- \nDuy\n"},{"id":"353404","messageId":"CAPig+cRsPV+PGtkEM_c1b4P-NF7Dk7A8DvPVuvdbx4ESr3pmqg@mail.gmail.com","threadId":"48936","inReplyTo":"CACsJy8C8RK6HkfoEYJGZg=sgtJS0WksHD3=7Souw3jYebRo=Sg@mail.gmail.com","subject":"Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-23T19:18:14Z","receivedAt":"2018-07-23T19:18:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 23, 2018 at 12:03 PM Duy Nguyen wrote:\n> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n> > @@ -0,0 +1,17 @@\n> > +void show_interdiff(struct rev_info *rev)\n> > +{\n> > + struct diff_options opts;\n> > +\n> > + memcpy(&opts, &rev->diffopt, sizeof(opts));\n> > + opts.output_format = DIFF_FORMAT_PATCH;\n> > + diff_setup_done(&opts);\n> > +\n> > + diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, \"\", &opts);\n> > + diffcore_std(&opts);\n> > + diff_flush(&opts);\n> > +}\n>\n> Is it worth adding a new file just for a single function? I haven't\n> read the rest of the series, but the cover letter's diffstat suggests\n> this is it. Is interdiff intended to become a lot more complicated in\n> the future? If not maybe just add this function in diff-lib.c\n\nGood question. The functionality originally lived in builtin/log.c but\nmoved to log-tree.c when I added the ability to embed an interdiff in\na single patch. However, it didn't \"feel\" right in log-tree.c, so I\nmoved it to its own file to mirror how the range-diff engine resides\nin its own file.\n\nAnd, the function actually did several more things as originally\nimplemented. For instance, it took care of not clobbering global\ndiff-queue state, and consulting 'reroll_count' and printing the\n\"Interdiff:\" header, but those bits eventually moved to live at more\n\"correct\" locations, leaving this relatively minimal function behind.\nIt does get a bit more complex in a later patch, but not significantly\nso.\n\nI wasn't aware of diff-lib.c, but it does seem like show_interdiff()\ncould be at home there.\n"},{"id":"353405","messageId":"CAPig+cS7xLgoL4SAihz0tjZur2c7Ff0YZWAgp0wTv0FCkYGZLQ@mail.gmail.com","threadId":"48936","inReplyTo":"CACsJy8AGwbrEFU2bTynXyQi1DmEYtwHrtmefMz+jD13GQs=9HQ@mail.gmail.com","subject":"Re: [PATCH 03/14] format-patch: teach --interdiff to respect -v/--reroll-count","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-23T19:32:19Z","receivedAt":"2018-07-23T19:32:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 23, 2018 at 12:12 PM Duy Nguyen wrote:\n> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n> > @@ -215,6 +215,7 @@ struct rev_info {\n> > /* interdiff */\n> > const struct object_id *idiff_oid1;\n> > const struct object_id *idiff_oid2;\n> > + const char *idiff_title;\n>\n> I feel we're abusing struct rev_info a bit for this since this\n> interdiff thing is very builtin/log.c's business and not at all\n> related to rev walk. Is it possible (and easy) to just pass\n> idfff_title from cmd_format_patch to make_cover_letter()? If it's a\n> lot of code, then I guess we can just leave it here.\n\nAs originally implemented, this information was passed directly to\nmake_cover_letter(), however, as you discovered in your review of\npatch 6/14[1], which makes it possible to embed an interdiff in the\ncommentary section of a lone patch, 'struct rev_info' is the only way\nto pass this information down that deeply in the patch-generation\nprocess. So, yes, this pretty much needs to use 'struct rev_info',\neven if that need doesn't exist at this early step in the series.\n\n[1]: https://public-inbox.org/git/CACsJy8Aw6R8-3kDfhCqunXziajCg9O_1WrEYc4rfKa+-=m1D5g@mail.gmail.com/\n"},{"id":"353409","messageId":"CAPig+cR=rcoLqtefWJMe0NY_7OX7WR6q1UoF-HnYrHZWtKuHAw@mail.gmail.com","threadId":"48936","inReplyTo":"CACsJy8Aw6R8-3kDfhCqunXziajCg9O_1WrEYc4rfKa+-=m1D5g@mail.gmail.com","subject":"Re: [PATCH 06/14] format-patch: allow --interdiff to apply to a lone-patch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-23T19:46:57Z","receivedAt":"2018-07-23T19:47:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 23, 2018 at 12:22 PM Duy Nguyen wrote:\n> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine wrote:\n> > + if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {\n>\n> OK putting idiff stuff in rev_info is probably the right choice. But\n> we all three fields prefixed with idiff_, maybe you could just add a\n> new struct \"idiff_options\" to contain them and add a pointer to that\n> struct in rev_info. Then \"opt->idiff\" is enough to know if idiff is\n> requested instead of relying on idiff_oid1 (seems too random).\n\nI have mixed feelings about this suggestion for the following reasons:\n\n* 'struct rev_info' already contains a number of specialized fields\nwhich apply in only certain use cases but not others, and those fields\noften are grouped textually to show relationship rather than being\nbundled in a struct.\n\n* These new fields are very specific to this particular operation and\nare unlikely to ever have wider use, so adding the extra level of\nindirection/abstraction (whatever you'd call it) feels overkill and,\nwhile nice theoretically, adds complexity without much practical\nvalue.\n\n* Bundling these fields in a struct might incorrectly imply to readers\nthat these items, collectively, can be used in some general-purpose\nfashion, which isn't at all the case; they are very specific to this\noperation and that struct would never be used elsewhere or for any\nother purpose.\n\nThe upside of bundling them in a struct, as you mention, is that\n\"opt->idiff\" would be slightly more obvious than \"opt->idiff_oid1\" as\na \"should we print an interdiff?\" conditional. On the other hand, this\ncase is so specific and narrow that I'm not sure it warrants such\ngenerality.\n"},{"id":"353411","messageId":"CAPig+cT_7eDyY6xGev4=dwpJnKufpMevO-+hGnXVt4ec0xhEiA@mail.gmail.com","threadId":"48936","inReplyTo":"CACsJy8AOeiKp2JnG0h9mw40TdsNft80vUu573ORtqKMor7B+vw@mail.gmail.com","subject":"Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-23T19:58:49Z","receivedAt":"2018-07-23T19:59:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen wrote:\n> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine wrote:\n> > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> > index f8a061794d..e7f404be3d 100644\n> > --- a/Documentation/git-format-patch.txt\n> > +++ b/Documentation/git-format-patch.txt\n> > @@ -24,6 +24,7 @@ SYNOPSIS\n> > [--to=] [--cc=]\n> > [--[no-]cover-letter] [--quiet] [--notes[=]]\n> > [--interdiff=]\n> > + [--range-diff=]\n>\n> I wonder if people will use both --interdiff= and\n> --range-diff= often enough to justify a shortcut\n> \"--all-kinds-of-diff=\" so that we don't have to type \n> twice. But I guess we don't have to worry about this right now.\n\nMy original thought was that --interdiff and --range-diff would be\nmutually exclusive, however, I quickly realized that some people might\nlike to use both options together since each format has its strengths\nand weaknesses. (I've used both types of diffs together when preparing\nrerolls of my own series and found that, together, they provided a\nbetter picture of the reroll than either would have alone.)\n\nBased upon experience on this mailing list, I'd guess that most people\nwould use only one or the other, though that doesn't speak for other\nprojects. And, as you note, it's something that can be added later if\nwarranted (plus, this series is already long and I'd like to avoid\nmaking it longer for something like this whose value is only\nspeculative).\n"},{"id":"353412","messageId":"CAPig+cTwZ2HCFgJmZR968uccsxfGndWKhOBPDrHsPhBRhLiogA@mail.gmail.com","threadId":"48936","inReplyTo":"CACsJy8BgAMYykkNOJc5NgMj-X6SMyskU3iCzTKnL0CLRTdF2oA@mail.gmail.com","subject":"Re: [PATCH 00/14] format-patch: add --interdiff and --range-diff options","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-23T20:03:36Z","receivedAt":"2018-07-23T20:03:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 23, 2018 at 12:32 PM Duy Nguyen wrote:\n> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n> > When re-submitting a patch series, it is often helpful (for reviewers)\n> > to include an interdiff or range-diff against the previous version.\n> > Doing so requires manually running git-diff or git-range-diff and\n> > copy/pasting the result into the cover letter of the new version.\n> >\n> > This series automates the process by introducing git-format-patch\n> > options --interdiff and --range-diff which insert such a diff into the\n> > cover-letter or into the commentary section of the lone patch of a\n> > 1-patch series. In the latter case, the interdiff or range-diff is\n> > indented to avoid confusing git-am and human readers.\n>\n> I gave up after 10/14. But what I've seen is nice (yes I have a couple\n> comments here and there but you probably won't need to update\n> anything).\n\nThanks for the review comments.\n"},{"id":"353423","messageId":"xmqqh8kp4otz.fsf@gitster-ct.c.googlers.com","threadId":"48936","inReplyTo":"CAPig+cRsPV+PGtkEM_c1b4P-NF7Dk7A8DvPVuvdbx4ESr3pmqg@mail.gmail.com","subject":"Re: [PATCH 02/14] format-patch: add --interdiff option to embed diff in cover letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-23T21:36:08Z","receivedAt":"2018-07-23T21:36:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Mon, Jul 23, 2018 at 12:03 PM Duy Nguyen wrote:\n>> On Sun, Jul 22, 2018 at 11:57 AM Eric Sunshine wrote:\n>> > @@ -0,0 +1,17 @@\n>> > +void show_interdiff(struct rev_info *rev)\n>> > +{\n>> > + struct diff_options opts;\n>> > +\n>> > + memcpy(&opts, &rev->diffopt, sizeof(opts));\n>> > + opts.output_format = DIFF_FORMAT_PATCH;\n>> > + diff_setup_done(&opts);\n>> > +\n>> > + diff_tree_oid(rev->idiff_oid1, rev->idiff_oid2, \"\", &opts);\n>> > + diffcore_std(&opts);\n>> > + diff_flush(&opts);\n>> > +}\n>>\n>> Is it worth adding a new file just for a single function? I haven't\n>> read the rest of the series, but the cover letter's diffstat suggests\n>> this is it. Is interdiff intended to become a lot more complicated in\n>> the future? If not maybe just add this function in diff-lib.c\n>\n> Good question. The functionality originally lived in builtin/log.c but\n> moved to log-tree.c when I added the ability to embed an interdiff in\n> a single patch. However, it didn't \"feel\" right in log-tree.c, so I\n> moved it to its own file to mirror how the range-diff engine resides\n> in its own file.\n\n> And, the function actually did several more things as originally\n> implemented. For instance, it took care of not clobbering global\n> diff-queue state, and consulting 'reroll_count' and printing the\n> \"Interdiff:\" header, but those bits eventually moved to live at more\n> \"correct\" locations, leaving this relatively minimal function behind.\n> It does get a bit more complex in a later patch, but not significantly\n> so.\n>\n> I wasn't aware of diff-lib.c, but it does seem like show_interdiff()\n> could be at home there.\n\nYeah, diff-lib.c is meant to be a home for the implementation of low\nlevel \"diff-{files,tree,index}\" plumbing that can be called from\nother routines; if you are adding \"take two things, compare them\"\nthat can be reused from places, then it is a good match.\n"},{"id":"353433","messageId":"CAGZ79kZ0OMrU_O5_rPdG7bNHRWQxVV1RguEg3g-pOa+kkgPCaA@mail.gmail.com","threadId":"48936","inReplyTo":"20180722095717.17912-8-sunshine@sunshineco.com","subject":"Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-23T22:59:39Z","receivedAt":"2018-07-23T22:59:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine wrote:\n>\n> The actual diffs output by range-diff respect diff_option.file, which\n> range-diff passes down the call-chain, thus are destination-agnostic.\n> However, output_pair_header() is hard-coded to emit to 'stdout'. Fix\n> this by making output_pair_header() respect diff_option.file, as well.\n>\n> Signed-off-by: Eric Sunshine \n\nDepending how much the range diff series has progressed\nalready, it might make sense to squash it there?\n\nThanks,\nStefan\n\n> ---\n> range-diff.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/range-diff.c b/range-diff.c\n> index 347b4a79f2..76e053c2b2 100644\n> --- a/range-diff.c\n> +++ b/range-diff.c\n> @@ -328,7 +328,7 @@ static void output_pair_header(struct diff_options *diffopt,\n> }\n> strbuf_addf(buf, \"%s\\n\", color_reset);\n>\n> - fwrite(buf->buf, buf->len, 1, stdout);\n> + fwrite(buf->buf, buf->len, 1, diffopt->file);\n> }\n>\n> static struct userdiff_driver no_func_name = {\n> --\n> 2.18.0.345.g5c9ce644c3\n>\n"},{"id":"353435","messageId":"CAPig+cQtzs5ENzOR2sJVkZu13OF7iMnEcf4GG1jKGrzRhE0AUg@mail.gmail.com","threadId":"48936","inReplyTo":"CAGZ79kZ0OMrU_O5_rPdG7bNHRWQxVV1RguEg3g-pOa+kkgPCaA@mail.gmail.com","subject":"Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-23T23:20:15Z","receivedAt":"2018-07-23T23:20:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 23, 2018 at 6:59 PM Stefan Beller wrote:\n> On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine wrote:\n> > The actual diffs output by range-diff respect diff_option.file, which\n> > range-diff passes down the call-chain, thus are destination-agnostic.\n> > However, output_pair_header() is hard-coded to emit to 'stdout'. Fix\n> > this by making output_pair_header() respect diff_option.file, as well.\n> >\n> > Signed-off-by: Eric Sunshine \n>\n> Depending how much the range diff series has progressed\n> already, it might make sense to squash it there?\n\nIt could be done that way, though it would be nice for Dscho's\nrange-diff series to land without another re-roll, and it looks like\nI'll probably be re-rolling this series, anyhow, to move\nshow_interdiff() to diff-lib.c and to fix its signature. Junio could\nmanage it as a \"squash\", but that's probably more work for him than\nfor me to retain it in this series. *And*, this change _is_ required\nfor this series, whereas it doesn't really matter for Dscho's series,\neven though it's an obvious \"fix\".\n\nAnyhow, whichever way Junio and Dscho would like to play it is fine with me.\n"},{"id":"353574","messageId":"xmqqva93usud.fsf@gitster-ct.c.googlers.com","threadId":"48936","inReplyTo":"CAPig+cT_7eDyY6xGev4=dwpJnKufpMevO-+hGnXVt4ec0xhEiA@mail.gmail.com","subject":"Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-25T17:29:30Z","receivedAt":"2018-07-25T17:29:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen wrote:\n>> On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine wrote:\n>> > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n>> > index f8a061794d..e7f404be3d 100644\n>> > --- a/Documentation/git-format-patch.txt\n>> > +++ b/Documentation/git-format-patch.txt\n>> > @@ -24,6 +24,7 @@ SYNOPSIS\n>> > [--to=] [--cc=]\n>> > [--[no-]cover-letter] [--quiet] [--notes[=]]\n>> > [--interdiff=]\n>> > + [--range-diff=]\n>>\n>> I wonder if people will use both --interdiff= and\n>> --range-diff= often enough to justify a shortcut\n>> \"--all-kinds-of-diff=\" so that we don't have to type \n>> twice. But I guess we don't have to worry about this right now.\n>\n> My original thought was that --interdiff and --range-diff would be\n> mutually exclusive, however, I quickly realized that some people might\n> like to use both options together since each format has its strengths\n> and weaknesses. (I've used both types of diffs together when preparing\n> rerolls of my own series and found that, together, they provided a\n> better picture of the reroll than either would have alone.)\n>\n> Based upon experience on this mailing list, I'd guess that most people\n> would use only one or the other, though that doesn't speak for other\n> projects. And, as you note, it's something that can be added later if\n> warranted (plus, this series is already long and I'd like to avoid\n> making it longer for something like this whose value is only\n> speculative).\n\nA few random thoughts.\n\n * I find it somewhat disturbing that use of dash is inconsistent\n between \"--interdiff=\" and \"--range-diff=\".\n\n * Perhaps \"--interdiff= --range-diff\" may be a possible\n way to say \"use the same and show both\"? Do we want\n \"--range-diff= --interdiff\" to mean the same two\n meta-diff but shown in different order?\n\n * Do we expect that we may find a third-kind of \"meta-diff\" that\n sits next to interdiff and range-diff in the future? I *think*\n two separate options \"--interdiff=...\" and \"--range-diff=...\" is\n still a good way forward, and we'd add \"--frotzdiff=...\" when\n such a third-kind of \"meta-diff\" turns out to be useful, without\n fearing proliferation of options, and that would be OK, but I am\n just thinking aloud to see if other people have better ideas.\n\n\n"},{"id":"353576","messageId":"CACsJy8BKa0WXGUo0LXF7fGGfHJMFmS_5YTT_QpOmGjdRBuYHog@mail.gmail.com","threadId":"48936","inReplyTo":"CAPig+cT_7eDyY6xGev4=dwpJnKufpMevO-+hGnXVt4ec0xhEiA@mail.gmail.com","subject":"Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-07-25T17:38:47Z","receivedAt":"2018-07-25T17:39:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 23, 2018 at 9:59 PM Eric Sunshine wrote:\n>\n> On Mon, Jul 23, 2018 at 12:28 PM Duy Nguyen wrote:\n> > On Sun, Jul 22, 2018 at 11:58 AM Eric Sunshine wrote:\n> > > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> > > index f8a061794d..e7f404be3d 100644\n> > > --- a/Documentation/git-format-patch.txt\n> > > +++ b/Documentation/git-format-patch.txt\n> > > @@ -24,6 +24,7 @@ SYNOPSIS\n> > > [--to=] [--cc=]\n> > > [--[no-]cover-letter] [--quiet] [--notes[=]]\n> > > [--interdiff=]\n> > > + [--range-diff=]\n> >\n> > I wonder if people will use both --interdiff= and\n> > --range-diff= often enough to justify a shortcut\n> > \"--all-kinds-of-diff=\" so that we don't have to type \n> > twice. But I guess we don't have to worry about this right now.\n>\n> My original thought was that --interdiff and --range-diff would be\n> mutually exclusive, however, I quickly realized that some people might\n> like to use both options together since each format has its strengths\n> and weaknesses. (I've used both types of diffs together when preparing\n> rerolls of my own series and found that, together, they provided a\n> better picture of the reroll than either would have alone.)\n\nI actually had another question that I answered myself: how do I know\nwhich one to choose? There's no preview option (and I'm lazy, I don't\nwant to do separate diff commands myself). So my answer was \"choose\nboth, then delete the one that does not look good (and explain it in\nthe cover too when I delete it)\"\n\n> And, as you note, it's something that can be added later if\n> warranted (plus, this series is already long and I'd like to avoid\n> making it longer for something like this whose value is only\n> speculative).\n\nYes of course. We can revisit this later.\n-- \nDuy\n"},{"id":"353580","messageId":"xmqqmuufuq8k.fsf@gitster-ct.c.googlers.com","threadId":"48936","inReplyTo":"CAPig+cQtzs5ENzOR2sJVkZu13OF7iMnEcf4GG1jKGrzRhE0AUg@mail.gmail.com","subject":"Re: [PATCH 07/14] range-diff: respect diff_option.file rather than assuming 'stdout'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-25T18:25:47Z","receivedAt":"2018-07-25T18:25:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Mon, Jul 23, 2018 at 6:59 PM Stefan Beller wrote:\n>> On Sun, Jul 22, 2018 at 2:58 AM Eric Sunshine wrote:\n>> > The actual diffs output by range-diff respect diff_option.file, which\n>> > range-diff passes down the call-chain, thus are destination-agnostic.\n>> > However, output_pair_header() is hard-coded to emit to 'stdout'. Fix\n>> > this by making output_pair_header() respect diff_option.file, as well.\n>> >\n>> > Signed-off-by: Eric Sunshine \n>>\n>> Depending how much the range diff series has progressed\n>> already, it might make sense to squash it there?\n>\n> It could be done that way, though it would be nice for Dscho's\n> range-diff series to land without another re-roll, and it looks like\n> I'll probably be re-rolling this series, anyhow, to move\n> show_interdiff() to diff-lib.c and to fix its signature. Junio could\n> manage it as a \"squash\", but that's probably more work for him than\n> for me to retain it in this series. *And*, this change _is_ required\n> for this series, whereas it doesn't really matter for Dscho's series,\n> even though it's an obvious \"fix\".\n>\n> Anyhow, whichever way Junio and Dscho would like to play it is fine with me.\n\nHaving it at the beginning of your series (I consider 1-6/14 to be a\nseparate series, on top of which another series whose first change\nis this 7/14 is builds upon) like you do here is probably fine.\n"},{"id":"353585","messageId":"CAPig+cS-haNwJha44gncB9PrXRPcEPuqDVGLgKii29h=abq6sA@mail.gmail.com","threadId":"48936","inReplyTo":"xmqqva93usud.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-25T19:30:44Z","receivedAt":"2018-07-25T19:30:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jul 25, 2018 at 1:29 PM Junio C Hamano wrote:\n> A few random thoughts.\n>\n> * I find it somewhat disturbing that use of dash is inconsistent\n> between \"--interdiff=\" and \"--range-diff=\".\n\nI went with the common spellings we've seen on the mailing lists.\n\"Interdiff\", in particular, seems well established. \"Range-diff\" is\nnew, so we don't have much data other than what we saw during the\ndiscussion when renaming git-branch-diff, and, of course,\ngit-branch-diff itself is hyphenated. Another consideration: \"inter\"\nis a prefix, whereas \"range\" stands on its own.\n\nI don't have a super strong opinion, as I'm used to both the\nhyphenated name (from discussion and the command name itself), and the\nunhyphenated name (\"rangediff\" was my local branch name). I'm open to\nopinions.\n\nWe probably wouldn't want to do so, but another possibility is to\nrecognize both --range-diff and --rangediff.\n\n> * Perhaps \"--interdiff= --range-diff\" may be a possible\n> way to say \"use the same and show both\"? Do we want\n> \"--range-diff= --interdiff\" to mean the same two\n> meta-diff but shown in different order?\n\nThat's not at all a bad shorthand. I like it better than the\n\"--all-kinds-of-diff=\" mentioned earlier. We don't need to make a\ndecision for this series since such functionality can be added later.\n\nThe order of specification of the two options affecting the order of\noutput is also an interesting idea. We may want to decide that before\ngraduating this topic.\n\n> * Do we expect that we may find a third-kind of \"meta-diff\" that\n> sits next to interdiff and range-diff in the future? I *think*\n> two separate options \"--interdiff=...\" and \"--range-diff=...\" is\n> still a good way forward, and we'd add \"--frotzdiff=...\" when\n> such a third-kind of \"meta-diff\" turns out to be useful, without\n> fearing proliferation of options, and that would be OK, but I am\n> just thinking aloud to see if other people have better ideas.\n\nI did consider other approaches, such as a more generic option. For\nexample, --embed=range-diff:. I also considered supporting more\ncomplex use-cases. For instance, git-range-diff itself accepts the two\nranges in several formats (\"a..b c..d\" or \"x...z\" or \"i j k\"), plus\nthe creation-factor can be tweaked, so I weighed ways of incorporating\nall that detail into the single argument to --range-diff.\n\nHowever, those are all difficult to use (onerous to type and remember)\nand difficult to document. Thus, I opted for simplicity of individual,\nstraight-forward options (--interdiff, --range-diff,\n--creation-factor), which are easy to type, remember, and document.\n\nWe may want to revisit this later if git-format-patch grows additional\nsimilar options.\n"},{"id":"353589","messageId":"xmqqeffrukd6.fsf@gitster-ct.c.googlers.com","threadId":"48936","inReplyTo":"CAPig+cS-haNwJha44gncB9PrXRPcEPuqDVGLgKii29h=abq6sA@mail.gmail.com","subject":"Re: [PATCH 10/14] format-patch: add --range-diff option to embed diff in cover letter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-25T20:32:37Z","receivedAt":"2018-07-25T20:32:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> I did consider other approaches, such as a more generic option. For\n> example, --embed=range-diff:. I also considered supporting more\n> complex use-cases. For instance, git-range-diff itself accepts the two\n> ranges in several formats (\"a..b c..d\" or \"x...z\" or \"i j k\"), plus\n> the creation-factor can be tweaked, so I weighed ways of incorporating\n> all that detail into the single argument to --range-diff.\n>\n> However, those are all difficult to use (onerous to type and remember)\n> and difficult to document. Thus, I opted for simplicity of individual,\n> straight-forward options (--interdiff, --range-diff,\n> --creation-factor), which are easy to type, remember, and document.\n\nYes, that matches my conclusion that it is OK to have these on\nseparate options that can grow as many meta-diff formats we would\nsupport.\n\n"},{"id":"353590","messageId":"xmqqva93t4u7.fsf@gitster-ct.c.googlers.com","threadId":"48936","inReplyTo":"20180722095717.17912-12-sunshine@sunshineco.com","subject":"Re: [PATCH 11/14] format-patch: extend --range-diff to accept revision range","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-25T20:53:20Z","receivedAt":"2018-07-25T20:53:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> When submitting a revised a patch series, the --range-diff option embeds\n> a range-diff in the cover letter showing changes since the previous\n> version of the patch series. The argument to --range-diff is a simple\n> revision naming the tip of the previous series, which works fine if the\n> previous and current versions of the patch series share a common base.\n>\n> However, it fails if the revision ranges of the old and new versions of\n> the series are disjoint. To address this shortcoming, extend\n> --range-diff to also accept an explicit revision range for the previous\n> series. For example:\n>\n> git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2\n>\n> Signed-off-by: Eric Sunshine \n> ---\n\nMakes sense. Even though a single \"common rev\" would serve as a\nfeature to discourage rebasing done \"just to catch up\" without a\ngood reason, it is a good idea to give an escape hatch like this\nto support a case where rebasing is the right thing to do.\n\n> static void infer_range_diff_ranges(struct strbuf *r1,\n> \t\t\t\t struct strbuf *r2,\n> \t\t\t\t const char *prev,\n> +\t\t\t\t struct commit *origin,\n> \t\t\t\t struct commit *head)\n> {\n> \tconst char *head_oid = oid_to_hex(&head->object.oid);\n> \n> -\tstrbuf_addf(r1, \"%s..%s\", head_oid, prev);\n> -\tstrbuf_addf(r2, \"%s..%s\", prev, head_oid);\n\nI thought \"git range-diff\" also took the three-dot notation as a\nshort-hand but there is no point using that in this application,\nwhich wants the RHS and the LHS range in separate output variables.\n\n> +\tif (!strstr(prev, \"..\")) {\n> +\t\tstrbuf_addf(r1, \"%s..%s\", head_oid, prev);\n> +\t\tstrbuf_addf(r2, \"%s..%s\", prev, head_oid);\n> +\t} else if (!origin) {\n> +\t\tdie(_(\"failed to infer range-diff ranges\"));\n> +\t} else {\n> +\t\tstrbuf_addstr(r1, prev);\n> +\t\tstrbuf_addf(r2, \"%s..%s\",\n> +\t\t\t oid_to_hex(&origin->object.oid), head_oid);\n\nInteresting.\n\nI actually would feel better to see the second range for the normal\ncase to be computed exactly the same way, i.e.\n\n\tint prev_is_range = strstr(prev, \"..\");\n\n\tif (prev_is_range)\n\t\tstrbuf_addstr(r1, prev);\n\telse\n\t\tstrbuf_addf(r1, \"%s..%s\", head_oid, prev);\n\n\tif (origin)\n\t\tstrbuf_addf(r2, \"%s..%s\", oid_to_hex(&origin->object.oid, head_oid);\n\telse if (prev_is_range)\n\t\tdie(\"cannot figure out the origin of new series\");\n\telse {\n\t\twarning(\"falling back to use '%s' as the origin of new series\",\tprev);\n\t\tstrbuf_addf(r2, \"%s..%s\", prev, head_oid);\n\t}\n\nbecause origin..HEAD is always the set of the \"new\" series, no\nmatter what \"prev\" the user chooses to compare that series against,\nwhen there is a single \"origin\".\n"},{"id":"353592","messageId":"xmqqr2jrt46j.fsf@gitster-ct.c.googlers.com","threadId":"48936","inReplyTo":"20180722095717.17912-15-sunshine@sunshineco.com","subject":"Re: [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-25T21:07:32Z","receivedAt":"2018-07-25T21:07:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> @@ -750,6 +751,20 @@ void show_log(struct rev_info *opt)\n> \n> \t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n> \t}\n> +\n> +\tif (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {\n> +\t\tstruct diff_queue_struct dq;\n> +\n> +\t\tmemcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n> +\t\tDIFF_QUEUE_CLEAR(&diff_queued_diff);\n> +\n> +\t\tnext_commentary_block(opt, NULL);\n> +\t\tfprintf_ln(opt->diffopt.file, \"%s\", opt->rdiff_title);\n> +\t\tshow_range_diff(opt->rdiff1, opt->rdiff2,\n> +\t\t\t\topt->creation_factor, 1, &opt->diffopt);\n> +\n> +\t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n> +\t}\n> }\n> \n> int log_tree_diff_flush(struct rev_info *opt)\n\nThis essentially repeats what is already done for \"interdiff\".\n\nDoes the global diff_queued_diff gets cleaned up when\nshow_interdiff() and show_range_diff() return, like diff_flush()\ndoes? Otherwise we'd be leaking the filepairs accumulated in the\ndiff_queued_diff.\n\n"},{"id":"357613","messageId":"CAPig+cS5VwmNnrpFwpm9Ko2xg3WVsfffDLpkJLtU9ziSFOoyUw@mail.gmail.com","threadId":"48936","inReplyTo":"xmqqr2jrt46j.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 14/14] format-patch: allow --range-diff to apply to a lone-patch","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-07T08:46:43Z","receivedAt":"2018-09-07T08:46:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jul 25, 2018 at 5:07 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > + if (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {\n> > + struct diff_queue_struct dq;\n> > +\n> > + memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n> > + DIFF_QUEUE_CLEAR(&diff_queued_diff);\n> > +\n> > + next_commentary_block(opt, NULL);\n> > + fprintf_ln(opt->diffopt.file, \"%s\", opt->rdiff_title);\n> > + show_range_diff(opt->rdiff1, opt->rdiff2,\n> > + opt->creation_factor, 1, &opt->diffopt);\n> > +\n> > + memcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n> > + }\n> > }\n>\n> This essentially repeats what is already done for \"interdiff\".\n\nYes, the two blocks are very similar, although they access different\nmembers of 'rev_info' and call different functions to perform the\nactual diff. I explored ways of avoiding the repeated boilerplate\n(using macros or passing a function pointer to a driver function\ncontaining the boilerplate), but the end result was uglier and harder\nto understand due to the added abstraction. Introducing\nnext_commentary_block()[1] reduced the repetition a bit.\n\n> Does the global diff_queued_diff gets cleaned up when\n> show_interdiff() and show_range_diff() return, like diff_flush()\n> does? Otherwise we'd be leaking the filepairs accumulated in the\n> diff_queued_diff.\n\nBoth show_interdiff() and show_range_diff() call diff_flush(). So, the\n\"temporary\" diff_queued_diff set up here does get cleaned up by\ndiff_flush(), as far as I understand (though this is my first foray\ninto the diff-generation code, so I may be missing something). And,\nthe diff_queued_diff which gets \"interrupted\" by this excursion into\ninterdiff/range-diff gets cleaned up normally when the interrupted\ndiff operation completes.\n\n[1]: https://public-inbox.org/git/20180722095717.17912-6-sunshine@sunshineco.com/\n"},{"id":"357614","messageId":"CAPig+cRfcJYux2Rfaq-2NhW-d9vD1J-Mg_NmjGMbrW2S0RWB4Q@mail.gmail.com","threadId":"48936","inReplyTo":"xmqqva93t4u7.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 11/14] format-patch: extend --range-diff to accept revision range","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-09-07T09:15:13Z","receivedAt":"2018-09-07T09:15:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jul 25, 2018 at 4:53 PM Junio C Hamano wrote:\n> Eric Sunshine writes:\n> > When submitting a revised a patch series, the --range-diff option embeds\n> > a range-diff in the cover letter showing changes since the previous\n> > version of the patch series. The argument to --range-diff is a simple\n> > revision naming the tip of the previous series [...]\n> >\n> > However, it fails if the revision ranges of the old and new versions of\n> > the series are disjoint. To address this shortcoming, extend\n> > --range-diff to also accept an explicit revision range for the previous\n> > series. [...]\n>\n> > static void infer_range_diff_ranges(struct strbuf *r1,\n> > {\n> > - strbuf_addf(r1, \"%s..%s\", head_oid, prev);\n> > - strbuf_addf(r2, \"%s..%s\", prev, head_oid);\n>\n> I thought \"git range-diff\" also took the three-dot notation as a\n> short-hand but there is no point using that in this application,\n> which wants the RHS and the LHS range in separate output variables.\n\nCorrect.\n\n> I actually would feel better to see the second range for the normal\n> case to be computed exactly the same way, i.e.\n>\n> int prev_is_range = strstr(prev, \"..\");\n>\n> if (prev_is_range)\n> strbuf_addstr(r1, prev);\n> else\n> strbuf_addf(r1, \"%s..%s\", head_oid, prev);\n>\n> if (origin)\n> strbuf_addf(r2, \"%s..%s\", oid_to_hex(&origin->object.oid, head_oid);\n> else if (prev_is_range)\n> die(\"cannot figure out the origin of new series\");\n> else {\n> warning(\"falling back to use '%s' as the origin of new series\", prev);\n> strbuf_addf(r2, \"%s..%s\", prev, head_oid);\n> }\n>\n> because origin..HEAD is always the set of the \"new\" series, no\n> matter what \"prev\" the user chooses to compare that series against,\n> when there is a single \"origin\".\n\nI plan to submit a short series as incremental changes atop\nes/format-patch-{interdiff,rangediff} to address the few review\ncomments[1] (including my own[2]), so I can make this change, as well.\n\n[1]: https://public-inbox.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grM1b8Gw@mail.gmail.com/T/#rfaf5a563e81b862bd8ed69232040056ab9a86dd8\n[2]: https://public-inbox.org/git/CAPig+cSuYUYSPTuKx08wcmQM-G12_-W2T4BS07fA=6grM1b8Gw@mail.gmail.com/\n"}]}