| Message ID | pull.1014.v2.git.1628725421868.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] commit: restore --edit when combined with --fixup | expand |
"Joel Klinghed via GitGitGadget" <gitgitgadget@gmail.com> writes: > diff --git a/builtin/commit.c b/builtin/commit.c > index 190d215d43b..4c5286840c5 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1333,7 +1333,8 @@ static int parse_and_validate_options(int argc, const char *argv[], > } else { > fixup_commit = fixup_message; > fixup_prefix = "fixup"; > - use_editor = 0; > + if (0 > edit_flag) Writing this as if (edit_flag < 0) makes it far easier to immediately see that we are talking about a nagetive edit_flag. > + use_editor = 0; > } > } > > diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh > index 7d02f79c0de..d71c7812180 100755 > --- a/t/t7500-commit-template-squash-signoff.sh > +++ b/t/t7500-commit-template-squash-signoff.sh > @@ -281,6 +281,21 @@ test_expect_success 'commit --fixup -m"something" -m"extra"' ' > > extra" > ' > +test_expect_success 'commit --fixup --edit' ' > + commit_for_rebase_autosquash_setup && > + cat >e-append <<-\EOF && > + #!/bin/sh > + sed -e "2a\\ > +something\\ > +extra" <"$1" >"$1-" > + mv "$1-" "$1" > + EOF > + chmod 755 e-append && Use write_script helper from test-lib-functions.sh here and lose the hardcoded reference to /bin/sh. > + EDITOR="./e-append" git commit --fixup HEAD~1 --edit && > + commit_msg_is "fixup! target message subject linesomething > +extra" > +' Thanks.
On Thu, Aug 12, 2021, at 07:21, Junio C Hamano wrote: > "Joel Klinghed via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > diff --git a/builtin/commit.c b/builtin/commit.c > > index 190d215d43b..4c5286840c5 100644 > > --- a/builtin/commit.c > > +++ b/builtin/commit.c > > @@ -1333,7 +1333,8 @@ static int parse_and_validate_options(int argc, const char *argv[], > > } else { > > fixup_commit = fixup_message; > > fixup_prefix = "fixup"; > > - use_editor = 0; > > + if (0 > edit_flag) > > Writing this as > > if (edit_flag < 0) > > makes it far easier to immediately see that we are talking about a > nagetive edit_flag. > Agree, I'll change it. I was unsure of the style and copied from the earlier condition: if (0 <= edit_flag) use_editor = edit_flag; > > + use_editor = 0; > > } > > } > > > > Use write_script helper from test-lib-functions.sh here and lose the > hardcoded reference to /bin/sh. > Ah, missed that one. Thanks. /JK
"Joel Klinghed" <the_jk@spawned.biz> writes: >> Writing this as >> >> if (edit_flag < 0) >> >> makes it far easier to immediately see that we are talking about a >> nagetive edit_flag. >> > > Agree, I'll change it. > I was unsure of the style and copied from the earlier condition: > if (0 <= edit_flag) > use_editor = edit_flag; There are two valid schools of thought when it comes to comparison. Some folks consider that a comparison between a variable and a constant is a statement about the variable, hence the expression should be if (VARIRABLE comparison-operator CONSTANT) They will write things like: if (edit_flag >= 0) if (edit_flag < 0) Other folks consider that textual order of the comparison should match the actual order of the things being compared, as if they are arranged on a number line, hence the expression should be if (SMALLER < LARGER) no matter which one is variable and which one is constant. They will write: if (0 <= edit_flag) if (edit_flag < 0) The case in question, asserting that edit_flag is negative, is what both camps agree how to write.
diff --git a/builtin/commit.c b/builtin/commit.c index 190d215d43b..4c5286840c5 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1333,7 +1333,8 @@ static int parse_and_validate_options(int argc, const char *argv[], } else { fixup_commit = fixup_message; fixup_prefix = "fixup"; - use_editor = 0; + if (0 > edit_flag) + use_editor = 0; } } diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh index 7d02f79c0de..d71c7812180 100755 --- a/t/t7500-commit-template-squash-signoff.sh +++ b/t/t7500-commit-template-squash-signoff.sh @@ -281,6 +281,21 @@ test_expect_success 'commit --fixup -m"something" -m"extra"' ' extra" ' +test_expect_success 'commit --fixup --edit' ' + commit_for_rebase_autosquash_setup && + cat >e-append <<-\EOF && + #!/bin/sh + sed -e "2a\\ +something\\ +extra" <"$1" >"$1-" + mv "$1-" "$1" + EOF + chmod 755 e-append && + EDITOR="./e-append" git commit --fixup HEAD~1 --edit && + commit_msg_is "fixup! target message subject linesomething +extra" +' + get_commit_msg () { rev="$1" && git log -1 --pretty=format:"%B" "$rev"