bug#23785: Emacs 25: 'Undo' overdoes things.

["Phillip Lord" <phillip.lord@russet.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

On Sat, June 18, 2016 8:52 pm, Stefan Monnier wrote:
>> My concern is not with the behavior the proposed change intends to get
>> us, the concern is with the unintended consequences of the change.  At
>> this late stage, I'd like to keep the risk of unintended consequences to
>> the minimum, unless we want this change so badly we agree to extend the
>> pretesting by another month or two.
>
> Yes, that's the other side of the coin, indeed.


Attached are another two patches which also solve the issue.

One adds an undo boundary specifically in revert-buffer. Works but is
questionable in that revert-buffer is pluggable -- it can call any
function, other than the default.

The other fiddles with insert-file-contents and adds an undo. It is this
function that has specialized handling for the undo list that is causing
the problem. My patch in this case is questionable in that I have randomly
pushed a call to undo-boundary near the end. It should probably be
somewere better.

Another possibility would be to have insert-file-contents call
"undo-auto--undoable-change" -- this is the root cause of the problem.
Changes are happening.

Also attached is a test case which demonstrates the problem.

So many possibilities!



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-missing-undo-boundary-after-revert-buffer.patch --]
[-- Type: text/x-patch; name="0001-Fix-missing-undo-boundary-after-revert-buffer.patch", Size: 1006 bytes --]

From 4250d150cfce6cd461dc6475bac08d7587842e84 Mon Sep 17 00:00:00 2001
From: Phillip Lord <phillip.lord@russet.org.uk>
Date: Sun, 19 Jun 2016 21:34:58 +0100
Subject: [PATCH] Fix missing undo-boundary after revert-buffer

 - lisp/files.el (revert-buffer): Ensure an undo-boundary after
   completion.

Addresses Bug#23785
---
 lisp/files.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/files.el b/lisp/files.el
index 1f97fa5..c5d2bab 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5533,7 +5533,10 @@ revert-buffer
   (let ((revert-buffer-in-progress-p t)
         (revert-buffer-preserve-modes preserve-modes))
     (funcall (or revert-buffer-function #'revert-buffer--default)
-             ignore-auto noconfirm)))
+             ignore-auto noconfirm)
+    ;; Ensure that we have a undo-boundary after reversion (see Bug
+    ;; #23785)
+    (undo-boundary)))
 
 (defun revert-buffer--default (ignore-auto noconfirm)
   "Default function for `revert-buffer'.
-- 
2.9.0

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-Fix-missing-undo-boundary-on-revert.patch --]
[-- Type: text/x-patch; name="0001-Fix-missing-undo-boundary-on-revert.patch", Size: 858 bytes --]

From 1b2cd06f4b6f993052632e24c50209816314c004 Mon Sep 17 00:00:00 2001
From: Phillip Lord <phillip.lord@russet.org.uk>
Date: Sun, 19 Jun 2016 23:33:22 +0100
Subject: [PATCH] Fix missing undo-boundary on revert

But badly, because I have pushed the undo_boundary call into a fairly
random place in a very long function.
---
 src/fileio.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/fileio.c b/src/fileio.c
index b11f923..e01045e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -4500,6 +4500,10 @@ by calling `format-decode', which see.  */)
                              current_buffer->newline_cache,
                              PT - BEG, Z - PT - inserted);
 
+  /* Random push an undo boundary here because I don't know where else
+     to put this call */
+  Fundo_boundary();
+
   if (read_quit)
     Fsignal (Qquit, Qnil);
 
-- 
2.9.0

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: revert-buffer-tests.el --]
[-- Type: text/x-emacs-lisp; name="revert-buffer-tests.el", Size: 338 bytes --]

(require 'ert)

(ert-deftest undo-after-revert-buffer ()
  (should-not
   (unwind-protect
       (progn
         (find-file "simple-file.txt")
         (goto-char (point-min))
         (insert "\n")
         (revert-buffer t t)
         (undo-auto--needs-boundary-p))
     (kill-buffer
      (get-file-buffer
       "simple-file.txt")))))

[-- Attachment #5: simple-file.txt --]
[-- Type: text/plain, Size: 9 bytes --]

line one