Hi Rubén

On 24/09/2024 23:54, Rubén Justo wrote:

On Mon, Sep 23, 2024 at 10:07:08AM +0100, phillip.wood123@gmail.com wrote:

 >

So, to me, it seems sensible to let the user review the faulty patch,
even if it's only to discard it.

I agree we could add that option but not as the default

quoted

quoted

If they really want to start over with a fresh patch they still can
say "no" to cancel the "edit" and start anew [*].

This is not very obvious to the user,

It has been so for a decade...

That does not make it obvious though

Keep in mind that this message will probably only be shown very _very_
rarely to users who are most likely very familiar with (e)dit.

I'd argue that users who are not familiar with (e)dit are more likely to 
make mistakes when editing hunks and are less likely to be able to fix them.

quoted

quoted

+
+	recolor_hunk(s, hunk);
+

This means we're now forking an external process when there is no hunk to
color. It would be better to avoid that by leaving this code where it was
and restoring the backup hunk above.

I don't see that external process. ¿?

Oh sorry, I thought we ran interactive.diffFilter on the edited hunk, 
but we don't. I'll try and find time to fix that.

quoted

This is still missing "n q". Apart from that the test is looking good.

I've been resisting the idea of "completeness", because I think "e y"
should also be fine.  But I'm not going to resist anymore here :-),
since I don't think the test has much more value without "n q".  So
I'll add it.

The reason I think we should have it is that the tests ought to be 
testing realistic user input and not rely on getting EOF which is 
unlikely to happen in real life.

Best Wishes

Phillip