From: Elijah Newren <newren@gmail.com>
Date: Thu, 27 Aug 2026 00:21:50 GMT
Subject: Re: [PATCH] commit: refuse to amend during conflict resolution
Message-ID: <CABPp-BHx7H5mF8tqs3JkK+Rqjn8hMSjXhohT9NW=NmPsU1by1Q@mail.gmail.com>
In-Reply-To: <4688ee19-b782-456a-bed2-8cd2a4415736@gmail.com>

On Wed, Aug 26, 2026 at 6:56 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
[...]
> > @@ -1336,6 +1337,46 @@ static int parse_and_validate_options(int argc, const char *argv[],
> >               else if (whence == FROM_REBASE_PICK)
> >                       die(_("You are in the middle of a rebase -- cannot amend."));
> >       }
> > +     if (amend && whence == FROM_COMMIT) {
> > +             char *applying, *apply_dir, *stopped_sha, *amend_marker;
> > +             int in_am, conflicted_stop;
> > +
> > +             /* Check middle of revert */
> > +             if (refs_ref_exists(get_main_ref_store(the_repository),
> > +                                 "REVERT_HEAD"))
> > +                     die(_("You are in the middle of a revert -- cannot amend."));
>
> I think it would be much cleaner to move this check and sequencer
> related ones below into sequencer_determine_whence() so that we don't
> have to hard code the paths here. It might be worth checking for "am"
> and the "apply" based rebase in that function as well.

Moving the logic makes sense.  I was a little unsure about putting it
in sequencer_determine_whence() since commit has its own
determine_whence() based on keeping the merge handling separate.  I
kind of wanted some function for ongoing_operation, so I just made a
new helper, and added merge, am, and the apply-based rebase to it.

> The logic looks sound to me

Thanks for taking a look!