Re: undo weirdness with insert-file-contents

[martin rudalics]

> Good, point so we just turn off undo during the dance and then insert
> the single undo-element (this is only valid for non-REPLACE, of course).

Hopefully for VISIT as well - we should not record deletions when
handling the VISIT&REPLACE case.  I see only three cases here:

(1) undo-list was t => leave it alone.

(2) nochange => restore undo-list to previous value.

(3) otherwise => reset undo-list to nil.

I also added a nochange arm for the non-decoding case - please have a
look.  BTW nochange makes an incompatible change to `revert-buffer': You
can no longer use `revert-buffer' to get rid of the undo list.  I think
this should be documented somehow.  Finally, we should document that
REPLACE doesn't handle differences in text properties and tell users to
write their own `revert-buffer-insert-file-contents-function' to handle
them.

martin, who still considers the non-VISIT&REPLACE case obscure and
cannot see any "example" in `smerge-resolve'.

*** fileio.c    Wed Feb 27 07:49:02 2008
--- fileio.c    Fri Mar  7 10:16:10 2008
***************
*** 3721,3727 ****
    register int unprocessed;
    int count = SPECPDL_INDEX ();
    struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5;
!   Lisp_Object handler, val, insval, orig_filename, old_undo;
    Lisp_Object p;
    int total = 0;
    int not_regular = 0;
--- 3721,3727 ----
    register int unprocessed;
    int count = SPECPDL_INDEX ();
    struct gcpro gcpro1, gcpro2, gcpro3, gcpro4, gcpro5;
!   Lisp_Object handler, val, insval, orig_filename, old_undo_list;
    Lisp_Object p;
    int total = 0;
    int not_regular = 0;
***************
*** 3734,3739 ****
--- 3734,3740 ----
    int read_quit = 0;
    Lisp_Object old_Vdeactivate_mark = Vdeactivate_mark;
    int we_locked_file = 0;
+   int buffer_was_unmodified;
  
    if (current_buffer->base_buffer && ! NILP (visit))
      error ("Cannot do file visiting in an indirect buffer");
***************
*** 3744,3752 ****
    val = Qnil;
    p = Qnil;
    orig_filename = Qnil;
!   old_undo = Qnil;
  
!   GCPRO5 (filename, val, p, orig_filename, old_undo);
  
    CHECK_STRING (filename);
    filename = Fexpand_file_name (filename, Qnil);
--- 3745,3754 ----
    val = Qnil;
    p = Qnil;
    orig_filename = Qnil;
!   old_undo_list = Qnil;
!   buffer_was_unmodified = MODIFF <= SAVE_MODIFF;
  
!   GCPRO5 (filename, val, p, orig_filename, old_undo_list);
  
    CHECK_STRING (filename);
    filename = Fexpand_file_name (filename, Qnil);
***************
*** 3984,3989 ****
--- 3986,3999 ----
        set_coding_system = 1;
      }
  
+   /* Don't record undo information when visiting or not replacing; we
+      restore the undo list manually after the format decode step.  */
+   if (! NILP (visit) || NILP (replace))
+     {
+       old_undo_list = current_buffer->undo_list;
+       current_buffer->undo_list = Qt;
+     }
+ 
    /* If requested, replace the accessible part of the buffer
       with the file contents.  Avoid replacing text at the
       beginning or end of the buffer that matches the file contents;
***************
*** 4067,4074 ****
        {
          emacs_close (fd);
          specpdl_ptr--;
!         /* Truncate the buffer to the size of the file.  */
!         del_range_1 (same_at_start, same_at_end, 0, 0);
          goto handled;
        }
        immediate_quit = 1;
--- 4077,4088 ----
        {
          emacs_close (fd);
          specpdl_ptr--;
!         if (same_at_start == same_at_end)
!           nochange = 1;
!         else
!           /* Truncate the buffer to the size of the file.  */
!           del_range_1 (same_at_start, same_at_end, 0, 0);
!         inserted = 0;
          goto handled;
        }
        immediate_quit = 1;
***************
*** 4281,4290 ****
        if (bufpos == inserted)
        {
          specpdl_ptr--;
-         /* Truncate the buffer to the size of the file.  */
          if (same_at_start == same_at_end)
            nochange = 1;
          else
            del_range_byte (same_at_start, same_at_end, 0);
          inserted = 0;
  
--- 4295,4304 ----
        if (bufpos == inserted)
        {
          specpdl_ptr--;
          if (same_at_start == same_at_end)
            nochange = 1;
          else
+           /* Truncate the buffer to the size of the file.  */
            del_range_byte (same_at_start, same_at_end, 0);
          inserted = 0;
  
***************
*** 4632,4640 ****
  
    if (!NILP (visit))
      {
-       if (!EQ (current_buffer->undo_list, Qt) && !nochange)
-       current_buffer->undo_list = Qnil;
- 
        if (NILP (handler))
        {
          current_buffer->modtime = st.st_mtime;
--- 4646,4651 ----
***************
*** 4679,4688 ****
        specbind (Qinhibit_point_motion_hooks, Qt);
        specbind (Qinhibit_modification_hooks, Qt);
  
-       /* Save old undo list and don't record undo for decoding. */
-       old_undo = current_buffer->undo_list;
-       current_buffer->undo_list = Qt;
- 
        if (NILP (replace))
        {
          insval = call3 (Qformat_decode,
--- 4690,4695 ----
***************
*** 4766,4793 ****
          p = XCDR (p);
        }
  
!       if (NILP (visit))
        {
!         Lisp_Object lbeg, lend;
!         XSETINT (lbeg, PT);
!         XSETINT (lend, PT + inserted);
!         if (CONSP (old_undo))
            {
!             Lisp_Object tem = XCAR (old_undo);
!             if (CONSP (tem) && INTEGERP (XCAR (tem)) &&
!                 INTEGERP (XCDR (tem)) && EQ (XCAR (tem), lbeg))
!               /* In the non-visiting case record only the final insertion. */
!               current_buffer->undo_list =
!                 Fcons (Fcons (lbeg, lend), Fcdr (old_undo));
            }
        }
-       else
-       /* If undo_list was Qt before, keep it that way.
-          Otherwise start with an empty undo_list. */
-       current_buffer->undo_list = EQ (old_undo, Qt) ? Qt : Qnil;
- 
        unbind_to (count, Qnil);
      }
  
    /* Call after-change hooks for the inserted text, aside from the case
       of normal visiting (not with REPLACE), which is done in a new buffer
--- 4773,4813 ----
          p = XCDR (p);
        }
  
!       if (! NILP (visit))
        {
!         if (! EQ (old_undo_list, Qt))
!           /* Reset undo list for visiting case.  */
!           current_buffer->undo_list =  Qnil;
!       }
!       else if (NILP (replace) && ! EQ (old_undo_list, Qt))
!       /* For the non-visiting, non-replacing case record the last
!          insertion only.  This avoids that intermediate steps get
!          recorded and show up during undo.  */
!       {
!         current_buffer->undo_list = old_undo_list;
!         if (inserted > 0)
            {
!             if (buffer_was_unmodified)
!               /* If the buffer was unmodified when this function was
!                  called record a first buffer change (this is *not*
!                  done by the record_insert below since the buffer is
!                  already modified).  */
!               record_first_change ();
!             record_insert (PT, inserted);
            }
        }
        unbind_to (count, Qnil);
      }
+   else if (! NILP (visit))
+     {
+       if (! EQ (old_undo_list, Qt))
+       /* For the visiting case restore old undo list iff replacing and
+          nothing changed.  Otherwise start with empty undo list.  */
+       current_buffer->undo_list = nochange ? old_undo_list : Qnil;
+     }
+   else if (NILP (replace))
+     /* Restore old undo list in non-visiting, non-replacing case.  */
+     current_buffer->undo_list = old_undo_list;
  
    /* Call after-change hooks for the inserted text, aside from the case
       of normal visiting (not with REPLACE), which is done in a new buffer