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

Previous Next

Full log

Message #38 received at 60467 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60467 <at> debbugs.gnu.org, yantar92 <at> posteo.net, monnier <at> iro.umontreal.ca
Subject: Re: bug#60467: 30.0.50; primitive-undo: Changes to be undone by
 function different from announced
Date: Tue, 03 Jan 2023 15:05:29 +0000

>> -			      ;; In case garbage collection has removed OLD-BUL.
>> -			      (cdr ptr)
>
> Why should we toss this precaution?
>

I don't even understand what this is supposed to do.  By definition, 
garbage collection cannot collect something that is still referred to.

>> -		  (unless (cdr ptr)
>> -		    (message "combine-change-calls: buffer-undo-list broken"))
>
> And this one?
>

This one is just plain wrong.  It assumes that buffer-undo-list is non-nil 
initially.

>> +	;; If buffer-undo-list is neither t (in which case undo
>> +	;; information is not recorded) nor equal to buffer-undo-list
>> +	;; before body was evaluated (in which case evaluating body
>> +	;; did not add items to buffer-undo-list) ...
>> +	(when (and (not (eq buffer-undo-list t))
>> +		   (not (eq buffer-undo-list old-bul)))
>> +	  (let ((ptr buffer-undo-list) body-undo-list)
>> +	    ;; ... then loop over buffer-undo-list, until the head of
>> +	    ;; buffer-undo-list before body was evaluated is found ...
>> +	    (while (not (eq ptr old-bul))
>> +	      ;; ... and add the entries to body-undo-list, unless
>> +	      ;; they are of the form (t . <something>), which are
>> +	      ;; entries that record buffer modification timestamps.
>> +	      (unless (and (consp (car ptr))
>> +			   (eq (caar ptr) t))
>> +		(push (car ptr) body-undo-list))
>> +	      (setq ptr (cdr ptr)))
>> +	    (setq body-undo-list (nreverse body-undo-list))
>> +	    ;; Add an (apply ...) entry to buffer-undo-list, using
>> +	    ;; body-undo-list ...
>> +	    (push (list 'apply
>> +			(- end end-marker)
>> +			beg
>> +			(marker-position end-marker)
>> +			#'undo--wrap-and-run-primitive-undo
>> +			beg (marker-position end-marker)
>> +			body-undo-list)
>> +		  buffer-undo-list)
>> +	    ;; ... and set the cdr of buffer-undo-list to
>> +	    ;; buffer-undo-list before body was evaluated.
>> +	    (setcdr buffer-undo-list old-bul)))
>
> So basically you are saying that the current code stops too early and 
> doesn't collect all the undo entries that need to be combined, because 
> timestamp entries get in the way, is that right?
>

Yes.  The current code apparently meant to skip these entries, but it does 
not work at all.  Replacing a broken code that does not work with a 
clean(er) code that does work seems the right thing.

Previous Next

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