From: Phillip Wood Date: Tue, 17 Aug 2021 10:06:58 GMT Subject: Re: [PATCH v5] commit: restore --edit when combined with --fixup Message-ID: <24356b20-45e2-5d60-434c-4f04d1b2055c@gmail.com> In-Reply-To: Hi Joel On 14/08/2021 22:40, Joel Klinghed via GitGitGadget wrote: > From: Joel Klinghed > > 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 moving the edit flag condition > later in the method. This version looks good, thanks for revising it Best Wishes Phillip > > Signed-off-by: Joel Klinghed > --- > commit: restore --edit when combined with --fixup > > 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. > > Changes since v1: Added test verifying that --fixup --edit brings up > editor. > > Changes since v2: Clarify if condition and use write_script helper in > test. > > Changes since v3: Simplify test. > > Changes since v4: Using cleaner fix by Phillip Wood instead and added an > explicit verification to a test for --fixup without --edit. > > Signed-off-by: Joel Klinghed the_jk@spawned.biz > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1014%2Fthejk%2Ffixup_edit-v5 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1014/thejk/fixup_edit-v5 > Pull-Request: https://github.com/gitgitgadget/git/pull/1014 > > Range-diff vs v4: > > 1: 0c0cb647e03 ! 1: 1c608daf0cd commit: restore --edit when combined with --fixup > @@ Commit message > --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. > + deciding the value of use_editor by moving the edit flag condition > + later in the method. > > Signed-off-by: Joel Klinghed > > ## builtin/commit.c ## > @@ builtin/commit.c: 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; > + > + if (logfile || have_option_m || use_message) > + use_editor = 0; > +- if (0 <= edit_flag) > +- use_editor = edit_flag; > + > + /* Sanity check options */ > + if (amend && !current_head) > +@@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[], > } > } > > ++ if (0 <= edit_flag) > ++ use_editor = edit_flag; > ++ > + cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor); > + > + handle_untracked_files_arg(s); > > ## t/t7500-commit-template-squash-signoff.sh ## > +@@ t/t7500-commit-template-squash-signoff.sh: EOF > + > + test_expect_success 'commit --fixup provides correct one-line commit message' ' > + commit_for_rebase_autosquash_setup && > +- git commit --fixup HEAD~1 && > ++ EDITOR="echo ignored >>" git commit --fixup HEAD~1 && > + commit_msg_is "fixup! target message subject line" > + ' > + > @@ t/t7500-commit-template-squash-signoff.sh: test_expect_success 'commit --fixup -m"something" -m"extra"' ' > > extra" > > > builtin/commit.c | 5 +++-- > t/t7500-commit-template-squash-signoff.sh | 9 ++++++++- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/builtin/commit.c b/builtin/commit.c > index 190d215d43b..854903ad113 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1246,8 +1246,6 @@ static int parse_and_validate_options(int argc, const char *argv[], > > if (logfile || have_option_m || use_message) > use_editor = 0; > - if (0 <= edit_flag) > - use_editor = edit_flag; > > /* Sanity check options */ > if (amend && !current_head) > @@ -1337,6 +1335,9 @@ static int parse_and_validate_options(int argc, const char *argv[], > } > } > > + if (0 <= edit_flag) > + use_editor = edit_flag; > + > cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor); > > handle_untracked_files_arg(s); > diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh > index 7d02f79c0de..8515736003a 100755 > --- a/t/t7500-commit-template-squash-signoff.sh > +++ b/t/t7500-commit-template-squash-signoff.sh > @@ -270,7 +270,7 @@ EOF > > test_expect_success 'commit --fixup provides correct one-line commit message' ' > commit_for_rebase_autosquash_setup && > - git commit --fixup HEAD~1 && > + EDITOR="echo ignored >>" git commit --fixup HEAD~1 && > commit_msg_is "fixup! target message subject line" > ' > > @@ -281,6 +281,13 @@ test_expect_success 'commit --fixup -m"something" -m"extra"' ' > > extra" > ' > +test_expect_success 'commit --fixup --edit' ' > + commit_for_rebase_autosquash_setup && > + EDITOR="printf \"something\nextra\" >>" 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 >