{"thread":{"id":"55268","subject":"[PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff","startedAt":"2021-03-07T07:05:44Z","lastAt":"2021-03-14T08:11:48Z","messageCount":16,"participants":["ZheNing Hu via GitGitGadget","Taylor Blau","ZheNing Hu","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"418414","messageId":"pull.898.git.1615100240295.gitgitgadget@gmail.com","threadId":"55268","inReplyTo":null,"subject":"[PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-07T06:57:19Z","receivedAt":"2021-03-07T07:05:44Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\nTaylor Blau proposing 'git format-patch --cover-letter\n--range-diff' may mistakenly place upstream commit in the\nrange-diff output. Teach `format-patch` pass `--right-only`\nto range-diff, maybe can avoid this kind of mistake.\n\nBecause `git rebase --apply` will internally call `git\nformat-patch -k --stdout --full-index --cherry-pick --right-only\n--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter\n--pretty=mboxrd --topo-order --no-base`, In order to avoid the\nambiguity of `--right-only`, users can call `format-patch\n--range-diff --only-right` instead of `--right-only` to only emit\noutput related to the second range.\n\nSigned-off-by: ZheNing Hu \n---\n [GSOC][RFC] format-patch: pass --right-only to range-diff\n \n Transfer parameters are relatively simple. But because I can' t\n reproduce the situation, There's no big difference between adding\n ---only-right and not adding ---only-right. So I haven't written the\n test files for the time being.\n \n I may need to ask reviewers opinion first.\n \n this want to fix #876 Thanks.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/898\n\n Documentation/git-format-patch.txt | 5 ++++-\n builtin/log.c | 15 ++++++++++-----\n 2 files changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 3e49bf221087..5dae34c3090b 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -27,7 +27,7 @@ SYNOPSIS\n \t\t [--[no-]encode-email-headers]\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n-\t\t [--range-diff= [--creation-factor=]]\n+\t\t [--range-diff= [--creation-factor=] [--right-only]]\n \t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n@@ -301,6 +301,9 @@ material (this may change in the future).\n \tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n \tfor details.\n \n+--only-right:\n+\tUsed with `--range-diff`, only emit output related to the second range.\n+\n --notes[=]::\n --no-notes::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f67b67d80ed1..5d2f39fd19a7 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n \t\t\t const char *branch_name,\n-\t\t\t int quiet)\n+\t\t\t int quiet, int right_only)\n {\n \tconst char *committer;\n \tstruct shortlog log;\n@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\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\t.other_arg = &other_arg,\n+\t\t\t.right_only = right_only\n \t\t};\n \n \t\tdiff_setup(&opts);\n@@ -1732,7 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tint creation_factor = -1;\n-\n+\tint right_only = 0;\n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n \t\t\t N_(\"use [PATCH n/m] even with a single patch\"),\n@@ -1814,6 +1815,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 or single patch\")),\n+\t\tOPT_BOOL(0, \"only-right\", &right_only,\n+\t\t\t N_(\"only emit output related to the second range\")),\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@@ -2087,7 +2090,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\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 (right_only && !rdiff_prev)\n+\t\tdie(_(\"--right_only requires --range-diff\"));\n \tif (rdiff_prev) {\n \t\tif (!cover_letter && total != 1)\n \t\t\tdie(_(\"--range-diff requires --cover-letter or single patch\"));\n@@ -2134,7 +2138,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t origin, nr, list, branch_name, quiet);\n+\t\t\t\t origin, nr, list, branch_name, quiet,\n+\t\t\t\t\tright_only);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\n\nbase-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50\n-- \ngitgitgadget\n"},{"id":"418524","messageId":"YEaGbn5rU3pU7/q5@nand.local","threadId":"55268","inReplyTo":"pull.898.git.1615100240295.gitgitgadget@gmail.com","subject":"Re: [PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff","fromName":"Taylor Blau","fromEmail":"ttaylorr@github.com","sentAt":"2021-03-08T20:18:04Z","receivedAt":"2021-03-08T20:19:07Z","isPatch":true,"sender":{"key":"ttaylorr@github.com","avatar":"https://gravatar.com/avatar/d5f3476f26b6f99cbb6b467e7ed7482f5762c8157bc73f569196e428bdcbea25?d=mp&s=160"},"body":"On Sun, Mar 07, 2021 at 06:57:19AM +0000, ZheNing Hu via GitGitGadget wrote:\n> From: ZheNing Hu \n>\n> In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n> Taylor Blau proposing 'git format-patch --cover-letter\n> --range-diff' may mistakenly place upstream commit in the\n> range-diff output. Teach `format-patch` pass `--right-only`\n> to range-diff, maybe can avoid this kind of mistake.\n>\n> Because `git rebase --apply` will internally call `git\n> format-patch -k --stdout --full-index --cherry-pick --right-only\n> --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter\n> --pretty=mboxrd --topo-order --no-base`, In order to avoid the\n> ambiguity of `--right-only`, users can call `format-patch\n> --range-diff --only-right` instead of `--right-only` to only emit\n> output related to the second range.\n\nA couple of times this new option is called \"--right-only\" and other\ntimes it is \"--only-right\". This comment makes me think that you meant\nto call it \"--only-right\", but...\n\n> Signed-off-by: ZheNing Hu \n> ---\n> [GSOC][RFC] format-patch: pass --right-only to range-diff\n>\n> Transfer parameters are relatively simple. But because I can' t\n> reproduce the situation, There's no big difference between adding\n> ---only-right and not adding ---only-right. So I haven't written the\n> test files for the time being.\n\nNote that I think (and Johannes--cc'd--could confirm) that what you want\nis '--left-only' to discard rebased changes from the upstream branch:\n\nIn either case, this should help you reproduce it (and would be a good\nstarting point for adding a test case). Here, we're using '--left-only'\nto discard changes from upstream (here, the 'main' branch) after\nrebasing:\n\n #!/bin/sh\n\n set -e\n\n rm -fr repo\n git init repo\n cd repo\n\n git branch -M main\n\n echo \"base\" >base\n git add base\n git commit -m \"base\"\n\n git checkout -b my-feature\n echo \"feature\" >feature\n git add feature\n git commit -m \"feature\"\n\n base=\"$(git rev-parse main)\"\n old=\"$(git rev-parse my-feature)\"\n\n git checkout main\n echo \"other\" >>base\n git add base\n git commit -m \"new\"\n\n git checkout my-feature\n git rebase $base --onto main\n\n tip=\"$(git rev-parse my-feature)\"\n\n git range-diff $base $old $tip\n git range-diff --left-only $base $old $tip\n\n> I may need to ask reviewers opinion first.\n>\n> this want to fix #876 Thanks.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/898\n>\n> Documentation/git-format-patch.txt | 5 ++++-\n> builtin/log.c | 15 ++++++++++-----\n> 2 files changed, 14 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index 3e49bf221087..5dae34c3090b 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -27,7 +27,7 @@ SYNOPSIS\n> \t\t [--[no-]encode-email-headers]\n> \t\t [--no-notes | --notes[=]]\n> \t\t [--interdiff=]\n> -\t\t [--range-diff= [--creation-factor=]]\n> +\t\t [--range-diff= [--creation-factor=] [--right-only]]\n> \t\t [--filename-max-length=]\n> \t\t [--progress]\n> \t\t []\n> @@ -301,6 +301,9 @@ material (this may change in the future).\n> \tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n> \tfor details.\n>\n> +--only-right:\n> +\tUsed with `--range-diff`, only emit output related to the second range.\n> +\n\ns/--only-right/--right-only?\n\n> --notes[=]::\n> --no-notes::\n> \tAppend the notes (see linkgit:git-notes[1]) for the commit\n> diff --git a/builtin/log.c b/builtin/log.c\n> index f67b67d80ed1..5d2f39fd19a7 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n> \t\t\t struct commit *origin,\n> \t\t\t int nr, struct commit **list,\n> \t\t\t const char *branch_name,\n> -\t\t\t int quiet)\n> +\t\t\t int quiet, int right_only)\n> {\n> \tconst char *committer;\n> \tstruct shortlog log;\n> @@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\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\t.other_arg = &other_arg,\n> +\t\t\t.right_only = right_only\n> \t\t};\n>\n> \t\tdiff_setup(&opts);\n> @@ -1732,7 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> \tstruct strbuf rdiff2 = STRBUF_INIT;\n> \tstruct strbuf rdiff_title = STRBUF_INIT;\n> \tint creation_factor = -1;\n> -\n> +\tint right_only = 0;\n\nNit: the newline between creation_factor's declaration and\nbuiltin_format_patch_options's is good to keep.\n\n> \tconst struct option builtin_format_patch_options[] = {\n> \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n> \t\t\t N_(\"use [PATCH n/m] even with a single patch\"),\n> @@ -1814,6 +1815,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 or single patch\")),\n> +\t\tOPT_BOOL(0, \"only-right\", &right_only,\n> +\t\t\t N_(\"only emit output related to the second range\")),\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> @@ -2087,7 +2090,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\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 (right_only && !rdiff_prev)\n> +\t\tdie(_(\"--right_only requires --range-diff\"));\n\nThese may be good to combine into one big conditional that dies whenever\nan option requiring --range-diff is set, but --range-diff is not:\n\n if (!rdiff_prev) {\n if (creation_factor)\n die(_(\"--creation-factor requires --range-diff\"));\n if (right_only)\n die(_(\"--right-only requires --range-diff\"));\n }\n\nThanks,\nTaylor\n"},{"id":"418580","messageId":"CAOLTT8REFb1QCpu=NQUBZJHy-uj_OhpNJJMu2PrTp6wkCDYr+w@mail.gmail.com","threadId":"55268","inReplyTo":"YEaGbn5rU3pU7/q5@nand.local","subject":"Re: [PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-03-09T07:28:24Z","receivedAt":"2021-03-09T07:29:33Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Taylor Blau 于2021年3月9日周二 上午4:18写道：\n>\n> On Sun, Mar 07, 2021 at 06:57:19AM +0000, ZheNing Hu via GitGitGadget wrote:\n> > From: ZheNing Hu \n> >\n> > In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n> > Taylor Blau proposing 'git format-patch --cover-letter\n> > --range-diff' may mistakenly place upstream commit in the\n> > range-diff output. Teach `format-patch` pass `--right-only`\n> > to range-diff, maybe can avoid this kind of mistake.\n> >\n> > Because `git rebase --apply` will internally call `git\n> > format-patch -k --stdout --full-index --cherry-pick --right-only\n> > --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter\n> > --pretty=mboxrd --topo-order --no-base`, In order to avoid the\n> > ambiguity of `--right-only`, users can call `format-patch\n> > --range-diff --only-right` instead of `--right-only` to only emit\n> > output related to the second range.\n>\n> A couple of times this new option is called \"--right-only\" and other\n> times it is \"--only-right\". This comment makes me think that you meant\n> to call it \"--only-right\", but...\n>\n> > Signed-off-by: ZheNing Hu \n> > ---\n> > [GSOC][RFC] format-patch: pass --right-only to range-diff\n> >\n> > Transfer parameters are relatively simple. But because I can' t\n> > reproduce the situation, There's no big difference between adding\n> > ---only-right and not adding ---only-right. So I haven't written the\n> > test files for the time being.\n>\n> Note that I think (and Johannes--cc'd--could confirm) that what you want\n> is '--left-only' to discard rebased changes from the upstream branch:\n>\n> In either case, this should help you reproduce it (and would be a good\n> starting point for adding a test case). Here, we're using '--left-only'\n> to discard changes from upstream (here, the 'main' branch) after\n> rebasing:\n>\n> #!/bin/sh\n>\n> set -e\n>\n> rm -fr repo\n> git init repo\n> cd repo\n>\n> git branch -M main\n>\n> echo \"base\" >base\n> git add base\n> git commit -m \"base\"\n>\n> git checkout -b my-feature\n> echo \"feature\" >feature\n> git add feature\n> git commit -m \"feature\"\n>\n> base=\"$(git rev-parse main)\"\n> old=\"$(git rev-parse my-feature)\"\n>\n> git checkout main\n> echo \"other\" >>base\n> git add base\n> git commit -m \"new\"\n>\n> git checkout my-feature\n> git rebase $base --onto main\n>\n> tip=\"$(git rev-parse my-feature)\"\n>\n> git range-diff $base $old $tip\n> git range-diff --left-only $base $old $tip\n>\nThanks, I have seen an obvious phenomenon from\nyour shell script, and I wrote test code based on it.\n\n> > I may need to ask reviewers opinion first.\n> >\n> > this want to fix #876 Thanks.\n> >\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/898\n> >\n> > Documentation/git-format-patch.txt | 5 ++++-\n> > builtin/log.c | 15 ++++++++++-----\n> > 2 files changed, 14 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> > index 3e49bf221087..5dae34c3090b 100644\n> > --- a/Documentation/git-format-patch.txt\n> > +++ b/Documentation/git-format-patch.txt\n> > @@ -27,7 +27,7 @@ SYNOPSIS\n> > [--[no-]encode-email-headers]\n> > [--no-notes | --notes[=]]\n> > [--interdiff=]\n> > - [--range-diff= [--creation-factor=]]\n> > + [--range-diff= [--creation-factor=] [--right-only]]\n> > [--filename-max-length=]\n> > [--progress]\n> > []\n> > @@ -301,6 +301,9 @@ material (this may change in the future).\n> > creation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n> > for details.\n> >\n> > +--only-right:\n> > + Used with `--range-diff`, only emit output related to the second range.\n> > +\n>\n> s/--only-right/--right-only?\nNow it's all \"--left-only\".\n>\n> > --notes[=]::\n> > --no-notes::\n> > Append the notes (see linkgit:git-notes[1]) for the commit\n> > diff --git a/builtin/log.c b/builtin/log.c\n> > index f67b67d80ed1..5d2f39fd19a7 100644\n> > --- a/builtin/log.c\n> > +++ b/builtin/log.c\n> > @@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n> > struct commit *origin,\n> > int nr, struct commit **list,\n> > const char *branch_name,\n> > - int quiet)\n> > + int quiet, int right_only)\n> > {\n> > const char *committer;\n> > struct shortlog log;\n> > @@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n> > .creation_factor = rev->creation_factor,\n> > .dual_color = 1,\n> > .diffopt = &opts,\n> > - .other_arg = &other_arg\n> > + .other_arg = &other_arg,\n> > + .right_only = right_only\n> > };\n> >\n> > diff_setup(&opts);\n> > @@ -1732,7 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> > struct strbuf rdiff2 = STRBUF_INIT;\n> > struct strbuf rdiff_title = STRBUF_INIT;\n> > int creation_factor = -1;\n> > -\n> > + int right_only = 0;\n>\n> Nit: the newline between creation_factor's declaration and\n> builtin_format_patch_options's is good to keep.\n>\n> > const struct option builtin_format_patch_options[] = {\n> > OPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n> > N_(\"use [PATCH n/m] even with a single patch\"),\n> > @@ -1814,6 +1815,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> > parse_opt_object_name),\n> > OPT_STRING(0, \"range-diff\", &rdiff_prev, N_(\"refspec\"),\n> > N_(\"show changes against in cover letter or single patch\")),\n> > + OPT_BOOL(0, \"only-right\", &right_only,\n> > + N_(\"only emit output related to the second range\")),\n> > OPT_INTEGER(0, \"creation-factor\", &creation_factor,\n> > N_(\"percentage by which creation is weighted\")),\n> > OPT_END()\n> > @@ -2087,7 +2090,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> > creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n> > else if (!rdiff_prev)\n> > die(_(\"--creation-factor requires --range-diff\"));\n> > -\n> > + if (right_only && !rdiff_prev)\n> > + die(_(\"--right_only requires --range-diff\"));\n>\n> These may be good to combine into one big conditional that dies whenever\n> an option requiring --range-diff is set, but --range-diff is not:\n>\nGood tip.\n> if (!rdiff_prev) {\n> if (creation_factor)\n> die(_(\"--creation-factor requires --range-diff\"));\n> if (right_only)\n> die(_(\"--right-only requires --range-diff\"));\n> }\n>\n> Thanks,\n> Taylor\n\nThanks,\nZheNing Hu\n"},{"id":"418582","messageId":"pull.898.v2.git.1615278830804.gitgitgadget@gmail.com","threadId":"55268","inReplyTo":"pull.898.git.1615100240295.gitgitgadget@gmail.com","subject":"[PATCH v2] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-09T08:33:50Z","receivedAt":"2021-03-09T08:34:49Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\nTaylor Blau proposing `git format-patch --cover-letter\n--range-diff` may mistakenly place upstream commit in the\nrange-diff output. Teach `format-patch` pass `--left-only`\nto range-diff,can avoid this kind of mistake.\n\nSigned-off-by: ZheNing Hu \n---\n [GSOC][RFC] format-patch: pass --left-only to range-diff\n \n With the help of Taylor Blau, I understood why the upstream commit\n appeared in the range-diff, and completed the writing of the test.\n \n this want to fix #876 Thanks.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/898\n\nRange-diff vs v1:\n\n 1: 590bd06ee5ee ! 1: 8daffd4f7546 [GSOC][RFC] format-patch: pass --right-only to range-diff\n @@ Metadata\n Author: ZheNing Hu \n \n ## Commit message ##\n - [GSOC][RFC] format-patch: pass --right-only to range-diff\n + [GSOC][RFC] format-patch: pass --left-only to range-diff\n \n In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n - Taylor Blau proposing 'git format-patch --cover-letter\n - --range-diff' may mistakenly place upstream commit in the\n - range-diff output. Teach `format-patch` pass `--right-only`\n - to range-diff, maybe can avoid this kind of mistake.\n -\n - Because `git rebase --apply` will internally call `git\n - format-patch -k --stdout --full-index --cherry-pick --right-only\n - --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter\n - --pretty=mboxrd --topo-order --no-base`, In order to avoid the\n - ambiguity of `--right-only`, users can call `format-patch\n - --range-diff --only-right` instead of `--right-only` to only emit\n - output related to the second range.\n + Taylor Blau proposing `git format-patch --cover-letter\n + --range-diff` may mistakenly place upstream commit in the\n + range-diff output. Teach `format-patch` pass `--left-only`\n + to range-diff,can avoid this kind of mistake.\n \n Signed-off-by: ZheNing Hu \n \n @@ Documentation/git-format-patch.txt: SYNOPSIS\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n -\t\t [--range-diff= [--creation-factor=]]\n -+\t\t [--range-diff= [--creation-factor=] [--right-only]]\n ++\t\t [--range-diff= [--creation-factor=] [--left-only]]\n \t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n @@ Documentation/git-format-patch.txt: material (this may change in the future).\n \tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n \tfor details.\n \n -+--only-right:\n -+\tUsed with `--range-diff`, only emit output related to the second range.\n ++--left-only:\n ++\tUsed with `--range-diff`, only emit output related to the first range.\n +\n --notes[=]::\n --no-notes::\n @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ\n \t\t\t int nr, struct commit **list,\n \t\t\t const char *branch_name,\n -\t\t\t int quiet)\n -+\t\t\t int quiet, int right_only)\n ++\t\t\t int quiet, int left_only)\n {\n \tconst char *committer;\n \tstruct shortlog log;\n @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ\n \t\t\t.diffopt = &opts,\n -\t\t\t.other_arg = &other_arg\n +\t\t\t.other_arg = &other_arg,\n -+\t\t\t.right_only = right_only\n ++\t\t\t.left_only = left_only\n \t\t};\n \n \t\tdiff_setup(&opts);\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tint creation_factor = -1;\n --\n -+\tint right_only = 0;\n ++\tint left_only = 0;\n + \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n - \t\t\t N_(\"use [PATCH n/m] even with a single patch\"),\n @@ builtin/log.c: 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 or single patch\")),\n -+\t\tOPT_BOOL(0, \"only-right\", &right_only,\n -+\t\t\t N_(\"only emit output related to the second range\")),\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_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\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 +-\telse if (!rdiff_prev)\n +-\t\tdie(_(\"--creation-factor requires --range-diff\"));\n -\n -+\tif (right_only && !rdiff_prev)\n -+\t\tdie(_(\"--right_only requires --range-diff\"));\n ++\telse if (!rdiff_prev) {\n ++\t\tif (creation_factor >= 0)\n ++\t\t\tdie(_(\"--creation-factor requires --range-diff\"));\n ++\t\tif (left_only)\n ++\t\t\tdie(_(\"--left-only requires --range-diff\"));\n ++\t}\n \tif (rdiff_prev) {\n \t\tif (!cover_letter && total != 1)\n \t\t\tdie(_(\"--range-diff requires --cover-letter or single patch\"));\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \t\tmake_cover_letter(&rev, !!output_directory,\n -\t\t\t\t origin, nr, list, branch_name, quiet);\n +\t\t\t\t origin, nr, list, branch_name, quiet,\n -+\t\t\t\t\tright_only);\n ++\t\t\t\t\tleft_only);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\n +\n + ## t/t3206-range-diff.sh ##\n +@@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '\n + \ttest_cmp expect actual\n + '\n + \n ++test_expect_success 'format-patch --range-diff --left-only' '\n ++\trm -fr repo &&\n ++\tgit init repo &&\n ++\tcd repo &&\n ++\tgit branch -M main &&\n ++\techo \"base\" >base &&\n ++\tgit add base &&\n ++\tgit commit -m \"base\" &&\n ++\tgit checkout -b my-feature &&\n ++\techo \"feature\" >feature &&\n ++\tgit add feature &&\n ++\tgit commit -m \"feature\" &&\n ++\tbase=\"$(git rev-parse main)\" &&\n ++\told=\"$(git rev-parse my-feature)\" &&\n ++\tgit checkout main &&\n ++\techo \"other\" >>base &&\n ++\tgit add base &&\n ++\tgit commit -m \"new\" &&\n ++\tgit checkout my-feature &&\n ++\tgit rebase $base --onto main &&\n ++\ttip=\"$(git rev-parse my-feature)\" &&\n ++\tgit format-patch --range-diff $base $old $tip --cover-letter &&\n ++\tgrep \"> 1: .* feature$\" 0000-cover-letter.patch &&\n ++\tgit format-patch --range-diff $base $old $tip --left-only --cover-letter &&\n ++\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n ++'\n ++\n ++\n + test_done\n\n\n Documentation/git-format-patch.txt | 5 ++++-\n builtin/log.c | 20 ++++++++++++++------\n t/t3206-range-diff.sh | 28 ++++++++++++++++++++++++++++\n 3 files changed, 46 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 3e49bf221087..8c5eca0ba1e3 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -27,7 +27,7 @@ SYNOPSIS\n \t\t [--[no-]encode-email-headers]\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n-\t\t [--range-diff= [--creation-factor=]]\n+\t\t [--range-diff= [--creation-factor=] [--left-only]]\n \t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n@@ -301,6 +301,9 @@ material (this may change in the future).\n \tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n \tfor details.\n \n+--left-only:\n+\tUsed with `--range-diff`, only emit output related to the first range.\n+\n --notes[=]::\n --no-notes::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f67b67d80ed1..5620dfafb37e 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n \t\t\t const char *branch_name,\n-\t\t\t int quiet)\n+\t\t\t int quiet, int left_only)\n {\n \tconst char *committer;\n \tstruct shortlog log;\n@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\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\t.other_arg = &other_arg,\n+\t\t\t.left_only = left_only\n \t\t};\n \n \t\tdiff_setup(&opts);\n@@ -1732,6 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tint creation_factor = -1;\n+\tint left_only = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1814,6 +1816,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 or single patch\")),\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_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n@@ -2085,9 +2089,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\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+\telse if (!rdiff_prev) {\n+\t\tif (creation_factor >= 0)\n+\t\t\tdie(_(\"--creation-factor requires --range-diff\"));\n+\t\tif (left_only)\n+\t\t\tdie(_(\"--left-only requires --range-diff\"));\n+\t}\n \tif (rdiff_prev) {\n \t\tif (!cover_letter && total != 1)\n \t\t\tdie(_(\"--range-diff requires --cover-letter or single patch\"));\n@@ -2134,7 +2141,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t origin, nr, list, branch_name, quiet);\n+\t\t\t\t origin, nr, list, branch_name, quiet,\n+\t\t\t\t\tleft_only);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 1b26c4c2ef91..abb0cf7fb465 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -748,4 +748,32 @@ test_expect_success '--left-only/--right-only' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch --range-diff --left-only' '\n+\trm -fr repo &&\n+\tgit init repo &&\n+\tcd repo &&\n+\tgit branch -M main &&\n+\techo \"base\" >base &&\n+\tgit add base &&\n+\tgit commit -m \"base\" &&\n+\tgit checkout -b my-feature &&\n+\techo \"feature\" >feature &&\n+\tgit add feature &&\n+\tgit commit -m \"feature\" &&\n+\tbase=\"$(git rev-parse main)\" &&\n+\told=\"$(git rev-parse my-feature)\" &&\n+\tgit checkout main &&\n+\techo \"other\" >>base &&\n+\tgit add base &&\n+\tgit commit -m \"new\" &&\n+\tgit checkout my-feature &&\n+\tgit rebase $base --onto main &&\n+\ttip=\"$(git rev-parse my-feature)\" &&\n+\tgit format-patch --range-diff $base $old $tip --cover-letter &&\n+\tgrep \"> 1: .* feature$\" 0000-cover-letter.patch &&\n+\tgit format-patch --range-diff $base $old $tip --left-only --cover-letter &&\n+\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n+'\n+\n+\n test_done\n\nbase-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50\n-- \ngitgitgadget\n"},{"id":"418583","messageId":"CAPig+cQ6oS8S1-AmRhM=Gb3EQ+7sdQxExoVPCoHKSRFivZq9Vw@mail.gmail.com","threadId":"55268","inReplyTo":"pull.898.v2.git.1615278830804.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-03-09T09:00:10Z","receivedAt":"2021-03-09T09:01:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 9, 2021 at 3:35 AM ZheNing Hu via GitGitGadget\n wrote:\n> In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n> Taylor Blau proposing `git format-patch --cover-letter\n> --range-diff` may mistakenly place upstream commit in the\n> range-diff output. Teach `format-patch` pass `--left-only`\n> to range-diff,can avoid this kind of mistake.\n>\n> Signed-off-by: ZheNing Hu \n> ---\n> diff --git a/builtin/log.c b/builtin/log.c\n> @@ -2085,9 +2089,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> if (creation_factor < 0)\n> creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n> - else if (!rdiff_prev)\n> - die(_(\"--creation-factor requires --range-diff\"));\n> -\n> + else if (!rdiff_prev) {\n> + if (creation_factor >= 0)\n> + die(_(\"--creation-factor requires --range-diff\"));\n> + if (left_only)\n> + die(_(\"--left-only requires --range-diff\"));\n> + }\n\nThis logic starts getting difficult to reason about. It would be\neasier to understand if written like this:\n\n if (!rdiff_prev) {\n if (creation_factor >= 0)\n die(_(\"--creation-factor requires --range-diff\"));\n if (left_only)\n die(_(\"--left-only requires --range-diff\"));\n }\n\n if (creation_factor < 0)\n creation_factor = RANGE_DIFF_..._DEFAULT;\n\nor this (which reduces the indentation a bit):\n\n if (creation_factor >= 0 && !rdiff_prev)\n die(_(\"--creation-factor requires --range-diff\"));\n if (left_only && !rdiff_prev)\n die(_(\"--left-only requires --range-diff\"));\n\n if (creation_factor < 0)\n creation_factor = RANGE_DIFF_..._DEFAULT;\n\nSubjective; not necessarily worth a re-roll.\n\n> @@ -2134,7 +2141,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> make_cover_letter(&rev, !!output_directory,\n> - origin, nr, list, branch_name, quiet);\n> + origin, nr, list, branch_name, quiet,\n> + left_only);\n\nNit: This indentation looks odd. One would expect `origin` and\n`left_only` to have the same indentation.\n\n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> @@ -748,4 +748,32 @@ test_expect_success '--left-only/--right-only' '\n> +test_expect_success 'format-patch --range-diff --left-only' '\n> + rm -fr repo &&\n> + git init repo &&\n> + ...\n> + ! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n> +'\n> +\n> +\n> test_done\n\nNit: One blank line before test_done() is typical, not two.\n"},{"id":"418584","messageId":"CAOLTT8SDs76dCdXi1rn_QVi924V7C5S4sLt8-EA1hFKPrFZKQQ@mail.gmail.com","threadId":"55268","inReplyTo":"CAPig+cQ6oS8S1-AmRhM=Gb3EQ+7sdQxExoVPCoHKSRFivZq9Vw@mail.gmail.com","subject":"Re: [PATCH v2] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-03-09T09:35:23Z","receivedAt":"2021-03-09T09:36:22Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Eric Sunshine 于2021年3月9日周二 下午5:00写道：\n>\n> On Tue, Mar 9, 2021 at 3:35 AM ZheNing Hu via GitGitGadget\n> wrote:\n> > In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n> > Taylor Blau proposing `git format-patch --cover-letter\n> > --range-diff` may mistakenly place upstream commit in the\n> > range-diff output. Teach `format-patch` pass `--left-only`\n> > to range-diff,can avoid this kind of mistake.\n> >\n> > Signed-off-by: ZheNing Hu \n> > ---\n> > diff --git a/builtin/log.c b/builtin/log.c\n> > @@ -2085,9 +2089,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> > if (creation_factor < 0)\n> > creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n> > - else if (!rdiff_prev)\n> > - die(_(\"--creation-factor requires --range-diff\"));\n> > -\n> > + else if (!rdiff_prev) {\n> > + if (creation_factor >= 0)\n> > + die(_(\"--creation-factor requires --range-diff\"));\n> > + if (left_only)\n> > + die(_(\"--left-only requires --range-diff\"));\n> > + }\n>\n> This logic starts getting difficult to reason about. It would be\n> easier to understand if written like this:\n>\n> if (!rdiff_prev) {\n> if (creation_factor >= 0)\n> die(_(\"--creation-factor requires --range-diff\"));\n> if (left_only)\n> die(_(\"--left-only requires --range-diff\"));\n> }\n>\n> if (creation_factor < 0)\n> creation_factor = RANGE_DIFF_..._DEFAULT;\n>\n> or this (which reduces the indentation a bit):\n>\n> if (creation_factor >= 0 && !rdiff_prev)\n> die(_(\"--creation-factor requires --range-diff\"));\n> if (left_only && !rdiff_prev)\n> die(_(\"--left-only requires --range-diff\"));\n>\n> if (creation_factor < 0)\n> creation_factor = RANGE_DIFF_..._DEFAULT;\n>\nEmm,I would prefer to use the above.\n> Subjective; not necessarily worth a re-roll.\n>\n> > @@ -2134,7 +2141,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n> > make_cover_letter(&rev, !!output_directory,\n> > - origin, nr, list, branch_name, quiet);\n> > + origin, nr, list, branch_name, quiet,\n> > + left_only);\n>\n> Nit: This indentation looks odd. One would expect `origin` and\n> `left_only` to have the same indentation.\n>\n> > diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> > @@ -748,4 +748,32 @@ test_expect_success '--left-only/--right-only' '\n> > +test_expect_success 'format-patch --range-diff --left-only' '\n> > + rm -fr repo &&\n> > + git init repo &&\n> > + ...\n> > + ! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n> > +'\n> > +\n> > +\n> > test_done\n>\n> Nit: One blank line before test_done() is typical, not two.\n\nThanks for the reminder.\nI'm going to modify these format errors.\n"},{"id":"418585","messageId":"pull.898.v3.git.1615285726482.gitgitgadget@gmail.com","threadId":"55268","inReplyTo":"pull.898.v2.git.1615278830804.gitgitgadget@gmail.com","subject":"[PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-09T10:28:46Z","receivedAt":"2021-03-09T10:29:35Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nIn https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\nTaylor Blau proposing `git format-patch --cover-letter\n--range-diff` may mistakenly place upstream commit in the\nrange-diff output. Teach `format-patch` pass `--left-only`\nto range-diff,can avoid this kind of mistake.\n\nSigned-off-by: ZheNing Hu \n---\n [GSOC][RFC] format-patch: pass --left-only to range-diff\n \n With the help of Taylor Blau, I understood why the upstream commit\n appeared in the range-diff, and completed the writing of the test.\n \n this want to fix #876 Thanks.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/898\n\nRange-diff vs v2:\n\n 1: 8daffd4f7546 ! 1: 5c58eb186d41 [GSOC][RFC] format-patch: pass --left-only to range-diff\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre\n \t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n @@ builtin/log.c: 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 -+\telse if (!rdiff_prev) {\n ++\tif (!rdiff_prev) {\n +\t\tif (creation_factor >= 0)\n +\t\t\tdie(_(\"--creation-factor requires --range-diff\"));\n +\t\tif (left_only)\n +\t\t\tdie(_(\"--left-only requires --range-diff\"));\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 && total != 1)\n - \t\t\tdie(_(\"--range-diff requires --cover-letter or single patch\"));\n @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n -\t\t\t\t origin, nr, list, branch_name, quiet);\n +\t\t\t\t origin, nr, list, branch_name, quiet,\n -+\t\t\t\t\tleft_only);\n ++\t\t\t\t left_only);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\n @@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '\n +\tgit rebase $base --onto main &&\n +\ttip=\"$(git rev-parse my-feature)\" &&\n +\tgit format-patch --range-diff $base $old $tip --cover-letter &&\n -+\tgrep \"> 1: .* feature$\" 0000-cover-letter.patch &&\n ++\tgrep \"> 1: .* feature$\" 0000-cover-letter.patch &&\n +\tgit format-patch --range-diff $base $old $tip --left-only --cover-letter &&\n -+\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n ++\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n +'\n -+\n +\n test_done\n\n\n Documentation/git-format-patch.txt | 5 ++++-\n builtin/log.c | 20 +++++++++++++++-----\n t/t3206-range-diff.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 46 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 3e49bf221087..8c5eca0ba1e3 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -27,7 +27,7 @@ SYNOPSIS\n \t\t [--[no-]encode-email-headers]\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n-\t\t [--range-diff= [--creation-factor=]]\n+\t\t [--range-diff= [--creation-factor=] [--left-only]]\n \t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n@@ -301,6 +301,9 @@ material (this may change in the future).\n \tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n \tfor details.\n \n+--left-only:\n+\tUsed with `--range-diff`, only emit output related to the first range.\n+\n --notes[=]::\n --no-notes::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f67b67d80ed1..21fed9db82d6 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n \t\t\t const char *branch_name,\n-\t\t\t int quiet)\n+\t\t\t int quiet, int left_only)\n {\n \tconst char *committer;\n \tstruct shortlog log;\n@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\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\t.other_arg = &other_arg,\n+\t\t\t.left_only = left_only\n \t\t};\n \n \t\tdiff_setup(&opts);\n@@ -1732,6 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tint creation_factor = -1;\n+\tint left_only = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1814,6 +1816,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 or single patch\")),\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_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n@@ -2083,10 +2087,15 @@ 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 (creation_factor >= 0)\n+\t\t\tdie(_(\"--creation-factor requires --range-diff\"));\n+\t\tif (left_only)\n+\t\t\tdie(_(\"--left-only requires --range-diff\"));\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 && total != 1)\n@@ -2134,7 +2143,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t origin, nr, list, branch_name, quiet);\n+\t\t\t\t origin, nr, list, branch_name, quiet,\n+\t\t\t\t left_only);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 1b26c4c2ef91..6178a12dd4b1 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -748,4 +748,31 @@ test_expect_success '--left-only/--right-only' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch --range-diff --left-only' '\n+\trm -fr repo &&\n+\tgit init repo &&\n+\tcd repo &&\n+\tgit branch -M main &&\n+\techo \"base\" >base &&\n+\tgit add base &&\n+\tgit commit -m \"base\" &&\n+\tgit checkout -b my-feature &&\n+\techo \"feature\" >feature &&\n+\tgit add feature &&\n+\tgit commit -m \"feature\" &&\n+\tbase=\"$(git rev-parse main)\" &&\n+\told=\"$(git rev-parse my-feature)\" &&\n+\tgit checkout main &&\n+\techo \"other\" >>base &&\n+\tgit add base &&\n+\tgit commit -m \"new\" &&\n+\tgit checkout my-feature &&\n+\tgit rebase $base --onto main &&\n+\ttip=\"$(git rev-parse my-feature)\" &&\n+\tgit format-patch --range-diff $base $old $tip --cover-letter &&\n+\tgrep \"> 1: .* feature$\" 0000-cover-letter.patch &&\n+\tgit format-patch --range-diff $base $old $tip --left-only --cover-letter &&\n+\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n+'\n+\n test_done\n\nbase-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50\n-- \ngitgitgadget\n"},{"id":"419002","messageId":"xmqq1rck6nr8.fsf@gitster.g","threadId":"55268","inReplyTo":"YEaGbn5rU3pU7/q5@nand.local","subject":"Re: [PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-12T21:55:07Z","receivedAt":"2021-03-12T21:55:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Note that I think (and Johannes--cc'd--could confirm) that what you want\n> is '--left-only' to discard rebased changes from the upstream branch:\n\nDoes that mean range-diff got --left-only and --right-only\nbackwards? IOW, doesn't\n\n$ git log --left-only A...B\n\nshow what is shown in \"git log B..A\", and if so, shouldn't\n\n$ git range-diff --left-only A...B\n\nwhich is a synonym for\n\n$ git range-diff B..A A..B\n\nalso give commits in the B..A range?\n\nPuzzled....\n\n"},{"id":"419003","messageId":"xmqqv99w58iq.fsf@gitster.g","threadId":"55268","inReplyTo":"xmqq1rck6nr8.fsf@gitster.g","subject":"Re: [PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-12T22:09:33Z","receivedAt":"2021-03-12T22:10:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Taylor Blau writes:\n>\n>> Note that I think (and Johannes--cc'd--could confirm) that what you want\n>> is '--left-only' to discard rebased changes from the upstream branch:\n>\n> Does that mean range-diff got --left-only and --right-only\n> backwards? IOW, doesn't\n>\n> $ git log --left-only A...B\n>\n> show what is shown in \"git log B..A\", and if so, shouldn't\n>\n> $ git range-diff --left-only A...B\n>\n> which is a synonym for\n>\n> $ git range-diff B..A A..B\n>\n> also give commits in the B..A range?\n>\n> Puzzled....\n\nAh, it was only that \"range-diff --help\" uses confusing\ndouble-negative phrasing. I.e.\n\n --left-only::\n Suppress commits that are missing from the first specified range\n (or the \"left range\" when using the `...` format).\n\nIf you \"suppress those that are missing from X\", you are effectively\nincluding only those that appear in X, so two conclusions I draw are\n\n - \"range-diff\" does not get \"--left/right-only\" backwards;\n\n - its documentation should be improved.\n\nPerhaps\n\n --left-only::\n\tInclude only commits that are in the first range (or the\n\t\"left range\" when using the `...` format).\n\nwouldn't have made me puzzled.\n\nThanks.\n"},{"id":"419013","messageId":"xmqqpn0456lr.fsf@gitster.g","threadId":"55268","inReplyTo":"pull.898.v3.git.1615285726482.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-12T22:50:56Z","receivedAt":"2021-03-12T22:51:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> From: ZheNing Hu \n>\n> In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n> Taylor Blau proposing `git format-patch --cover-letter\n> --range-diff` may mistakenly place upstream commit in the\n> range-diff output. Teach `format-patch` pass `--left-only`\n> to range-diff,can avoid this kind of mistake.\n\nThe above is a bit too dense for average readers to grok. Even if\nthe readers refer to the external reference, it is unclear where the\n\"may mistakenly\" can come from and why \"--left-only\" would be\nuseful (and our log message should not depend on external material\nso heavily to begin with).\n\nSo let's think aloud to see what use case this may be helpful, and\nhow the proposed solution makes the world a better place.\n\nIf I understand correctly, the use case this tries to help is this:\n\n * You had sent the v1 iteration of topic. It was in the range\n B1..T1 where B1 is the tip of the integration branch (like\n 'master') from the upstream.\n\n * To prepare for the v2 iteration, not only you updated individual\n commits, you rebased the series on a new upstream. Now the topic\n is in the range B2..T2, where B2 is the tip of the integration\n branch from the upstream, and it is very likely that B2 is a\n descendant of B1.\n\nAnd you want to find out how your commits in T2 (new iteration)\ncompares with those in T1 (old iteration). Normally,\n\n $ git range-diff T1...T2\n\nwould be the shortest-to-type and correct version but that is\ninvalidated because you rebased.\n\n ---o---B1--b---b---b---B2\n \\ \\\n\t t---t---T1 s---s---s---T2\n\nYou'd have commits B1..T1 on the left hand side of the range-diff,\nwhile the right hand side has not just B2..T2 but also commits in\nthe range B1..B2, too.\n\nBy using --left-only (i.e. show only those pair that maps from\ncommits in the left range), you can exclude the commits in the\nB1..B2.\n\n $ git range-diff --left-only T1...T2\n\nI however wonder what --left-only (Suppress commits that are missing\nfrom the first range) would do to commits in range B2..T2 (they are\nall yours) that are (1) added since the v1 iteration, or (2)\nmodified so drastically that no matching commit is found. With the\nright invocation, of course,\n\n $ git range-diff B1..T1 B2..T2\n\nyou would not have such a problem. If 2 't's in B1..T1 correspond\nto 2 of the 3 's's in B2..T2, at least the presense of the third 's'\nthat did not match would show up in the output, making it clear that\nyou have one more commit relative to the earlier iteration. If use\nof --left-only filters it out, the output may be misleading to the\nreaders, no?\n\nI started writing (or \"thinking aloud\") hoping that I can help\ncoming up with a better log message to describe the problem being\nsolved, but I ended up with \"does this make the system better?\"\n"},{"id":"419024","messageId":"CAOLTT8QhgeGim6ujqqyXwQ=bmQtJ43T5i3CPmNMPmBr0amR-HQ@mail.gmail.com","threadId":"55268","inReplyTo":"xmqqpn0456lr.fsf@gitster.g","subject":"Re: [PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-03-13T04:01:50Z","receivedAt":"2021-03-13T04:04:25Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年3月13日周六 上午6:51写道：\n>\n> \"ZheNing Hu via GitGitGadget\" writes:\n>\n> > From: ZheNing Hu \n> >\n> > In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n> > Taylor Blau proposing `git format-patch --cover-letter\n> > --range-diff` may mistakenly place upstream commit in the\n> > range-diff output. Teach `format-patch` pass `--left-only`\n> > to range-diff,can avoid this kind of mistake.\n>\n> The above is a bit too dense for average readers to grok. Even if\n> the readers refer to the external reference, it is unclear where the\n> \"may mistakenly\" can come from and why \"--left-only\" would be\n> useful (and our log message should not depend on external material\n> so heavily to begin with).\n>\n\nYou are right, commit information with the original thread link may make\nit difficult for readers to read. I will pay attention.\n\n> So let's think aloud to see what use case this may be helpful, and\n> how the proposed solution makes the world a better place.\n>\n> If I understand correctly, the use case this tries to help is this:\n>\n> * You had sent the v1 iteration of topic. It was in the range\n> B1..T1 where B1 is the tip of the integration branch (like\n> 'master') from the upstream.\n>\n> * To prepare for the v2 iteration, not only you updated individual\n> commits, you rebased the series on a new upstream. Now the topic\n> is in the range B2..T2, where B2 is the tip of the integration\n> branch from the upstream, and it is very likely that B2 is a\n> descendant of B1.\n>\n> And you want to find out how your commits in T2 (new iteration)\n> compares with those in T1 (old iteration). Normally,\n>\n> $ git range-diff T1...T2\n>\n> would be the shortest-to-type and correct version but that is\n> invalidated because you rebased.\n>\n> ---o---B1--b---b---b---B2\n> \\ \\\n> t---t---T1 s---s---s---T2\n>\n> You'd have commits B1..T1 on the left hand side of the range-diff,\n> while the right hand side has not just B2..T2 but also commits in\n> the range B1..B2, too.\n>\n> By using --left-only (i.e. show only those pair that maps from\n> commits in the left range), you can exclude the commits in the\n> B1..B2.\n>\n> $ git range-diff --left-only T1...T2\n>\n> I however wonder what --left-only (Suppress commits that are missing\n> from the first range) would do to commits in range B2..T2 (they are\n> all yours) that are (1) added since the v1 iteration, or (2)\n> modified so drastically that no matching commit is found. With the\n> right invocation, of course,\n>\n> $ git range-diff B1..T1 B2..T2\n>\n> you would not have such a problem. If 2 't's in B1..T1 correspond\n> to 2 of the 3 's's in B2..T2, at least the presense of the third 's'\n> that did not match would show up in the output, making it clear that\n> you have one more commit relative to the earlier iteration. If use\n> of --left-only filters it out, the output may be misleading to the\n> readers, no?\n>\n> I started writing (or \"thinking aloud\") hoping that I can help\n> coming up with a better log message to describe the problem being\n> solved, but I ended up with \"does this make the system better?\"\n\nJunio, thank you for elaborating this issue in detail and clearly.\nI probably understand what you mean by \"git range-diff B1..T1 B2..T2\"\n to correctly output the commits on my two version topic branch, without\nincluding the upstream commits of B1..B2.So we don’t even need to specify\nthe `--left-only` to avoid the output of B1...B2, right?\n\nThe only thing I can think of now is that if users tend to use T1...T2\nto compare\n the differences between the two topics, will the upstream commit in\nB1...B2 appear\nmore abrupt?\n\nThanks.\n"},{"id":"419060","messageId":"xmqqwnua4ozy.fsf@gitster.g","threadId":"55268","inReplyTo":"CAOLTT8QhgeGim6ujqqyXwQ=bmQtJ43T5i3CPmNMPmBr0amR-HQ@mail.gmail.com","subject":"Re: [PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-13T23:23:29Z","receivedAt":"2021-03-13T23:24:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ZheNing Hu writes:\n\n> Junio C Hamano 于2021年3月13日周六 上午6:51写道：\n>>\n>> \"ZheNing Hu via GitGitGadget\" writes:\n>>\n>> > From: ZheNing Hu \n>> >\n>> > In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n>> > Taylor Blau proposing `git format-patch --cover-letter\n>> > --range-diff` may mistakenly place upstream commit in the\n>> > range-diff output. Teach `format-patch` pass `--left-only`\n>> > to range-diff,can avoid this kind of mistake.\n>>\n>> The above is a bit too dense for average readers to grok. Even if\n>> the readers refer to the external reference, it is unclear where the\n>> \"may mistakenly\" can come from and why \"--left-only\" would be\n>> useful (and our log message should not depend on external material\n>> so heavily to begin with).\n>>\n>\n> You are right, commit information with the original thread link may make\n> it difficult for readers to read. I will pay attention.\n>\n>> So let's think aloud to see what use case this may be helpful, and\n>> how the proposed solution makes the world a better place.\n>>\n>> If I understand correctly, the use case this tries to help is this:\n>>\n>> * You had sent the v1 iteration of topic. It was in the range\n>> B1..T1 where B1 is the tip of the integration branch (like\n>> 'master') from the upstream.\n>>\n>> * To prepare for the v2 iteration, not only you updated individual\n>> commits, you rebased the series on a new upstream. Now the topic\n>> is in the range B2..T2, where B2 is the tip of the integration\n>> branch from the upstream, and it is very likely that B2 is a\n>> descendant of B1.\n>>\n>> And you want to find out how your commits in T2 (new iteration)\n>> compares with those in T1 (old iteration). Normally,\n>>\n>> $ git range-diff T1...T2\n>>\n>> would be the shortest-to-type and correct version but that is\n>> invalidated because you rebased.\n>>\n>> ---o---B1--b---b---b---B2\n>> \\ \\\n>> t---t---T1 s---s---s---T2\n>>\n>> You'd have commits B1..T1 on the left hand side of the range-diff,\n>> while the right hand side has not just B2..T2 but also commits in\n>> the range B1..B2, too.\n>>\n>> By using --left-only (i.e. show only those pair that maps from\n>> commits in the left range), you can exclude the commits in the\n>> B1..B2.\n>>\n>> $ git range-diff --left-only T1...T2\n>>\n>> I however wonder what --left-only (Suppress commits that are missing\n>> from the first range) would do to commits in range B2..T2 (they are\n>> all yours) that are (1) added since the v1 iteration, or (2)\n>> modified so drastically that no matching commit is found. With the\n>> right invocation, of course,\n>>\n>> $ git range-diff B1..T1 B2..T2\n>>\n>> you would not have such a problem. If 2 't's in B1..T1 correspond\n>> to 2 of the 3 's's in B2..T2, at least the presense of the third 's'\n>> that did not match would show up in the output, making it clear that\n>> you have one more commit relative to the earlier iteration. If use\n>> of --left-only filters it out, the output may be misleading to the\n>> readers, no?\n>>\n>> I started writing (or \"thinking aloud\") hoping that I can help\n>> coming up with a better log message to describe the problem being\n>> solved, but I ended up with \"does this make the system better?\"\n>\n> Junio, thank you for elaborating this issue in detail and clearly.\n> I probably understand what you mean by \"git range-diff B1..T1 B2..T2\"\n> to correctly output the commits on my two version topic branch, without\n> including the upstream commits of B1..B2.So we don’t even need to specify\n> the `--left-only` to avoid the output of B1...B2, right?\n>\n> The only thing I can think of now is that if users tend to use T1...T2\n> to compare\n> the differences between the two topics, will the upstream commit in\n> B1...B2 appear\n> more abrupt?\n\nYes, it would be, but that is why you need to educate users what\ncauses it, and what the right way to avoid unrelated commits from\nappearing, and how this --left-only fits in the solution.\n\nIf some of the time, \"--left-only T1...T2\" would give you the same\nresult as the more strict \"B1..T1 B2..T2\", that may be why users may\nwant to use the \"--left-only\" instead as an easy/lazy alternative.\n\nBut I suspect that it would give an incorrect result some of the\ntime---for example, in the above example, wouldn't one of the\ncommits labeled as 's' be completely hidden? And if that is the\ncase, the end-user documentation would need to warn about it, and\nexplain that it is a easy/lazy alternative that can produce\nincorrect result in the log message.\n"},{"id":"419066","messageId":"CAOLTT8TT6rKdF-quS-t3X31PfQcrHPom4Rk5LcrMUBKha78MGg@mail.gmail.com","threadId":"55268","inReplyTo":"xmqqwnua4ozy.fsf@gitster.g","subject":"Re: [PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-03-14T02:16:48Z","receivedAt":"2021-03-14T02:20:09Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano 于2021年3月14日周日 上午7:23写道：\n\n>\n> Yes, it would be, but that is why you need to educate users what\n> causes it, and what the right way to avoid unrelated commits from\n> appearing, and how this --left-only fits in the solution.\n>\n> If some of the time, \"--left-only T1...T2\" would give you the same\n> result as the more strict \"B1..T1 B2..T2\", that may be why users may\n> want to use the \"--left-only\" instead as an easy/lazy alternative.\n>\n> But I suspect that it would give an incorrect result some of the\n> time---for example, in the above example, wouldn't one of the\n> commits labeled as 's' be completely hidden? And if that is the\n> case, the end-user documentation would need to warn about it, and\n> explain that it is a easy/lazy alternative that can produce\n> incorrect result in the log message.\n\nThanks, I will try to illustrate these issues in the document.\n\nMy another thinking is:\nSince `--left-only` inhibits \"B1..B2\" and \"B2..T2\" ( let the user\nchoose the left B1..T1 ), To some extent, `--right-only` can also\nadd ( let the user choose the right B2..T2 ). A separate `--left-only`\nwill be strange to the user ( If user call T2...T1 ). Since the\n`git rebase --apply` will internal call `git format-patch -k --stdout\n--full-index --cherry-pick --right-only ...`, I don't know what to deal\nwith this `--right-only` yet, because I don't how to teach git to judge\nif the `--right-only` is pass from user or `git rebase --apply`, Is there\nany good way? If can solve this problem, the user can choose the left\nor right side of the free choice. (Or users don't need `--right-only?`)\n"},{"id":"419068","messageId":"CAOLTT8RS_FDGEJZ-ogfMf-QJAUEkycju3757uTcQ+Tczub+gqQ@mail.gmail.com","threadId":"55268","inReplyTo":"CAOLTT8TT6rKdF-quS-t3X31PfQcrHPom4Rk5LcrMUBKha78MGg@mail.gmail.com","subject":"Re: [PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-03-14T02:37:40Z","receivedAt":"2021-03-14T02:38:50Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"ZheNing Hu 于2021年3月14日周日 上午10:16写道：\n>\n> Junio C Hamano 于2021年3月14日周日 上午7:23写道：\n>\n> >\n> > Yes, it would be, but that is why you need to educate users what\n> > causes it, and what the right way to avoid unrelated commits from\n> > appearing, and how this --left-only fits in the solution.\n> >\n> > If some of the time, \"--left-only T1...T2\" would give you the same\n> > result as the more strict \"B1..T1 B2..T2\", that may be why users may\n> > want to use the \"--left-only\" instead as an easy/lazy alternative.\n> >\n> > But I suspect that it would give an incorrect result some of the\n> > time---for example, in the above example, wouldn't one of the\n> > commits labeled as 's' be completely hidden? And if that is the\n> > case, the end-user documentation would need to warn about it, and\n> > explain that it is a easy/lazy alternative that can produce\n> > incorrect result in the log message.\n>\n> Thanks, I will try to illustrate these issues in the document.\n>\n> My another thinking is:\n> Since `--left-only` inhibits \"B1..B2\" and \"B2..T2\" ( let the user\n> choose the left B1..T1 ), To some extent, `--right-only` can also\n> add ( let the user choose the right B2..T2 ). A separate `--left-only`\n> will be strange to the user ( If user call T2...T1 ). Since the\n> `git rebase --apply` will internal call `git format-patch -k --stdout\n> --full-index --cherry-pick --right-only ...`, I don't know what to deal\n> with this `--right-only` yet, because I don't how to teach git to judge\n> if the `--right-only` is pass from user or `git rebase --apply`, Is there\n> any good way? If can solve this problem, the user can choose the left\n> or right side of the free choice. (Or users don't need `--right-only?`)\n\nLet me refute my own point just now :`--right-only` can not show \"B2..T2\",\nbut \"B1..T1\", and it may be useful only when user want an inverted \"T2...T1\".\n"},{"id":"419069","messageId":"CAOLTT8TG-NP=W4G5R2GTO+QPdHQ5dL0eLYV7Fof_co4XaCjTxg@mail.gmail.com","threadId":"55268","inReplyTo":"CAOLTT8RS_FDGEJZ-ogfMf-QJAUEkycju3757uTcQ+Tczub+gqQ@mail.gmail.com","subject":"Re: [PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-03-14T02:41:44Z","receivedAt":"2021-03-14T02:42:47Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"> Let me refute my own point just now :`--right-only` can not show \"B2..T2\",\n> but \"B1..T1\", and it may be useful only when user want an inverted \"T2...T1\".\n\nsorry,\ns/B1..T1/B1..T2/\n"},{"id":"419072","messageId":"pull.898.v4.git.1615709438971.gitgitgadget@gmail.com","threadId":"55268","inReplyTo":"pull.898.v3.git.1615285726482.gitgitgadget@gmail.com","subject":"[PATCH v4] [GSOC] format-patch: pass --left-only to range-diff","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-14T08:10:38Z","receivedAt":"2021-03-14T08:11:48Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nWhen two different iterative versions t1 and t2 are base upstream\ncommit b1 and b2, b2 inherits from b1. If the user base on b1 to\ngenerate cover-letter, but `git format-patch --range-diff=b1..t1\nb1..t2 --cover-letter` may mistakenly place upstream commit in the\nrange-diff output in cover-letter.\n\nTeaching `format-patch` pass `--left-only` to `range-diff`, it will\nsuppress the output on the upstream commits.At the same time, it has\na disadvantage: the iterative version on the right side will be ignored\ntoo. So using `--left-only` is just a lazy way to avoid upstream output.\n\nSigned-off-by: ZheNing Hu \n---\n [GSOC][RFC] format-patch: pass --left-only to range-diff\n \n With the help of Taylor Blau and Junio, I understand why the upstream\n commit appeared in the range-diff, and completed the writing of the\n test.\n \n But I notice that in https://github.com/gitgitgadget/git/issues/876 and\n https://lore.kernel.org/git/xmqqpn0456lr.fsf@gitster.g/ Both Junio and\n Johannes Schindelin have questions about the practicality of this\n --left-only, and I am also starting to have confusion now, and\n format-patch --range-diff is more suitable for two commit ranges to\n compare the differences. It is not suitable for T1...T2, which further\n proves that the practicability of this patch may not be as good as\n previously imagined. Should I close it?\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/898\n\nRange-diff vs v3:\n\n 1: 5c58eb186d41 ! 1: 3738a00129b5 [GSOC][RFC] format-patch: pass --left-only to range-diff\n @@ Metadata\n Author: ZheNing Hu \n \n ## Commit message ##\n - [GSOC][RFC] format-patch: pass --left-only to range-diff\n + [GSOC] format-patch: pass --left-only to range-diff\n \n - In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,\n - Taylor Blau proposing `git format-patch --cover-letter\n - --range-diff` may mistakenly place upstream commit in the\n - range-diff output. Teach `format-patch` pass `--left-only`\n - to range-diff,can avoid this kind of mistake.\n + When two different iterative versions t1 and t2 are base upstream\n + commit b1 and b2, b2 inherits from b1. If the user base on b1 to\n + generate cover-letter, but `git format-patch --range-diff=b1..t1\n + b1..t2 --cover-letter` may mistakenly place upstream commit in the\n + range-diff output in cover-letter.\n +\n + Teaching `format-patch` pass `--left-only` to `range-diff`, it will\n + suppress the output on the upstream commits.At the same time, it has\n + a disadvantage: the iterative version on the right side will be ignored\n + too. So using `--left-only` is just a lazy way to avoid upstream output.\n \n Signed-off-by: ZheNing Hu \n \n @@ Documentation/git-format-patch.txt: material (this may change in the future).\n \n +--left-only:\n +\tUsed with `--range-diff`, only emit output related to the first range.\n ++\tThis option only can be used in this situation: The first iteration `t1`\n ++\tbased on the upstream commit `b1`, the second iteration `t2` based on the\n ++\tupstream commit `b2`, `b2` inherited from the `b1`, then If the user want\n ++\toutput the range-diff between this two iterations (t1 and t2) in cover-letter,\n ++\tand just use `b1` as the same base for `--range-diff`, but they don't want the\n ++\tupstream range `b1..b2` included on the right side of the range-diff output.\n ++\tBy using `git format-patch --range-diff=b1..t1 b1..t2 --cover-letter --left-only`,\n ++\tall content on the right side will be removed, leaving only the range\n ++\t`b1..t1` on the left side.\n +++\n ++Note that this `--left-only` is just a lazy way to let user use same base\n ++and avoid outputting upstream commits in cover-letter, and the side effect\n ++is that `b2..t2` on the right side will not be outputted.\n ++\n +\n --notes[=]::\n --no-notes::\n @@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '\n +\tgit checkout my-feature &&\n +\tgit rebase $base --onto main &&\n +\ttip=\"$(git rev-parse my-feature)\" &&\n -+\tgit format-patch --range-diff $base $old $tip --cover-letter &&\n -+\tgrep \"> 1: .* feature$\" 0000-cover-letter.patch &&\n -+\tgit format-patch --range-diff $base $old $tip --left-only --cover-letter &&\n ++\tgit format-patch --range-diff $base..$old $base..$tip --cover-letter &&\n ++\tgrep \"> 1: .* new$\" 0000-cover-letter.patch &&\n ++\tgit format-patch --range-diff $base..$old $base..$tip --left-only --cover-letter &&\n +\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n +'\n +\n\n\n Documentation/git-format-patch.txt | 19 ++++++++++++++++++-\n builtin/log.c | 20 +++++++++++++++-----\n t/t3206-range-diff.sh | 27 +++++++++++++++++++++++++++\n 3 files changed, 60 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 3e49bf221087..99a2d87a859f 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -27,7 +27,7 @@ SYNOPSIS\n \t\t [--[no-]encode-email-headers]\n \t\t [--no-notes | --notes[=]]\n \t\t [--interdiff=]\n-\t\t [--range-diff= [--creation-factor=]]\n+\t\t [--range-diff= [--creation-factor=] [--left-only]]\n \t\t [--filename-max-length=]\n \t\t [--progress]\n \t\t []\n@@ -301,6 +301,23 @@ material (this may change in the future).\n \tcreation/deletion cost fudge factor. See linkgit:git-range-diff[1])\n \tfor details.\n \n+--left-only:\n+\tUsed with `--range-diff`, only emit output related to the first range.\n+\tThis option only can be used in this situation: The first iteration `t1`\n+\tbased on the upstream commit `b1`, the second iteration `t2` based on the\n+\tupstream commit `b2`, `b2` inherited from the `b1`, then If the user want\n+\toutput the range-diff between this two iterations (t1 and t2) in cover-letter,\n+\tand just use `b1` as the same base for `--range-diff`, but they don't want the\n+\tupstream range `b1..b2` included on the right side of the range-diff output.\n+\tBy using `git format-patch --range-diff=b1..t1 b1..t2 --cover-letter --left-only`,\n+\tall content on the right side will be removed, leaving only the range\n+\t`b1..t1` on the left side.\n++\n+Note that this `--left-only` is just a lazy way to let user use same base\n+and avoid outputting upstream commits in cover-letter, and the side effect\n+is that `b2..t2` on the right side will not be outputted.\n+\n+\n --notes[=]::\n --no-notes::\n \tAppend the notes (see linkgit:git-notes[1]) for the commit\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f67b67d80ed1..21fed9db82d6 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\n \t\t\t struct commit *origin,\n \t\t\t int nr, struct commit **list,\n \t\t\t const char *branch_name,\n-\t\t\t int quiet)\n+\t\t\t int quiet, int left_only)\n {\n \tconst char *committer;\n \tstruct shortlog log;\n@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,\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\t.other_arg = &other_arg,\n+\t\t\t.left_only = left_only\n \t\t};\n \n \t\tdiff_setup(&opts);\n@@ -1732,6 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf rdiff2 = STRBUF_INIT;\n \tstruct strbuf rdiff_title = STRBUF_INIT;\n \tint creation_factor = -1;\n+\tint left_only = 0;\n \n \tconst struct option builtin_format_patch_options[] = {\n \t\tOPT_CALLBACK_F('n', \"numbered\", &numbered, NULL,\n@@ -1814,6 +1816,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 or single patch\")),\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_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t N_(\"percentage by which creation is weighted\")),\n \t\tOPT_END()\n@@ -2083,10 +2087,15 @@ 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 (creation_factor >= 0)\n+\t\t\tdie(_(\"--creation-factor requires --range-diff\"));\n+\t\tif (left_only)\n+\t\t\tdie(_(\"--left-only requires --range-diff\"));\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 && total != 1)\n@@ -2134,7 +2143,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, !!output_directory,\n-\t\t\t\t origin, nr, list, branch_name, quiet);\n+\t\t\t\t origin, nr, list, branch_name, quiet,\n+\t\t\t\t left_only);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 1b26c4c2ef91..8e537793947b 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -748,4 +748,31 @@ test_expect_success '--left-only/--right-only' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'format-patch --range-diff --left-only' '\n+\trm -fr repo &&\n+\tgit init repo &&\n+\tcd repo &&\n+\tgit branch -M main &&\n+\techo \"base\" >base &&\n+\tgit add base &&\n+\tgit commit -m \"base\" &&\n+\tgit checkout -b my-feature &&\n+\techo \"feature\" >feature &&\n+\tgit add feature &&\n+\tgit commit -m \"feature\" &&\n+\tbase=\"$(git rev-parse main)\" &&\n+\told=\"$(git rev-parse my-feature)\" &&\n+\tgit checkout main &&\n+\techo \"other\" >>base &&\n+\tgit add base &&\n+\tgit commit -m \"new\" &&\n+\tgit checkout my-feature &&\n+\tgit rebase $base --onto main &&\n+\ttip=\"$(git rev-parse my-feature)\" &&\n+\tgit format-patch --range-diff $base..$old $base..$tip --cover-letter &&\n+\tgrep \"> 1: .* new$\" 0000-cover-letter.patch &&\n+\tgit format-patch --range-diff $base..$old $base..$tip --left-only --cover-letter &&\n+\t! grep \"> 1: .* feature$\" 0000-cover-letter.patch\n+'\n+\n test_done\n\nbase-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50\n-- \ngitgitgadget\n"}]}