bug#60467: 30.0.50; primitive-undo: Changes to be undone by function different from announced

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Tue, Jan 03, 2023 at 10:16:54 -0500, Stefan Monnier wrote:
> > <undo element 1> <timestamp> <undo element 2> <timestamp> nil <undo element 3>

> > ofter body has been evaluated, the buffer-undo-list after
> > combine-change-call is

> > (apply ... #'undo--wrap-and-run-primitive-undo ... (<undo element 1>))
> > <timestamp>  <undo element 2> <timestamp> nil <undo element 3>

> > which is clearly wrong

> Indeed.  Which begs the question: why does the current code stop when it
> sees a timestamp?

> Alan?  Do you remember why you did that?

I'm afraid not.  On 2018-04-01, I noted down that timestamps get "caught
up inside the undo--wrap-and-run-primitive-undo entry.".  But I neglected
to be more specific, and can no longer remember exactly why.

> What would go wrong if we applied a patch like the one below?

I don't know.  Possibly nothing.  Maybe the problem that that code was
meant to solve was also solved by some other code.

>         Stefan


> diff --git a/lisp/subr.el b/lisp/subr.el
> index 5fb150994ec..6f51ac90ce5 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -4972,10 +4972,11 @@ combine-change-calls-1
>  			      ;; In case garbage collection has removed OLD-BUL.
>  			      (cdr ptr)
>  			      ;; Don't include a timestamp entry.
> -			      (not (and (consp (cdr ptr))
> -					(consp (cadr ptr))
> -					(eq (caadr ptr) t)
> -					(setq old-bul (cdr ptr)))))
> +			      ;; (not (and (consp (cdr ptr))
> +			      ;;   	(consp (cadr ptr))
> +			      ;;   	(eq (caadr ptr) t)
> +			      ;;   	(setq old-bul (cdr ptr))))
> +			      )
>  		    (setq ptr (cdr ptr)))
>  		  (unless (cdr ptr)
>  		    (message "combine-change-calls: buffer-undo-list broken"))

-- 
Alan Mackenzie (Nuremberg, Germany).