The *Warnings* buffer and undo

The *Warnings* buffer and undo

[Lennart Borgman (gmail)]

Does the *Warnings* buffer really need to save undo info? Why not set 
buffer-undo-list to t in this buffer?

Re: The *Warnings* buffer and undo

[Richard Stallman]

    Does the *Warnings* buffer really need to save undo info? Why not set 
    buffer-undo-list to t in this buffer?

I agree completely.  Would someone please do that?

Re: The *Warnings* buffer and undo

[Glenn Morris]

Richard Stallman wrote:

>     Does the *Warnings* buffer really need to save undo info? Why not set 
>     buffer-undo-list to t in this buffer?
>
> I agree completely.  Would someone please do that?

Do you mean this:


*** warnings.el 21 Jan 2007 02:44:23 -0000 1.11
--- warnings.el 28 Mar 2007 00:47:05 -0000
***************
*** 237,242 ****
--- 237,243 ----
         (level-info (assq level warning-levels))
              start end)
              (with-current-buffer buffer
+           (let ((buffer-undo-list t))
              (goto-char (point-max))
              (when (and warning-series (symbolp warning-series))
                (setq warning-series
***************
*** 260,266 ****
      (setq end (point))
        (when (and (markerp warning-series)
                   (eq (marker-buffer warning-series) buffer))
!                      (goto-char warning-series)))
                       (if (nth 2 level-info)
                           (funcall (nth 2 level-info)))
                           (if noninteractive
--- 261,267 ----
              (setq end (point))
              (when (and (markerp warning-series)
                         (eq (marker-buffer warning-series) buffer))
!               (goto-char warning-series))))
                (if (nth 2 level-info)
                    (funcall (nth 2 level-info)))
                    (if noninteractive



Maybe you want this too, I don't know:

*** bytecomp.el     26 Mar 2007 03:30:53 -0000 2.196
--- bytecomp.el     28 Mar 2007 00:48:18 -0000
***************
*** 1011,1016 ****
--- 1011,1017 ----
     (set-buffer (get-buffer-create "*Compile-Log*"))
      (goto-char (point-max))
       (let* ((inhibit-read-only t)
+                 (buffer-undo-list t)
                                    (dir (and
                                    byte-compile-current-file
                                        (file-name-directory
                                        byte-compile-current-file)))
                                         (was-same (equal
                                         default-directory dir))

Re: The *Warnings* buffer and undo

[David Kastrup]

Glenn Morris <rgm@gnu.org> writes:

> Richard Stallman wrote:
>
>>     Does the *Warnings* buffer really need to save undo info? Why not set 
>>     buffer-undo-list to t in this buffer?
>>
>> I agree completely.  Would someone please do that?
>
> Do you mean this:
>
>
> *** warnings.el 21 Jan 2007 02:44:23 -0000 1.11
> --- warnings.el 28 Mar 2007 00:47:05 -0000
> ***************
> *** 237,242 ****
> --- 237,243 ----
>          (level-info (assq level warning-levels))
>               start end)
>               (with-current-buffer buffer
> +           (let ((buffer-undo-list t))

I don't think it is a good idea to let-bind buffer-undo-list here:
instead it should generally be set to t in the buffer.

Otherwise undo information will be recorded sparsely (for example, for
user input), leading to inconsistent results.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: The *Warnings* buffer and undo

[Glenn Morris]

David Kastrup wrote:

> I don't think it is a good idea to let-bind buffer-undo-list here:
> instead it should generally be set to t in the buffer.

Maybe the following is better then. It would seem odd to me if
display-warning permanently disabled undo in pre-existing buffers.


*** warnings.el	21 Jan 2007 02:44:23 -0000	1.11
--- warnings.el	28 Mar 2007 20:49:50 -0000
***************
*** 218,225 ****
  	      but raise suspicion of a possible problem.
  :debug     -- info for debugging only.
  
! BUFFER-NAME, if specified, is the name of the buffer for logging the
! warning.  By default, it is `*Warnings*'.
  
  See the `warnings' custom group for user customization features.
  
--- 218,226 ----
  	      but raise suspicion of a possible problem.
  :debug     -- info for debugging only.
  
! BUFFER-NAME, if specified, is the name of the buffer for logging
! the warning.  By default, it is `*Warnings*'.  If this function
! has to create the buffer, it disables undo in the buffer.
  
  See the `warnings' custom group for user customization features.
  
***************
*** 227,242 ****
  `warning-fill-prefix' for additional programming features."
    (unless level
      (setq level :warning))
    (if (assq level warning-level-aliases)
        (setq level (cdr (assq level warning-level-aliases))))
    (or (< (warning-numeric-level level)
           (warning-numeric-level warning-minimum-log-level))
        (warning-suppress-p type warning-suppress-log-types)
        (let* ((typename (if (consp type) (car type) type))
! 	     (buffer (get-buffer-create (or buffer-name "*Warnings*")))
  	     (level-info (assq level warning-levels))
  	     start end)
  	(with-current-buffer buffer
  	  (goto-char (point-max))
  	  (when (and warning-series (symbolp warning-series))
  	    (setq warning-series
--- 228,249 ----
  `warning-fill-prefix' for additional programming features."
    (unless level
      (setq level :warning))
+   (unless buffer-name
+     (setq buffer-name "*Warnings*"))
    (if (assq level warning-level-aliases)
        (setq level (cdr (assq level warning-level-aliases))))
    (or (< (warning-numeric-level level)
           (warning-numeric-level warning-minimum-log-level))
        (warning-suppress-p type warning-suppress-log-types)
        (let* ((typename (if (consp type) (car type) type))
!              (old (get-buffer buffer-name))
! 	     (buffer (get-buffer-create buffer-name))
  	     (level-info (assq level warning-levels))
  	     start end)
  	(with-current-buffer buffer
+           ;; If we created the buffer, disable undo.
+           (unless old
+             (setq buffer-undo-list t))
  	  (goto-char (point-max))
  	  (when (and warning-series (symbolp warning-series))
  	    (setq warning-series

Re: The *Warnings* buffer and undo

[Richard Stallman]

These changes are good.  Would you please install them?

Re: The *Warnings* buffer and undo

[Richard Stallman]

    I don't think it is a good idea to let-bind buffer-undo-list here:
    instead it should generally be set to t in the buffer.

    Otherwise undo information will be recorded sparsely (for example, for
    user input), leading to inconsistent results.

That was my first reaction, but then it occurred to me that recording
undo info only for user editing might be just the right thing.

Re: The *Warnings* buffer and undo

[David Kastrup]

Richard Stallman <rms@gnu.org> writes:

>     I don't think it is a good idea to let-bind buffer-undo-list here:
>     instead it should generally be set to t in the buffer.
>
>     Otherwise undo information will be recorded sparsely (for example, for
>     user input), leading to inconsistent results.
>
> That was my first reaction, but then it occurred to me that recording
> undo info only for user editing might be just the right thing.

But the undo info will then not be able to _undo_ older changes, but
will rather damage the buffer in unpredictable ways if you try using
it.

Unrecorded changes turn the existing undo history useless.  While it
could conceivably be nice if user changes could be undone, this won't
work out.  The best that a program could do in this regard is set
buffer-undo-list to t before doing its changes, and to nil afterwards.

That way the user will be able to undo his changes as long as no new
output appears.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: The *Warnings* buffer and undo

[Miles Bader]

Richard Stallman <rms@gnu.org> writes:
>     Otherwise undo information will be recorded sparsely (for example, for
>     user input), leading to inconsistent results.
>
> That was my first reaction, but then it occurred to me that recording
> undo info only for user editing might be just the right thing.

There's already another thread on exactly this topic (see the subject
"A new(?) warning of erase-buffer, which was not seen before.").  I want
to add this feature for comint, and ERC already uses it.

Basically you can't just bind buffer-undo-list; there are complications
because undo information holds absolute positions (not, e.g., markers).
If any changes are shielded from the undo recording mechanism, the
positions in buffer-undo-list get screwed up.

Possible solution to this problem are:

  (1) Instead of binding buffer-undo-list, have special insert/delete
      functions that do the operation and then update buffer-undo-list
      accordingly.  This only works if you have control over all the
      relevant insert/delete calls.

  (2) Have an explicit function for fixing up buffer-undo-list.  The
      caller would then let-bind buffer-undo-list around the
      insertions/deletions and then call the function to fix things up.
      This only would work if it's very simple to describe (to the fixup
      function) where the ignored operations happened; that might be
      true of e.g., comint, but maybe not in other contexts.

  (3) Modify the emacs insert/delete primitives to do the job, e.g.,
      they could look for a variable like `inhibit-undo', and if
      non-nil, fixup buffer-undo-list to account for the new operation
      instead of actually recording the new operation in it.

All of this for after the release of course. :-)

-Miles

-- 
Love is a snowmobile racing across the tundra.  Suddenly it flips over,
pinning you underneath.  At night the ice weasels come.  --Nietzsche

Re: The *Warnings* buffer and undo

[Richard Stallman]

    Unrecorded changes turn the existing undo history useless.  While it
    could conceivably be nice if user changes could be undone, this won't
    work out.  The best that a program could do in this regard is set
    buffer-undo-list to t before doing its changes, and to nil afterwards.

That sounds like a good solution -- better than suppressing undo 100%
in that buffer.

Re: The *Warnings* buffer and undo

[Glenn Morris]

Richard Stallman wrote:

>     work out.  The best that a program could do in this regard is set
>     buffer-undo-list to t before doing its changes, and to nil afterwards.
>
> That sounds like a good solution -- better than suppressing undo 100%
> in that buffer.

Can we just leave this alone for Emacs 22? There is no actual problem
we are trying to fix, other than the general sentiment that changes
produced by display-warning should not be undoable. I don't see any
harm in the current behaviour, and there seems to be no good, simple
solution.

Re: The *Warnings* buffer and undo

[Lennart Borgman (gmail)]

Glenn Morris wrote:
> Richard Stallman wrote:
> 
>>     work out.  The best that a program could do in this regard is set
>>     buffer-undo-list to t before doing its changes, and to nil afterwards.
>>
>> That sounds like a good solution -- better than suppressing undo 100%
>> in that buffer.
> 
> Can we just leave this alone for Emacs 22? There is no actual problem
> we are trying to fix, other than the general sentiment that changes
> produced by display-warning should not be undoable. I don't see any
> harm in the current behaviour, and there seems to be no good, simple
> solution.

It is not a big problem, but perhaps it would then be best to just 
disable undo in the *Warnings* buffer?

Re: The *Warnings* buffer and undo

[Richard Stallman]

      (3) Modify the emacs insert/delete primitives to do the job, e.g.,
	  they could look for a variable like `inhibit-undo', and if
	  non-nil, fixup buffer-undo-list to account for the new operation
	  instead of actually recording the new operation in it.

This seems like a good feature to perhaps add.

However, it occurs to me that the insertion of text (such as, new
warnings) uses rather little space in an undo list.  So I wonder how
it happened that the undo list in the warnings buffer got big enough
to trigger the warning.  Did that really happen?

Re: The *Warnings* buffer and undo

[Glenn Morris]

Richard Stallman wrote:

> However, it occurs to me that the insertion of text (such as, new
> warnings) uses rather little space in an undo list.  So I wonder how
> it happened that the undo list in the warnings buffer got big enough
> to trigger the warning.  Did that really happen?

Nobody in this thread ever said it did. So can we forget about this
until after Emacs 22?

Re: The *Warnings* buffer and undo

[Miles Bader]

Richard Stallman <rms@gnu.org> writes:
>       (3) Modify the emacs insert/delete primitives to do the job, e.g.,
> 	  they could look for a variable like `inhibit-undo', and if
> 	  non-nil, fixup buffer-undo-list to account for the new operation
> 	  instead of actually recording the new operation in it.
>
> This seems like a good feature to perhaps add.
>
> However, it occurs to me that the insertion of text (such as, new
> warnings) uses rather little space in an undo list.  So I wonder how
> it happened that the undo list in the warnings buffer got big enough
> to trigger the warning.  Did that really happen?

I don't know about the *Warning* buffer (I don't think I've ever even seen
it), but in the case of comint:

 (1) If you use `comint-truncate-buffer' (in comint-output-filter-functions)
     to keep comint buffers from getting too big (and in many cases it's a
     very good idea), a line gets deleted at the beginning of the buffer for
     every new line that gets inserted at the end, which can use massive
     amounts of space in the undo list.

 (2) The interleaving of "user input" and "process output" in the undo list
     can be very annoying for the user -- when you try to undo something you
     just typed, you can end up accidentally undoing process output instead.
     That's not only very confusing, but can actually be quite hard to
     properly recover from (despite undo) if the process is continually
     sending output.

-Miles
-- 
Come now, if we were really planning to harm you, would we be waiting here,
 beside the path, in the very darkest part of the forest?

Re: The *Warnings* buffer and undo

[Richard Stallman]

    > Can we just leave this alone for Emacs 22? There is no actual problem
    > we are trying to fix, other than the general sentiment that changes
    > produced by display-warning should not be undoable. I don't see any
    > harm in the current behaviour, and there seems to be no good, simple
    > solution.

    It is not a big problem, but perhaps it would then be best to just 
    disable undo in the *Warnings* buffer?

I agree.

Re: The *Warnings* buffer and undo

[Glenn Morris]

Richard Stallman wrote:

>     It is not a big problem, but perhaps it would then be best to just 
>     disable undo in the *Warnings* buffer?
>
> I agree.

I don't really know what either of you actually want (nor why it is
important), but I checked in the change to display-warning I
previously proposed. I hope this suffices.

Re: The *Warnings* buffer and undo

[Richard Stallman]

    I don't know about the *Warning* buffer (I don't think I've ever even seen
    it), but in the case of comint:

     (1) If you use `comint-truncate-buffer' (in comint-output-filter-functions)
	 to keep comint buffers from getting too big (and in many cases it's a
	 very good idea), a line gets deleted at the beginning of the buffer for
	 every new line that gets inserted at the end, which can use massive
	 amounts of space in the undo list.

Perhaps `comint-truncate-buffer' should discard its own undo entries.
It could use `inhibit-undo', if we add such a feature.
Alternatively we could add a feature which says, "discard this undo entry,
and fix up the older entries so that they still work."  That could
be written in Lisp.

Re: The *Warnings* buffer and undo

[Richard Stallman]

    I don't really know what either of you actually want (nor why it is
    important), but I checked in the change to display-warning I
    previously proposed. I hope this suffices.

I think it does.  Thanks.

Re: The *Warnings* buffer and undo

[Miles Bader]

Richard Stallman <rms@gnu.org> writes:
>      (1) If you use `comint-truncate-buffer' (in comint-output-filter-functions)
> 	 to keep comint buffers from getting too big (and in many cases it's a
> 	 very good idea), a line gets deleted at the beginning of the buffer for
> 	 every new line that gets inserted at the end, which can use massive
> 	 amounts of space in the undo list.
>
> Perhaps `comint-truncate-buffer' should discard its own undo entries.
> It could use `inhibit-undo', if we add such a feature.
> Alternatively we could add a feature which says, "discard this undo entry,
> and fix up the older entries so that they still work."  That could
> be written in Lisp.

It's arguably less reliable to rely on comint-truncate-buffer to worry
about it, because hooks on comint-output-filter-functions are not
generally controlled by the emacs distribution -- the user can write their
own hooks, and so might not know too take care of the necessary details.

The lisp solution which I've worked on a slight bit (and which
apparently is also done in ERC to some degree) inhibits undo while
calling hooks (and during new output insertion), and then tries to
measure the resulting amount of buffer change and fix up the
buffer-undo-list to reflect it.

This is probably doable in comint because the "undoable" and "undo
inhibited" buffer modifications are separated into two easily
distinguished areas.

However in some other potential applications (someone mentioned
customization buffers, where you probably want to restrict undo to the
user-editable fields), things are not so simple, so having something
which works at a primitive level (like "inhibit-undo") might be nice in
general.

-Miles

-- 
"1971 pickup truck; will trade for guns"

Re: The *Warnings* buffer and undo

[Miles Bader]

I wrote:
> However in some other potential applications (someone mentioned
> customization buffers, where you probably want to restrict undo to the
> user-editable fields), things are not so simple, so having something
> which works at a primitive level (like "inhibit-undo") might be nice in
> general.

I think I slightly mis-read your message.  The method you mention --
looking at buffer-undo-list after the fact, and then removing some
entries and using them to fix up the rest of the entries -- sounds like
a very workable approach in lisp, since the to-be-discarded entries will
contain all the information you need.

A possible interface could be:

  (let ((keep-undo buffer-undo-list))

    ;; do some modifications which shouldn't be kept in the undo list
    (do-some-buffer-modifications)

    (discard-undo-entries keep-undo))

"discard-undo-entries" would just take a tail of the undo list as an
argument and discard everything before it (fixing up the rest).

-Miles

-- 
A zen-buddhist walked into a pizza shop and
said, "Make me one with everything."