From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: Tue, 09 Jun 2026 17:12:17 GMT
Subject: Re: [PATCH RFC v2 2/2] builtin/history: abort reword on same message
Message-ID: <CAN5EUNRz9F+njb_O=Q4DzVMec-q+rDf83Ow+MPJE4yLCBq9qww@mail.gmail.com>
In-Reply-To: <xmqq4ijbsn2m.fsf@gitster.g>

El mar, 9 jun 2026 a las 18:20, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> > Hi Pablo
> >
> > On 09/06/2026 11:42, Pablo Sabater wrote:
> >>   static int commit_tree_ext(struct repository *repo,
> >> @@ -135,6 +136,13 @@ static int commit_tree_ext(struct repository *repo,
> >>                                        original_body, action, &commit_message);
> >>              if (ret < 0)
> >>                      goto out;
> >> +
> >> +            if (flags & COMMIT_TREE_ABORT_ON_SAME_MESSAGE &&
> >> +                !strcmp(original_body, commit_message.buf)) {
> >> +                    fprintf(stderr, _("Message unchanged, aborting reword.\n"));
> >> +                    ret = 1;
> >> +                    goto out;
> >> +            }
> >
> > I wonder if we should check that the committer identity is unchanged as
> > well in case anyone is using this to fix commits after committing with
> > the wrong identity.

I think that if you reword a commit committed by someone else but end
up with no changes I want it to be kept as it was.

> >
> > Aborting when the message and committer identity are unchanged seems
> > like a good idea.
>
> I am not sure why it would be a good idea.  The user wanted to make
> the commit have this message, and the commit ended up having the
> same message as the user gave.  That message may have been identical
> to what the commit originally had, or it may be different.  Why is
> the former an abort-worthy event?  A simple note, I may understand,
> but aborting with an error message?

With what you said at [1], having this in an
"--avoid-unnecessary-rewrite" I think that the abort might be too much
as with the flag the user already expects this to happen and silent
might be better.

By the way, I feel that "--avoid-unnecessary-rewrite" is too long,
could it be something shorter? If not it could be set "-r" as the
short and leave the long as it is.

>
> Thanks.

[1]: https://lore.kernel.org/git/xmqqtsrbsvcm.fsf@gitster.g/

Thanks,
Pablo