[PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff

Subsystems: documentation, the rest

STALE2041d

8 messages, 4 authors, 2021-03-14 · open the first message on its own page

[PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff

From: ZheNing Hu via GitGitGadget <hidden>
Date: 2021-03-07 07:05:44

From: ZheNing Hu <redacted>

In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing 'git format-patch --cover-letter
--range-diff' may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--right-only`
to range-diff, maybe can avoid this kind of mistake.

Because `git rebase --apply` will internally call `git
format-patch -k --stdout --full-index --cherry-pick --right-only
--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter
--pretty=mboxrd --topo-order --no-base`, In order to avoid the
ambiguity of `--right-only`, users can call `format-patch
--range-diff --only-right` instead of `--right-only` to only emit
output related to the second range.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC][RFC] format-patch: pass --right-only to range-diff
    
    Transfer parameters are relatively simple. But because I can' t
    reproduce the situation, There's no big difference between adding
    ---only-right and not adding ---only-right. So I haven't written the
    test files for the time being.
    
    I may need to ask reviewers opinion first.
    
    this want to fix #876 Thanks.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/898

 Documentation/git-format-patch.txt |  5 ++++-
 builtin/log.c                      | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 3e49bf221087..5dae34c3090b 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -27,7 +27,7 @@ SYNOPSIS
 		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous> [--creation-factor=<percent>]]
+		   [--range-diff=<previous> [--creation-factor=<percent>] [--right-only]]
 		   [--filename-max-length=<n>]
 		   [--progress]
 		   [<common diff options>]
@@ -301,6 +301,9 @@ material (this may change in the future).
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.
 
+--only-right:
+	Used with `--range-diff`, only emit output related to the second range.
+
 --notes[=<ref>]::
 --no-notes::
 	Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index f67b67d80ed1..5d2f39fd19a7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet, int right_only)
 {
 	const char *committer;
 	struct shortlog log;
@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.diffopt = &opts,
-			.other_arg = &other_arg
+			.other_arg = &other_arg,
+			.right_only = right_only
 		};
 
 		diff_setup(&opts);
@@ -1732,7 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	int creation_factor = -1;
-
+	int right_only = 0;
 	const struct option builtin_format_patch_options[] = {
 		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
 			    N_("use [PATCH n/m] even with a single patch"),
@@ -1814,6 +1815,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
 			   N_("show changes against <refspec> in cover letter or single patch")),
+		OPT_BOOL(0, "only-right", &right_only,
+			 N_("only emit output related to the second range")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
 		OPT_END()
@@ -2087,7 +2090,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
 	else if (!rdiff_prev)
 		die(_("--creation-factor requires --range-diff"));
-
+	if (right_only && !rdiff_prev)
+		die(_("--right_only requires --range-diff"));
 	if (rdiff_prev) {
 		if (!cover_letter && total != 1)
 			die(_("--range-diff requires --cover-letter or single patch"));
@@ -2134,7 +2138,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, !!output_directory,
-				  origin, nr, list, branch_name, quiet);
+				  origin, nr, list, branch_name, quiet,
+					right_only);
 		print_bases(&bases, rev.diffopt.file);
 		print_signature(rev.diffopt.file);
 		total++;
base-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50
-- 
gitgitgadget

Re: [PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff

From: Taylor Blau <hidden>
Date: 2021-03-08 20:19:07

On Sun, Mar 07, 2021 at 06:57:19AM +0000, ZheNing Hu via GitGitGadget wrote:
From: ZheNing Hu <redacted>

In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing 'git format-patch --cover-letter
--range-diff' may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--right-only`
to range-diff, maybe can avoid this kind of mistake.

Because `git rebase --apply` will internally call `git
format-patch -k --stdout --full-index --cherry-pick --right-only
--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter
--pretty=mboxrd --topo-order --no-base`, In order to avoid the
ambiguity of `--right-only`, users can call `format-patch
--range-diff --only-right` instead of `--right-only` to only emit
output related to the second range.
A couple of times this new option is called "--right-only" and other
times it is "--only-right". This comment makes me think that you meant
to call it "--only-right", but...
Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC][RFC] format-patch: pass --right-only to range-diff

    Transfer parameters are relatively simple. But because I can' t
    reproduce the situation, There's no big difference between adding
    ---only-right and not adding ---only-right. So I haven't written the
    test files for the time being.
Note that I think (and Johannes--cc'd--could confirm) that what you want
is '--left-only' to discard rebased changes from the upstream branch:

In either case, this should help you reproduce it (and would be a good
starting point for adding a test case). Here, we're using '--left-only'
to discard changes from upstream (here, the 'main' branch) after
rebasing:

    #!/bin/sh

    set -e

    rm -fr repo
    git init repo
    cd repo

    git branch -M main

    echo "base" >base
    git add base
    git commit -m "base"

    git checkout -b my-feature
    echo "feature" >feature
    git add feature
    git commit -m "feature"

    base="$(git rev-parse main)"
    old="$(git rev-parse my-feature)"

    git checkout main
    echo "other" >>base
    git add base
    git commit -m "new"

    git checkout my-feature
    git rebase $base --onto main

    tip="$(git rev-parse my-feature)"

    git range-diff $base $old $tip
    git range-diff --left-only $base $old $tip
quoted hunk
    I may need to ask reviewers opinion first.

    this want to fix #876 Thanks.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/898

 Documentation/git-format-patch.txt |  5 ++++-
 builtin/log.c                      | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 3e49bf221087..5dae34c3090b 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -27,7 +27,7 @@ SYNOPSIS
 		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous> [--creation-factor=<percent>]]
+		   [--range-diff=<previous> [--creation-factor=<percent>] [--right-only]]
 		   [--filename-max-length=<n>]
 		   [--progress]
 		   [<common diff options>]
@@ -301,6 +301,9 @@ material (this may change in the future).
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.

+--only-right:
+	Used with `--range-diff`, only emit output related to the second range.
+
s/--only-right/--right-only?
quoted hunk
 --notes[=<ref>]::
 --no-notes::
 	Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index f67b67d80ed1..5d2f39fd19a7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet, int right_only)
 {
 	const char *committer;
 	struct shortlog log;
@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.diffopt = &opts,
-			.other_arg = &other_arg
+			.other_arg = &other_arg,
+			.right_only = right_only
 		};

 		diff_setup(&opts);
@@ -1732,7 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	int creation_factor = -1;
-
+	int right_only = 0;
Nit: the newline between creation_factor's declaration and
builtin_format_patch_options's is good to keep.
quoted hunk
 	const struct option builtin_format_patch_options[] = {
 		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
 			    N_("use [PATCH n/m] even with a single patch"),
@@ -1814,6 +1815,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
 			   N_("show changes against <refspec> in cover letter or single patch")),
+		OPT_BOOL(0, "only-right", &right_only,
+			 N_("only emit output related to the second range")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
 		OPT_END()
@@ -2087,7 +2090,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
 	else if (!rdiff_prev)
 		die(_("--creation-factor requires --range-diff"));
-
+	if (right_only && !rdiff_prev)
+		die(_("--right_only requires --range-diff"));
These may be good to combine into one big conditional that dies whenever
an option requiring --range-diff is set, but --range-diff is not:

  if (!rdiff_prev) {
    if (creation_factor)
      die(_("--creation-factor requires --range-diff"));
    if (right_only)
      die(_("--right-only requires --range-diff"));
  }

Thanks,
Taylor

Re: [PATCH] [GSOC][RFC] format-patch: pass --right-only to range-diff

From: ZheNing Hu <hidden>
Date: 2021-03-09 07:29:33

Taylor Blau [off-list ref] 于2021年3月9日周二 上午4:18写道:
On Sun, Mar 07, 2021 at 06:57:19AM +0000, ZheNing Hu via GitGitGadget wrote:
quoted
From: ZheNing Hu <redacted>

In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing 'git format-patch --cover-letter
--range-diff' may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--right-only`
to range-diff, maybe can avoid this kind of mistake.

Because `git rebase --apply` will internally call `git
format-patch -k --stdout --full-index --cherry-pick --right-only
--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter
--pretty=mboxrd --topo-order --no-base`, In order to avoid the
ambiguity of `--right-only`, users can call `format-patch
--range-diff --only-right` instead of `--right-only` to only emit
output related to the second range.
A couple of times this new option is called "--right-only" and other
times it is "--only-right". This comment makes me think that you meant
to call it "--only-right", but...
quoted
Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC][RFC] format-patch: pass --right-only to range-diff

    Transfer parameters are relatively simple. But because I can' t
    reproduce the situation, There's no big difference between adding
    ---only-right and not adding ---only-right. So I haven't written the
    test files for the time being.
Note that I think (and Johannes--cc'd--could confirm) that what you want
is '--left-only' to discard rebased changes from the upstream branch:

In either case, this should help you reproduce it (and would be a good
starting point for adding a test case). Here, we're using '--left-only'
to discard changes from upstream (here, the 'main' branch) after
rebasing:

    #!/bin/sh

    set -e

    rm -fr repo
    git init repo
    cd repo

    git branch -M main

    echo "base" >base
    git add base
    git commit -m "base"

    git checkout -b my-feature
    echo "feature" >feature
    git add feature
    git commit -m "feature"

    base="$(git rev-parse main)"
    old="$(git rev-parse my-feature)"

    git checkout main
    echo "other" >>base
    git add base
    git commit -m "new"

    git checkout my-feature
    git rebase $base --onto main

    tip="$(git rev-parse my-feature)"

    git range-diff $base $old $tip
    git range-diff --left-only $base $old $tip
Thanks, I have seen an obvious phenomenon from
your shell script, and I wrote test code based on it.
quoted
    I may need to ask reviewers opinion first.

    this want to fix #876 Thanks.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/898

 Documentation/git-format-patch.txt |  5 ++++-
 builtin/log.c                      | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 3e49bf221087..5dae34c3090b 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -27,7 +27,7 @@ SYNOPSIS
                 [--[no-]encode-email-headers]
                 [--no-notes | --notes[=<ref>]]
                 [--interdiff=<previous>]
-                [--range-diff=<previous> [--creation-factor=<percent>]]
+                [--range-diff=<previous> [--creation-factor=<percent>] [--right-only]]
                 [--filename-max-length=<n>]
                 [--progress]
                 [<common diff options>]
@@ -301,6 +301,9 @@ material (this may change in the future).
      creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
      for details.

+--only-right:
+     Used with `--range-diff`, only emit output related to the second range.
+
s/--only-right/--right-only?
Now it's all "--left-only".
quoted
 --notes[=<ref>]::
 --no-notes::
      Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index f67b67d80ed1..5d2f39fd19a7 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
                            struct commit *origin,
                            int nr, struct commit **list,
                            const char *branch_name,
-                           int quiet)
+                           int quiet, int right_only)
 {
      const char *committer;
      struct shortlog log;
@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
                      .creation_factor = rev->creation_factor,
                      .dual_color = 1,
                      .diffopt = &opts,
-                     .other_arg = &other_arg
+                     .other_arg = &other_arg,
+                     .right_only = right_only
              };

              diff_setup(&opts);
@@ -1732,7 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
      struct strbuf rdiff2 = STRBUF_INIT;
      struct strbuf rdiff_title = STRBUF_INIT;
      int creation_factor = -1;
-
+     int right_only = 0;
Nit: the newline between creation_factor's declaration and
builtin_format_patch_options's is good to keep.
quoted
      const struct option builtin_format_patch_options[] = {
              OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
                          N_("use [PATCH n/m] even with a single patch"),
@@ -1814,6 +1815,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                           parse_opt_object_name),
              OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
                         N_("show changes against <refspec> in cover letter or single patch")),
+             OPT_BOOL(0, "only-right", &right_only,
+                      N_("only emit output related to the second range")),
              OPT_INTEGER(0, "creation-factor", &creation_factor,
                          N_("percentage by which creation is weighted")),
              OPT_END()
@@ -2087,7 +2090,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
              creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
      else if (!rdiff_prev)
              die(_("--creation-factor requires --range-diff"));
-
+     if (right_only && !rdiff_prev)
+             die(_("--right_only requires --range-diff"));
These may be good to combine into one big conditional that dies whenever
an option requiring --range-diff is set, but --range-diff is not:
Good tip.
  if (!rdiff_prev) {
    if (creation_factor)
      die(_("--creation-factor requires --range-diff"));
    if (right_only)
      die(_("--right-only requires --range-diff"));
  }

Thanks,
Taylor
Thanks,
ZheNing Hu

[PATCH v2] [GSOC][RFC] format-patch: pass --left-only to range-diff

From: ZheNing Hu via GitGitGadget <hidden>
Date: 2021-03-09 08:34:49

From: ZheNing Hu <redacted>

In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing `git format-patch --cover-letter
--range-diff` may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--left-only`
to range-diff,can avoid this kind of mistake.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC][RFC] format-patch: pass --left-only to range-diff
    
    With the help of Taylor Blau, I understood why the upstream commit
    appeared in the range-diff, and completed the writing of the test.
    
    this want to fix #876 Thanks.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/898

Range-diff vs v1:

 1:  590bd06ee5ee ! 1:  8daffd4f7546 [GSOC][RFC] format-patch: pass --right-only to range-diff
     @@ Metadata
      Author: ZheNing Hu [off-list ref]
      
       ## Commit message ##
     -    [GSOC][RFC] format-patch: pass --right-only to range-diff
     +    [GSOC][RFC] format-patch: pass --left-only to range-diff
      
          In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
     -    Taylor Blau proposing 'git format-patch --cover-letter
     -    --range-diff' may mistakenly place upstream commit in the
     -    range-diff output. Teach `format-patch` pass `--right-only`
     -    to range-diff, maybe can avoid this kind of mistake.
     -
     -    Because `git rebase --apply` will internally call `git
     -    format-patch -k --stdout --full-index --cherry-pick --right-only
     -    --src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter
     -    --pretty=mboxrd --topo-order --no-base`, In order to avoid the
     -    ambiguity of `--right-only`, users can call `format-patch
     -    --range-diff --only-right` instead of `--right-only` to only emit
     -    output related to the second range.
     +    Taylor Blau proposing `git format-patch --cover-letter
     +    --range-diff` may mistakenly place upstream commit in the
     +    range-diff output. Teach `format-patch` pass `--left-only`
     +    to range-diff,can avoid this kind of mistake.
      
          Signed-off-by: ZheNing Hu [off-list ref]
      
     @@ Documentation/git-format-patch.txt: SYNOPSIS
       		   [--no-notes | --notes[=<ref>]]
       		   [--interdiff=<previous>]
      -		   [--range-diff=<previous> [--creation-factor=<percent>]]
     -+		   [--range-diff=<previous> [--creation-factor=<percent>] [--right-only]]
     ++		   [--range-diff=<previous> [--creation-factor=<percent>] [--left-only]]
       		   [--filename-max-length=<n>]
       		   [--progress]
       		   [<common diff options>]
     @@ Documentation/git-format-patch.txt: material (this may change in the future).
       	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
       	for details.
       
     -+--only-right:
     -+	Used with `--range-diff`, only emit output related to the second range.
     ++--left-only:
     ++	Used with `--range-diff`, only emit output related to the first range.
      +
       --notes[=<ref>]::
       --no-notes::
     @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ
       			      int nr, struct commit **list,
       			      const char *branch_name,
      -			      int quiet)
     -+			      int quiet, int right_only)
     ++			      int quiet, int left_only)
       {
       	const char *committer;
       	struct shortlog log;
     @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_separ
       			.diffopt = &opts,
      -			.other_arg = &other_arg
      +			.other_arg = &other_arg,
     -+			.right_only = right_only
     ++			.left_only = left_only
       		};
       
       		diff_setup(&opts);
     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
       	struct strbuf rdiff2 = STRBUF_INIT;
       	struct strbuf rdiff_title = STRBUF_INIT;
       	int creation_factor = -1;
     --
     -+	int right_only = 0;
     ++	int left_only = 0;
     + 
       	const struct option builtin_format_patch_options[] = {
       		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
     - 			    N_("use [PATCH n/m] even with a single patch"),
      @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
       			     parse_opt_object_name),
       		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
       			   N_("show changes against <refspec> in cover letter or single patch")),
     -+		OPT_BOOL(0, "only-right", &right_only,
     -+			 N_("only emit output related to the second range")),
     ++		OPT_BOOL(0, "left-only", &left_only,
     ++			 N_("only emit output related to the first range")),
       		OPT_INTEGER(0, "creation-factor", &creation_factor,
       			    N_("percentage by which creation is weighted")),
       		OPT_END()
      @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
     + 
     + 	if (creation_factor < 0)
       		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
     - 	else if (!rdiff_prev)
     - 		die(_("--creation-factor requires --range-diff"));
     +-	else if (!rdiff_prev)
     +-		die(_("--creation-factor requires --range-diff"));
      -
     -+	if (right_only && !rdiff_prev)
     -+		die(_("--right_only requires --range-diff"));
     ++	else if (!rdiff_prev) {
     ++		if (creation_factor >= 0)
     ++			die(_("--creation-factor requires --range-diff"));
     ++		if (left_only)
     ++			die(_("--left-only requires --range-diff"));
     ++	}
       	if (rdiff_prev) {
       		if (!cover_letter && total != 1)
       			die(_("--range-diff requires --cover-letter or single patch"));
     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
       		make_cover_letter(&rev, !!output_directory,
      -				  origin, nr, list, branch_name, quiet);
      +				  origin, nr, list, branch_name, quiet,
     -+					right_only);
     ++					left_only);
       		print_bases(&bases, rev.diffopt.file);
       		print_signature(rev.diffopt.file);
       		total++;
     +
     + ## t/t3206-range-diff.sh ##
     +@@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '
     + 	test_cmp expect actual
     + '
     + 
     ++test_expect_success 'format-patch --range-diff --left-only' '
     ++	rm -fr repo &&
     ++	git init repo &&
     ++	cd repo &&
     ++	git branch -M main &&
     ++	echo "base" >base &&
     ++	git add base &&
     ++	git commit -m "base" &&
     ++	git checkout -b my-feature &&
     ++	echo "feature" >feature &&
     ++	git add feature &&
     ++	git commit -m "feature" &&
     ++	base="$(git rev-parse main)" &&
     ++	old="$(git rev-parse my-feature)" &&
     ++	git checkout main &&
     ++	echo "other" >>base &&
     ++	git add base &&
     ++	git commit -m "new" &&
     ++	git checkout my-feature &&
     ++	git rebase $base --onto main &&
     ++	tip="$(git rev-parse my-feature)" &&
     ++	git format-patch --range-diff $base $old $tip --cover-letter  &&
     ++	grep  "> 1: .* feature$"  0000-cover-letter.patch &&
     ++	git format-patch --range-diff $base $old $tip --left-only --cover-letter &&
     ++	! grep  "> 1: .* feature$"  0000-cover-letter.patch
     ++'
     ++
     ++
     + test_done


 Documentation/git-format-patch.txt |  5 ++++-
 builtin/log.c                      | 20 ++++++++++++++------
 t/t3206-range-diff.sh              | 28 ++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 3e49bf221087..8c5eca0ba1e3 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -27,7 +27,7 @@ SYNOPSIS
 		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous> [--creation-factor=<percent>]]
+		   [--range-diff=<previous> [--creation-factor=<percent>] [--left-only]]
 		   [--filename-max-length=<n>]
 		   [--progress]
 		   [<common diff options>]
@@ -301,6 +301,9 @@ material (this may change in the future).
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.
 
+--left-only:
+	Used with `--range-diff`, only emit output related to the first range.
+
 --notes[=<ref>]::
 --no-notes::
 	Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index f67b67d80ed1..5620dfafb37e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet, int left_only)
 {
 	const char *committer;
 	struct shortlog log;
@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.diffopt = &opts,
-			.other_arg = &other_arg
+			.other_arg = &other_arg,
+			.left_only = left_only
 		};
 
 		diff_setup(&opts);
@@ -1732,6 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	int creation_factor = -1;
+	int left_only = 0;
 
 	const struct option builtin_format_patch_options[] = {
 		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
@@ -1814,6 +1816,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
 			   N_("show changes against <refspec> in cover letter or single patch")),
+		OPT_BOOL(0, "left-only", &left_only,
+			 N_("only emit output related to the first range")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
 		OPT_END()
@@ -2085,9 +2089,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 	if (creation_factor < 0)
 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
-	else if (!rdiff_prev)
-		die(_("--creation-factor requires --range-diff"));
-
+	else if (!rdiff_prev) {
+		if (creation_factor >= 0)
+			die(_("--creation-factor requires --range-diff"));
+		if (left_only)
+			die(_("--left-only requires --range-diff"));
+	}
 	if (rdiff_prev) {
 		if (!cover_letter && total != 1)
 			die(_("--range-diff requires --cover-letter or single patch"));
@@ -2134,7 +2141,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, !!output_directory,
-				  origin, nr, list, branch_name, quiet);
+				  origin, nr, list, branch_name, quiet,
+					left_only);
 		print_bases(&bases, rev.diffopt.file);
 		print_signature(rev.diffopt.file);
 		total++;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 1b26c4c2ef91..abb0cf7fb465 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -748,4 +748,32 @@ test_expect_success '--left-only/--right-only' '
 	test_cmp expect actual
 '
 
+test_expect_success 'format-patch --range-diff --left-only' '
+	rm -fr repo &&
+	git init repo &&
+	cd repo &&
+	git branch -M main &&
+	echo "base" >base &&
+	git add base &&
+	git commit -m "base" &&
+	git checkout -b my-feature &&
+	echo "feature" >feature &&
+	git add feature &&
+	git commit -m "feature" &&
+	base="$(git rev-parse main)" &&
+	old="$(git rev-parse my-feature)" &&
+	git checkout main &&
+	echo "other" >>base &&
+	git add base &&
+	git commit -m "new" &&
+	git checkout my-feature &&
+	git rebase $base --onto main &&
+	tip="$(git rev-parse my-feature)" &&
+	git format-patch --range-diff $base $old $tip --cover-letter  &&
+	grep  "> 1: .* feature$"  0000-cover-letter.patch &&
+	git format-patch --range-diff $base $old $tip --left-only --cover-letter &&
+	! grep  "> 1: .* feature$"  0000-cover-letter.patch
+'
+
+
 test_done
base-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50
-- 
gitgitgadget

Re: [PATCH v2] [GSOC][RFC] format-patch: pass --left-only to range-diff

From: Eric Sunshine <hidden>
Date: 2021-03-09 09:01:22

On Tue, Mar 9, 2021 at 3:35 AM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
quoted hunk
In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing `git format-patch --cover-letter
--range-diff` may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--left-only`
to range-diff,can avoid this kind of mistake.

Signed-off-by: ZheNing Hu <redacted>
---
diff --git a/builtin/log.c b/builtin/log.c
@@ -2085,9 +2089,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        if (creation_factor < 0)
                creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
-       else if (!rdiff_prev)
-               die(_("--creation-factor requires --range-diff"));
-
+       else if (!rdiff_prev) {
+               if (creation_factor >= 0)
+                       die(_("--creation-factor requires --range-diff"));
+               if (left_only)
+                       die(_("--left-only requires --range-diff"));
+       }
This logic starts getting difficult to reason about. It would be
easier to understand if written like this:

    if (!rdiff_prev) {
        if (creation_factor >= 0)
            die(_("--creation-factor requires --range-diff"));
        if (left_only)
            die(_("--left-only requires --range-diff"));
    }

    if (creation_factor < 0)
        creation_factor = RANGE_DIFF_..._DEFAULT;

or this (which reduces the indentation a bit):

    if (creation_factor >= 0 && !rdiff_prev)
        die(_("--creation-factor requires --range-diff"));
    if (left_only && !rdiff_prev)
        die(_("--left-only requires --range-diff"));

    if (creation_factor < 0)
        creation_factor = RANGE_DIFF_..._DEFAULT;

Subjective; not necessarily worth a re-roll.
quoted hunk
@@ -2134,7 +2141,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                make_cover_letter(&rev, !!output_directory,
-                                 origin, nr, list, branch_name, quiet);
+                                 origin, nr, list, branch_name, quiet,
+                                       left_only);
Nit: This indentation looks odd. One would expect `origin` and
`left_only` to have the same indentation.
quoted hunk
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
@@ -748,4 +748,32 @@ test_expect_success '--left-only/--right-only' '
+test_expect_success 'format-patch --range-diff --left-only' '
+       rm -fr repo &&
+       git init repo &&
+       ...
+       ! grep  "> 1: .* feature$"  0000-cover-letter.patch
+'
+
+
 test_done
Nit: One blank line before test_done() is typical, not two.

Re: [PATCH v2] [GSOC][RFC] format-patch: pass --left-only to range-diff

From: ZheNing Hu <hidden>
Date: 2021-03-09 09:36:22

Eric Sunshine [off-list ref] 于2021年3月9日周二 下午5:00写道:
On Tue, Mar 9, 2021 at 3:35 AM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
quoted
In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing `git format-patch --cover-letter
--range-diff` may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--left-only`
to range-diff,can avoid this kind of mistake.

Signed-off-by: ZheNing Hu <redacted>
---
diff --git a/builtin/log.c b/builtin/log.c
@@ -2085,9 +2089,12 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
        if (creation_factor < 0)
                creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
-       else if (!rdiff_prev)
-               die(_("--creation-factor requires --range-diff"));
-
+       else if (!rdiff_prev) {
+               if (creation_factor >= 0)
+                       die(_("--creation-factor requires --range-diff"));
+               if (left_only)
+                       die(_("--left-only requires --range-diff"));
+       }
This logic starts getting difficult to reason about. It would be
easier to understand if written like this:

    if (!rdiff_prev) {
        if (creation_factor >= 0)
            die(_("--creation-factor requires --range-diff"));
        if (left_only)
            die(_("--left-only requires --range-diff"));
    }

    if (creation_factor < 0)
        creation_factor = RANGE_DIFF_..._DEFAULT;

or this (which reduces the indentation a bit):

    if (creation_factor >= 0 && !rdiff_prev)
        die(_("--creation-factor requires --range-diff"));
    if (left_only && !rdiff_prev)
        die(_("--left-only requires --range-diff"));

    if (creation_factor < 0)
        creation_factor = RANGE_DIFF_..._DEFAULT;
Emm,I would prefer to use the above.
Subjective; not necessarily worth a re-roll.
quoted
@@ -2134,7 +2141,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                make_cover_letter(&rev, !!output_directory,
-                                 origin, nr, list, branch_name, quiet);
+                                 origin, nr, list, branch_name, quiet,
+                                       left_only);
Nit: This indentation looks odd. One would expect `origin` and
`left_only` to have the same indentation.
quoted
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
@@ -748,4 +748,32 @@ test_expect_success '--left-only/--right-only' '
+test_expect_success 'format-patch --range-diff --left-only' '
+       rm -fr repo &&
+       git init repo &&
+       ...
+       ! grep  "> 1: .* feature$"  0000-cover-letter.patch
+'
+
+
 test_done
Nit: One blank line before test_done() is typical, not two.
Thanks for the reminder.
I'm going to modify these format errors.

[PATCH v3] [GSOC][RFC] format-patch: pass --left-only to range-diff

From: ZheNing Hu via GitGitGadget <hidden>
Date: 2021-03-09 10:29:35

From: ZheNing Hu <redacted>

In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
Taylor Blau proposing `git format-patch --cover-letter
--range-diff` may mistakenly place upstream commit in the
range-diff output. Teach `format-patch` pass `--left-only`
to range-diff,can avoid this kind of mistake.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC][RFC] format-patch: pass --left-only to range-diff
    
    With the help of Taylor Blau, I understood why the upstream commit
    appeared in the range-diff, and completed the writing of the test.
    
    this want to fix #876 Thanks.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/898

Range-diff vs v2:

 1:  8daffd4f7546 ! 1:  5c58eb186d41 [GSOC][RFC] format-patch: pass --left-only to range-diff
     @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *pre
       			    N_("percentage by which creation is weighted")),
       		OPT_END()
      @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
     + 					     _("Interdiff against v%d:"));
     + 	}
       
     - 	if (creation_factor < 0)
     - 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
     --	else if (!rdiff_prev)
     --		die(_("--creation-factor requires --range-diff"));
     --
     -+	else if (!rdiff_prev) {
     ++	if (!rdiff_prev) {
      +		if (creation_factor >= 0)
      +			die(_("--creation-factor requires --range-diff"));
      +		if (left_only)
      +			die(_("--left-only requires --range-diff"));
      +	}
     ++
     + 	if (creation_factor < 0)
     + 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
     +-	else if (!rdiff_prev)
     +-		die(_("--creation-factor requires --range-diff"));
     + 
       	if (rdiff_prev) {
       		if (!cover_letter && total != 1)
     - 			die(_("--range-diff requires --cover-letter or single patch"));
      @@ builtin/log.c: int cmd_format_patch(int argc, const char **argv, const char *prefix)
       		if (thread)
       			gen_message_id(&rev, "cover");
       		make_cover_letter(&rev, !!output_directory,
      -				  origin, nr, list, branch_name, quiet);
      +				  origin, nr, list, branch_name, quiet,
     -+					left_only);
     ++				  left_only);
       		print_bases(&bases, rev.diffopt.file);
       		print_signature(rev.diffopt.file);
       		total++;
     @@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '
      +	git rebase $base --onto main &&
      +	tip="$(git rev-parse my-feature)" &&
      +	git format-patch --range-diff $base $old $tip --cover-letter  &&
     -+	grep  "> 1: .* feature$"  0000-cover-letter.patch &&
     ++	grep "> 1: .* feature$" 0000-cover-letter.patch &&
      +	git format-patch --range-diff $base $old $tip --left-only --cover-letter &&
     -+	! grep  "> 1: .* feature$"  0000-cover-letter.patch
     ++	! grep "> 1: .* feature$" 0000-cover-letter.patch
      +'
     -+
      +
       test_done


 Documentation/git-format-patch.txt |  5 ++++-
 builtin/log.c                      | 20 +++++++++++++++-----
 t/t3206-range-diff.sh              | 27 +++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 3e49bf221087..8c5eca0ba1e3 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -27,7 +27,7 @@ SYNOPSIS
 		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous> [--creation-factor=<percent>]]
+		   [--range-diff=<previous> [--creation-factor=<percent>] [--left-only]]
 		   [--filename-max-length=<n>]
 		   [--progress]
 		   [<common diff options>]
@@ -301,6 +301,9 @@ material (this may change in the future).
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.
 
+--left-only:
+	Used with `--range-diff`, only emit output related to the first range.
+
 --notes[=<ref>]::
 --no-notes::
 	Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index f67b67d80ed1..21fed9db82d6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet, int left_only)
 {
 	const char *committer;
 	struct shortlog log;
@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.diffopt = &opts,
-			.other_arg = &other_arg
+			.other_arg = &other_arg,
+			.left_only = left_only
 		};
 
 		diff_setup(&opts);
@@ -1732,6 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	int creation_factor = -1;
+	int left_only = 0;
 
 	const struct option builtin_format_patch_options[] = {
 		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
@@ -1814,6 +1816,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
 			   N_("show changes against <refspec> in cover letter or single patch")),
+		OPT_BOOL(0, "left-only", &left_only,
+			 N_("only emit output related to the first range")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
 		OPT_END()
@@ -2083,10 +2087,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 					     _("Interdiff against v%d:"));
 	}
 
+	if (!rdiff_prev) {
+		if (creation_factor >= 0)
+			die(_("--creation-factor requires --range-diff"));
+		if (left_only)
+			die(_("--left-only requires --range-diff"));
+	}
+
 	if (creation_factor < 0)
 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
-	else if (!rdiff_prev)
-		die(_("--creation-factor requires --range-diff"));
 
 	if (rdiff_prev) {
 		if (!cover_letter && total != 1)
@@ -2134,7 +2143,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, !!output_directory,
-				  origin, nr, list, branch_name, quiet);
+				  origin, nr, list, branch_name, quiet,
+				  left_only);
 		print_bases(&bases, rev.diffopt.file);
 		print_signature(rev.diffopt.file);
 		total++;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 1b26c4c2ef91..6178a12dd4b1 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -748,4 +748,31 @@ test_expect_success '--left-only/--right-only' '
 	test_cmp expect actual
 '
 
+test_expect_success 'format-patch --range-diff --left-only' '
+	rm -fr repo &&
+	git init repo &&
+	cd repo &&
+	git branch -M main &&
+	echo "base" >base &&
+	git add base &&
+	git commit -m "base" &&
+	git checkout -b my-feature &&
+	echo "feature" >feature &&
+	git add feature &&
+	git commit -m "feature" &&
+	base="$(git rev-parse main)" &&
+	old="$(git rev-parse my-feature)" &&
+	git checkout main &&
+	echo "other" >>base &&
+	git add base &&
+	git commit -m "new" &&
+	git checkout my-feature &&
+	git rebase $base --onto main &&
+	tip="$(git rev-parse my-feature)" &&
+	git format-patch --range-diff $base $old $tip --cover-letter  &&
+	grep "> 1: .* feature$" 0000-cover-letter.patch &&
+	git format-patch --range-diff $base $old $tip --left-only --cover-letter &&
+	! grep "> 1: .* feature$" 0000-cover-letter.patch
+'
+
 test_done
base-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50
-- 
gitgitgadget

[PATCH v4] [GSOC] format-patch: pass --left-only to range-diff

From: ZheNing Hu via GitGitGadget <hidden>
Date: 2021-03-14 08:11:48

From: ZheNing Hu <redacted>

When two different iterative versions t1 and t2 are base upstream
commit b1 and b2, b2 inherits from b1. If the user base on b1 to
generate cover-letter, but `git format-patch --range-diff=b1..t1
b1..t2 --cover-letter` may mistakenly place upstream commit in the
range-diff output in cover-letter.

Teaching `format-patch` pass `--left-only` to `range-diff`, it will
suppress the output on the upstream commits.At the same time, it has
a disadvantage: the iterative version on the right side will be ignored
too. So using `--left-only` is just a lazy way to avoid upstream output.

Signed-off-by: ZheNing Hu <redacted>
---
    [GSOC][RFC] format-patch: pass --left-only to range-diff
    
    With the help of Taylor Blau and Junio, I understand why the upstream
    commit appeared in the range-diff, and completed the writing of the
    test.
    
    But I notice that in https://github.com/gitgitgadget/git/issues/876 and
    https://lore.kernel.org/git/xmqqpn0456lr.fsf@gitster.g/ Both Junio and
    Johannes Schindelin have questions about the practicality of this
    --left-only, and I am also starting to have confusion now, and
    format-patch --range-diff is more suitable for two commit ranges to
    compare the differences. It is not suitable for T1...T2, which further
    proves that the practicability of this patch may not be as good as
    previously imagined. Should I close it?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-898%2Fadlternative%2Fformat-patch-range-diff-right-only-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-898/adlternative/format-patch-range-diff-right-only-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/898

Range-diff vs v3:

 1:  5c58eb186d41 ! 1:  3738a00129b5 [GSOC][RFC] format-patch: pass --left-only to range-diff
     @@ Metadata
      Author: ZheNing Hu [off-list ref]
      
       ## Commit message ##
     -    [GSOC][RFC] format-patch: pass --left-only to range-diff
     +    [GSOC] format-patch: pass --left-only to range-diff
      
     -    In https://lore.kernel.org/git/YBx5rmVsg1LJhSKN@nand.local/,
     -    Taylor Blau proposing `git format-patch --cover-letter
     -    --range-diff` may mistakenly place upstream commit in the
     -    range-diff output. Teach `format-patch` pass `--left-only`
     -    to range-diff,can avoid this kind of mistake.
     +    When two different iterative versions t1 and t2 are base upstream
     +    commit b1 and b2, b2 inherits from b1. If the user base on b1 to
     +    generate cover-letter, but `git format-patch --range-diff=b1..t1
     +    b1..t2 --cover-letter` may mistakenly place upstream commit in the
     +    range-diff output in cover-letter.
     +
     +    Teaching `format-patch` pass `--left-only` to `range-diff`, it will
     +    suppress the output on the upstream commits.At the same time, it has
     +    a disadvantage: the iterative version on the right side will be ignored
     +    too. So using `--left-only` is just a lazy way to avoid upstream output.
      
          Signed-off-by: ZheNing Hu [off-list ref]
      
     @@ Documentation/git-format-patch.txt: material (this may change in the future).
       
      +--left-only:
      +	Used with `--range-diff`, only emit output related to the first range.
     ++	This option only can be used in this situation: The first iteration `t1`
     ++	based on the upstream commit `b1`, the second iteration `t2` based on the
     ++	upstream commit `b2`, `b2` inherited from the `b1`, then If the user want
     ++	output the range-diff between this two iterations (t1 and t2) in cover-letter,
     ++	and just use `b1` as the same base for `--range-diff`, but they don't want the
     ++	upstream range `b1..b2` included on the right side of the range-diff output.
     ++	By using `git format-patch --range-diff=b1..t1 b1..t2 --cover-letter --left-only`,
     ++	all content on the right side will be removed, leaving only the range
     ++	`b1..t1` on the left side.
     +++
     ++Note that this `--left-only` is just a lazy way to let user use same base
     ++and avoid outputting upstream commits in cover-letter, and the side effect
     ++is that `b2..t2` on the right side will not be outputted.
     ++
      +
       --notes[=<ref>]::
       --no-notes::
     @@ t/t3206-range-diff.sh: test_expect_success '--left-only/--right-only' '
      +	git checkout my-feature &&
      +	git rebase $base --onto main &&
      +	tip="$(git rev-parse my-feature)" &&
     -+	git format-patch --range-diff $base $old $tip --cover-letter  &&
     -+	grep "> 1: .* feature$" 0000-cover-letter.patch &&
     -+	git format-patch --range-diff $base $old $tip --left-only --cover-letter &&
     ++	git format-patch --range-diff $base..$old $base..$tip --cover-letter  &&
     ++	grep "> 1: .* new$" 0000-cover-letter.patch &&
     ++	git format-patch --range-diff $base..$old $base..$tip --left-only --cover-letter &&
      +	! grep "> 1: .* feature$" 0000-cover-letter.patch
      +'
      +


 Documentation/git-format-patch.txt | 19 ++++++++++++++++++-
 builtin/log.c                      | 20 +++++++++++++++-----
 t/t3206-range-diff.sh              | 27 +++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 3e49bf221087..99a2d87a859f 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -27,7 +27,7 @@ SYNOPSIS
 		   [--[no-]encode-email-headers]
 		   [--no-notes | --notes[=<ref>]]
 		   [--interdiff=<previous>]
-		   [--range-diff=<previous> [--creation-factor=<percent>]]
+		   [--range-diff=<previous> [--creation-factor=<percent>] [--left-only]]
 		   [--filename-max-length=<n>]
 		   [--progress]
 		   [<common diff options>]
@@ -301,6 +301,23 @@ material (this may change in the future).
 	creation/deletion cost fudge factor. See linkgit:git-range-diff[1])
 	for details.
 
+--left-only:
+	Used with `--range-diff`, only emit output related to the first range.
+	This option only can be used in this situation: The first iteration `t1`
+	based on the upstream commit `b1`, the second iteration `t2` based on the
+	upstream commit `b2`, `b2` inherited from the `b1`, then If the user want
+	output the range-diff between this two iterations (t1 and t2) in cover-letter,
+	and just use `b1` as the same base for `--range-diff`, but they don't want the
+	upstream range `b1..b2` included on the right side of the range-diff output.
+	By using `git format-patch --range-diff=b1..t1 b1..t2 --cover-letter --left-only`,
+	all content on the right side will be removed, leaving only the range
+	`b1..t1` on the left side.
++
+Note that this `--left-only` is just a lazy way to let user use same base
+and avoid outputting upstream commits in cover-letter, and the side effect
+is that `b2..t2` on the right side will not be outputted.
+
+
 --notes[=<ref>]::
 --no-notes::
 	Append the notes (see linkgit:git-notes[1]) for the commit
diff --git a/builtin/log.c b/builtin/log.c
index f67b67d80ed1..21fed9db82d6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1153,7 +1153,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet, int left_only)
 {
 	const char *committer;
 	struct shortlog log;
@@ -1228,7 +1228,8 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			.creation_factor = rev->creation_factor,
 			.dual_color = 1,
 			.diffopt = &opts,
-			.other_arg = &other_arg
+			.other_arg = &other_arg,
+			.left_only = left_only
 		};
 
 		diff_setup(&opts);
@@ -1732,6 +1733,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf rdiff2 = STRBUF_INIT;
 	struct strbuf rdiff_title = STRBUF_INIT;
 	int creation_factor = -1;
+	int left_only = 0;
 
 	const struct option builtin_format_patch_options[] = {
 		OPT_CALLBACK_F('n', "numbered", &numbered, NULL,
@@ -1814,6 +1816,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			     parse_opt_object_name),
 		OPT_STRING(0, "range-diff", &rdiff_prev, N_("refspec"),
 			   N_("show changes against <refspec> in cover letter or single patch")),
+		OPT_BOOL(0, "left-only", &left_only,
+			 N_("only emit output related to the first range")),
 		OPT_INTEGER(0, "creation-factor", &creation_factor,
 			    N_("percentage by which creation is weighted")),
 		OPT_END()
@@ -2083,10 +2087,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 					     _("Interdiff against v%d:"));
 	}
 
+	if (!rdiff_prev) {
+		if (creation_factor >= 0)
+			die(_("--creation-factor requires --range-diff"));
+		if (left_only)
+			die(_("--left-only requires --range-diff"));
+	}
+
 	if (creation_factor < 0)
 		creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
-	else if (!rdiff_prev)
-		die(_("--creation-factor requires --range-diff"));
 
 	if (rdiff_prev) {
 		if (!cover_letter && total != 1)
@@ -2134,7 +2143,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, !!output_directory,
-				  origin, nr, list, branch_name, quiet);
+				  origin, nr, list, branch_name, quiet,
+				  left_only);
 		print_bases(&bases, rev.diffopt.file);
 		print_signature(rev.diffopt.file);
 		total++;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 1b26c4c2ef91..8e537793947b 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -748,4 +748,31 @@ test_expect_success '--left-only/--right-only' '
 	test_cmp expect actual
 '
 
+test_expect_success 'format-patch --range-diff --left-only' '
+	rm -fr repo &&
+	git init repo &&
+	cd repo &&
+	git branch -M main &&
+	echo "base" >base &&
+	git add base &&
+	git commit -m "base" &&
+	git checkout -b my-feature &&
+	echo "feature" >feature &&
+	git add feature &&
+	git commit -m "feature" &&
+	base="$(git rev-parse main)" &&
+	old="$(git rev-parse my-feature)" &&
+	git checkout main &&
+	echo "other" >>base &&
+	git add base &&
+	git commit -m "new" &&
+	git checkout my-feature &&
+	git rebase $base --onto main &&
+	tip="$(git rev-parse my-feature)" &&
+	git format-patch --range-diff $base..$old $base..$tip --cover-letter  &&
+	grep "> 1: .* new$" 0000-cover-letter.patch &&
+	git format-patch --range-diff $base..$old $base..$tip --left-only --cover-letter &&
+	! grep "> 1: .* feature$" 0000-cover-letter.patch
+'
+
 test_done
base-commit: be7935ed8bff19f481b033d0d242c5d5f239ed50
-- 
gitgitgadget
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help