bug#21523: 25.0.50; Undo with active region adds extra text

[Lars Ingebrigtsen <larsi@gnus.org>]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Actually, the problem seems to be in undo-adjust-elt, which was
> rewritten in 2014 to fix bug#17235.
>
> I've now added a test case (commented out), but I don't quite understand
> the logic in undo-adjust-elt...  anybody see something obviously wrong?

(defun undo-adjust-elt (elt deltas)

[...]

    ;; (TEXT . POSITION)
    (`(,(and text (pred stringp)) . ,(and pos (pred integerp)))
     (cons text (* (if (< pos 0) -1 1)
                   (undo-adjust-pos (abs pos) deltas))))

The problem seems to be here.  In my test case, this make the ("This"
. 1) entry into a ("This" . 5) entry, which is then included in the
region.  Using < instead if <= works for this particular test case, but
not for undo-test-region-eob.

I've added Barry to the CCs; perhaps he has some insights here.

For reference, this is the test case:

(ert-deftest test-undo-region ()
  (with-temp-buffer
    (insert "This is a test\n")
    (goto-char (point-min))
    (setq buffer-undo-list nil)
    (downcase-word 1)
    (should (= (length (delq nil (undo-make-selective-list 1 9))) 2))
    ;; FIXME: These should give 0, but currently give 1.
    ;;(should (= (length (delq nil (undo-make-selective-list 4 9))) 0))
    ;;(should (= (length (delq nil (undo-make-selective-list 5 9))) 0))
    (should (= (length (delq nil (undo-make-selective-list 6 9))) 0))))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no