GNU bug report logs - #11773
24.1; [PATCH] Handle first change undo marker correctly in case of insert-file-contents

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11773 in the body.
You can then email your comments to 11773 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Gergely Risko <gergely <at> risko.hu>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.1; [PATCH] Handle first change undo marker correctly in case of
	insert-file-contents
Date: Sun, 24 Jun 2012 19:43:16 +0200

Tags: patch

I've run into the following problem the other day:
-=-
(defun common ()
  (save-buffer)
  (setq buffer-undo-list nil))

(defun insert-bad ()
  (interactive)
  (common)
  (insert-file-contents "/etc/fstab"))

(defun insert-good ()
  (interactive)
  (common)
  (insert "foobar"))
-=-

If I open a file and run insert-good and then I undo, the file goes back
to unmodified state.

If I open a file and run insert-bad and then I undo, the file doesn't
go back to unmodified state.

Investigating, it turned out that if insert-file-contents is ran for
an unmodified buffer, the first change marker of (t HIGH . LOW) is not
inserted although it should've been.

This is because of a bug in decode_string at coding.c, where by the time
the record_insert is ran at the end, MODIFF is already too high.
MODIFF gets increased in insert_from_gap (used by produce_chars).  But
at the point of increase there is a record_insert before:
-=-
  record_insert (GPT, nchars);
  MODIFF++;
-=-

So why it's not having the effect of recording first change?  Because
decode_coding disables undo list handling at the beginning of the
function.  Therefore, my solution is to record the first change (if
MODIFF <= SAVE_MODIFF) before the undo list handling gets disabled.

I attached the patch, please consider applying it.

Thanks,
Gergely Risko

diff --git a/src/coding.c b/src/coding.c
index eb89563..212e73e 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -7083,6 +7083,15 @@ decode_coding (struct coding_system *coding)
        set_buffer_internal (XBUFFER (coding->dst_object));
       if (GPT != PT)
        move_gap_both (PT, PT_BYTE);
+
+      /* We have to disable undo_list, so we can record the whole
+        insert transaction via record_insert at the end.
+        Unfortunately, disabling the undo recording also disables the
+        recording of the first change in the undo_list.  Therefore
+        we check for first change here and record it via
+        record_first_change if needed.  */
+      if (MODIFF <= SAVE_MODIFF)
+       record_first_change ();
       undo_list = BVAR (current_buffer, undo_list);
       BVAR (current_buffer, undo_list) = Qt;
     }

Message #8 received at 11773 <at> debbugs.gnu.org (full text, mbox):

From: Chong Yidong <cyd <at> gnu.org>
To: Gergely Risko <gergely <at> risko.hu>
Cc: 11773 <at> debbugs.gnu.org
Subject: Re: bug#11773: 24.1;
	[PATCH] Handle first change undo marker correctly in case of
	insert-file-contents
Date: Tue, 14 Aug 2012 13:09:47 +0800

Gergely Risko <gergely <at> risko.hu> writes:

> decode_coding disables undo list handling at the beginning of the
> function.  Therefore, my solution is to record the first change (if
> MODIFF <= SAVE_MODIFF) before the undo list handling gets disabled.
>
> I attached the patch, please consider applying it.

Thanks, I've committed your patch to trunk.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.