Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 于2021年3月9日周二 下午5:00写道： > > On Tue, Mar 9, 2021 at 3:35 AM ZheNing Hu via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: > > 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 <adlternative@xxxxxxxxx> > > --- > > 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. > > > @@ -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. > > > 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.
