timers and undo

timers and undo

[Luc Teirlinck]

The way timers treat undo is really peculiar.

Do `emacs -q'.  Then:

M-: (run-at-time 5 5 (lambda () (with-current-buffer "*scratch*"
(insert "aaaaaaa\n"))))

Wait till enough lines are inserted.  C-/.  The entire block of
inserted lines gets deleted all at once.  This apparently causes
problems with undo-outer-limit as I pointed out before.

But there is more.

Wait until once more a bunch of lines are inserted.  I now expected
that C-/ would get rid of these inserted lines once more.  But no.
It deletes the introductory comment lines.  The "aaaaaaa" lines
stay and they are impossible to undo after repaeting C-/

undo-more: No further undo information

It seems hard to consider this a deliberate "feature".

Sincerely,

Luc.

Re: timers and undo

[Richard Stallman]

    Wait until once more a bunch of lines are inserted.  I now expected
    that C-/ would get rid of these inserted lines once more.  But no.
    It deletes the introductory comment lines.  The "aaaaaaa" lines
    stay and they are impossible to undo after repaeting C-/

This is because it sees that the previous command was also `undo'.
So it continues the undo sequence previously started.

One fix is to make the command `undo' defensively notice that the undo
list has changed since its previous execution.  Does the fix below
give good results?

One drawback of this patch is that the space in the undo list
will still be wasted if buffer-undo-list is cleared out

*** simple.el	20 Dec 2004 15:37:08 -0500	1.673
--- simple.el	26 Dec 2004 15:12:32 -0500	
*** 1215,1220 ****
--- 1234,1243 ----
  (defvar undo-no-redo nil
    "If t, `undo' doesn't go through redo entries.")
  
+ (defvar undo-list-saved nil
+   "The value of `buffer-undo-list' saved by the last undo command.")
+ (make-variable-buffer-local 'undo-list-saved)
+ 
  (defun undo (&optional arg)
    "Undo some previous changes.
  Repeat this command to undo more changes.
***************
*** 1237,1243 ****
      ;; So set `this-command' to something other than `undo'.
      (setq this-command 'undo-start)
  
!     (unless (eq last-command 'undo)
        (setq undo-in-region
  	    (if transient-mark-mode mark-active (and arg (not (numberp arg)))))
        (if undo-in-region
--- 1260,1267 ----
      ;; So set `this-command' to something other than `undo'.
      (setq this-command 'undo-start)
  
!     (unless (and (eq last-command 'undo)
! 		 (eq undo-list-saved buffer-undo-list))
        (setq undo-in-region
  	    (if transient-mark-mode mark-active (and arg (not (numberp arg)))))
        (if undo-in-region
***************
*** 1289,1295 ****
  	      (setq tail (cdr tail)))
  	    (setq tail nil)))
  	(setq prev tail tail (cdr tail))))
! 
      (and modified (not (buffer-modified-p))
  	 (delete-auto-save-file-if-necessary recent-save))))
  
--- 1313,1319 ----
  	      (setq tail (cdr tail)))
  	    (setq tail nil)))
  	(setq prev tail tail (cdr tail))))
!     (setq saved-undo-list buffer-undo-list)
      (and modified (not (buffer-modified-p))
  	 (delete-auto-save-file-if-necessary recent-save))))

Re: timers and undo

[Stefan Monnier]

> This is because it sees that the previous command was also `undo'.
> So it continues the undo sequence previously started.

> One fix is to make the command `undo' defensively notice that the undo
> list has changed since its previous execution.  Does the fix below
> give good results?

You could use the info that's already in undo-equiv-table (and which uses
a weak hash table so there's no impact on the GC behavior) to check whether
changes have occured since the last undo command or not (I use it here to
allow the user to continue a previous undo sequence even if it was
interrupted by some cursor movement).


        Stefan

Re: timers and undo

[Luc Teirlinck]

Richard Stallman wrote:

   One fix is to make the command `undo' defensively notice that the undo
   list has changed since its previous execution.  Does the fix below
   give good results?

Sorry for waiting nearly two days to check this.  But yes, the fix you
already committed to simple.el seems to work, as long as the timer
puts in undo boundaries before and after modification of the buffer,
as it should.

Sincerely,

Luc.