On Fri, Aug 13, 2021, at 15:06, Phillip Wood wrote:
> On 12/08/2021 11:01, Joel Klinghed wrote:
> > 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
