Re: undo weirdness with insert-file-contents

[martin rudalics]

>>+     current_buffer->undo_list =
>>+       Fcons (Qnil, current_buffer->undo_list);
>
>
> We should use Fundo_boundary here.

OK - tho' I'm not sure whether pending_boundary should be considered
here.  Note that I could remove that boundary and also combine the
insertion with a previous one if that's desired.  Personally, I'd prefer
to undo two separate insertions from files separately.  In this context
note that record_insert combines two insertions only if they happen in a
left-to-right way.  IIUC this should be eventually changed when Emacs
goes bi-directional.

>>+   else if (inserted > 0
>>+          && (! EQ (original_undo_list, Qt))
>>+          && (NILP (visit)) && (NILP (replace)))
>>+     /* Assure that the first insertion is counted as a change. */
>>+     {
>>+       current_buffer->undo_list = Qnil;
>>+       record_first_change ();
>>+     }
>
>
> You use `else' between the two, but it's not clear why the two should be
> mutually exclusive.  And please mention `visit' in the comment.

It's hopefully a bit clearer now although I'm not sure whether I need
the empty_undo_list check at all.  BTW, what is the semantics of
`insert-file-contents' with VISIT nil and REPLACE non-nil?

> Also if the file is empty, is this going to mark the buffer as modified
> even though nothing was changed?

I'm afraid I don't understand the question - do you mean the case where
inserted equals zero?  I don't handle that.  But maybe I'm missing
something.

*** fileio.c.~1.602.~   Thu Feb 14 20:41:44 2008
--- fileio.c    Sun Mar  2 12:03:00 2008
***************
*** 3730,3735 ****
--- 3730,3736 ----
    int read_quit = 0;
    Lisp_Object old_Vdeactivate_mark = Vdeactivate_mark;
    int we_locked_file = 0;
+   int empty_undo_list = NILP (current_buffer->undo_list);
  
    if (current_buffer->base_buffer && ! NILP (visit))
      error ("Cannot do file visiting in an indirect buffer");
***************
*** 4593,4598 ****
--- 4594,4622 ----
    if (CODING_MAY_REQUIRE_DECODING (&coding)
        && (inserted > 0 || CODING_REQUIRE_FLUSHING (&coding)))
      {
+       if (CONSP (current_buffer->undo_list)
+         && ! NILP (XCAR (current_buffer->undo_list)))
+       /* If the undo list is non-empty and undo information is
+          recorded, insert an undo boundary unless we already have one.
+          This is needed to avoid that record_insert combines the
+          present insertion with an earlier one and consequently the
+          mechanism fails that compares the start position of the
+          insertion before vs that after the format-decode step.  */
+       Fundo_boundary ();
+       else if (empty_undo_list
+              && MODIFF <= SAVE_MODIFF
+              && NILP (visit) && NILP (replace))
+       /* Else if the present insertion is the very first change of the
+          current buffer, undo information is recorded, and we are
+          neither visiting a file nor replacing buffer contents, make
+          sure we record the insertion as a first change in the
+          undo-list.  This is needed in order to avoid that undoing the
+          insertion leaves the buffer's modified state set.  */
+       {
+         current_buffer->undo_list = Qnil;
+         record_first_change ();
+       }
+ 
        move_gap_both (PT, PT_BYTE);
        GAP_SIZE += inserted;
        ZV_BYTE -= inserted;
***************
*** 4604,4611 ****
        coding_system = CODING_ID_NAME (coding.id);
      }
    else if (inserted > 0)
!     adjust_after_insert (PT, PT_BYTE, PT + inserted, PT_BYTE + inserted,
!                        inserted);
  
    /* Now INSERTED is measured in characters.  */
  
--- 4628,4651 ----
        coding_system = CODING_ID_NAME (coding.id);
      }
    else if (inserted > 0)
!     {
!       if (CONSP (current_buffer->undo_list)
!         && ! NILP (XCAR (current_buffer->undo_list)))
!       /* Insert an undo boundary unless there's one here.  See
!          explanation above.  */
!       Fundo_boundary ();
!       else if (empty_undo_list
!              && MODIFF <= SAVE_MODIFF
!              && NILP (visit) && NILP (replace))
!       /* Make sure the first insertion is counted as a change.  See
!          explanation above.  */
!       {
!         current_buffer->undo_list = Qnil;
!         record_first_change ();
!       }
!       adjust_after_insert (PT, PT_BYTE, PT + inserted, PT_BYTE + inserted,
!                          inserted);
!     }
  
    /* Now INSERTED is measured in characters.  */