diff options
| author | Barry O'Reilly | 2014-03-24 22:47:39 -0400 |
|---|---|---|
| committer | Barry O'Reilly | 2014-03-24 22:47:39 -0400 |
| commit | 37ea8275f7faad1192ddaba9f4a0789580675e17 (patch) | |
| tree | cb6242b0298180f32d8c253c1b3aa8af3c6572fa | |
| parent | 3e2377ce2f4eeb141ffbd000723c55813f78b08f (diff) | |
| download | emacs-37ea8275f7faad1192ddaba9f4a0789580675e17.tar.gz emacs-37ea8275f7faad1192ddaba9f4a0789580675e17.zip | |
Undo in region after markers in undo history relocated
* simple.el (primitive-undo): Only process marker adjustments
validated against their corresponding (TEXT . POS). Issue warning
for lone marker adjustments in undo history. (Bug#16818)
(undo-make-selective-list): Add marker adjustments to selective
undo list based on whether their corresponding (TEXT . POS) is in
the region. Remove variable adjusted-markers, which was unused
and only non nil during undo-make-selective-list.
(undo-elt-in-region): Return nil when passed a marker adjustment
and explain in function doc.
Have (MARKER . ADJUSTMENT) undo records always be immediately
after their corresponding (TEXT . POS) record in undo list.
(Bug#16818)
* lisp.h (record-delete): New arg record_markers.
(record_marker_adjustment): No longer needed outside undo.c.
* insdel.c (adjust_markers_for_delete): Move calculation of marker
adjustments to undo.c's record_marker_adjustments. Note that
fileio.c's decide_coding_unwind is another caller to
adjust_markers_for_delete. Because it has undo list bound to t,
it does not rely on adjust_markers_for_delete to record marker
adjustments.
(del_range_2): Swap call to record_delete and
adjust_markers_for_delete so as undo marker adjustments are
recorded before current deletion's adjustments, as before.
(adjust_after_replace):
(replace_range): Pass value for new record_markers arg to
delete_record.
* undo.c (record_marker_adjustment): Renamed to
record_marker_adjustments and made static.
(record_delete): Check record_markers arg and call
record_marker_adjustments.
(record_change): Pass value for new record_markers arg to
delete_record.
(record_point): at_boundary calculation no longer needs to account
for marker adjustments.
* undo-tests.el (undo-test-marker-adjustment-nominal):
(undo-test-region-t-marker): New tests of marker adjustments.
(undo-test-marker-adjustment-moved):
(undo-test-region-mark-adjustment): New tests to demonstrate
bug#16818, which fail without the fix.
* markers.texi (Moving Marker Positions): The 2014-03-02 doc
change mentioning undo's inability to handle relocated markers no
longer applies. See bug#16818.
* text.texi (Undo): Expand documentation of (TEXT . POS) and
(MARKER . ADJUSTMENT) undo elements.
| -rw-r--r-- | doc/lispref/ChangeLog | 8 | ||||
| -rw-r--r-- | doc/lispref/markers.texi | 10 | ||||
| -rw-r--r-- | doc/lispref/text.texi | 9 | ||||
| -rw-r--r-- | lisp/ChangeLog | 12 | ||||
| -rw-r--r-- | lisp/simple.el | 93 | ||||
| -rw-r--r-- | src/ChangeLog | 28 | ||||
| -rw-r--r-- | src/insdel.c | 40 | ||||
| -rw-r--r-- | src/lisp.h | 3 | ||||
| -rw-r--r-- | src/undo.c | 112 | ||||
| -rw-r--r-- | test/ChangeLog | 8 | ||||
| -rw-r--r-- | test/automated/undo-tests.el | 98 |
11 files changed, 294 insertions, 127 deletions
diff --git a/doc/lispref/ChangeLog b/doc/lispref/ChangeLog index 1e57d4f0db2..3dfc0dffd76 100644 --- a/doc/lispref/ChangeLog +++ b/doc/lispref/ChangeLog | |||
| @@ -1,3 +1,11 @@ | |||
| 1 | 2014-03-24 Barry O'Reilly <gundaetiapo@gmail.com> | ||
| 2 | |||
| 3 | * markers.texi (Moving Marker Positions): The 2014-03-02 doc | ||
| 4 | change mentioning undo's inability to handle relocated markers no | ||
| 5 | longer applies. See bug#16818. | ||
| 6 | * text.texi (Undo): Expand documentation of (TEXT . POS) and | ||
| 7 | (MARKER . ADJUSTMENT) undo elements. | ||
| 8 | |||
| 1 | 2014-03-22 Glenn Morris <rgm@gnu.org> | 9 | 2014-03-22 Glenn Morris <rgm@gnu.org> |
| 2 | 10 | ||
| 3 | * commands.texi (Defining Commands): List interactive-only values. | 11 | * commands.texi (Defining Commands): List interactive-only values. |
diff --git a/doc/lispref/markers.texi b/doc/lispref/markers.texi index 19386d638fe..51b87ab1e5b 100644 --- a/doc/lispref/markers.texi +++ b/doc/lispref/markers.texi | |||
| @@ -344,12 +344,10 @@ specify the insertion type, create them with insertion type | |||
| 344 | @section Moving Marker Positions | 344 | @section Moving Marker Positions |
| 345 | 345 | ||
| 346 | This section describes how to change the position of an existing | 346 | This section describes how to change the position of an existing |
| 347 | marker. When you do this, be sure you know how the marker is used | 347 | marker. When you do this, be sure you know whether the marker is used |
| 348 | outside of your program. For example, moving a marker to an unrelated | 348 | outside of your program, and, if so, what effects will result from |
| 349 | new position can cause undo to later adjust the marker incorrectly. | 349 | moving it---otherwise, confusing things may happen in other parts of |
| 350 | Often when you wish to relocate a marker to an unrelated position, it | 350 | Emacs. |
| 351 | is preferable to make a new marker and set the prior one to point | ||
| 352 | nowhere. | ||
| 353 | 351 | ||
| 354 | @defun set-marker marker position &optional buffer | 352 | @defun set-marker marker position &optional buffer |
| 355 | This function moves @var{marker} to @var{position} | 353 | This function moves @var{marker} to @var{position} |
diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi index f8a3e873449..7c5603fd645 100644 --- a/doc/lispref/text.texi +++ b/doc/lispref/text.texi | |||
| @@ -1270,7 +1270,8 @@ This kind of element indicates how to reinsert text that was deleted. | |||
| 1270 | The deleted text itself is the string @var{text}. The place to | 1270 | The deleted text itself is the string @var{text}. The place to |
| 1271 | reinsert it is @code{(abs @var{position})}. If @var{position} is | 1271 | reinsert it is @code{(abs @var{position})}. If @var{position} is |
| 1272 | positive, point was at the beginning of the deleted text, otherwise it | 1272 | positive, point was at the beginning of the deleted text, otherwise it |
| 1273 | was at the end. | 1273 | was at the end. Zero or more (@var{marker} . @var{adjustment}) |
| 1274 | elements follow immediately after this element. | ||
| 1274 | 1275 | ||
| 1275 | @item (t . @var{time-flag}) | 1276 | @item (t . @var{time-flag}) |
| 1276 | This kind of element indicates that an unmodified buffer became | 1277 | This kind of element indicates that an unmodified buffer became |
| @@ -1296,8 +1297,10 @@ Here's how you might undo the change: | |||
| 1296 | @item (@var{marker} . @var{adjustment}) | 1297 | @item (@var{marker} . @var{adjustment}) |
| 1297 | This kind of element records the fact that the marker @var{marker} was | 1298 | This kind of element records the fact that the marker @var{marker} was |
| 1298 | relocated due to deletion of surrounding text, and that it moved | 1299 | relocated due to deletion of surrounding text, and that it moved |
| 1299 | @var{adjustment} character positions. Undoing this element moves | 1300 | @var{adjustment} character positions. If the marker's location is |
| 1300 | @var{marker} @minus{} @var{adjustment} characters. | 1301 | consistent with the (@var{text} . @var{position}) element preceding it |
| 1302 | in the undo list, then undoing this element moves @var{marker} | ||
| 1303 | @minus{} @var{adjustment} characters. | ||
| 1301 | 1304 | ||
| 1302 | @item (apply @var{funname} . @var{args}) | 1305 | @item (apply @var{funname} . @var{args}) |
| 1303 | This is an extensible undo item, which is undone by calling | 1306 | This is an extensible undo item, which is undone by calling |
diff --git a/lisp/ChangeLog b/lisp/ChangeLog index 667b85729d7..b34a0b8bd7e 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog | |||
| @@ -1,3 +1,15 @@ | |||
| 1 | 2014-03-24 Barry O'Reilly <gundaetiapo@gmail.com> | ||
| 2 | |||
| 3 | * simple.el (primitive-undo): Only process marker adjustments | ||
| 4 | validated against their corresponding (TEXT . POS). Issue warning | ||
| 5 | for lone marker adjustments in undo history. (Bug#16818) | ||
| 6 | (undo-make-selective-list): Add marker adjustments to selective | ||
| 7 | undo list based on whether their corresponding (TEXT . POS) is in | ||
| 8 | the region. Remove variable adjusted-markers, which was unused | ||
| 9 | and only non nil during undo-make-selective-list. | ||
| 10 | (undo-elt-in-region): Return nil when passed a marker adjustment | ||
| 11 | and explain in function doc. | ||
| 12 | |||
| 1 | 2014-03-24 Dmitry Gutov <dgutov@yandex.ru> | 13 | 2014-03-24 Dmitry Gutov <dgutov@yandex.ru> |
| 2 | 14 | ||
| 3 | * emacs-lisp/package.el (package--add-to-archive-contents): | 15 | * emacs-lisp/package.el (package--add-to-archive-contents): |
diff --git a/lisp/simple.el b/lisp/simple.el index c8350005acc..7be1f1f6399 100644 --- a/lisp/simple.el +++ b/lisp/simple.el | |||
| @@ -2289,24 +2289,41 @@ Return what remains of the list." | |||
| 2289 | (when (let ((apos (abs pos))) | 2289 | (when (let ((apos (abs pos))) |
| 2290 | (or (< apos (point-min)) (> apos (point-max)))) | 2290 | (or (< apos (point-min)) (> apos (point-max)))) |
| 2291 | (error "Changes to be undone are outside visible portion of buffer")) | 2291 | (error "Changes to be undone are outside visible portion of buffer")) |
| 2292 | (if (< pos 0) | 2292 | (let (valid-marker-adjustments) |
| 2293 | (progn | 2293 | ;; Check that marker adjustments which were recorded |
| 2294 | (goto-char (- pos)) | 2294 | ;; with the (STRING . POS) record are still valid, ie |
| 2295 | (insert string)) | 2295 | ;; the markers haven't moved. We check their validity |
| 2296 | (goto-char pos) | 2296 | ;; before reinserting the string so as we don't need to |
| 2297 | ;; Now that we record marker adjustments | 2297 | ;; mind marker insertion-type. |
| 2298 | ;; (caused by deletion) for undo, | 2298 | (while (and (markerp (car-safe (car list))) |
| 2299 | ;; we should always insert after markers, | 2299 | (integerp (cdr-safe (car list)))) |
| 2300 | ;; so that undoing the marker adjustments | 2300 | (let* ((marker-adj (pop list)) |
| 2301 | ;; put the markers back in the right place. | 2301 | (m (car marker-adj))) |
| 2302 | (insert string) | 2302 | (and (eq (marker-buffer m) (current-buffer)) |
| 2303 | (goto-char pos))) | 2303 | (= pos m) |
| 2304 | (push marker-adj valid-marker-adjustments)))) | ||
| 2305 | ;; Insert string and adjust point | ||
| 2306 | (if (< pos 0) | ||
| 2307 | (progn | ||
| 2308 | (goto-char (- pos)) | ||
| 2309 | (insert string)) | ||
| 2310 | (goto-char pos) | ||
| 2311 | (insert string) | ||
| 2312 | (goto-char pos)) | ||
| 2313 | ;; Adjust the valid marker adjustments | ||
| 2314 | (dolist (adj valid-marker-adjustments) | ||
| 2315 | (set-marker (car adj) | ||
| 2316 | (- (car adj) (cdr adj)))))) | ||
| 2304 | ;; (MARKER . OFFSET) means a marker MARKER was adjusted by OFFSET. | 2317 | ;; (MARKER . OFFSET) means a marker MARKER was adjusted by OFFSET. |
| 2305 | (`(,(and marker (pred markerp)) . ,(and offset (pred integerp))) | 2318 | (`(,(and marker (pred markerp)) . ,(and offset (pred integerp))) |
| 2306 | (when (marker-buffer marker) | 2319 | (warn "Encountered %S entry in undo list with no matching (TEXT . POS) entry" |
| 2307 | (set-marker marker | 2320 | next) |
| 2308 | (- marker offset) | 2321 | ;; Even though these elements are not expected in the undo |
| 2309 | (marker-buffer marker)))) | 2322 | ;; list, adjust them to be conservative for the 24.4 |
| 2323 | ;; release. (Bug#16818) | ||
| 2324 | (set-marker marker | ||
| 2325 | (- marker offset) | ||
| 2326 | (marker-buffer marker))) | ||
| 2310 | (_ (error "Unrecognized entry in undo list %S" next)))) | 2327 | (_ (error "Unrecognized entry in undo list %S" next)))) |
| 2311 | (setq arg (1- arg))) | 2328 | (setq arg (1- arg))) |
| 2312 | ;; Make sure an apply entry produces at least one undo entry, | 2329 | ;; Make sure an apply entry produces at least one undo entry, |
| @@ -2341,8 +2358,6 @@ are ignored. If BEG and END are nil, all undo elements are used." | |||
| 2341 | (undo-make-selective-list (min beg end) (max beg end)) | 2358 | (undo-make-selective-list (min beg end) (max beg end)) |
| 2342 | buffer-undo-list))) | 2359 | buffer-undo-list))) |
| 2343 | 2360 | ||
| 2344 | (defvar undo-adjusted-markers) | ||
| 2345 | |||
| 2346 | (defun undo-make-selective-list (start end) | 2361 | (defun undo-make-selective-list (start end) |
| 2347 | "Return a list of undo elements for the region START to END. | 2362 | "Return a list of undo elements for the region START to END. |
| 2348 | The elements come from `buffer-undo-list', but we keep only | 2363 | The elements come from `buffer-undo-list', but we keep only |
| @@ -2351,7 +2366,6 @@ If we find an element that crosses an edge of this region, | |||
| 2351 | we stop and ignore all further elements." | 2366 | we stop and ignore all further elements." |
| 2352 | (let ((undo-list-copy (undo-copy-list buffer-undo-list)) | 2367 | (let ((undo-list-copy (undo-copy-list buffer-undo-list)) |
| 2353 | (undo-list (list nil)) | 2368 | (undo-list (list nil)) |
| 2354 | undo-adjusted-markers | ||
| 2355 | some-rejected | 2369 | some-rejected |
| 2356 | undo-elt temp-undo-list delta) | 2370 | undo-elt temp-undo-list delta) |
| 2357 | (while undo-list-copy | 2371 | (while undo-list-copy |
| @@ -2361,15 +2375,30 @@ we stop and ignore all further elements." | |||
| 2361 | ;; This is a "was unmodified" element. | 2375 | ;; This is a "was unmodified" element. |
| 2362 | ;; Keep it if we have kept everything thus far. | 2376 | ;; Keep it if we have kept everything thus far. |
| 2363 | (not some-rejected)) | 2377 | (not some-rejected)) |
| 2378 | ;; Skip over marker adjustments, instead relying on | ||
| 2379 | ;; finding them after (TEXT . POS) elements | ||
| 2380 | ((markerp (car-safe undo-elt)) | ||
| 2381 | nil) | ||
| 2364 | (t | 2382 | (t |
| 2365 | (undo-elt-in-region undo-elt start end))))) | 2383 | (undo-elt-in-region undo-elt start end))))) |
| 2366 | (if keep-this | 2384 | (if keep-this |
| 2367 | (progn | 2385 | (progn |
| 2368 | (setq end (+ end (cdr (undo-delta undo-elt)))) | 2386 | (setq end (+ end (cdr (undo-delta undo-elt)))) |
| 2369 | ;; Don't put two nils together in the list | 2387 | ;; Don't put two nils together in the list |
| 2370 | (if (not (and (eq (car undo-list) nil) | 2388 | (when (not (and (eq (car undo-list) nil) |
| 2371 | (eq undo-elt nil))) | 2389 | (eq undo-elt nil))) |
| 2372 | (setq undo-list (cons undo-elt undo-list)))) | 2390 | (setq undo-list (cons undo-elt undo-list)) |
| 2391 | ;; If (TEXT . POS), "keep" its subsequent (MARKER | ||
| 2392 | ;; . ADJUSTMENT) whose markers haven't moved. | ||
| 2393 | (when (and (stringp (car-safe undo-elt)) | ||
| 2394 | (integerp (cdr-safe undo-elt))) | ||
| 2395 | (let ((list-i (cdr undo-list-copy))) | ||
| 2396 | (while (markerp (car-safe (car list-i))) | ||
| 2397 | (let* ((adj-elt (pop list-i)) | ||
| 2398 | (m (car adj-elt))) | ||
| 2399 | (and (eq (marker-buffer m) (current-buffer)) | ||
| 2400 | (= (cdr undo-elt) m) | ||
| 2401 | (push adj-elt undo-list)))))))) | ||
| 2373 | (if (undo-elt-crosses-region undo-elt start end) | 2402 | (if (undo-elt-crosses-region undo-elt start end) |
| 2374 | (setq undo-list-copy nil) | 2403 | (setq undo-list-copy nil) |
| 2375 | (setq some-rejected t) | 2404 | (setq some-rejected t) |
| @@ -2417,7 +2446,12 @@ we stop and ignore all further elements." | |||
| 2417 | 2446 | ||
| 2418 | (defun undo-elt-in-region (undo-elt start end) | 2447 | (defun undo-elt-in-region (undo-elt start end) |
| 2419 | "Determine whether UNDO-ELT falls inside the region START ... END. | 2448 | "Determine whether UNDO-ELT falls inside the region START ... END. |
| 2420 | If it crosses the edge, we return nil." | 2449 | If it crosses the edge, we return nil. |
| 2450 | |||
| 2451 | Generally this function is not useful for determining | ||
| 2452 | whether (MARKER . ADJUSTMENT) undo elements are in the region, | ||
| 2453 | because markers can be arbitrarily relocated. Instead, pass the | ||
| 2454 | marker adjustment's corresponding (TEXT . POS) element." | ||
| 2421 | (cond ((integerp undo-elt) | 2455 | (cond ((integerp undo-elt) |
| 2422 | (and (>= undo-elt start) | 2456 | (and (>= undo-elt start) |
| 2423 | (<= undo-elt end))) | 2457 | (<= undo-elt end))) |
| @@ -2430,17 +2464,8 @@ If it crosses the edge, we return nil." | |||
| 2430 | (and (>= (abs (cdr undo-elt)) start) | 2464 | (and (>= (abs (cdr undo-elt)) start) |
| 2431 | (<= (abs (cdr undo-elt)) end))) | 2465 | (<= (abs (cdr undo-elt)) end))) |
| 2432 | ((and (consp undo-elt) (markerp (car undo-elt))) | 2466 | ((and (consp undo-elt) (markerp (car undo-elt))) |
| 2433 | ;; This is a marker-adjustment element (MARKER . ADJUSTMENT). | 2467 | ;; (MARKER . ADJUSTMENT) |
| 2434 | ;; See if MARKER is inside the region. | 2468 | (<= start (car undo-elt) end)) |
| 2435 | (let ((alist-elt (assq (car undo-elt) undo-adjusted-markers))) | ||
| 2436 | (unless alist-elt | ||
| 2437 | (setq alist-elt (cons (car undo-elt) | ||
| 2438 | (marker-position (car undo-elt)))) | ||
| 2439 | (setq undo-adjusted-markers | ||
| 2440 | (cons alist-elt undo-adjusted-markers))) | ||
| 2441 | (and (cdr alist-elt) | ||
| 2442 | (>= (cdr alist-elt) start) | ||
| 2443 | (<= (cdr alist-elt) end)))) | ||
| 2444 | ((null (car undo-elt)) | 2469 | ((null (car undo-elt)) |
| 2445 | ;; (nil PROPERTY VALUE BEG . END) | 2470 | ;; (nil PROPERTY VALUE BEG . END) |
| 2446 | (let ((tail (nthcdr 3 undo-elt))) | 2471 | (let ((tail (nthcdr 3 undo-elt))) |
diff --git a/src/ChangeLog b/src/ChangeLog index da82eca9df2..cbe10d89690 100644 --- a/src/ChangeLog +++ b/src/ChangeLog | |||
| @@ -1,3 +1,31 @@ | |||
| 1 | 2014-03-24 Barry O'Reilly <gundaetiapo@gmail.com> | ||
| 2 | |||
| 3 | Have (MARKER . ADJUSTMENT) undo records always be immediately | ||
| 4 | after their corresponding (TEXT . POS) record in undo list. | ||
| 5 | (Bug#16818) | ||
| 6 | * lisp.h (record-delete): New arg record_markers. | ||
| 7 | (record_marker_adjustment): No longer needed outside undo.c. | ||
| 8 | * insdel.c (adjust_markers_for_delete): Move calculation of marker | ||
| 9 | adjustments to undo.c's record_marker_adjustments. Note that | ||
| 10 | fileio.c's decide_coding_unwind is another caller to | ||
| 11 | adjust_markers_for_delete. Because it has undo list bound to t, | ||
| 12 | it does not rely on adjust_markers_for_delete to record marker | ||
| 13 | adjustments. | ||
| 14 | (del_range_2): Swap call to record_delete and | ||
| 15 | adjust_markers_for_delete so as undo marker adjustments are | ||
| 16 | recorded before current deletion's adjustments, as before. | ||
| 17 | (adjust_after_replace): | ||
| 18 | (replace_range): Pass value for new record_markers arg to | ||
| 19 | delete_record. | ||
| 20 | * undo.c (record_marker_adjustment): Renamed to | ||
| 21 | record_marker_adjustments and made static. | ||
| 22 | (record_delete): Check record_markers arg and call | ||
| 23 | record_marker_adjustments. | ||
| 24 | (record_change): Pass value for new record_markers arg to | ||
| 25 | delete_record. | ||
| 26 | (record_point): at_boundary calculation no longer needs to account | ||
| 27 | for marker adjustments. | ||
| 28 | |||
| 1 | 2014-03-24 Martin Rudalics <rudalics@gmx.at> | 29 | 2014-03-24 Martin Rudalics <rudalics@gmx.at> |
| 2 | 30 | ||
| 3 | * w32term.c (x_set_window_size): Refine fix from 2014-03-14 | 31 | * w32term.c (x_set_window_size): Refine fix from 2014-03-14 |
diff --git a/src/insdel.c b/src/insdel.c index 1c9bafd6004..eb1ad627f66 100644 --- a/src/insdel.c +++ b/src/insdel.c | |||
| @@ -233,34 +233,9 @@ adjust_markers_for_delete (ptrdiff_t from, ptrdiff_t from_byte, | |||
| 233 | /* Here's the case where a marker is inside text being deleted. */ | 233 | /* Here's the case where a marker is inside text being deleted. */ |
| 234 | else if (charpos > from) | 234 | else if (charpos > from) |
| 235 | { | 235 | { |
| 236 | if (! m->insertion_type) | ||
| 237 | { /* Normal markers will end up at the beginning of the | ||
| 238 | re-inserted text after undoing a deletion, and must be | ||
| 239 | adjusted to move them to the correct place. */ | ||
| 240 | XSETMISC (marker, m); | ||
| 241 | record_marker_adjustment (marker, from - charpos); | ||
| 242 | } | ||
| 243 | else if (charpos < to) | ||
| 244 | { /* Before-insertion markers will automatically move forward | ||
| 245 | upon re-inserting the deleted text, so we have to arrange | ||
| 246 | for them to move backward to the correct position. */ | ||
| 247 | XSETMISC (marker, m); | ||
| 248 | record_marker_adjustment (marker, to - charpos); | ||
| 249 | } | ||
| 250 | m->charpos = from; | 236 | m->charpos = from; |
| 251 | m->bytepos = from_byte; | 237 | m->bytepos = from_byte; |
| 252 | } | 238 | } |
| 253 | /* Here's the case where a before-insertion marker is immediately | ||
| 254 | before the deleted region. */ | ||
| 255 | else if (charpos == from && m->insertion_type) | ||
| 256 | { | ||
| 257 | /* Undoing the change uses normal insertion, which will | ||
| 258 | incorrectly make MARKER move forward, so we arrange for it | ||
| 259 | to then move backward to the correct place at the beginning | ||
| 260 | of the deleted region. */ | ||
| 261 | XSETMISC (marker, m); | ||
| 262 | record_marker_adjustment (marker, to - from); | ||
| 263 | } | ||
| 264 | } | 239 | } |
| 265 | } | 240 | } |
| 266 | 241 | ||
| @@ -1219,7 +1194,7 @@ adjust_after_replace (ptrdiff_t from, ptrdiff_t from_byte, | |||
| 1219 | from + len, from_byte + len_byte, 0); | 1194 | from + len, from_byte + len_byte, 0); |
| 1220 | 1195 | ||
| 1221 | if (nchars_del > 0) | 1196 | if (nchars_del > 0) |
| 1222 | record_delete (from, prev_text); | 1197 | record_delete (from, prev_text, false); |
| 1223 | record_insert (from, len); | 1198 | record_insert (from, len); |
| 1224 | 1199 | ||
| 1225 | if (len > nchars_del) | 1200 | if (len > nchars_del) |
| @@ -1384,7 +1359,7 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new, | |||
| 1384 | if (!NILP (deletion)) | 1359 | if (!NILP (deletion)) |
| 1385 | { | 1360 | { |
| 1386 | record_insert (from + SCHARS (deletion), inschars); | 1361 | record_insert (from + SCHARS (deletion), inschars); |
| 1387 | record_delete (from, deletion); | 1362 | record_delete (from, deletion, false); |
| 1388 | } | 1363 | } |
| 1389 | 1364 | ||
| 1390 | GAP_SIZE -= outgoing_insbytes; | 1365 | GAP_SIZE -= outgoing_insbytes; |
| @@ -1716,13 +1691,14 @@ del_range_2 (ptrdiff_t from, ptrdiff_t from_byte, | |||
| 1716 | else | 1691 | else |
| 1717 | deletion = Qnil; | 1692 | deletion = Qnil; |
| 1718 | 1693 | ||
| 1719 | /* Relocate all markers pointing into the new, larger gap | 1694 | /* Record marker adjustments, and text deletion into undo |
| 1720 | to point at the end of the text before the gap. | 1695 | history. */ |
| 1721 | Do this before recording the deletion, | 1696 | record_delete (from, deletion, true); |
| 1722 | so that undo handles this after reinserting the text. */ | 1697 | |
| 1698 | /* Relocate all markers pointing into the new, larger gap to point | ||
| 1699 | at the end of the text before the gap. */ | ||
| 1723 | adjust_markers_for_delete (from, from_byte, to, to_byte); | 1700 | adjust_markers_for_delete (from, from_byte, to, to_byte); |
| 1724 | 1701 | ||
| 1725 | record_delete (from, deletion); | ||
| 1726 | MODIFF++; | 1702 | MODIFF++; |
| 1727 | CHARS_MODIFF = MODIFF; | 1703 | CHARS_MODIFF = MODIFF; |
| 1728 | 1704 | ||
diff --git a/src/lisp.h b/src/lisp.h index 2f9a30fdfe9..30f52b9070c 100644 --- a/src/lisp.h +++ b/src/lisp.h | |||
| @@ -4198,9 +4198,8 @@ extern void syms_of_macros (void); | |||
| 4198 | extern Lisp_Object Qapply; | 4198 | extern Lisp_Object Qapply; |
| 4199 | extern Lisp_Object Qinhibit_read_only; | 4199 | extern Lisp_Object Qinhibit_read_only; |
| 4200 | extern void truncate_undo_list (struct buffer *); | 4200 | extern void truncate_undo_list (struct buffer *); |
| 4201 | extern void record_marker_adjustment (Lisp_Object, ptrdiff_t); | ||
| 4202 | extern void record_insert (ptrdiff_t, ptrdiff_t); | 4201 | extern void record_insert (ptrdiff_t, ptrdiff_t); |
| 4203 | extern void record_delete (ptrdiff_t, Lisp_Object); | 4202 | extern void record_delete (ptrdiff_t, Lisp_Object, bool); |
| 4204 | extern void record_first_change (void); | 4203 | extern void record_first_change (void); |
| 4205 | extern void record_change (ptrdiff_t, ptrdiff_t); | 4204 | extern void record_change (ptrdiff_t, ptrdiff_t); |
| 4206 | extern void record_property_change (ptrdiff_t, ptrdiff_t, | 4205 | extern void record_property_change (ptrdiff_t, ptrdiff_t, |
diff --git a/src/undo.c b/src/undo.c index 7286d40b2e5..2dde02b99a9 100644 --- a/src/undo.c +++ b/src/undo.c | |||
| @@ -75,27 +75,8 @@ record_point (ptrdiff_t pt) | |||
| 75 | Fundo_boundary (); | 75 | Fundo_boundary (); |
| 76 | last_undo_buffer = current_buffer; | 76 | last_undo_buffer = current_buffer; |
| 77 | 77 | ||
| 78 | if (CONSP (BVAR (current_buffer, undo_list))) | 78 | at_boundary = ! CONSP (BVAR (current_buffer, undo_list)) |
| 79 | { | 79 | || NILP (XCAR (BVAR (current_buffer, undo_list))); |
| 80 | /* Set AT_BOUNDARY only when we have nothing other than | ||
| 81 | marker adjustment before undo boundary. */ | ||
| 82 | |||
| 83 | Lisp_Object tail = BVAR (current_buffer, undo_list), elt; | ||
| 84 | |||
| 85 | while (1) | ||
| 86 | { | ||
| 87 | if (NILP (tail)) | ||
| 88 | elt = Qnil; | ||
| 89 | else | ||
| 90 | elt = XCAR (tail); | ||
| 91 | if (NILP (elt) || ! (CONSP (elt) && MARKERP (XCAR (elt)))) | ||
| 92 | break; | ||
| 93 | tail = XCDR (tail); | ||
| 94 | } | ||
| 95 | at_boundary = NILP (elt); | ||
| 96 | } | ||
| 97 | else | ||
| 98 | at_boundary = 1; | ||
| 99 | 80 | ||
| 100 | if (MODIFF <= SAVE_MODIFF) | 81 | if (MODIFF <= SAVE_MODIFF) |
| 101 | record_first_change (); | 82 | record_first_change (); |
| @@ -147,11 +128,61 @@ record_insert (ptrdiff_t beg, ptrdiff_t length) | |||
| 147 | Fcons (Fcons (lbeg, lend), BVAR (current_buffer, undo_list))); | 128 | Fcons (Fcons (lbeg, lend), BVAR (current_buffer, undo_list))); |
| 148 | } | 129 | } |
| 149 | 130 | ||
| 150 | /* Record that a deletion is about to take place, | 131 | /* Record the fact that markers in the region of FROM, TO are about to |
| 151 | of the characters in STRING, at location BEG. */ | 132 | be adjusted. This is done only when a marker points within text |
| 133 | being deleted, because that's the only case where an automatic | ||
| 134 | marker adjustment won't be inverted automatically by undoing the | ||
| 135 | buffer modification. */ | ||
| 136 | |||
| 137 | static void | ||
| 138 | record_marker_adjustments (ptrdiff_t from, ptrdiff_t to) | ||
| 139 | { | ||
| 140 | Lisp_Object marker; | ||
| 141 | register struct Lisp_Marker *m; | ||
| 142 | register ptrdiff_t charpos, adjustment; | ||
| 143 | |||
| 144 | /* Allocate a cons cell to be the undo boundary after this command. */ | ||
| 145 | if (NILP (pending_boundary)) | ||
| 146 | pending_boundary = Fcons (Qnil, Qnil); | ||
| 147 | |||
| 148 | if (current_buffer != last_undo_buffer) | ||
| 149 | Fundo_boundary (); | ||
| 150 | last_undo_buffer = current_buffer; | ||
| 151 | |||
| 152 | for (m = BUF_MARKERS (current_buffer); m; m = m->next) | ||
| 153 | { | ||
| 154 | charpos = m->charpos; | ||
| 155 | eassert (charpos <= Z); | ||
| 156 | |||
| 157 | if (from <= charpos && charpos <= to) | ||
| 158 | { | ||
| 159 | /* insertion_type nil markers will end up at the beginning of | ||
| 160 | the re-inserted text after undoing a deletion, and must be | ||
| 161 | adjusted to move them to the correct place. | ||
| 162 | |||
| 163 | insertion_type t markers will automatically move forward | ||
| 164 | upon re-inserting the deleted text, so we have to arrange | ||
| 165 | for them to move backward to the correct position. */ | ||
| 166 | adjustment = (m->insertion_type ? to : from) - charpos; | ||
| 167 | |||
| 168 | if (adjustment) | ||
| 169 | { | ||
| 170 | XSETMISC (marker, m); | ||
| 171 | bset_undo_list | ||
| 172 | (current_buffer, | ||
| 173 | Fcons (Fcons (marker, make_number (adjustment)), | ||
| 174 | BVAR (current_buffer, undo_list))); | ||
| 175 | } | ||
| 176 | } | ||
| 177 | } | ||
| 178 | } | ||
| 179 | |||
| 180 | /* Record that a deletion is about to take place, of the characters in | ||
| 181 | STRING, at location BEG. Optionally record adjustments for markers | ||
| 182 | in the region STRING occupies in the current buffer. */ | ||
| 152 | 183 | ||
| 153 | void | 184 | void |
| 154 | record_delete (ptrdiff_t beg, Lisp_Object string) | 185 | record_delete (ptrdiff_t beg, Lisp_Object string, bool record_markers) |
| 155 | { | 186 | { |
| 156 | Lisp_Object sbeg; | 187 | Lisp_Object sbeg; |
| 157 | 188 | ||
| @@ -169,34 +200,15 @@ record_delete (ptrdiff_t beg, Lisp_Object string) | |||
| 169 | record_point (beg); | 200 | record_point (beg); |
| 170 | } | 201 | } |
| 171 | 202 | ||
| 172 | bset_undo_list | 203 | /* primitive-undo assumes marker adjustments are recorded |
| 173 | (current_buffer, | 204 | immediately before the deletion is recorded. See bug 16818 |
| 174 | Fcons (Fcons (string, sbeg), BVAR (current_buffer, undo_list))); | 205 | discussion. */ |
| 175 | } | 206 | if (record_markers) |
| 176 | 207 | record_marker_adjustments (beg, beg + SCHARS (string)); | |
| 177 | /* Record the fact that MARKER is about to be adjusted by ADJUSTMENT. | ||
| 178 | This is done only when a marker points within text being deleted, | ||
| 179 | because that's the only case where an automatic marker adjustment | ||
| 180 | won't be inverted automatically by undoing the buffer modification. */ | ||
| 181 | |||
| 182 | void | ||
| 183 | record_marker_adjustment (Lisp_Object marker, ptrdiff_t adjustment) | ||
| 184 | { | ||
| 185 | if (EQ (BVAR (current_buffer, undo_list), Qt)) | ||
| 186 | return; | ||
| 187 | |||
| 188 | /* Allocate a cons cell to be the undo boundary after this command. */ | ||
| 189 | if (NILP (pending_boundary)) | ||
| 190 | pending_boundary = Fcons (Qnil, Qnil); | ||
| 191 | |||
| 192 | if (current_buffer != last_undo_buffer) | ||
| 193 | Fundo_boundary (); | ||
| 194 | last_undo_buffer = current_buffer; | ||
| 195 | 208 | ||
| 196 | bset_undo_list | 209 | bset_undo_list |
| 197 | (current_buffer, | 210 | (current_buffer, |
| 198 | Fcons (Fcons (marker, make_number (adjustment)), | 211 | Fcons (Fcons (string, sbeg), BVAR (current_buffer, undo_list))); |
| 199 | BVAR (current_buffer, undo_list))); | ||
| 200 | } | 212 | } |
| 201 | 213 | ||
| 202 | /* Record that a replacement is about to take place, | 214 | /* Record that a replacement is about to take place, |
| @@ -206,7 +218,7 @@ record_marker_adjustment (Lisp_Object marker, ptrdiff_t adjustment) | |||
| 206 | void | 218 | void |
| 207 | record_change (ptrdiff_t beg, ptrdiff_t length) | 219 | record_change (ptrdiff_t beg, ptrdiff_t length) |
| 208 | { | 220 | { |
| 209 | record_delete (beg, make_buffer_string (beg, beg + length, 1)); | 221 | record_delete (beg, make_buffer_string (beg, beg + length, 1), false); |
| 210 | record_insert (beg, length); | 222 | record_insert (beg, length); |
| 211 | } | 223 | } |
| 212 | 224 | ||
diff --git a/test/ChangeLog b/test/ChangeLog index 15677e76468..e5df7536d0b 100644 --- a/test/ChangeLog +++ b/test/ChangeLog | |||
| @@ -1,3 +1,11 @@ | |||
| 1 | 2014-03-24 Barry O'Reilly <gundaetiapo@gmail.com> | ||
| 2 | |||
| 3 | * undo-tests.el (undo-test-marker-adjustment-nominal): | ||
| 4 | (undo-test-region-t-marker): New tests of marker adjustments. | ||
| 5 | (undo-test-marker-adjustment-moved): | ||
| 6 | (undo-test-region-mark-adjustment): New tests to demonstrate | ||
| 7 | bug#16818, which fail without the fix. | ||
| 8 | |||
| 1 | 2014-03-23 Daniel Colascione <dancol@dancol.org> | 9 | 2014-03-23 Daniel Colascione <dancol@dancol.org> |
| 2 | 10 | ||
| 3 | * automated/cl-lib.el (cl-lib-keyword-names-versus-values): New | 11 | * automated/cl-lib.el (cl-lib-keyword-names-versus-values): New |
diff --git a/test/automated/undo-tests.el b/test/automated/undo-tests.el index 8a963f10028..6ecac36b6b3 100644 --- a/test/automated/undo-tests.el +++ b/test/automated/undo-tests.el | |||
| @@ -268,6 +268,104 @@ | |||
| 268 | (should (string= (buffer-string) | 268 | (should (string= (buffer-string) |
| 269 | "This sentence corrupted?aaa")))) | 269 | "This sentence corrupted?aaa")))) |
| 270 | 270 | ||
| 271 | (ert-deftest undo-test-marker-adjustment-nominal () | ||
| 272 | "Test nominal behavior of marker adjustments." | ||
| 273 | (with-temp-buffer | ||
| 274 | (buffer-enable-undo) | ||
| 275 | (insert "abcdefg") | ||
| 276 | (undo-boundary) | ||
| 277 | (let ((m (make-marker))) | ||
| 278 | (set-marker m 2 (current-buffer)) | ||
| 279 | (goto-char (point-min)) | ||
| 280 | (delete-forward-char 3) | ||
| 281 | (undo-boundary) | ||
| 282 | (should (= (point-min) (marker-position m))) | ||
| 283 | (undo) | ||
| 284 | (undo-boundary) | ||
| 285 | (should (= 2 (marker-position m)))))) | ||
| 286 | |||
| 287 | (ert-deftest undo-test-region-t-marker () | ||
| 288 | "Test undo in region containing marker with t insertion-type." | ||
| 289 | (with-temp-buffer | ||
| 290 | (buffer-enable-undo) | ||
| 291 | (transient-mark-mode 1) | ||
| 292 | (insert "abcdefg") | ||
| 293 | (undo-boundary) | ||
| 294 | (let ((m (make-marker))) | ||
| 295 | (set-marker-insertion-type m t) | ||
| 296 | (set-marker m (point-min) (current-buffer)) ; m at a | ||
| 297 | (goto-char (+ 2 (point-min))) | ||
| 298 | (push-mark (point) t t) | ||
| 299 | (setq mark-active t) | ||
| 300 | (goto-char (point-min)) | ||
| 301 | (delete-forward-char 1) ;; delete region covering "ab" | ||
| 302 | (undo-boundary) | ||
| 303 | (should (= (point-min) (marker-position m))) | ||
| 304 | ;; Resurrect "ab". m's insertion type means the reinsertion | ||
| 305 | ;; moves it forward 2, and then the marker adjustment returns it | ||
| 306 | ;; to its rightful place. | ||
| 307 | (undo) | ||
| 308 | (undo-boundary) | ||
| 309 | (should (= (point-min) (marker-position m)))))) | ||
| 310 | |||
| 311 | (ert-deftest undo-test-marker-adjustment-moved () | ||
| 312 | "Test marker adjustment behavior when the marker moves. | ||
| 313 | Demonstrates bug 16818." | ||
| 314 | (with-temp-buffer | ||
| 315 | (buffer-enable-undo) | ||
| 316 | (insert "abcdefghijk") | ||
| 317 | (undo-boundary) | ||
| 318 | (let ((m (make-marker))) | ||
| 319 | (set-marker m 2 (current-buffer)) ; m at b | ||
| 320 | (goto-char (point-min)) | ||
| 321 | (delete-forward-char 3) ; m at d | ||
| 322 | (undo-boundary) | ||
| 323 | (set-marker m 4) ; m at g | ||
| 324 | (undo) | ||
| 325 | (undo-boundary) | ||
| 326 | ;; m still at g, but shifted 3 because deletion undone | ||
| 327 | (should (= 7 (marker-position m)))))) | ||
| 328 | |||
| 329 | (ert-deftest undo-test-region-mark-adjustment () | ||
| 330 | "Test that the mark's marker adjustment in undo history doesn't | ||
| 331 | obstruct undo in region from finding the correct change group. | ||
| 332 | Demonstrates bug 16818." | ||
| 333 | (with-temp-buffer | ||
| 334 | (buffer-enable-undo) | ||
| 335 | (transient-mark-mode 1) | ||
| 336 | (insert "First line\n") | ||
| 337 | (insert "Second line\n") | ||
| 338 | (undo-boundary) | ||
| 339 | |||
| 340 | (goto-char (point-min)) | ||
| 341 | (insert "aaa") | ||
| 342 | (undo-boundary) | ||
| 343 | |||
| 344 | (undo) | ||
| 345 | (undo-boundary) | ||
| 346 | |||
| 347 | (goto-char (point-max)) | ||
| 348 | (insert "bbb") | ||
| 349 | (undo-boundary) | ||
| 350 | |||
| 351 | (push-mark (point) t t) | ||
| 352 | (setq mark-active t) | ||
| 353 | (goto-char (- (point) 3)) | ||
| 354 | (delete-forward-char 1) | ||
| 355 | (undo-boundary) | ||
| 356 | |||
| 357 | (insert "bbb") | ||
| 358 | (undo-boundary) | ||
| 359 | |||
| 360 | (goto-char (point-min)) | ||
| 361 | (push-mark (point) t t) | ||
| 362 | (setq mark-active t) | ||
| 363 | (goto-char (+ (point) 3)) | ||
| 364 | (undo) | ||
| 365 | (undo-boundary) | ||
| 366 | |||
| 367 | (should (string= (buffer-string) "aaaFirst line\nSecond line\nbbb")))) | ||
| 368 | |||
| 271 | (defun undo-test-all (&optional interactive) | 369 | (defun undo-test-all (&optional interactive) |
| 272 | "Run all tests for \\[undo]." | 370 | "Run all tests for \\[undo]." |
| 273 | (interactive "p") | 371 | (interactive "p") |