Re: [PATCH v3] commit: restore --edit when combined with --fixup
From: Joel Klinghed <hidden>
Date: 2021-08-13 15:35:59
On Fri, Aug 13, 2021, at 15:06, Phillip Wood wrote:
quoted hunk ↗ jump to hunk
On 12/08/2021 11:01, Joel Klinghed wrote:quoted
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.I don't think you need to duplicate the check for a suboption, can't you just do this on top of master (i.e without you patch applied)?diff --git a/builtin/commit.c b/builtin/commit.c index 243c626307..67a84ff6e4 100644 --- a/builtin/commit.c +++ b/builtin/commit.c@@ -1251,11 +1251,6 @@ 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) - use_editor = 0; - if (0 <= edit_flag) - use_editor = edit_flag; - /* Sanity check options */ if (amend && !current_head) die(_("You have nothing to amend."));@@ -1344,6 +1339,11 @@ 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; + cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor); handle_untracked_files_arg(s); I chose to move the other clause that sets use_editor as well so they stay together.
With the above change use_editor no longer defaults to 0 for --fixup as it used to do. My expected behavior (based on old versions): git commit --fixup <hash> /// No editor git commit --fixup <hash> --edit /// Editor As far as I can see your change would display an editor in both cases. An alternative would be: + if (logfile || have_option_m || use_message || fixup_message) + use_editor = 0; + if (0 <= edit_flag) + use_editor = edit_flag; That would fix the above cases but in "commit: add amend suboption to --fixup to create amend! commit" the implementation left git commit --fixup amend:<hash> // Editor and I didn't want to change that. But if the default should be no editor here as well then the above would be a better patch. /JK