Re: New undo element (fun . args)

[Kim F. Storm]

Richard Stallman <address@hidden> writes:

>       /* Make sure this produces at least one undo entry,
>        so the test in `undo' for continuing an undo series
>        will work right.  */
>       if (EQ (oldlist, current_buffer->undo_list))
>       current_buffer->undo_list
>         = Fcons (list3 (Qapply, Qcdr, nil), current_buffer->undo_list);
>
>     I don't understand why this is necessary.  The primitive-undo
>     continues to the next undo boundary, so why is it important what is
>     recorded "between undo boundaries" ?
>
> I think it is necessary for this code to work:
>
>                    ;; If something (a timer or filter?) changed the buffer
>                    ;; since the previous command, don't continue the undo seq.
>                    (let ((list buffer-undo-list))
>                      (while (eq (car list) nil)
>                        (setq list (cdr list)))
>                      ;; If the last undo record made was made by undo
>                      ;; it shows nothing else happened in between.
>                      (gethash list undo-equiv-table))))
>
> If undoing does not generate any undo entries, there will be nothing
> for this hash mark to ride on.  However, I may have misunderstood
> something.

I may have misunderstood something too, but IIUC, this checks to see
if the buffer-undo-list has not been modified since the last undo --
by comparing the current head of buffer-undo-list with the the head of
the buffer-undo-list at the end of the last undo as it was recorded in
undo-equiv-table.

So whether that last undo command added anything to buffer-undo-list
or not doesn't seem relevant, as the head of the buffer undo list
should be in the undo-equiv-table in both cases, and gethash should
succeed in both cases.

I will experiment with removing the (apply cdr nil) element and see
if it breaks things.



BTW, I suppose that the apply undo function is not allowed to change
current buffer.  And maybe we should signal an error in primitive-undo
if it does.



>
>       It works
>     to some extent, but as soon as undo hits one of the entries I added to
>     the undo list, it gets stuck -- and the pending undo list seems to
>     be stuck at that specific (apply cdr ...) entry.
>
> Could you debug precisely what is happening when it is "stuck" as you
> say?

I'm trying :-)

>   In what way does the undo process fail to work?  Where is the bug?

I'm not sure it is a bug in the undo machinery or the way I try to use
it in cua, or a combination...


PS: If it turns out that my understanding is wrong, my concern with
adding (apply cdr nil) is when the (apply ...) element is only one
among several undo elements between two undo boundaries--as long as
just one of those elements generate an undo element in
buffer-undo-list it should be sufficient to fulfill that requirement.

So this would be a problem only in the case where (apply ...) is the
only action between two undo boundaries. To work around this, it seems
to be sufficient to add just one element at the end of Fprimitive_undo
in case the undo list hasn't changed at all.


-- 
Kim F. Storm <address@hidden> http://www.cua.dk