Re: [PATCH v3] commit: restore --edit when combined with --fixup
From: Phillip Wood <hidden>
Date: 2021-08-12 09:33:06
Hi Joel On 12/08/2021 09:02, Joel Klinghed via GitGitGadget wrote:
From: Joel Klinghed <redacted> Recent changes to --fixup, adding amend suboption, caused the --edit flag to be ignored as use_editor was always set to zero. Restore edit_flag having higher priority than fixup_message when deciding the value of use_editor by only changing the default if edit_flag is not set.
Thanks for fixing this
quoted hunk ↗ jump to hunk
diff --git a/builtin/commit.c b/builtin/commit.c index 190d215d43b..560aecd21b1 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 (edit_flag < 0) + use_editor = 0; } }
Commit 494d314a05 ("commit: add amend suboption to --fixup to create
amend! commit", 2021-03-15) that broke this has the following hunk above
this change
@@ -1170,7 +1206,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
if (force_author && renew_authorship)
die(_("Using both --reset-author and --author does not
make sense"));
- if (logfile || have_option_m || use_message || fixup_message)
+ if (logfile || have_option_m || use_message)
use_editor = 0;
if (0 <= edit_flag)
use_editor = edit_flag;
I think it should have moved those last two context lines that set
`use_editor` to below the part that you are fixing. Then the `use_editor
= 0` line that you are changing continues to work as before. (As you see
there are quite a few legacy Yoda conditions in this file, nowadays we
avoid adding new ones). I'm not sure if it is worth re working this
patch to do that, but it does have the advantage of only testing
edit_flag once.
quoted hunk ↗ jump to hunk
diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh index 7d02f79c0de..a48fe859235 100755 --- a/t/t7500-commit-template-squash-signoff.sh +++ b/t/t7500-commit-template-squash-signoff.sh@@ -281,6 +281,19 @@ test_expect_success 'commit --fixup -m"something" -m"extra"' ' extra" ' +test_expect_success 'commit --fixup --edit' ' + commit_for_rebase_autosquash_setup && + write_script e-append <<-\EOF && + sed -e "2a\\ +something\\ +extra" <"$1" >"$1-" + mv "$1-" "$1" + EOF
Again I'm not sure it is worth changing it now but for future reference
this is a rather complicated way of appending to the commit message. The
test file has an example using set_fake_editor() together with
FAKE_COMMIT_AMEND just below where you have added this test or you can
just have
EDITOR="echo something extra >>" git commit --fixup=HEAD~1 --edit
Best Wishes
Phillip
+ 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"
base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb