GNU bug report logs - #77580
[PATCH] New command ediff-undo

Previous Next

Full log

View this message in rfc822 format

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: "Paul D. Nelson" <ultrono <at> gmail.com>
Cc: 77580 <at> debbugs.gnu.org
Subject: bug#77580: [PATCH] New command ediff-undo
Date: Tue, 08 Apr 2025 11:04:12 +0800

Hello,

On Mon 07 Apr 2025 at 08:20am +02, Paul D. Nelson wrote:

> Hi Sean, thanks for your comments.
>
>> - Why ediff?  Why not plain diff?  The latter is lighter weight, and is
>>   what diff-buffer-with-file uses, and your new command feels similar to
>>   diff-buffer-with-file, to me.
>
> "ediff" because that's what I've typically used more, but I agree that
> it makes sense to have a "diff" analogue, so I've added one (attached).

Cool.  There is ediff-with-current-buffer so probably it should be named
ediff-with-undo, like you already have diff-with-undo.

Btw I noticed that in undo-to-buffer it would be better to use
'(when (minusp' instead of '(unless (>='

>> - Have you thought about automatically detecting how far to go back,
>>   somehow?  We already have a feature whereby autosaves are disabled if
>>   the buffer text has shrunk a lot; maybe that heuristic could help
>>   here.  Probably this would want to be a separate command so the manual
>>   version was still available.
>>
>
> In practice, I've just spammed C-u enough times.  I considered allowing
> a negative argument to signal "undo everything", which would be most
> useful with an active region; how does that sound?

Wouldn't undoing everything just be the existing diff-buffer-with-file?
That doesn't seem right.

> I hadn't thought about automatic detection along the lines that you
> suggest.  I think I'm happy to leave that for now, either to a
> refinement of this command or a future one.

To be honest, I don't like adding a command where the normal way to use
it is to spam C-u.  The normal way to use it should be easy to access.
So let's try to think of something better.

>>> +         (orig-buf (current-buffer))
>>> +         (current-major major-mode)
>>> +         (undo-buf-name (concat "UNDO=" (buffer-name orig-buf)))
>>
>> A more usual name would be " *ediff-undo*<N>", I think.  The space means
>> it is considered an internal buffer.  (I know you're copying other ediff
>> practice with this FOO= thing but let's not introduce more.)
>
> I'm hesitant to mark it as internal since other Ediff functions don't
> automatically delete modified non-indirect buffers without user-defined
> cleanup hooks.  In particular, I think it would be confusing if this
> function behaved differently from ediff-current-file, although I see
> your point about naming conventions.  Perhaps "*ediff-undo*<N>" would be
> more suitable?

Yes, sure, sounds good.

>>> +      (funcall current-major)
>>
>> Could you explain why the temporary buffer needs to be set to the major
>> mode?  Is it for font lock/syntax highlighting, essentially?
>
> Another reason is that users might edit the "undo" buffer during the
> Ediff session, in which case standard keybindings are useful.  Other
> suggestions would be welcome here.

Cool.

-- 
Sean Whitton

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.