{"thread":{"id":"55109","subject":"[PATCH 3/6] range-diff: simplify code spawning `git log`","startedAt":"2021-02-04T20:15:08Z","lastAt":"2021-02-08T22:37:44Z","messageCount":22,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Taylor Blau","Eric Sunshine","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"416183","messageId":"d6c582c25df0ff57072c4170203b77a6334d006a.1612469275.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH 3/6] range-diff: simplify code spawning `git log`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:51Z","receivedAt":"2021-02-04T20:15:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nPreviously, we waited for the child process to be finished in every\nfailing code path as well as at the end of the function\n`show_range_diff()`.\n\nHowever, we do not need to wait that long. Directly after reading the\noutput of the child process, we can wrap up the child process.\n\nThis also has the advantage that we don't do a bunch of unnecessary work\nin case `finish_command()` returns with an error anyway.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 6707b728a07f..3919d56e3716 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -80,6 +80,8 @@ static int read_patches(const char *range, struct string_list *list,\n \t\tfinish_command(&cp);\n \t\treturn -1;\n \t}\n+\tif (finish_command(&cp))\n+\t\treturn -1;\n \n \tline = contents.buf;\n \tsize = contents.len;\n@@ -101,7 +103,6 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\t\tstring_list_clear(list, 1);\n \t\t\t\tstrbuf_release(&buf);\n \t\t\t\tstrbuf_release(&contents);\n-\t\t\t\tfinish_command(&cp);\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t\tutil->matching = -1;\n@@ -117,7 +118,6 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tstring_list_clear(list, 1);\n \t\t\tstrbuf_release(&buf);\n \t\t\tstrbuf_release(&contents);\n-\t\t\tfinish_command(&cp);\n \t\t\treturn -1;\n \t\t}\n \n@@ -227,9 +227,6 @@ static int read_patches(const char *range, struct string_list *list,\n \tstrbuf_release(&buf);\n \tfree(current_filename);\n \n-\tif (finish_command(&cp))\n-\t\treturn -1;\n-\n \treturn 0;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"416184","messageId":"5f2f06b79e208b68f3c753538f2b081729765b82.1612469275.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH 1/6] range-diff: avoid leaking memory in two error code paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:49Z","receivedAt":"2021-02-04T20:15:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn the code paths in question, we already release a lot of memory, but\nthe `current_filename` variable was missed. Fix that.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 0c6ac4f954d1..963cdecb4cbb 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -97,6 +97,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tif (get_oid(p, &util->oid)) {\n \t\t\t\terror(_(\"could not parse commit '%s'\"), p);\n \t\t\t\tfree(util);\n+\t\t\t\tfree(current_filename);\n \t\t\t\tstring_list_clear(list, 1);\n \t\t\t\tstrbuf_release(&buf);\n \t\t\t\tstrbuf_release(&contents);\n@@ -112,6 +113,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\terror(_(\"could not parse first line of `log` output: \"\n \t\t\t\t\"did not start with 'commit ': '%s'\"),\n \t\t\t      line);\n+\t\t\tfree(current_filename);\n \t\t\tstring_list_clear(list, 1);\n \t\t\tstrbuf_release(&buf);\n \t\t\tstrbuf_release(&contents);\n-- \ngitgitgadget\n\n"},{"id":"416185","messageId":"0d25dd2a9c2d15479d098b10a85cf14144329a57.1612469275.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH 2/6] range-diff: libify the read_patches() function again","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:50Z","receivedAt":"2021-02-04T20:15:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn library functions, we do want to avoid the (simple, but rather final)\n`die()` calls, instead returning with a value indicating an error.\n\nLet's do exactly that in the code introduced in b66885a30cb8\n(range-diff: add section header instead of diff header, 2019-07-11) that\nwants to error out if a diff header could not be parsed.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 963cdecb4cbb..6707b728a07f 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -135,9 +135,16 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\torig_len = len;\n \t\t\tlen = parse_git_diff_header(&root, &linenr, 0, line,\n \t\t\t\t\t\t    len, size, &patch);\n-\t\t\tif (len < 0)\n-\t\t\t\tdie(_(\"could not parse git header '%.*s'\"),\n-\t\t\t\t    orig_len, line);\n+\t\t\tif (len < 0) {\n+\t\t\t\terror(_(\"could not parse git header '%.*s'\"),\n+\t\t\t\t      orig_len, line);\n+\t\t\t\tfree(util);\n+\t\t\t\tfree(current_filename);\n+\t\t\t\tstring_list_clear(list, 1);\n+\t\t\t\tstrbuf_release(&buf);\n+\t\t\t\tstrbuf_release(&contents);\n+\t\t\t\treturn -1;\n+\t\t\t}\n \t\t\tstrbuf_addstr(&buf, \" ## \");\n \t\t\tif (patch.is_new > 0)\n \t\t\t\tstrbuf_addf(&buf, \"%s (new)\", patch.new_name);\n-- \ngitgitgadget\n\n"},{"id":"416186","messageId":"pull.869.git.1612469275.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":null,"subject":"[PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:48Z","receivedAt":"2021-02-04T20:15:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"One of my quite common workflows is to see whether an ancient topic branch I\nhave lying about has made it into Git. Since my local commit OIDs have\nnothing to do with the OIDs of the corresponding commits in git/git, my only\nway is to fire up git range-diff ...upstream/master, but of course that\noutput contains way more commits than I care about.\n\nTo help this use case, here is a patch series that teaches git range-diff\nthe --left-only and --right-only options in the end, restricting the output\nto those commits and commit pairs that correspond to the commits in the\nfirst and the second range, respectively.\n\nThe first part of the series contains cleanup patches that are not strictly\nrelated to the feature I implemented here, but since I already have them, I\nfigured I could just as well contribute them all together.\n\nThis patch series is based on js/range-diff-wo-dotdot.\n\nJohannes Schindelin (6):\n  range-diff: avoid leaking memory in two error code paths\n  range-diff: libify the read_patches() function again\n  range-diff: simplify code spawning `git log`\n  range-diff: combine all options in a single data structure\n  range-diff: move the diffopt initialization down one layer\n  range-diff: offer --left-only/--right-only options\n\n Documentation/git-range-diff.txt |   9 +++\n builtin/log.c                    |  10 ++-\n builtin/range-diff.c             |  21 +++++--\n log-tree.c                       |   8 ++-\n range-diff.c                     | 101 +++++++++++++++++--------------\n range-diff.h                     |  12 +++-\n t/t3206-range-diff.sh            |  15 +++++\n 7 files changed, 118 insertions(+), 58 deletions(-)\n\n\nbase-commit: 43718f6741a87f87bd400bdf5264394e980583c5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-869%2Fdscho%2Frange-diff-left-and-right-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-869/dscho/range-diff-left-and-right-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/869\n-- \ngitgitgadget\n"},{"id":"416187","messageId":"b620be042eb31c0e771230434951d983f4173ecf.1612469275.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH 4/6] range-diff: combine all options in a single data structure","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:52Z","receivedAt":"2021-02-04T20:22:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis will make it easier to implement the `--left-only` and\n`--right-only` options.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/log.c        | 10 ++++++++--\n builtin/range-diff.c | 13 +++++++++----\n log-tree.c           |  8 ++++++--\n range-diff.c         | 18 +++++++++---------\n range-diff.h         | 11 ++++++++---\n 5 files changed, 40 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 91466c059c79..a06e5385689b 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1231,14 +1231,20 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t */\n \t\tstruct diff_options opts;\n \t\tstruct strvec other_arg = STRVEC_INIT;\n+\t\tstruct range_diff_options range_diff_opts = {\n+\t\t\t.creation_factor = rev->creation_factor,\n+\t\t\t.dual_color = 1,\n+\t\t\t.diffopt = &opts,\n+\t\t\t.other_arg = &other_arg\n+\t\t};\n+\n \t\tdiff_setup(&opts);\n \t\topts.file = rev->diffopt.file;\n \t\topts.use_color = rev->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n \t\tget_notes_args(&other_arg, rev);\n-\t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n-\t\t\t\trev->creation_factor, 1, &opts, &other_arg);\n+\t\tshow_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);\n \t\tstrvec_clear(&other_arg);\n \t}\n }\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 5b1f6326322f..80fcdc6ad42d 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -14,12 +14,17 @@ NULL\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 \tstruct diff_options diffopt = { NULL };\n \tstruct strvec other_arg = STRVEC_INIT;\n+\tstruct range_diff_options range_diff_opts = {\n+\t\t.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,\n+\t\t.diffopt = &diffopt,\n+\t\t.other_arg = &other_arg\n+\t};\n \tint simple_color = -1;\n \tstruct option range_diff_options[] = {\n-\t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n+\t\tOPT_INTEGER(0, \"creation-factor\",\n+\t\t\t    &range_diff_opts.creation_factor,\n \t\t\t    N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n \t\t\t    N_(\"use simple diff colors\")),\n@@ -82,8 +87,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t}\n \tFREE_AND_NULL(options);\n \n-\tres = show_range_diff(range1.buf, range2.buf, creation_factor,\n-\t\t\t      simple_color < 1, &diffopt, &other_arg);\n+\trange_diff_opts.dual_color = simple_color < 1;\n+\tres = show_range_diff(range1.buf, range2.buf, &range_diff_opts);\n \n \tstrvec_clear(&other_arg);\n \tstrbuf_release(&range1);\ndiff --git a/log-tree.c b/log-tree.c\nindex fd0dde97ec32..eeacba15dc94 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -808,6 +808,11 @@ void show_log(struct rev_info *opt)\n \tif (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {\n \t\tstruct diff_queue_struct dq;\n \t\tstruct diff_options opts;\n+\t\tstruct range_diff_options range_diff_opts = {\n+\t\t\t.creation_factor = opt->creation_factor,\n+\t\t\t.dual_color = 1,\n+\t\t\t.diffopt = &opts\n+\t\t};\n \n \t\tmemcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n \t\tDIFF_QUEUE_CLEAR(&diff_queued_diff);\n@@ -822,8 +827,7 @@ void show_log(struct rev_info *opt)\n \t\topts.file = opt->diffopt.file;\n \t\topts.use_color = opt->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n-\t\tshow_range_diff(opt->rdiff1, opt->rdiff2,\n-\t\t\t\topt->creation_factor, 1, &opts, NULL);\n+\t\tshow_range_diff(opt->rdiff1, opt->rdiff2, &range_diff_opts);\n \n \t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n \t}\ndiff --git a/range-diff.c b/range-diff.c\nindex 3919d56e3716..58528c43a3e8 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -525,33 +525,32 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n }\n \n int show_range_diff(const char *range1, const char *range2,\n-\t\t    int creation_factor, int dual_color,\n-\t\t    const struct diff_options *diffopt,\n-\t\t    const struct strvec *other_arg)\n+\t\t    struct range_diff_options *range_diff_opts)\n {\n \tint res = 0;\n \n \tstruct string_list branch1 = STRING_LIST_INIT_DUP;\n \tstruct string_list branch2 = STRING_LIST_INIT_DUP;\n \n-\tif (read_patches(range1, &branch1, other_arg))\n+\tif (read_patches(range1, &branch1, range_diff_opts->other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range1);\n-\tif (!res && read_patches(range2, &branch2, other_arg))\n+\tif (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))\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\tif (diffopt)\n-\t\t\tmemcpy(&opts, diffopt, sizeof(opts));\n+\t\tif (range_diff_opts->diffopt)\n+\t\t\tmemcpy(&opts, range_diff_opts->diffopt, sizeof(opts));\n \t\telse\n \t\t\tdiff_setup(&opts);\n \n \t\tif (!opts.output_format)\n \t\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.flags.dual_color_diffed_diffs =\n+\t\t\trange_diff_opts->dual_color;\n \t\topts.flags.suppress_hunk_header_line_count = 1;\n \t\topts.output_prefix = output_prefix_cb;\n \t\tstrbuf_addstr(&indent, \"    \");\n@@ -559,7 +558,8 @@ int show_range_diff(const char *range1, const char *range2,\n \t\tdiff_setup_done(&opts);\n \n \t\tfind_exact_matches(&branch1, &branch2);\n-\t\tget_correspondences(&branch1, &branch2, creation_factor);\n+\t\tget_correspondences(&branch1, &branch2,\n+\t\t\t\t    range_diff_opts->creation_factor);\n \t\toutput(&branch1, &branch2, &opts);\n \n \t\tstrbuf_release(&indent);\ndiff --git a/range-diff.h b/range-diff.h\nindex c17dbc2e75a8..8fb2ff05865d 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -6,15 +6,20 @@\n \n #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n \n+struct range_diff_options {\n+\tint creation_factor;\n+\tunsigned dual_color:1;\n+\tconst struct diff_options *diffopt;\n+\tconst struct strvec *other_arg;\n+};\n+\n /*\n  * Compare series of commits in RANGE1 and RANGE2, and emit to the\n  * standard output.  NULL can be passed to DIFFOPT to use the built-in\n  * default.\n  */\n int show_range_diff(const char *range1, const char *range2,\n-\t\t    int creation_factor, int dual_color,\n-\t\t    const struct diff_options *diffopt,\n-\t\t    const struct strvec *other_arg);\n+\t\t    struct range_diff_options *opts);\n \n /*\n  * Determine whether the given argument is usable as a range argument of `git\n-- \ngitgitgadget\n\n"},{"id":"416188","messageId":"1c599abdbb6fd304dd340ecbf903082306cf9138.1612469275.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH 6/6] range-diff: offer --left-only/--right-only options","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:54Z","receivedAt":"2021-02-04T20:23:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen comparing commit ranges, one is frequently interested only in one\nside, such as asking the question \"Has this patch that I submitted to\nthe Git mailing list been applied?\": one would only care about the part\nof the output that corresponds to the commits in a local branch.\n\nTo make that possible, imitate the `git rev-list` options `--left-only`\nand `--right-only`.\n\nThis addresses https://github.com/gitgitgadget/git/issues/206\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-range-diff.txt |  9 +++++++++\n builtin/range-diff.c             |  8 +++++++-\n range-diff.c                     | 11 ++++++++---\n range-diff.h                     |  1 +\n t/t3206-range-diff.sh            | 15 +++++++++++++++\n 5 files changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex 14bffb272a06..e3bd93515819 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n [verse]\n 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]\n \t[--no-dual-color] [--creation-factor=<factor>]\n+\t[--left-only | --right-only]\n \t( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )\n \n DESCRIPTION\n@@ -69,6 +70,14 @@ to revert to color all lines according to the outer diff markers\n \tSee the ``Algorithm`` section below for an explanation why this is\n \tneeded.\n \n+--left-only::\n+\tSuppress commits that are missing from the first specified range\n+\t(or the \"left range\" when using the `<rev1>...<rev2>` format).\n+\n+--right-only::\n+\tSuppress commits that are missing from the second specified range\n+\t(or the \"right range\" when using the `<rev1>...<rev2>` format).\n+\n --[no-]notes[=<ref>]::\n \tThis flag is passed to the `git log` program\n \t(see linkgit:git-log[1]) that generates the patches.\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 80fcdc6ad42d..78bc9fa77062 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -21,7 +21,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\t.diffopt = &diffopt,\n \t\t.other_arg = &other_arg\n \t};\n-\tint simple_color = -1;\n+\tint simple_color = -1, left_only = 0, right_only = 0;\n \tstruct option range_diff_options[] = {\n \t\tOPT_INTEGER(0, \"creation-factor\",\n \t\t\t    &range_diff_opts.creation_factor,\n@@ -31,6 +31,10 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tOPT_PASSTHRU_ARGV(0, \"notes\", &other_arg,\n \t\t\t\t  N_(\"notes\"), N_(\"passed to 'git log'\"),\n \t\t\t\t  PARSE_OPT_OPTARG),\n+\t\tOPT_BOOL(0, \"left-only\", &left_only,\n+\t\t\t N_(\"only emit output related to the first range\")),\n+\t\tOPT_BOOL(0, \"right-only\", &right_only,\n+\t\t\t N_(\"only emit output related to the second range\")),\n \t\tOPT_END()\n \t};\n \tstruct option *options;\n@@ -88,6 +92,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \tFREE_AND_NULL(options);\n \n \trange_diff_opts.dual_color = simple_color < 1;\n+\trange_diff_opts.left_only = left_only;\n+\trange_diff_opts.right_only = right_only;\n \tres = show_range_diff(range1.buf, range2.buf, &range_diff_opts);\n \n \tstrvec_clear(&other_arg);\ndiff --git a/range-diff.c b/range-diff.c\nindex 5455cbb9521b..6f52dce0f6b6 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -513,7 +513,8 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched LHS commit whose predecessors were shown. */\n \t\tif (i < a->nr && a_util->matching < 0) {\n-\t\t\toutput_pair_header(&opts, patch_no_width,\n+\t\t\tif (!range_diff_opts->right_only)\n+\t\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, NULL);\n \t\t\ti++;\n \t\t\tcontinue;\n@@ -521,7 +522,8 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched RHS commits. */\n \t\twhile (j < b->nr && b_util->matching < 0) {\n-\t\t\toutput_pair_header(&opts, patch_no_width,\n+\t\t\tif (!range_diff_opts->left_only)\n+\t\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, NULL, b_util);\n \t\t\tb_util = ++j < b->nr ? b->items[j].util : NULL;\n \t\t}\n@@ -551,7 +553,10 @@ int show_range_diff(const char *range1, const char *range2,\n \tstruct string_list branch1 = STRING_LIST_INIT_DUP;\n \tstruct string_list branch2 = STRING_LIST_INIT_DUP;\n \n-\tif (read_patches(range1, &branch1, range_diff_opts->other_arg))\n+\tif (range_diff_opts->left_only && range_diff_opts->right_only)\n+\t\tres = error(_(\"--left-only and --right-only are mutually exclusive\"));\n+\n+\tif (!res && read_patches(range1, &branch1, range_diff_opts->other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range1);\n \tif (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range2);\ndiff --git a/range-diff.h b/range-diff.h\nindex 8fb2ff05865d..66aacd47d149 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -9,6 +9,7 @@\n struct range_diff_options {\n \tint creation_factor;\n \tunsigned dual_color:1;\n+\tunsigned left_only:1, right_only:1;\n \tconst struct diff_options *diffopt;\n \tconst struct strvec *other_arg;\n };\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex e217cecac9ed..41cc1077a18f 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -725,4 +725,19 @@ test_expect_success 'format-patch --range-diff with multiple notes' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--left-only/--right-only' '\n+\tgit switch --orphan left-right &&\n+\ttest_commit first &&\n+\ttest_commit unmatched &&\n+\ttest_commit common &&\n+\tgit switch -C left-right first &&\n+\tgit cherry-pick common &&\n+\n+\tgit range-diff -s --left-only ...common >actual &&\n+\thead_oid=$(git rev-parse --short HEAD) &&\n+\tcommon_oid=$(git rev-parse --short common) &&\n+\techo \"1:  $head_oid = 2:  $common_oid common\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"416189","messageId":"9fa945db5f135d3b829aef70da490184f898e276.1612469275.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH 5/6] range-diff: move the diffopt initialization down one layer","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-04T20:07:53Z","receivedAt":"2021-02-04T20:24:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIt is actually only the `output()` function that uses those diffopts. By\nmoving the diffopt initialization down into that function, it is\nencapsulated better.\n\nIncidentally, it will also make it easier to implement the `--left-only`\nand `--right-only` options in `git range-diff` because the `output()`\nfunction is now receiving all range-diff options as a parameter, not\njust the diffopts.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 64 +++++++++++++++++++++++++---------------------------\n 1 file changed, 31 insertions(+), 33 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 58528c43a3e8..5455cbb9521b 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -464,12 +464,35 @@ static void patch_diff(const char *a, const char *b,\n \tdiff_flush(diffopt);\n }\n \n+static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n+{\n+\treturn data;\n+}\n+\n static void output(struct string_list *a, struct string_list *b,\n-\t\t   struct diff_options *diffopt)\n+\t\t   struct range_diff_options *range_diff_opts)\n {\n \tstruct strbuf buf = STRBUF_INIT, dashes = STRBUF_INIT;\n \tint patch_no_width = decimal_width(1 + (a->nr > b->nr ? a->nr : b->nr));\n \tint i = 0, j = 0;\n+\tstruct diff_options opts;\n+\tstruct strbuf indent = STRBUF_INIT;\n+\n+\tif (range_diff_opts->diffopt)\n+\t\tmemcpy(&opts, range_diff_opts->diffopt, sizeof(opts));\n+\telse\n+\t\tdiff_setup(&opts);\n+\n+\tif (!opts.output_format)\n+\t\topts.output_format = DIFF_FORMAT_PATCH;\n+\topts.flags.suppress_diff_headers = 1;\n+\topts.flags.dual_color_diffed_diffs =\n+\t\trange_diff_opts->dual_color;\n+\topts.flags.suppress_hunk_header_line_count = 1;\n+\topts.output_prefix = output_prefix_cb;\n+\tstrbuf_addstr(&indent, \"    \");\n+\topts.output_prefix_data = &indent;\n+\tdiff_setup_done(&opts);\n \n \t/*\n \t * We assume the user is really more interested in the second argument\n@@ -490,7 +513,7 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched LHS commit whose predecessors were shown. */\n \t\tif (i < a->nr && a_util->matching < 0) {\n-\t\t\toutput_pair_header(diffopt, patch_no_width,\n+\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, NULL);\n \t\t\ti++;\n \t\t\tcontinue;\n@@ -498,7 +521,7 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched RHS commits. */\n \t\twhile (j < b->nr && b_util->matching < 0) {\n-\t\t\toutput_pair_header(diffopt, patch_no_width,\n+\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, NULL, b_util);\n \t\t\tb_util = ++j < b->nr ? b->items[j].util : NULL;\n \t\t}\n@@ -506,22 +529,18 @@ static void output(struct string_list *a, struct string_list *b,\n \t\t/* Show matching LHS/RHS pair. */\n \t\tif (j < b->nr) {\n \t\t\ta_util = a->items[b_util->matching].util;\n-\t\t\toutput_pair_header(diffopt, patch_no_width,\n+\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, b_util);\n-\t\t\tif (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))\n+\t\t\tif (!(opts.output_format & DIFF_FORMAT_NO_OUTPUT))\n \t\t\t\tpatch_diff(a->items[b_util->matching].string,\n-\t\t\t\t\t   b->items[j].string, diffopt);\n+\t\t\t\t\t   b->items[j].string, &opts);\n \t\t\ta_util->shown = 1;\n \t\t\tj++;\n \t\t}\n \t}\n \tstrbuf_release(&buf);\n \tstrbuf_release(&dashes);\n-}\n-\n-static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n-{\n-\treturn data;\n+\tstrbuf_release(&indent);\n }\n \n int show_range_diff(const char *range1, const char *range2,\n@@ -538,31 +557,10 @@ 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\tif (range_diff_opts->diffopt)\n-\t\t\tmemcpy(&opts, range_diff_opts->diffopt, sizeof(opts));\n-\t\telse\n-\t\t\tdiff_setup(&opts);\n-\n-\t\tif (!opts.output_format)\n-\t\t\topts.output_format = DIFF_FORMAT_PATCH;\n-\t\topts.flags.suppress_diff_headers = 1;\n-\t\topts.flags.dual_color_diffed_diffs =\n-\t\t\trange_diff_opts->dual_color;\n-\t\topts.flags.suppress_hunk_header_line_count = 1;\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,\n \t\t\t\t    range_diff_opts->creation_factor);\n-\t\toutput(&branch1, &branch2, &opts);\n-\n-\t\tstrbuf_release(&indent);\n+\t\toutput(&branch1, &branch2, range_diff_opts);\n \t}\n \n \tstring_list_clear(&branch1, 1);\n-- \ngitgitgadget\n\n"},{"id":"416214","messageId":"xmqqtuqr4frw.fsf@gitster.c.googlers.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T22:41:39Z","receivedAt":"2021-02-04T22:42:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> One of my quite common workflows is to see whether an ancient topic branch I\n> have lying about has made it into Git. Since my local commit OIDs have\n> nothing to do with the OIDs of the corresponding commits in git/git, my only\n> way is to fire up git range-diff ...upstream/master, but of course that\n> output contains way more commits than I care about.\n>\n> To help this use case, here is a patch series that teaches git range-diff\n> the --left-only and --right-only options in the end, restricting the output\n> to those commits and commit pairs that correspond to the commits in the\n> first and the second range, respectively.\n\nMakes sense.\n"},{"id":"416216","messageId":"YBx5rmVsg1LJhSKN@nand.local","threadId":"55109","inReplyTo":"xmqqtuqr4frw.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-02-04T22:48:14Z","receivedAt":"2021-02-04T22:49:01Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Feb 04, 2021 at 02:41:39PM -0800, Junio C Hamano wrote:\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > One of my quite common workflows is to see whether an ancient topic branch I\n> > have lying about has made it into Git. Since my local commit OIDs have\n> > nothing to do with the OIDs of the corresponding commits in git/git, my only\n> > way is to fire up git range-diff ...upstream/master, but of course that\n> > output contains way more commits than I care about.\n> >\n> > To help this use case, here is a patch series that teaches git range-diff\n> > the --left-only and --right-only options in the end, restricting the output\n> > to those commits and commit pairs that correspond to the commits in the\n> > first and the second range, respectively.\n>\n> Makes sense.\n\nI'd add an additional use-case, which is ignoring new commits from\nupstream when displaying a range-diff in rerolled patch series.\n\nOftentimes I'll find that the automatically-prepared range diff that\n'git format-patch --cover-letter --range-diff' generates will include\nnew commits from upstream, so these new options should help me ignore\nthose in the output.\n\nAs an aside: I am curious if I'm missing something when you say the\n\"only way\" is to ask for a 'git range-diff ...@{u}'. IIUC what you're\ndescribing, I often resort to using 'git cherry' for that exact thing.\nBut, I may not be quite understanding your use-case (and why git-cherry\ndoesn't do what you want already).\n\nMy latter question is purely for satisfying my own curiosity; I don't\nhave any problem with a '--{left,right}-only' option in range-diff. From\nmy quick read of the patches, it all looks pretty sane to me.\n\n\nThanks,\nTaylor\n"},{"id":"416223","messageId":"CAPig+cSNCVT8h-wFBGvDWB4u0OUQM-Q-1DwuLepnjTdwEpk_4A@mail.gmail.com","threadId":"55109","inReplyTo":"b620be042eb31c0e771230434951d983f4173ecf.1612469275.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/6] range-diff: combine all options in a single data structure","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-02-04T23:56:36Z","receivedAt":"2021-02-04T23:57:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Feb 4, 2021 at 3:24 PM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> This will make it easier to implement the `--left-only` and\n> `--right-only` options.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> diff --git a/range-diff.h b/range-diff.h\n> @@ -6,15 +6,20 @@\n> +struct range_diff_options {\n> +       int creation_factor;\n> +       unsigned dual_color:1;\n> +       const struct diff_options *diffopt;\n> +       const struct strvec *other_arg;\n> +};\n> +\n>  /*\n>   * Compare series of commits in RANGE1 and RANGE2, and emit to the\n>   * standard output.  NULL can be passed to DIFFOPT to use the built-in\n>   * default.\n>   */\n>  int show_range_diff(const char *range1, const char *range2,\n> -                   int creation_factor, int dual_color,\n> -                   const struct diff_options *diffopt,\n> -                   const struct strvec *other_arg);\n> +                   struct range_diff_options *opts);\n\nThe function comment's mention of DIFFOPT becomes outdated with this\nchange. Perhaps update it to say `opts.diffopt` or something.\n"},{"id":"416227","messageId":"xmqqtuqr2uz3.fsf@gitster.c.googlers.com","threadId":"55109","inReplyTo":"YBx5rmVsg1LJhSKN@nand.local","subject":"Re: [PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-05T00:56:16Z","receivedAt":"2021-02-05T00:57:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Thu, Feb 04, 2021 at 02:41:39PM -0800, Junio C Hamano wrote:\n>> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n>> writes:\n>>\n>> > One of my quite common workflows is to see whether an ancient topic branch I\n>> > have lying about has made it into Git. Since my local commit OIDs have\n>> > nothing to do with the OIDs of the corresponding commits in git/git, my only\n>> > way is to fire up git range-diff ...upstream/master, but of course that\n>> > output contains way more commits than I care about.\n>> > ...\n>> Makes sense.\n>\n> I'd add an additional use-case, which is ignoring new commits from\n> upstream when displaying a range-diff in rerolled patch series.\n>\n> Oftentimes I'll find that the automatically-prepared range diff that\n> 'git format-patch --cover-letter --range-diff' generates will include\n> new commits from upstream, so these new options should help me ignore\n> those in the output.\n\nDo you mean that the new round is based on an updated upstream\ncommit, while the old series was based on a bit older upstream?\nAfter rebasing your topic, \"range-diff @{1}...\" would find the\nupdates in the base (made in the upstream) plus the new round of\nyour work on the right hand side of the symmetric range, while the\nleft hand side solely consists of your old round (unless the\nupstream rewound their work, which should not happen).  But that\nmust not be it, I guess, because in such a case, among the commits\nin @{1}..HEAD, we cannot (eh, at least range-diff cannot) tell which\none came from upstream and which one came from our fingers.\n\nSo I am a bit puzzled there.\n\n> As an aside: I am curious if I'm missing something when you say the\n> \"only way\" is to ask for a 'git range-diff ...@{u}'. IIUC what you're\n> describing, I often resort to using 'git cherry' for that exact thing.\n> But, I may not be quite understanding your use-case (and why git-cherry\n> doesn't do what you want already).\n>\n> My latter question is purely for satisfying my own curiosity; I don't\n> have any problem with a '--{left,right}-only' option in range-diff. From\n> my quick read of the patches, it all looks pretty sane to me.\n\nThe question is addressed to Dscho, and I am also somewhat curious.\nPerhaps the reason would be that the output from cherry is not as\neasy to read as range-diff, without any post-processing.\n\nI do find \"range-diff ...@{u}\" a bit too blunt and heavy a hammer\nfor that task, but as they say, when you are familiar with and fond\nof a hammer, all tasks look like nails ;-).\n\n"},{"id":"416251","messageId":"YB0Z5JXbe22ko2Mj@coredump.intra.peff.net","threadId":"55109","inReplyTo":"xmqqtuqr2uz3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-05T10:11:48Z","receivedAt":"2021-02-05T10:16:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:\n\n> > As an aside: I am curious if I'm missing something when you say the\n> > \"only way\" is to ask for a 'git range-diff ...@{u}'. IIUC what you're\n> > describing, I often resort to using 'git cherry' for that exact thing.\n> > But, I may not be quite understanding your use-case (and why git-cherry\n> > doesn't do what you want already).\n> >\n> > My latter question is purely for satisfying my own curiosity; I don't\n> > have any problem with a '--{left,right}-only' option in range-diff. From\n> > my quick read of the patches, it all looks pretty sane to me.\n> \n> The question is addressed to Dscho, and I am also somewhat curious.\n> Perhaps the reason would be that the output from cherry is not as\n> easy to read as range-diff, without any post-processing.\n\nI had the same curiosity; I'd use git-cherry (or rev-list --cherry) for\nthis.\n\nI suspect the big difference is the quality of the matching. git-cherry\nis purely looking at patch-ids. So it is quite likely to say \"this was\nnot applied upstream\" when what got applied differed slightly (e.g.,\nfixups upstream, applied to a different base, etc). Whereas range-diff\nhas some cost heuristics for deciding that two patches are basically the\nsame thing.  So it would find more cases (and as a bonus, give you the\ndiff to see what tweaks were made upstream).\n\nIt does make me wonder if it would be useful for rev-list, etc to have\nan option to make \"--cherry\" use the more clever heuristics instead of\njust a patch-id. It would never show the same diff output as range-diff,\nbut maybe more scripts would find the advanced heuristic useful.\n\nI know it would probably make rebase's \"ignore if in upstream\" feature\nless clunky when I rebase topics. But it would also make it more\ndangerous! E.g., right now I see any upstream tweaks as potential\nconflicts when I rebase, and I manually review them for sanity.\n\n-Peff\n"},{"id":"416283","messageId":"YB2lHmLxe8ArFz0v@nand.local","threadId":"55109","inReplyTo":"xmqqtuqr2uz3.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-02-05T20:05:50Z","receivedAt":"2021-02-05T20:09:06Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:\n> > I'd add an additional use-case, which is ignoring new commits from\n> > upstream when displaying a range-diff in rerolled patch series.\n> >\n> > Oftentimes I'll find that the automatically-prepared range diff that\n> > 'git format-patch --cover-letter --range-diff' generates will include\n> > new commits from upstream, so these new options should help me ignore\n> > those in the output.\n>\n> Do you mean that the new round is based on an updated upstream\n> commit, while the old series was based on a bit older upstream?\n> After rebasing your topic, \"range-diff @{1}...\" would find the\n> updates in the base (made in the upstream) plus the new round of\n> your work on the right hand side of the symmetric range, while the\n> left hand side solely consists of your old round (unless the\n> upstream rewound their work, which should not happen).  But that\n> must not be it, I guess, because in such a case, among the commits\n> in @{1}..HEAD, we cannot (eh, at least range-diff cannot) tell which\n> one came from upstream and which one came from our fingers.\n>\n> So I am a bit puzzled there.\n\nI'm talking about a situation where a later re-roll is based of of a\nnewer upstream. But your judgement is right: upstream's updates look\nlike \"new\" commits on the right-hand side.\n\nI have some scripts built around this, but they all boil down to passing\n'--range-diff=@{1}' (where @{1} is the tip of the previous reroll) to\nformat-patch. See:\n\n    https://github.com/ttaylorr/dotfiles/blob/work-gh/bin/git-mail#L8-L10\n\nfor details.\n\nIIUC this series, I think I'd also want to start passing '--left-only'\nto ignore the new commits from upstream in a range-diff, no?\n\nThanks,\nTaylor\n"},{"id":"416285","messageId":"a36631b663e2310f60c5477df5efeda9dccfcce9.1612536373.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","subject":"[PATCH v2 3/6] range-diff: simplify code spawning `git log`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:10Z","receivedAt":"2021-02-05T20:40:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nPreviously, we waited for the child process to be finished in every\nfailing code path as well as at the end of the function\n`show_range_diff()`.\n\nHowever, we do not need to wait that long. Directly after reading the\noutput of the child process, we can wrap up the child process.\n\nThis also has the advantage that we don't do a bunch of unnecessary work\nin case `finish_command()` returns with an error anyway.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 8844359d416f..d0d941a25add 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -81,6 +81,8 @@ static int read_patches(const char *range, struct string_list *list,\n \t\tfinish_command(&cp);\n \t\treturn -1;\n \t}\n+\tif (finish_command(&cp))\n+\t\treturn -1;\n \n \tline = contents.buf;\n \tsize = contents.len;\n@@ -102,7 +104,6 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\t\tstring_list_clear(list, 1);\n \t\t\t\tstrbuf_release(&buf);\n \t\t\t\tstrbuf_release(&contents);\n-\t\t\t\tfinish_command(&cp);\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t\tutil->matching = -1;\n@@ -118,7 +119,6 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tstring_list_clear(list, 1);\n \t\t\tstrbuf_release(&buf);\n \t\t\tstrbuf_release(&contents);\n-\t\t\tfinish_command(&cp);\n \t\t\treturn -1;\n \t\t}\n \n@@ -228,9 +228,6 @@ static int read_patches(const char *range, struct string_list *list,\n \tstrbuf_release(&buf);\n \tfree(current_filename);\n \n-\tif (finish_command(&cp))\n-\t\treturn -1;\n-\n \treturn 0;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"416296","messageId":"a6285292b4f12bf3e57aed115595bc254a63dabf.1612536373.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","subject":"[PATCH v2 5/6] range-diff: move the diffopt initialization down one layer","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:12Z","receivedAt":"2021-02-05T21:52:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIt is actually only the `output()` function that uses those diffopts. By\nmoving the diffopt initialization down into that function, it is\nencapsulated better.\n\nIncidentally, it will also make it easier to implement the `--left-only`\nand `--right-only` options in `git range-diff` because the `output()`\nfunction is now receiving all range-diff options as a parameter, not\njust the diffopts.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 64 +++++++++++++++++++++++++---------------------------\n 1 file changed, 31 insertions(+), 33 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 25d4c244799c..001b6e174079 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -465,12 +465,35 @@ static void patch_diff(const char *a, const char *b,\n \tdiff_flush(diffopt);\n }\n \n+static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n+{\n+\treturn data;\n+}\n+\n static void output(struct string_list *a, struct string_list *b,\n-\t\t   struct diff_options *diffopt)\n+\t\t   struct range_diff_options *range_diff_opts)\n {\n \tstruct strbuf buf = STRBUF_INIT, dashes = STRBUF_INIT;\n \tint patch_no_width = decimal_width(1 + (a->nr > b->nr ? a->nr : b->nr));\n \tint i = 0, j = 0;\n+\tstruct diff_options opts;\n+\tstruct strbuf indent = STRBUF_INIT;\n+\n+\tif (range_diff_opts->diffopt)\n+\t\tmemcpy(&opts, range_diff_opts->diffopt, sizeof(opts));\n+\telse\n+\t\tdiff_setup(&opts);\n+\n+\tif (!opts.output_format)\n+\t\topts.output_format = DIFF_FORMAT_PATCH;\n+\topts.flags.suppress_diff_headers = 1;\n+\topts.flags.dual_color_diffed_diffs =\n+\t\trange_diff_opts->dual_color;\n+\topts.flags.suppress_hunk_header_line_count = 1;\n+\topts.output_prefix = output_prefix_cb;\n+\tstrbuf_addstr(&indent, \"    \");\n+\topts.output_prefix_data = &indent;\n+\tdiff_setup_done(&opts);\n \n \t/*\n \t * We assume the user is really more interested in the second argument\n@@ -491,7 +514,7 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched LHS commit whose predecessors were shown. */\n \t\tif (i < a->nr && a_util->matching < 0) {\n-\t\t\toutput_pair_header(diffopt, patch_no_width,\n+\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, NULL);\n \t\t\ti++;\n \t\t\tcontinue;\n@@ -499,7 +522,7 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched RHS commits. */\n \t\twhile (j < b->nr && b_util->matching < 0) {\n-\t\t\toutput_pair_header(diffopt, patch_no_width,\n+\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, NULL, b_util);\n \t\t\tb_util = ++j < b->nr ? b->items[j].util : NULL;\n \t\t}\n@@ -507,22 +530,18 @@ static void output(struct string_list *a, struct string_list *b,\n \t\t/* Show matching LHS/RHS pair. */\n \t\tif (j < b->nr) {\n \t\t\ta_util = a->items[b_util->matching].util;\n-\t\t\toutput_pair_header(diffopt, patch_no_width,\n+\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, b_util);\n-\t\t\tif (!(diffopt->output_format & DIFF_FORMAT_NO_OUTPUT))\n+\t\t\tif (!(opts.output_format & DIFF_FORMAT_NO_OUTPUT))\n \t\t\t\tpatch_diff(a->items[b_util->matching].string,\n-\t\t\t\t\t   b->items[j].string, diffopt);\n+\t\t\t\t\t   b->items[j].string, &opts);\n \t\t\ta_util->shown = 1;\n \t\t\tj++;\n \t\t}\n \t}\n \tstrbuf_release(&buf);\n \tstrbuf_release(&dashes);\n-}\n-\n-static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n-{\n-\treturn data;\n+\tstrbuf_release(&indent);\n }\n \n int show_range_diff(const char *range1, const char *range2,\n@@ -539,31 +558,10 @@ 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\tif (range_diff_opts->diffopt)\n-\t\t\tmemcpy(&opts, range_diff_opts->diffopt, sizeof(opts));\n-\t\telse\n-\t\t\tdiff_setup(&opts);\n-\n-\t\tif (!opts.output_format)\n-\t\t\topts.output_format = DIFF_FORMAT_PATCH;\n-\t\topts.flags.suppress_diff_headers = 1;\n-\t\topts.flags.dual_color_diffed_diffs =\n-\t\t\trange_diff_opts->dual_color;\n-\t\topts.flags.suppress_hunk_header_line_count = 1;\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,\n \t\t\t\t    range_diff_opts->creation_factor);\n-\t\toutput(&branch1, &branch2, &opts);\n-\n-\t\tstrbuf_release(&indent);\n+\t\toutput(&branch1, &branch2, range_diff_opts);\n \t}\n \n \tstring_list_clear(&branch1, 1);\n-- \ngitgitgadget\n\n"},{"id":"416298","messageId":"99e466ef32d77f45de5c3aae5f44bea017b622cf.1612536373.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","subject":"[PATCH v2 2/6] range-diff: libify the read_patches() function again","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:09Z","receivedAt":"2021-02-05T22:01:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn library functions, we do want to avoid the (simple, but rather final)\n`die()` calls, instead returning with a value indicating an error.\n\nLet's do exactly that in the code introduced in b66885a30cb8\n(range-diff: add section header instead of diff header, 2019-07-11) that\nwants to error out if a diff header could not be parsed.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 9972808fe4cf..8844359d416f 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -136,9 +136,16 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\torig_len = len;\n \t\t\tlen = parse_git_diff_header(&root, &linenr, 0, line,\n \t\t\t\t\t\t    len, size, &patch);\n-\t\t\tif (len < 0)\n-\t\t\t\tdie(_(\"could not parse git header '%.*s'\"),\n-\t\t\t\t    orig_len, line);\n+\t\t\tif (len < 0) {\n+\t\t\t\terror(_(\"could not parse git header '%.*s'\"),\n+\t\t\t\t      orig_len, line);\n+\t\t\t\tfree(util);\n+\t\t\t\tfree(current_filename);\n+\t\t\t\tstring_list_clear(list, 1);\n+\t\t\t\tstrbuf_release(&buf);\n+\t\t\t\tstrbuf_release(&contents);\n+\t\t\t\treturn -1;\n+\t\t\t}\n \t\t\tstrbuf_addstr(&buf, \" ## \");\n \t\t\tif (patch.is_new > 0)\n \t\t\t\tstrbuf_addf(&buf, \"%s (new)\", patch.new_name);\n-- \ngitgitgadget\n\n"},{"id":"416302","messageId":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.git.1612469275.gitgitgadget@gmail.com","subject":"[PATCH v2 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:07Z","receivedAt":"2021-02-05T22:05:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"One of my quite common workflows is to see whether an ancient topic branch I\nhave lying about has made it into Git. Since my local commit OIDs have\nnothing to do with the OIDs of the corresponding commits in git/git, my only\nway is to fire up git range-diff ...upstream/master, but of course that\noutput contains way more commits than I care about.\n\nTo help this use case, here is a patch series that teaches git range-diff\nthe --left-only and --right-only options in the end, restricting the output\nto those commits and commit pairs that correspond to the commits in the\nfirst and the second range, respectively.\n\nThe first part of the series contains cleanup patches that are not strictly\nrelated to the feature I implemented here, but since I already have them, I\nfigured I could just as well contribute them all together.\n\nThis patch series is based on js/range-diff-wo-dotdot.\n\nChanges since v1:\n\n * Adjusted the comment above show_range_diff() to reflect the new\n   signature.\n\nJohannes Schindelin (6):\n  range-diff: avoid leaking memory in two error code paths\n  range-diff: libify the read_patches() function again\n  range-diff: simplify code spawning `git log`\n  range-diff: combine all options in a single data structure\n  range-diff: move the diffopt initialization down one layer\n  range-diff: offer --left-only/--right-only options\n\n Documentation/git-range-diff.txt |   9 +++\n builtin/log.c                    |  10 ++-\n builtin/range-diff.c             |  21 +++++--\n log-tree.c                       |   8 ++-\n range-diff.c                     | 101 +++++++++++++++++--------------\n range-diff.h                     |  17 ++++--\n t/t3206-range-diff.sh            |  15 +++++\n 7 files changed, 120 insertions(+), 61 deletions(-)\n\n\nbase-commit: 35fac18ea5ccab7524501bcb4542473ef8195541\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-869%2Fdscho%2Frange-diff-left-and-right-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-869/dscho/range-diff-left-and-right-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/869\n\nRange-diff vs v1:\n\n 1:  5f2f06b79e20 = 1:  15f4f6abdaa2 range-diff: avoid leaking memory in two error code paths\n 2:  0d25dd2a9c2d = 2:  99e466ef32d7 range-diff: libify the read_patches() function again\n 3:  d6c582c25df0 = 3:  a36631b663e2 range-diff: simplify code spawning `git log`\n 4:  b620be042eb3 ! 4:  7367eadfe243 range-diff: combine all options in a single data structure\n     @@ range-diff.h\n      +struct range_diff_options {\n      +\tint creation_factor;\n      +\tunsigned dual_color:1;\n     -+\tconst struct diff_options *diffopt;\n     -+\tconst struct strvec *other_arg;\n     ++\tconst struct diff_options *diffopt; /* may be NULL */\n     ++\tconst struct strvec *other_arg; /* may be NULL */\n      +};\n      +\n       /*\n     -  * Compare series of commits in RANGE1 and RANGE2, and emit to the\n     -  * standard output.  NULL can be passed to DIFFOPT to use the built-in\n     -  * default.\n     +- * Compare series of commits in RANGE1 and RANGE2, and emit to the\n     +- * standard output.  NULL can be passed to DIFFOPT to use the built-in\n     +- * default.\n     ++ * Compare series of commits in `range1` and `range2`, and emit to the\n     ++ * standard output.\n        */\n       int show_range_diff(const char *range1, const char *range2,\n      -\t\t    int creation_factor, int dual_color,\n 5:  9fa945db5f13 = 5:  a6285292b4f1 range-diff: move the diffopt initialization down one layer\n 6:  1c599abdbb6f ! 6:  8357d3c94f17 range-diff: offer --left-only/--right-only options\n     @@ range-diff.h\n       \tint creation_factor;\n       \tunsigned dual_color:1;\n      +\tunsigned left_only:1, right_only:1;\n     - \tconst struct diff_options *diffopt;\n     - \tconst struct strvec *other_arg;\n     + \tconst struct diff_options *diffopt; /* may be NULL */\n     + \tconst struct strvec *other_arg; /* may be NULL */\n       };\n      \n       ## t/t3206-range-diff.sh ##\n\n-- \ngitgitgadget\n"},{"id":"416304","messageId":"8357d3c94f175b2354465dab3c9a6e331ab082ee.1612536373.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","subject":"[PATCH v2 6/6] range-diff: offer --left-only/--right-only options","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:13Z","receivedAt":"2021-02-05T22:08:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen comparing commit ranges, one is frequently interested only in one\nside, such as asking the question \"Has this patch that I submitted to\nthe Git mailing list been applied?\": one would only care about the part\nof the output that corresponds to the commits in a local branch.\n\nTo make that possible, imitate the `git rev-list` options `--left-only`\nand `--right-only`.\n\nThis addresses https://github.com/gitgitgadget/git/issues/206\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-range-diff.txt |  9 +++++++++\n builtin/range-diff.c             |  8 +++++++-\n range-diff.c                     | 11 ++++++++---\n range-diff.h                     |  1 +\n t/t3206-range-diff.sh            | 15 +++++++++++++++\n 5 files changed, 40 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\nindex a968d5237dae..fe350d7f4056 100644\n--- a/Documentation/git-range-diff.txt\n+++ b/Documentation/git-range-diff.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n [verse]\n 'git range-diff' [--color=[<when>]] [--no-color] [<diff-options>]\n \t[--no-dual-color] [--creation-factor=<factor>]\n+\t[--left-only | --right-only]\n \t( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )\n \n DESCRIPTION\n@@ -68,6 +69,14 @@ to revert to color all lines according to the outer diff markers\n \tSee the ``Algorithm`` section below for an explanation why this is\n \tneeded.\n \n+--left-only::\n+\tSuppress commits that are missing from the first specified range\n+\t(or the \"left range\" when using the `<rev1>...<rev2>` format).\n+\n+--right-only::\n+\tSuppress commits that are missing from the second specified range\n+\t(or the \"right range\" when using the `<rev1>...<rev2>` format).\n+\n --[no-]notes[=<ref>]::\n \tThis flag is passed to the `git log` program\n \t(see linkgit:git-log[1]) that generates the patches.\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 80fcdc6ad42d..78bc9fa77062 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -21,7 +21,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\t.diffopt = &diffopt,\n \t\t.other_arg = &other_arg\n \t};\n-\tint simple_color = -1;\n+\tint simple_color = -1, left_only = 0, right_only = 0;\n \tstruct option range_diff_options[] = {\n \t\tOPT_INTEGER(0, \"creation-factor\",\n \t\t\t    &range_diff_opts.creation_factor,\n@@ -31,6 +31,10 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\tOPT_PASSTHRU_ARGV(0, \"notes\", &other_arg,\n \t\t\t\t  N_(\"notes\"), N_(\"passed to 'git log'\"),\n \t\t\t\t  PARSE_OPT_OPTARG),\n+\t\tOPT_BOOL(0, \"left-only\", &left_only,\n+\t\t\t N_(\"only emit output related to the first range\")),\n+\t\tOPT_BOOL(0, \"right-only\", &right_only,\n+\t\t\t N_(\"only emit output related to the second range\")),\n \t\tOPT_END()\n \t};\n \tstruct option *options;\n@@ -88,6 +92,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \tFREE_AND_NULL(options);\n \n \trange_diff_opts.dual_color = simple_color < 1;\n+\trange_diff_opts.left_only = left_only;\n+\trange_diff_opts.right_only = right_only;\n \tres = show_range_diff(range1.buf, range2.buf, &range_diff_opts);\n \n \tstrvec_clear(&other_arg);\ndiff --git a/range-diff.c b/range-diff.c\nindex 001b6e174079..ed19b4729845 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -514,7 +514,8 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched LHS commit whose predecessors were shown. */\n \t\tif (i < a->nr && a_util->matching < 0) {\n-\t\t\toutput_pair_header(&opts, patch_no_width,\n+\t\t\tif (!range_diff_opts->right_only)\n+\t\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, NULL);\n \t\t\ti++;\n \t\t\tcontinue;\n@@ -522,7 +523,8 @@ static void output(struct string_list *a, struct string_list *b,\n \n \t\t/* Show unmatched RHS commits. */\n \t\twhile (j < b->nr && b_util->matching < 0) {\n-\t\t\toutput_pair_header(&opts, patch_no_width,\n+\t\t\tif (!range_diff_opts->left_only)\n+\t\t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, NULL, b_util);\n \t\t\tb_util = ++j < b->nr ? b->items[j].util : NULL;\n \t\t}\n@@ -552,7 +554,10 @@ int show_range_diff(const char *range1, const char *range2,\n \tstruct string_list branch1 = STRING_LIST_INIT_DUP;\n \tstruct string_list branch2 = STRING_LIST_INIT_DUP;\n \n-\tif (read_patches(range1, &branch1, range_diff_opts->other_arg))\n+\tif (range_diff_opts->left_only && range_diff_opts->right_only)\n+\t\tres = error(_(\"--left-only and --right-only are mutually exclusive\"));\n+\n+\tif (!res && read_patches(range1, &branch1, range_diff_opts->other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range1);\n \tif (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range2);\ndiff --git a/range-diff.h b/range-diff.h\nindex a595f4e8db2d..04ffe217be67 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -9,6 +9,7 @@\n struct range_diff_options {\n \tint creation_factor;\n \tunsigned dual_color:1;\n+\tunsigned left_only:1, right_only:1;\n \tconst struct diff_options *diffopt; /* may be NULL */\n \tconst struct strvec *other_arg; /* may be NULL */\n };\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 2b518378d4a0..04aa9aed6bde 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -730,4 +730,19 @@ test_expect_success 'format-patch --range-diff with multiple notes' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--left-only/--right-only' '\n+\tgit switch --orphan left-right &&\n+\ttest_commit first &&\n+\ttest_commit unmatched &&\n+\ttest_commit common &&\n+\tgit switch -C left-right first &&\n+\tgit cherry-pick common &&\n+\n+\tgit range-diff -s --left-only ...common >actual &&\n+\thead_oid=$(git rev-parse --short HEAD) &&\n+\tcommon_oid=$(git rev-parse --short common) &&\n+\techo \"1:  $head_oid = 2:  $common_oid common\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"416306","messageId":"15f4f6abdaa206ca9b89f4f2444b88a77ef1060c.1612536373.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","subject":"[PATCH v2 1/6] range-diff: avoid leaking memory in two error code paths","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:08Z","receivedAt":"2021-02-05T22:08:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn the code paths in question, we already release a lot of memory, but\nthe `current_filename` variable was missed. Fix that.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n range-diff.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 7a38dc871543..9972808fe4cf 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -98,6 +98,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\tif (get_oid(p, &util->oid)) {\n \t\t\t\terror(_(\"could not parse commit '%s'\"), p);\n \t\t\t\tfree(util);\n+\t\t\t\tfree(current_filename);\n \t\t\t\tstring_list_clear(list, 1);\n \t\t\t\tstrbuf_release(&buf);\n \t\t\t\tstrbuf_release(&contents);\n@@ -113,6 +114,7 @@ static int read_patches(const char *range, struct string_list *list,\n \t\t\terror(_(\"could not parse first line of `log` output: \"\n \t\t\t\t\"did not start with 'commit ': '%s'\"),\n \t\t\t      line);\n+\t\t\tfree(current_filename);\n \t\t\tstring_list_clear(list, 1);\n \t\t\tstrbuf_release(&buf);\n \t\t\tstrbuf_release(&contents);\n-- \ngitgitgadget\n\n"},{"id":"416321","messageId":"nycvar.QRO.7.76.6.2102051513050.54@tvgsbejvaqbjf.bet","threadId":"55109","inReplyTo":"CAPig+cSNCVT8h-wFBGvDWB4u0OUQM-Q-1DwuLepnjTdwEpk_4A@mail.gmail.com","subject":"Re: [PATCH 4/6] range-diff: combine all options in a single data structure","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-05T14:13:31Z","receivedAt":"2021-02-05T22:28:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Thu, 4 Feb 2021, Eric Sunshine wrote:\n\n> On Thu, Feb 4, 2021 at 3:24 PM Johannes Schindelin via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > This will make it easier to implement the `--left-only` and\n> > `--right-only` options.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> > diff --git a/range-diff.h b/range-diff.h\n> > @@ -6,15 +6,20 @@\n> > +struct range_diff_options {\n> > +       int creation_factor;\n> > +       unsigned dual_color:1;\n> > +       const struct diff_options *diffopt;\n> > +       const struct strvec *other_arg;\n> > +};\n> > +\n> >  /*\n> >   * Compare series of commits in RANGE1 and RANGE2, and emit to the\n> >   * standard output.  NULL can be passed to DIFFOPT to use the built-in\n> >   * default.\n> >   */\n> >  int show_range_diff(const char *range1, const char *range2,\n> > -                   int creation_factor, int dual_color,\n> > -                   const struct diff_options *diffopt,\n> > -                   const struct strvec *other_arg);\n> > +                   struct range_diff_options *opts);\n>\n> The function comment's mention of DIFFOPT becomes outdated with this\n> change. Perhaps update it to say `opts.diffopt` or something.\n\nI actually moved the comment to the new `struct`.\n\nThanks,\nDscho\n"},{"id":"416323","messageId":"7367eadfe2430fbcd60e7b7abf7cb432488b9c39.1612536373.git.gitgitgadget@gmail.com","threadId":"55109","inReplyTo":"pull.869.v2.git.1612536373.gitgitgadget@gmail.com","subject":"[PATCH v2 4/6] range-diff: combine all options in a single data structure","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-05T14:46:11Z","receivedAt":"2021-02-05T22:29:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis will make it easier to implement the `--left-only` and\n`--right-only` options.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n builtin/log.c        | 10 ++++++++--\n builtin/range-diff.c | 13 +++++++++----\n log-tree.c           |  8 ++++++--\n range-diff.c         | 18 +++++++++---------\n range-diff.h         | 16 ++++++++++------\n 5 files changed, 42 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 91466c059c79..a06e5385689b 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1231,14 +1231,20 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t */\n \t\tstruct diff_options opts;\n \t\tstruct strvec other_arg = STRVEC_INIT;\n+\t\tstruct range_diff_options range_diff_opts = {\n+\t\t\t.creation_factor = rev->creation_factor,\n+\t\t\t.dual_color = 1,\n+\t\t\t.diffopt = &opts,\n+\t\t\t.other_arg = &other_arg\n+\t\t};\n+\n \t\tdiff_setup(&opts);\n \t\topts.file = rev->diffopt.file;\n \t\topts.use_color = rev->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n \t\tfprintf_ln(rev->diffopt.file, \"%s\", rev->rdiff_title);\n \t\tget_notes_args(&other_arg, rev);\n-\t\tshow_range_diff(rev->rdiff1, rev->rdiff2,\n-\t\t\t\trev->creation_factor, 1, &opts, &other_arg);\n+\t\tshow_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts);\n \t\tstrvec_clear(&other_arg);\n \t}\n }\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex 5b1f6326322f..80fcdc6ad42d 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -14,12 +14,17 @@ NULL\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 \tstruct diff_options diffopt = { NULL };\n \tstruct strvec other_arg = STRVEC_INIT;\n+\tstruct range_diff_options range_diff_opts = {\n+\t\t.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,\n+\t\t.diffopt = &diffopt,\n+\t\t.other_arg = &other_arg\n+\t};\n \tint simple_color = -1;\n \tstruct option range_diff_options[] = {\n-\t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n+\t\tOPT_INTEGER(0, \"creation-factor\",\n+\t\t\t    &range_diff_opts.creation_factor,\n \t\t\t    N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n \t\t\t    N_(\"use simple diff colors\")),\n@@ -82,8 +87,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t}\n \tFREE_AND_NULL(options);\n \n-\tres = show_range_diff(range1.buf, range2.buf, creation_factor,\n-\t\t\t      simple_color < 1, &diffopt, &other_arg);\n+\trange_diff_opts.dual_color = simple_color < 1;\n+\tres = show_range_diff(range1.buf, range2.buf, &range_diff_opts);\n \n \tstrvec_clear(&other_arg);\n \tstrbuf_release(&range1);\ndiff --git a/log-tree.c b/log-tree.c\nindex fd0dde97ec32..eeacba15dc94 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -808,6 +808,11 @@ void show_log(struct rev_info *opt)\n \tif (cmit_fmt_is_mail(ctx.fmt) && opt->rdiff1) {\n \t\tstruct diff_queue_struct dq;\n \t\tstruct diff_options opts;\n+\t\tstruct range_diff_options range_diff_opts = {\n+\t\t\t.creation_factor = opt->creation_factor,\n+\t\t\t.dual_color = 1,\n+\t\t\t.diffopt = &opts\n+\t\t};\n \n \t\tmemcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff));\n \t\tDIFF_QUEUE_CLEAR(&diff_queued_diff);\n@@ -822,8 +827,7 @@ void show_log(struct rev_info *opt)\n \t\topts.file = opt->diffopt.file;\n \t\topts.use_color = opt->diffopt.use_color;\n \t\tdiff_setup_done(&opts);\n-\t\tshow_range_diff(opt->rdiff1, opt->rdiff2,\n-\t\t\t\topt->creation_factor, 1, &opts, NULL);\n+\t\tshow_range_diff(opt->rdiff1, opt->rdiff2, &range_diff_opts);\n \n \t\tmemcpy(&diff_queued_diff, &dq, sizeof(diff_queued_diff));\n \t}\ndiff --git a/range-diff.c b/range-diff.c\nindex d0d941a25add..25d4c244799c 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -526,33 +526,32 @@ static struct strbuf *output_prefix_cb(struct diff_options *opt, void *data)\n }\n \n int show_range_diff(const char *range1, const char *range2,\n-\t\t    int creation_factor, int dual_color,\n-\t\t    const struct diff_options *diffopt,\n-\t\t    const struct strvec *other_arg)\n+\t\t    struct range_diff_options *range_diff_opts)\n {\n \tint res = 0;\n \n \tstruct string_list branch1 = STRING_LIST_INIT_DUP;\n \tstruct string_list branch2 = STRING_LIST_INIT_DUP;\n \n-\tif (read_patches(range1, &branch1, other_arg))\n+\tif (read_patches(range1, &branch1, range_diff_opts->other_arg))\n \t\tres = error(_(\"could not parse log for '%s'\"), range1);\n-\tif (!res && read_patches(range2, &branch2, other_arg))\n+\tif (!res && read_patches(range2, &branch2, range_diff_opts->other_arg))\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\tif (diffopt)\n-\t\t\tmemcpy(&opts, diffopt, sizeof(opts));\n+\t\tif (range_diff_opts->diffopt)\n+\t\t\tmemcpy(&opts, range_diff_opts->diffopt, sizeof(opts));\n \t\telse\n \t\t\tdiff_setup(&opts);\n \n \t\tif (!opts.output_format)\n \t\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.flags.dual_color_diffed_diffs =\n+\t\t\trange_diff_opts->dual_color;\n \t\topts.flags.suppress_hunk_header_line_count = 1;\n \t\topts.output_prefix = output_prefix_cb;\n \t\tstrbuf_addstr(&indent, \"    \");\n@@ -560,7 +559,8 @@ int show_range_diff(const char *range1, const char *range2,\n \t\tdiff_setup_done(&opts);\n \n \t\tfind_exact_matches(&branch1, &branch2);\n-\t\tget_correspondences(&branch1, &branch2, creation_factor);\n+\t\tget_correspondences(&branch1, &branch2,\n+\t\t\t\t    range_diff_opts->creation_factor);\n \t\toutput(&branch1, &branch2, &opts);\n \n \t\tstrbuf_release(&indent);\ndiff --git a/range-diff.h b/range-diff.h\nindex 4abd70c40fed..a595f4e8db2d 100644\n--- a/range-diff.h\n+++ b/range-diff.h\n@@ -6,15 +6,19 @@\n \n #define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60\n \n+struct range_diff_options {\n+\tint creation_factor;\n+\tunsigned dual_color:1;\n+\tconst struct diff_options *diffopt; /* may be NULL */\n+\tconst struct strvec *other_arg; /* may be NULL */\n+};\n+\n /*\n- * Compare series of commits in RANGE1 and RANGE2, and emit to the\n- * standard output.  NULL can be passed to DIFFOPT to use the built-in\n- * default.\n+ * Compare series of commits in `range1` and `range2`, and emit to the\n+ * standard output.\n  */\n int show_range_diff(const char *range1, const char *range2,\n-\t\t    int creation_factor, int dual_color,\n-\t\t    const struct diff_options *diffopt,\n-\t\t    const struct strvec *other_arg);\n+\t\t    struct range_diff_options *opts);\n \n /*\n  * Determine whether the given argument is usable as a range argument of `git\n-- \ngitgitgadget\n\n"},{"id":"416452","messageId":"nycvar.QRO.7.76.6.2102082329460.29765@tvgsbejvaqbjf.bet","threadId":"55109","inReplyTo":"YB0Z5JXbe22ko2Mj@coredump.intra.peff.net","subject":"Re: [PATCH 0/6] Optionally restrict range-diff output to \"left\" or \"right\" range only","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-08T22:36:12Z","receivedAt":"2021-02-08T22:37:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 5 Feb 2021, Jeff King wrote:\n\n> On Thu, Feb 04, 2021 at 04:56:16PM -0800, Junio C Hamano wrote:\n>\n> > > As an aside: I am curious if I'm missing something when you say the\n> > > \"only way\" is to ask for a 'git range-diff ...@{u}'. IIUC what you're\n> > > describing, I often resort to using 'git cherry' for that exact thing.\n> > > But, I may not be quite understanding your use-case (and why git-cherry\n> > > doesn't do what you want already).\n> > >\n> > > My latter question is purely for satisfying my own curiosity; I don't\n> > > have any problem with a '--{left,right}-only' option in range-diff. From\n> > > my quick read of the patches, it all looks pretty sane to me.\n> >\n> > The question is addressed to Dscho, and I am also somewhat curious.\n> > Perhaps the reason would be that the output from cherry is not as\n> > easy to read as range-diff, without any post-processing.\n>\n> I had the same curiosity; I'd use git-cherry (or rev-list --cherry) for\n> this.\n>\n> I suspect the big difference is the quality of the matching. git-cherry\n> is purely looking at patch-ids.\n\nIndeed. Whenever I had tried `git cherry` in the past (which, admittedly,\nhas been with geometrically decreasing frequency given the results), it\ncompletely failed to help me. And it's not only its reliance on perfect\nmatches of the diff _with context lines_, it is also that the commit\nmessages are completely ignored.\n\n`git cherry`'s track record with me is so perfect that I want to put this\nline into all my Bash profiles:\n\n\teval \"$(set | sed -n '/^__git_main /,/^}$/{s/--list-cmds=list-mainporcelain[^)]*/& | grep -v ^cherry\\$/;p}')\"\n\n> So it is quite likely to say \"this was not applied upstream\" when what\n> got applied differed slightly (e.g., fixups upstream, applied to a\n> different base, etc). Whereas range-diff has some cost heuristics for\n> deciding that two patches are basically the same thing.  So it would\n> find more cases (and as a bonus, give you the diff to see what tweaks\n> were made upstream).\n>\n> It does make me wonder if it would be useful for rev-list, etc to have\n> an option to make \"--cherry\" use the more clever heuristics instead of\n> just a patch-id. It would never show the same diff output as range-diff,\n> but maybe more scripts would find the advanced heuristic useful.\n>\n> I know it would probably make rebase's \"ignore if in upstream\" feature\n> less clunky when I rebase topics. But it would also make it more\n> dangerous! E.g., right now I see any upstream tweaks as potential\n> conflicts when I rebase, and I manually review them for sanity.\n\nYeah, I thought the same when I read the paragraphs before this one. It\nmight sound convenient, but there _are_ false positives in `git\nrange-diff`'s output, therefore I would recommend never using\n`git range-diff --left-only` or `[...] --right-only` with `-s`. IOW\n_always_ inspect the differences.\n\nCiao,\nDscho\n"}]}