Re: [PATCH] add-patch: edit the hunk again

From: Rubén Justo <rjusto@gmail.com>
Date: Sep 18, 2024, 17:46 UTC
Message-ID: <08b29649-eeeb-49e4-82ac-2a3473dd2ad5@gmail.com>
In-Reply-To: <d20d030b-7d3e-49c6-a988-ab7fe480dd47@gmail.com>

On Wed, Sep 18, 2024 at 11:06:34AM +0100, phillip.wood123@gmail.com wrote:

Show 9 quoted lines

> Hi Rubén
> 
> On 16/09/2024 23:09, Rubén Justo wrote:
> > On Mon, Sep 16, 2024 at 02:33:54PM +0100, Phillip Wood wrote:
> > 
> > I can imagine that we could give the flawed and annotated patch back to
> > the user, if they want to fix it and try again.
> 
> Exactly

So we agree on where we're going ...

Show 7 quoted lines

> 
> > At any rate, I'm thinking about small fixes and/or avoiding to use a
> > backup (":w! /tmp/patch" + ":r /tmp/patch") if I have doubts about
> > making a mistake after spending some time thinking about a hunk, so as
> > not to lose some work.
> 
> The problem is there is no good solution at the moment.

although not in the length of the stride :)

Maybe in the future we can provide better error descriptions, or even add annotations to the faulty patch explaining the faults.

But we're not there yet, and honestly, it's not my intention to work on that.

> Either we keep the broken
> patch and say "this is broken, please figure out what's wrong with it and
> fix it"

Yes, we should keep the users's work if they say "yes" to:

    Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]?

> or throw away
> the user's work if the edited patch does not apply.

Only if the user says "no" (as we say in the message).

After that "no", the user has another opportunity to decide about the hunk:

    Your edited hunk does not apply. Edit again (saying "no" discards!) [y/n]? no

    (n/m) Stage this hunk [y,n,q,a,d,e,p,?]

And then, "edit" will allow them to start over and edit the original hunk, again.

> As I explained previously fixing a broken patch is not necessarily
> straight forward especially for new users.

Very true. But I don't think that should be a reason to stop the user from trying.

Show 5 quoted lines

> A few times when editing patches
> that are going to be applied in reverse (from "git checkout HEAD -- <path>")
> I've found it impossible to figure out why a particular edit was being
> rejected. In that case starting again with the original patch is my only
> hope.

My experience is usually small last-minute adjustments that aren't worth canceling the interactive session for, and I don't want to have to remember to make them later.

A small error in a large hunk has been frustrating several times because I have to go back and review the whole thing.

> If you want to be able to re-edit a broken hunk perhaps we should add
> an option for that when we ask the user if they want to try again.

As we commented in a previous message, this is what we are regaining with this patch. The option was introduced in ac083c47ea (git-add--interactive: manual hunk editing mode, 2008-07-03) and lost in 2b8ea7f3c7 (add -p: calculate offset delta for edited patches, 2018-03-05).

Thanks.