| Message ID | pull.1014.v5.git.1628977230247.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8ef6aad664715c16caf6e36a7f6b174a06574477 |
| Headers | show |
| Series | [v5] commit: restore --edit when combined with --fixup | expand |
Hi Joel On 14/08/2021 22:40, Joel Klinghed via GitGitGadget wrote: > From: Joel Klinghed <the_jk@spawned.biz> > > 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 <the_jk@spawned.biz> > --- > 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 <the_jk@spawned.biz> > > ## 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 >
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"