[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Emacs-diffs] fix/undo-point-in-wrong-place d4b252c 1/2: Working on stor
From: |
Phillip Lord |
Subject: |
[Emacs-diffs] fix/undo-point-in-wrong-place d4b252c 1/2: Working on storing point. |
Date: |
Mon, 23 Nov 2015 17:17:55 +0000 |
branch: fix/undo-point-in-wrong-place
commit d4b252c84279fb5c1b9d1431763fa1ea9d011d95
Author: Phillip Lord <address@hidden>
Commit: Phillip Lord <address@hidden>
Working on storing point.
I think we already have these stored in keyboard.c, so we can probably
reuse these.
---
problem.org | 56 +++++++++++++++++++++++++++++++++++++++++
src/undo.c | 13 +++++++--
test/automated/simple-test.el | 21 ++++++++++++++-
3 files changed, 86 insertions(+), 4 deletions(-)
diff --git a/problem.org b/problem.org
index f75df0c..d4c9aee 100644
--- a/problem.org
+++ b/problem.org
@@ -276,3 +276,59 @@ So, we only actually record point in this way before a
delete, and
only from one of the two calls from record_delete
So, we can rewrite record_point into two functions
+
+
+* Stefan
+
+> 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. */
+
+This comment is invalidated by your change.
+
+> + record_point (beg + SCHARS (string));
+
+Hmm I thought the sign on sbeg took care of this case already (i.e. the
+record_point should only record something when point was neither at the
+beginning nor at the end of the deleted string).
+
+As for the source of the bug (i.e. what change caused the new behavior):
+in the old code, undo-boundary was called right before every command
+(whether there was a need to push a boundary or not), so contrary to the
+comment in the code, last_boundary_position was actually recording
+"position of point at beginning of the command" rather than "position of
+point last time we inserted a boundary".
+
+So the hunk below should recover the old behavior (well, more or less:
+it wouldn't compile as is, but I hope you get the idea). But to fix it
+right, we should rename these vars and adjust their comment to better
+reflect the way they're really used.
+
+
+ Stefan
+
+
+diff --git a/src/keyboard.c b/src/keyboard.c
+index 849066c..125091e 100644
+--- a/src/keyboard.c
++++ b/src/keyboard.c
+@@ -1448,6 +1448,8 @@ command_loop_1 (void)
+ /* Ensure that we have added appropriate undo-boundaries as a
+ result of changes from the last command. */
+ call0 (Qundo_auto__add_boundary);
++ last_boundary_position = PT;
++ last_boundary_buffer = current_buffer;
+
+ call1 (Qcommand_execute, Vthis_command);
+
+
+
+But how does this solve the problem where things change in other bufers?
+
+It doesn't -- but that is more complicated, so don't worry.
+
+Are these things not already available here?
+
+
+ prev_buffer = current_buffer;
+ prev_modiff = MODIFF;
+ last_point_position = PT;
diff --git a/src/undo.c b/src/undo.c
index ade47d1..fbaa066 100644
--- a/src/undo.c
+++ b/src/undo.c
@@ -22,6 +22,7 @@ along with GNU Emacs. If not, see
<http://www.gnu.org/licenses/>. */
#include "lisp.h"
#include "buffer.h"
+#include <stdio.h>
/* The first time a command records something for undo.
it also allocates the undo-boundary object
@@ -69,10 +70,13 @@ record_point (ptrdiff_t pt)
/* If we are just after an undo boundary, and
point wasn't at start of deleted range, record where it was. */
- if (at_boundary)
+ if (at_boundary
+ && current_buffer == last_boundary_buffer
+ && last_boundary_position != pt){
bset_undo_list (current_buffer,
Fcons (make_number (pt),
BVAR (current_buffer, undo_list)));
+ }
}
/* Record an insertion that just happened or is about to happen,
@@ -162,7 +166,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)
{
@@ -171,6 +174,10 @@ record_delete (ptrdiff_t beg, Lisp_Object string, bool
record_markers)
if (EQ (BVAR (current_buffer, undo_list), Qt))
return;
+ if (PT != beg ){
+ record_point (PT);
+ }
+
if (PT == beg + SCHARS (string))
{
XSETINT (sbeg, -beg);
@@ -179,7 +186,7 @@ record_delete (ptrdiff_t beg, Lisp_Object string, bool
record_markers)
else
{
XSETFASTINT (sbeg, beg);
- record_point (beg + SCHARS (string));
+ prepare_record ();
}
/* primitive-undo assumes marker adjustments are recorded
diff --git a/test/automated/simple-test.el b/test/automated/simple-test.el
index 2fd68a1..5f035c0 100644
--- a/test/automated/simple-test.el
+++ b/test/automated/simple-test.el
@@ -274,11 +274,30 @@
])
(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))))
+ (undo-test-kill-c-a-then-undo)))
+ (should
+ (= 6
+ (undo-test-point-after-forward-kill))))
(provide 'simple-test)