Re: [PATCH v3] commit: restore --edit when combined with --fixup
From: Joel Klinghed <hidden>
Date: 2021-08-12 10:01:34
On Thu, Aug 12, 2021, at 11:32, Phillip Wood wrote:
quoted hunk ↗ jump to hunk
Hi Joel@@ -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.
I looked at moving the condition to one place but as use_editor = 0 is only set for --fixup if there isn't a suboption specified I didn't want to have to duplicate the check for a suboption when deciding if use_editor should default to zero.
quoted
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" + EOFAgain 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
Yeah, especially getting sed in a POSIX and BSD compatible mode took some doing. I wanted to have a similar output to the test above this one, with a line break between something and extra, and frankly, my shell-foo lacked for getting either FAKE_COMMIT_AMEND or EDITOR="... >>" to include a newline. But looking at it again, I realize that EDITOR="printf \"something\nextra\" >>" works just fine. /JK