Changing of line format and undo

Changing of line format and undo

[Lennart Borgman]

If you change line endings this change is not saved to undo. Is this the 
expected behaviour?

Re: Changing of line format and undo

[Eli Zaretskii]

> Date: Wed, 21 Jun 2006 00:15:17 +0200
> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
> 
> If you change line endings this change is not saved to undo. Is this the 
> expected behaviour?

If you mean "C-x RET f", then it doesn't really change anything.  It
just sets a buffer-local variable to some value.  So there's nothing
to undo.

Re: Changing of line format and undo

[Lennart Borgman]

Eli Zaretskii wrote:
>> Date: Wed, 21 Jun 2006 00:15:17 +0200
>> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
>>
>> If you change line endings this change is not saved to undo. Is this the 
>> expected behaviour?
>>     
>
> If you mean "C-x RET f", then it doesn't really change anything.  It
> just sets a buffer-local variable to some value.  So there's nothing
> to undo.
>   
That is an internal Emacs perpective. From a users point of view there 
is truly a change. If you for example change the line endings the file 
will be saved with this new line endings.

BTW should not changes of this kind also set the buffer to modified?

Re: Changing of line format and undo

[David Kastrup]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> Eli Zaretskii wrote:
>>> Date: Wed, 21 Jun 2006 00:15:17 +0200
>>> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
>>>
>>> If you change line endings this change is not saved to undo. Is
>>> this the expected behaviour?
>>>     
>>
>> If you mean "C-x RET f", then it doesn't really change anything.  It
>> just sets a buffer-local variable to some value.  So there's nothing
>> to undo.
>>   
> That is an internal Emacs perpective. From a users point of view there
> is truly a change. If you for example change the line endings the file
> will be saved with this new line endings.
>
> BTW should not changes of this kind also set the buffer to modified?

What makes you think they don't?

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Changing of line format and undo

[Lennart Borgman]

David Kastrup wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>> Eli Zaretskii wrote:
>>     
>>>> Date: Wed, 21 Jun 2006 00:15:17 +0200
>>>> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
>>>>
>>>> If you change line endings this change is not saved to undo. Is
>>>> this the expected behaviour?
>>>>     
>>>>         
>>> If you mean "C-x RET f", then it doesn't really change anything.  It
>>> just sets a buffer-local variable to some value.  So there's nothing
>>> to undo.
>>>   
>>>       
>> That is an internal Emacs perpective. From a users point of view there
>> is truly a change. If you for example change the line endings the file
>> will be saved with this new line endings.
>>
>> BTW should not changes of this kind also set the buffer to modified?
>>     
>
> What makes you think they don't?
>
>   
Test this for example:

    (defun test-set-eol()
      (interactive)
      (let* ((coding buffer-file-coding-system)
             (new-coding (coding-system-change-eol-conversion coding 2)))
        (setq buffer-file-coding-system new-coding)))

Re: Changing of line format and undo

[David Kastrup]

Lennart Borgman <lennart.borgman.073@student.lu.se> writes:

> David Kastrup wrote:
>> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>>
>>   
>>> Eli Zaretskii wrote:
>>>     
>>>>> Date: Wed, 21 Jun 2006 00:15:17 +0200
>>>>> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
>>>>>
>>>>> If you change line endings this change is not saved to undo. Is
>>>>> this the expected behaviour?
>>>>>             
>>>> If you mean "C-x RET f", then it doesn't really change anything.  It
>>>> just sets a buffer-local variable to some value.  So there's nothing
>>>> to undo.
>>>>         
>>> That is an internal Emacs perpective. From a users point of view there
>>> is truly a change. If you for example change the line endings the file
>>> will be saved with this new line endings.
>>>
>>> BTW should not changes of this kind also set the buffer to modified?
>>>     
>>
>> What makes you think they don't?
>>
>>   
> Test this for example:
>
>    (defun test-set-eol()
>      (interactive)
>      (let* ((coding buffer-file-coding-system)
>             (new-coding (coding-system-change-eol-conversion coding 2)))
>        (setq buffer-file-coding-system new-coding)))

Very funny.  setq most certainly does not trigger effects like a
changed buffer modification flag.  But setq is not a user-level
command.  Here is one that is:

C-x RET f (translated from C-x <return> f) runs the command set-buffer-file-coding-system
   which is an interactive compiled Lisp function in `mule.el'.
It is bound to C-x RET f, <menu-bar> <options> <mule> <set-various-coding-system> <set-buffer-file-coding-system>.
(set-buffer-file-coding-system CODING-SYSTEM &optional FORCE NOMODIFY)

Set the file coding-system of the current buffer to CODING-SYSTEM.
This means that when you save the buffer, it will be converted
according to CODING-SYSTEM.  For a list of possible values of CODING-SYSTEM,
use M-x list-coding-systems.

If CODING-SYSTEM leaves the text conversion unspecified, or if it
leaves the end-of-line conversion unspecified, FORCE controls what to
do.  If FORCE is nil, get the unspecified aspect (or aspects) from the
buffer's previous `buffer-file-coding-system' value (if it is
specified there).  Otherwise, leave it unspecified.

This marks the buffer modified so that the succeeding M-x save-buffer
surely saves the buffer with CODING-SYSTEM.  From a program, if you
don't want to mark the buffer modified, specify t for NOMODIFY.
If you know exactly what coding system you want to use,
just set the variable `buffer-file-coding-system' directly.


-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Changing of line format and undo

[Lennart Borgman]

David Kastrup wrote:
> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>
>   
>> David Kastrup wrote:
>>     
>>> Lennart Borgman <lennart.borgman.073@student.lu.se> writes:
>>>
>>>   
>>>       
>>>> Eli Zaretskii wrote:
>>>>     
>>>>         
>>>>>> Date: Wed, 21 Jun 2006 00:15:17 +0200
>>>>>> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
>>>>>>
>>>>>> If you change line endings this change is not saved to undo. Is
>>>>>> this the expected behaviour?
>>>>>>             
>>>>>>             
>>>>> If you mean "C-x RET f", then it doesn't really change anything.  It
>>>>> just sets a buffer-local variable to some value.  So there's nothing
>>>>> to undo.
>>>>>         
>>>>>           
>>>> That is an internal Emacs perpective. From a users point of view there
>>>> is truly a change. If you for example change the line endings the file
>>>> will be saved with this new line endings.
>>>>
>>>> BTW should not changes of this kind also set the buffer to modified?
>>>>     
>>>>         
>>> What makes you think they don't?
>>>
>>>   
>>>       
>> Test this for example:
>>
>>    (defun test-set-eol()
>>      (interactive)
>>      (let* ((coding buffer-file-coding-system)
>>             (new-coding (coding-system-change-eol-conversion coding 2)))
>>        (setq buffer-file-coding-system new-coding)))
>>     
>
> Very funny.  setq most certainly does not trigger effects like a
> changed buffer modification flag.  But setq is not a user-level
> command.  Here is one that is:
>   
Thanks, you are right ;-) Now I see what Eli meant. I still believe that 
everything that can result in changing an external file should go into 
the undo history but this is really a corner case of course.

Re: Changing of line format and undo

[Eli Zaretskii]

> Date: Wed, 21 Jun 2006 18:47:27 +0200
> From: Lennart Borgman <lennart.borgman.073@student.lu.se>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> Thanks, you are right ;-) Now I see what Eli meant. I still believe that 
> everything that can result in changing an external file should go into 
> the undo history

Undo saves changes to the _buffer_text_, not changes to the file on
disk.  Undo has no reasonable way of knowing whether the file on disk
will change.

Re: Changing of line format and undo

[Stuart D. Herring]

Lennart Borgman wrote:
> Eli Zaretskii wrote:
>> If you mean "C-x RET f", then it doesn't really change anything.  It
>> just sets a buffer-local variable to some value.  So there's nothing
>> to undo.
>>
> That is an internal Emacs perpective. From a users point of view there
> is truly a change. If you for example change the line endings the file
> will be saved with this new line endings.

Doing

(add-hook 'local-write-file-functions 'erase-buffer)

also would change how files were written out.  Changing how Emacs works
(via variables like the coding systems, hooks, or outright rewriting
functions) is not within the scope of the undo mechanism, which is for
modifications to text, not Emacs.

> BTW should not changes of this kind also set the buffer to modified?

If and only if they should be undoable (which they shouldn't, IMHO).  It
would be very odd to have a buffer unmodified before and after an undo, or
to be marked modified with no way (via undo) to remove that marking.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

Re: Changing of line format and undo

[Lennart Borgman]

Stuart D. Herring wrote:
> Lennart Borgman wrote:
>   
>> Eli Zaretskii wrote:
>>     
>>> If you mean "C-x RET f", then it doesn't really change anything.  It
>>> just sets a buffer-local variable to some value.  So there's nothing
>>> to undo.
>>>
>>>       
>> That is an internal Emacs perpective. From a users point of view there
>> is truly a change. If you for example change the line endings the file
>> will be saved with this new line endings.
>>     
>
> Doing
>
> (add-hook 'local-write-file-functions 'erase-buffer)
>
> also would change how files were written out.  Changing how Emacs works
> (via variables like the coding systems, hooks, or outright rewriting
> functions) is not within the scope of the undo mechanism, which is for
> modifications to text, not Emacs.
>   
I would say that the line endings is a border case. I would expect most 
users to look like it like changing the text (even if that is not what 
is done internally). However I realize it is a lot of work to fix this 
border case and it is perhaps not worth the trouble.
>   
>> BTW should not changes of this kind also set the buffer to modified?
>>     
>
> If and only if they should be undoable (which they shouldn't, IMHO).  It
> would be very odd to have a buffer unmodified before and after an undo, or
> to be marked modified with no way (via undo) to remove that marking.
>   
My mistake. As David pointed out the buffer is marked modified if you 
change the line endings in a supported way. It is natural to make the 
buffer modified since perhaps it will not get saved otherwise. 
Unfortunately this change is not undoable and that is a bit confusing 
sometimes.

Re: Changing of line format and undo

[Eli Zaretskii]

> Date: Wed, 21 Jun 2006 14:54:07 -0700 (PDT)
> From: "Stuart D. Herring" <herring@lanl.gov>
> Cc: emacs-devel@gnu.org
> 
> > BTW should not changes of this kind also set the buffer to modified?
> 
> If and only if they should be undoable

??? Why?

> It would be very odd to have a buffer unmodified before and after
> an undo, or to be marked modified with no way (via undo) to remove
> that marking.

It is very easy to ``undo'' the buffer-modified flag: type "M-~".

Re: Changing of line format and undo

[Stefan Monnier]

> Thanks, you are right ;-) Now I see what Eli meant. I still believe that
> everything that can result in changing an external file should go into the
> undo history but this is really a corner case of course.

You can change C-x RET f to do what you want by having it add (manually in
elisp) an `apply' entry to buufer-undo-list.

E.g. I've changed my set-buffer-multibyte to do that (waiting for post-22
to commit it).


        Stefan


--- orig/src/buffer.c
+++ mod/src/buffer.c
@@ -2112,10 +2100,11 @@
 {
   struct Lisp_Marker *tail, *markers;
   struct buffer *other;
-  int undo_enabled_p = !EQ (current_buffer->undo_list, Qt);
   int begv, zv;
   int narrowed = (BEG != BEGV || Z != ZV);
   int modified_p = !NILP (Fbuffer_modified_p (Qnil));
+  Lisp_Object old_undo = current_buffer->undo_list;
+  struct gcpro gcpro1;
 
   if (current_buffer->base_buffer)
     error ("Cannot do `set-buffer-multibyte' on an indirect buffer");
@@ -2124,10 +2113,11 @@
   if (NILP (flag) == NILP (current_buffer->enable_multibyte_characters))
     return flag;
 
-  /* It would be better to update the list,
-     but this is good enough for now.  */
-  if (undo_enabled_p)
-    current_buffer->undo_list = Qt;
+  GCPRO1 (old_undo);
+
+  /* Don't record these buffer changes.  We will put a special undo entry
+     instead.  */
+  current_buffer->undo_list = Qt;
 
   /* If the cached position is for this buffer, clear it out.  */
   clear_charpos_cache (current_buffer);
@@ -2327,8 +2317,18 @@
       set_intervals_multibyte (1);
     }
 
-  if (undo_enabled_p)
-    current_buffer->undo_list = Qnil;
+  if (!EQ (old_undo, Qt))
+    {
+      /* Represent all the above changes by a special undo entry.  */
+      extern Lisp_Object Qapply;
+      Lisp_Object args[3];
+      args[0] = Qapply;
+      args[1] = Qset_buffer_multibyte;
+      args[2] = NILP (flag) ? Qt : Qnil;
+      current_buffer->undo_list = Fcons (Flist (3, args), old_undo);
+    }
+
+  UNGCPRO;
 
   /* Changing the multibyteness of a buffer means that all windows
      showing that buffer must be updated thoroughly.  */

Re: Changing of line format and undo

[Kim F. Storm]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Thanks, you are right ;-) Now I see what Eli meant. I still believe that
>> everything that can result in changing an external file should go into the
>> undo history but this is really a corner case of course.
>
> You can change C-x RET f to do what you want by having it add (manually in
> elisp) an `apply' entry to buufer-undo-list.
>
> E.g. I've changed my set-buffer-multibyte to do that (waiting for post-22
> to commit it).

I think this is an _excellent_ idea (and IMO the current behaviour is faulty).

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

Re: Changing of line format and undo

[Richard Stallman]

    > E.g. I've changed my set-buffer-multibyte to do that (waiting for post-22
    > to commit it).

    I think this is an _excellent_ idea (and IMO the current behaviour is faulty).

Maybe this is a bug that should be fixed; can someone explain the
problem to me?

Re: Changing of line format and undo

[Lennart Borgman]

Richard Stallman wrote:
>     > E.g. I've changed my set-buffer-multibyte to do that (waiting for post-22
>     > to commit it).
>
>     I think this is an _excellent_ idea (and IMO the current behaviour is faulty).
>
> Maybe this is a bug that should be fixed; can someone explain the
> problem to me?
>   
I started the thread, see here: 
http://lists.gnu.org/archive/html/emacs-devel/2006-06/msg00512.html

If you change line endings this change is not saved to undo. However it 
is a change that is written to the edited file. Therefore I think it 
should be saved to undo.

It seemed quite hard to fix this but it seems like Stefan has a very 
good idea of how to do this.

Re: Changing of line format and undo

[Richard Stallman]

    If you change line endings this change is not saved to undo. However it 
    is a change that is written to the edited file. Therefore I think it 
    should be saved to undo.

    It seemed quite hard to fix this but it seems like Stefan has a very 
    good idea of how to do this.

Let's try that change, and see how it works out.

Re: Changing of line format and undo

[Kim F. Storm]

In case you forgot:

RMS agreed to install the following change in emacs 22.



Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Thanks, you are right ;-) Now I see what Eli meant. I still believe that
>> everything that can result in changing an external file should go into the
>> undo history but this is really a corner case of course.
>
> You can change C-x RET f to do what you want by having it add (manually in
> elisp) an `apply' entry to buufer-undo-list.
>
> E.g. I've changed my set-buffer-multibyte to do that (waiting for post-22
> to commit it).
>
>
>         Stefan
>
>
> --- orig/src/buffer.c
> +++ mod/src/buffer.c
> @@ -2112,10 +2100,11 @@
>  {
>    struct Lisp_Marker *tail, *markers;
>    struct buffer *other;
> -  int undo_enabled_p = !EQ (current_buffer->undo_list, Qt);
>    int begv, zv;
>    int narrowed = (BEG != BEGV || Z != ZV);
>    int modified_p = !NILP (Fbuffer_modified_p (Qnil));
> +  Lisp_Object old_undo = current_buffer->undo_list;
> +  struct gcpro gcpro1;
>  
>    if (current_buffer->base_buffer)
>      error ("Cannot do `set-buffer-multibyte' on an indirect buffer");
> @@ -2124,10 +2113,11 @@
>    if (NILP (flag) == NILP (current_buffer->enable_multibyte_characters))
>      return flag;
>  
> -  /* It would be better to update the list,
> -     but this is good enough for now.  */
> -  if (undo_enabled_p)
> -    current_buffer->undo_list = Qt;
> +  GCPRO1 (old_undo);
> +
> +  /* Don't record these buffer changes.  We will put a special undo entry
> +     instead.  */
> +  current_buffer->undo_list = Qt;
>  
>    /* If the cached position is for this buffer, clear it out.  */
>    clear_charpos_cache (current_buffer);
> @@ -2327,8 +2317,18 @@
>        set_intervals_multibyte (1);
>      }
>  
> -  if (undo_enabled_p)
> -    current_buffer->undo_list = Qnil;
> +  if (!EQ (old_undo, Qt))
> +    {
> +      /* Represent all the above changes by a special undo entry.  */
> +      extern Lisp_Object Qapply;
> +      Lisp_Object args[3];
> +      args[0] = Qapply;
> +      args[1] = Qset_buffer_multibyte;
> +      args[2] = NILP (flag) ? Qt : Qnil;
> +      current_buffer->undo_list = Fcons (Flist (3, args), old_undo);
> +    }
> +
> +  UNGCPRO;
>  
>    /* Changing the multibyteness of a buffer means that all windows
>       showing that buffer must be updated thoroughly.  */

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

Re: Changing of line format and undo

[Stefan Monnier]

> RMS agreed to install the following change in Emacs 22.

Oh, did he?  I must have missed it.  If you want to install it, then please
go ahead (I'm on the road right now).  Otherwise I'll get to it later.
Thanks,


        Stefan


> Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Thanks, you are right ;-) Now I see what Eli meant. I still believe that
>>> everything that can result in changing an external file should go into the
>>> undo history but this is really a corner case of course.
>> 
>> You can change C-x RET f to do what you want by having it add (manually in
>> elisp) an `apply' entry to buufer-undo-list.
>> 
>> E.g. I've changed my set-buffer-multibyte to do that (waiting for post-22
>> to commit it).
>> 
>> 
>> Stefan
>> 
>> 
>> --- orig/src/buffer.c
>> +++ mod/src/buffer.c
>> @@ -2112,10 +2100,11 @@
>> {
>> struct Lisp_Marker *tail, *markers;
>> struct buffer *other;
>> -  int undo_enabled_p = !EQ (current_buffer->undo_list, Qt);
>> int begv, zv;
>> int narrowed = (BEG != BEGV || Z != ZV);
>> int modified_p = !NILP (Fbuffer_modified_p (Qnil));
>> +  Lisp_Object old_undo = current_buffer->undo_list;
>> +  struct gcpro gcpro1;
>> 
>> if (current_buffer->base_buffer)
>> error ("Cannot do `set-buffer-multibyte' on an indirect buffer");
>> @@ -2124,10 +2113,11 @@
>> if (NILP (flag) == NILP (current_buffer->enable_multibyte_characters))
>> return flag;
>> 
>> -  /* It would be better to update the list,
>> -     but this is good enough for now.  */
>> -  if (undo_enabled_p)
>> -    current_buffer->undo_list = Qt;
>> +  GCPRO1 (old_undo);
>> +
>> +  /* Don't record these buffer changes.  We will put a special undo entry
>> +     instead.  */
>> +  current_buffer->undo_list = Qt;
>> 
>> /* If the cached position is for this buffer, clear it out.  */
>> clear_charpos_cache (current_buffer);
>> @@ -2327,8 +2317,18 @@
>> set_intervals_multibyte (1);
>> }
>> 
>> -  if (undo_enabled_p)
>> -    current_buffer->undo_list = Qnil;
>> +  if (!EQ (old_undo, Qt))
>> +    {
>> +      /* Represent all the above changes by a special undo entry.  */
>> +      extern Lisp_Object Qapply;
>> +      Lisp_Object args[3];
>> +      args[0] = Qapply;
>> +      args[1] = Qset_buffer_multibyte;
>> +      args[2] = NILP (flag) ? Qt : Qnil;
>> +      current_buffer->undo_list = Fcons (Flist (3, args), old_undo);
>> +    }
>> +
>> +  UNGCPRO;
>> 
>> /* Changing the multibyteness of a buffer means that all windows
>> showing that buffer must be updated thoroughly.  */

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

Re: Changing of line format and undo

[Richard Stallman]

    RMS agreed to install the following change in emacs 22.

Would you please install that change?

Re: Changing of line format and undo

[Kim F. Storm]

Richard Stallman <rms@gnu.org> writes:

>     RMS agreed to install the following change in emacs 22.
>
> Would you please install that change?

Done.

But I had to replace undefined symbol
  Qset_buffer_multibyte  
by
  intern ("set-buffer-multibyte")


Stefan, is that ok?


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

Re: Changing of line format and undo

[Kim F. Storm]

storm@cua.dk (Kim F. Storm) writes:

> Richard Stallman <rms@gnu.org> writes:
>
>>     RMS agreed to install the following change in emacs 22.
>>
>> Would you please install that change?
>
> Done.

BTW, this change does not fix the original problem of recording undo
information for changing line endings; it only addresses undo for
multibyte.

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

Re: Changing of line format and undo

[Richard Stallman]

    BTW, this change does not fix the original problem of recording undo
    information for changing line endings; it only addresses undo for
    multibyte.

set-buffer-multibyte changes the buffer contents, so it ought to be
undoable.  Changing buffer-file-coding-system does not change the buffer
contents, so I am not sure it ought to be undoable.  It only affects
how the buffer will be saved.  set-visited-file-name also affects
how the buffer will be saved, and we did not make it undoable.

I am not convinced it is correct to try to make undo cover changes
in how the buffer would be saved.

Re: Changing of line format and undo

[Lennart Borgman]

Richard Stallman wrote:
> I am not convinced it is correct to try to make undo cover changes
> in how the buffer would be saved.
>   

See it from the users point of view. Is it not desireable then to be 
able to undo all changes that will be saved to disk?

Or are there any logical problems regarding this (since undo is buffer 
specific) that can cause problems later? I can not find any. Saving is 
buffer specific.

Re: Changing of line format and undo

[Richard Stallman]

    > I am not convinced it is correct to try to make undo cover changes
    > in how the buffer would be saved.
    >   

    See it from the users point of view. Is it not desireable then to be 
    able to undo all changes that will be saved to disk?

The argument seems powerful, but it would seem to apply just as
strongly to set-visited-file-name.  Do you think set-visited-file-name
should be an undoable buffer change?