[Emacs-diffs] emacs-25 7592cb9 1/3: After delete, record point location in undo.

[Phillip Lord]

branch: emacs-25
commit 7592cb9d2a5d68dcb556c87226e38588ce555bd9
Author: Phillip Lord <address@hidden>
Commit: Phillip Lord <address@hidden>

    After delete, record point location in undo.
    
    Addresses Bug #21968.
    
        * lisp/simple.el (undo-auto--add-boundary): Clean up code to
        better support intercalating calls.
        * src/keyboard.c,src/keyboard.h (command_loop_1): Store value of
        point and current buffer before each command.
        * src/undo.c (record_point): Now only record the point.
        * src/undo.c (prepare_record): Functionality removed form
        record_point.
        * src/undo.c (record_delete): Check if point needs recording.
        * src/undo.c (undo-boundary): Record value of point before each
        boundary.
        * test/automated/simple-test.el: New tests.
    
    Conflicts:
        src/undo.c
---
 lisp/simple.el                |    9 +++---
 src/keyboard.c                |    7 ++++-
 src/keyboard.h                |   13 +++++++++
 src/undo.c                    |   57 +++++++++++++++++++++++-----------------
 test/automated/simple-test.el |   48 ++++++++++++++++++++++++++++++++++
 5 files changed, 105 insertions(+), 29 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index 6a745c7..b6bf010 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2872,10 +2872,11 @@ See also `undo-auto--buffer-undoably-changed'.")
 (defun undo-auto--add-boundary ()
   "Add an `undo-boundary' in appropriate buffers."
   (undo-auto--boundaries
-   (if undo-auto--this-command-amalgamating
-       'amalgamate
-     'command))
-  (setq undo-auto--this-command-amalgamating nil))
+   (let ((amal undo-auto--this-command-amalgamating))
+       (setq undo-auto--this-command-amalgamating nil)
+       (if amal
+           'amalgamate
+         'command))))
 
 (defun undo-auto--amalgamate ()
   "Amalgamate undo if necessary.
diff --git a/src/keyboard.c b/src/keyboard.c
index c9e58e7..02bc7d2 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -202,7 +202,7 @@ uintmax_t num_input_events;
 
 static EMACS_INT last_auto_save;
 
-/* The value of point when the last command was started.  */
+/* The value of point when the last command was started. */
 static ptrdiff_t last_point_position;
 
 /* The frame in which the last input event occurred, or Qmacro if the
@@ -1449,6 +1449,11 @@ command_loop_1 (void)
                result of changes from the last command. */
             call0 (Qundo_auto__add_boundary);
 
+            /* Record point and buffer, so we can put point into the undo
+               information if necessary. */
+            point_before_last_command_or_undo = PT;
+            buffer_before_last_command_or_undo = current_buffer;
+
             call1 (Qcommand_execute, Vthis_command);
 
 #ifdef HAVE_WINDOW_SYSTEM
diff --git a/src/keyboard.h b/src/keyboard.h
index 98bc86b..6c715a4 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -245,6 +245,19 @@ extern KBOARD *current_kboard;
 /* Total number of times read_char has returned, modulo UINTMAX_MAX + 1.  */
 extern uintmax_t num_input_events;
 
+
+/* The location of point immediately before the last command was
+   executed, or the last time the undo-boundary command added a
+   boundary.*/
+ptrdiff_t point_before_last_command_or_undo;
+
+/* The value of current_buffer immediately before the last command was
+   executed, or the last time the undo-boundary command added a
+   boundary.*/
+struct buffer *buffer_before_last_command_or_undo;
+
+extern struct buffer *prev_buffer;
+
 /* Nonzero means polling for input is temporarily suppressed.  */
 extern int poll_suppress_count;
 
diff --git a/src/undo.c b/src/undo.c
index 060dbfc..6806575 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -22,10 +22,7 @@ along with GNU Emacs.  If not, see 
<http://www.gnu.org/licenses/>.  */
 
 #include "lisp.h"
 #include "buffer.h"
-
-/* Position of point last time we inserted a boundary.  */
-static struct buffer *last_boundary_buffer;
-static ptrdiff_t last_boundary_position;
+#include "keyboard.h"
 
 /* The first time a command records something for undo.
    it also allocates the undo-boundary object
@@ -36,36 +33,44 @@ static Lisp_Object pending_boundary;
 
 /* Record point as it was at beginning of this command (if necessary)
    and prepare the undo info for recording a change.
+/* Prepare the undo info for recording a change. */
+static void
+prepare_record ()
+{
+  /* Allocate a cons cell to be the undo boundary after this command.  */
+  if (NILP (pending_boundary))
+    pending_boundary = Fcons (Qnil, Qnil);
+
+  run_undoable_change ();
+
+  if (MODIFF <= SAVE_MODIFF)
+    record_first_change ();
+}
+
+/* Record point as it was at beginning of this command.
    PT is the position of point that will naturally occur as a result of the
    undo record that will be added just after this command terminates.  */
-
 static void
 record_point (ptrdiff_t pt)
 {
-  bool at_boundary;
-
   /* Don't record position of pt when undo_inhibit_record_point holds.  */
   if (undo_inhibit_record_point)
     return;
 
-  /* Allocate a cons cell to be the undo boundary after this command.  */
-  if (NILP (pending_boundary))
-    pending_boundary = Fcons (Qnil, Qnil);
+  bool at_boundary;
 
   at_boundary = ! CONSP (BVAR (current_buffer, undo_list))
                 || NILP (XCAR (BVAR (current_buffer, undo_list)));
 
-  if (MODIFF <= SAVE_MODIFF)
-    record_first_change ();
+  prepare_record();
 
   /* If we are just after an undo boundary, and
      point wasn't at start of deleted range, record where it was.  */
-  if (at_boundary
-      && current_buffer == last_boundary_buffer
-      && last_boundary_position != pt)
+  if (at_boundary){
     bset_undo_list (current_buffer,
-                   Fcons (make_number (last_boundary_position),
+                   Fcons (make_number (pt),
                           BVAR (current_buffer, undo_list)));
+  }
 }
 
 /* Record an insertion that just happened or is about to happen,
@@ -81,7 +86,7 @@ record_insert (ptrdiff_t beg, ptrdiff_t length)
   if (EQ (BVAR (current_buffer, undo_list), Qt))
     return;
 
-  record_point (beg);
+  prepare_record ();
 
   /* If this is following another insertion and consecutive with it
      in the buffer, combine the two.  */
@@ -153,7 +158,6 @@ record_marker_adjustments (ptrdiff_t from, ptrdiff_t to)
 /* Record that a deletion is about to take place, of the characters in
    STRING, at location BEG.  Optionally record adjustments for markers
    in the region STRING occupies in the current buffer.  */
-
 void
 record_delete (ptrdiff_t beg, Lisp_Object string, bool record_markers)
 {
@@ -162,15 +166,21 @@ record_delete (ptrdiff_t beg, Lisp_Object string, bool 
record_markers)
   if (EQ (BVAR (current_buffer, undo_list), Qt))
     return;
 
+  if (point_before_last_command_or_undo != beg &&
+      buffer_before_last_command_or_undo == current_buffer)
+    {
+      record_point (point_before_last_command_or_undo);
+    }
+
   if (PT == beg + SCHARS (string))
     {
       XSETINT (sbeg, -beg);
-      record_point (PT);
+      prepare_record ();
     }
   else
     {
       XSETFASTINT (sbeg, beg);
-      record_point (beg);
+      prepare_record ();
     }
 
   /* primitive-undo assumes marker adjustments are recorded
@@ -268,10 +278,11 @@ but another undo command will undo to the previous 
boundary.  */)
        bset_undo_list (current_buffer,
                        Fcons (Qnil, BVAR (current_buffer, undo_list)));
     }
-  last_boundary_position = PT;
-  last_boundary_buffer = current_buffer;
 
   Fset (Qundo_auto__last_boundary_cause, Qexplicit);
+  point_before_last_command_or_undo = PT;
+  buffer_before_last_command_or_undo = current_buffer;
+
   return Qnil;
 }
 
@@ -423,8 +434,6 @@ syms_of_undo (void)
   pending_boundary = Qnil;
   staticpro (&pending_boundary);
 
-  last_boundary_buffer = NULL;
-
   defsubr (&Sundo_boundary);
 
   DEFVAR_INT ("undo-limit", undo_limit,
diff --git a/test/automated/simple-test.el b/test/automated/simple-test.el
index a3931ef..c758d7c 100644
--- a/test/automated/simple-test.el
+++ b/test/automated/simple-test.el
@@ -263,5 +263,53 @@
                  '("(s1) (s4)" . " (s2) (s3) (s5)"))))
 
 
+;; Test for a regression introduced by undo-auto--boundaries changes.
+;; https://lists.gnu.org/archive/html/emacs-devel/2015-11/msg01652.html
+(defun undo-test-kill-c-a-then-undo ()
+  (with-temp-buffer
+    (switch-to-buffer (current-buffer))
+    (setq buffer-undo-list nil)
+    (insert "a\nb\n\c\n")
+    (goto-char (point-max))
+    ;; We use a keyboard macro because it adds undo events in the same
+    ;; way as if a user were involved.
+    (kmacro-call-macro nil nil nil
+                       [left
+                        ;; Delete "c"
+                        backspace
+                        left left left
+                        ;; Delete "a"
+                        backspace
+                        ;; C-/ or undo
+                        67108911
+                        ])
+    (point)))
+
+(defun undo-test-point-after-forward-kill ()
+  (with-temp-buffer
+    (switch-to-buffer (current-buffer))
+    (setq buffer-undo-list nil)
+    (insert "kill word forward")
+    ;; Move to word "word".
+    (goto-char 6)
+    (kmacro-call-macro nil nil nil
+                       [
+                        ;; kill-word
+                        C-delete
+                        ;; undo
+                        67108911
+                        ])
+    (point)))
+
+(ert-deftest undo-point-in-wrong-place ()
+  (should
+   ;; returns 5 with the bug
+   (= 2
+      (undo-test-kill-c-a-then-undo)))
+  (should
+   (= 6
+      (undo-test-point-after-forward-kill))))
+
+
 (provide 'simple-test)
 ;;; simple-test.el ends here