From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Newsgroups: gmane.emacs.bugs Subject: bug#16818: Acknowledgement (Undo in region after markers in undo history relocated) Date: Mon, 17 Mar 2014 20:02:43 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1395101001 15685 80.91.229.3 (18 Mar 2014 00:03:21 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 18 Mar 2014 00:03:21 +0000 (UTC) Cc: 16818@debbugs.gnu.org To: Barry OReilly Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Mar 18 01:03:29 2014 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WPhVA-0007fM-HD for geb-bug-gnu-emacs@m.gmane.org; Tue, 18 Mar 2014 01:03:28 +0100 Original-Received: from localhost ([::1]:60962 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPhVA-00021V-3k for geb-bug-gnu-emacs@m.gmane.org; Mon, 17 Mar 2014 20:03:28 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:40616) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPhUx-0000nS-7c for bug-gnu-emacs@gnu.org; Mon, 17 Mar 2014 20:03:22 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPhUl-00059W-4s for bug-gnu-emacs@gnu.org; Mon, 17 Mar 2014 20:03:15 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:38172) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPhUl-00059S-2Q for bug-gnu-emacs@gnu.org; Mon, 17 Mar 2014 20:03:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1WPhUk-0001Ek-RQ for bug-gnu-emacs@gnu.org; Mon, 17 Mar 2014 20:03:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 18 Mar 2014 00:03:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 16818 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 16818-submit@debbugs.gnu.org id=B16818.13951009674715 (code B ref 16818); Tue, 18 Mar 2014 00:03:02 +0000 Original-Received: (at 16818) by debbugs.gnu.org; 18 Mar 2014 00:02:47 +0000 Original-Received: from localhost ([127.0.0.1]:39352 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WPhUU-0001Dw-Fy for submit@debbugs.gnu.org; Mon, 17 Mar 2014 20:02:46 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:41672) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WPhUS-0001Do-NC for 16818@debbugs.gnu.org; Mon, 17 Mar 2014 20:02:45 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Av8EABK/CFHO+KKg/2dsb2JhbABEuzWDWRdzgh4BAQQBViMQCzQSFBgNJIgeBsEtjQ2DfQOOGJZigV6DE4FK X-IPAS-Result: Av8EABK/CFHO+KKg/2dsb2JhbABEuzWDWRdzgh4BAQQBViMQCzQSFBgNJIgeBsEtjQ2DfQOOGJZigV6DE4FK X-IronPort-AV: E=Sophos;i="4.84,565,1355115600"; d="scan'208";a="52406720" Original-Received: from 206-248-162-160.dsl.teksavvy.com (HELO pastel.home) ([206.248.162.160]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 17 Mar 2014 20:02:43 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id B01E26028B; Mon, 17 Mar 2014 20:02:43 -0400 (EDT) In-Reply-To: (Barry OReilly's message of "Mon, 17 Mar 2014 19:05:34 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:86929 Archived-At: >> The primitive-undo fix should be safe enough for 24.4, so if you >> want to code this up and install it, feel free. > I have attached the patch for this, which I'll install if nothing > comes up from review. Thanks. A few comments below. --- a/lisp/simple.el +++ b/lisp/simple.el @@ -2229,6 +2229,7 @@ (did-apply nil) (next nil)) (while (> arg 0) + (let (del-pos) (while (setq next (pop list)) ;Exit inner loop at undo boundary. ;; Handle an integer by setting point to that value. (pcase next @@ -2289,6 +2290,7 @@ (when (let ((apos (abs pos))) (or (< apos (point-min)) (> apos (point-max)))) (error "Changes to be undone are outside visible portion of buffer")) + (setq del-pos pos) (if (< pos 0) (progn (goto-char (- pos)) @@ -2303,11 +2305,14 @@ (goto-char pos))) ;; (MARKER . OFFSET) means a marker MARKER was adjusted by OFFSET. (`(,(and marker (pred markerp)) . ,(and offset (pred integerp))) - (when (marker-buffer marker) + (when (and del-pos + (integerp (marker-position marker)) + (= del-pos marker) + (marker-buffer marker)) (set-marker marker (- marker offset) (marker-buffer marker)))) - (_ (error "Unrecognized entry in undo list %S" next)))) + (_ (error "Unrecognized entry in undo list %S" next))))) (setq arg (1- arg))) ;; Make sure an apply entry produces at least one undo entry, ;; so the test in `undo' for continuing an undo series The "move-after" markers will be at (+ del-pos (length inserted-text)) rather than at del-pos. Also, a lone (MARKER . OFFSET) entry will arbitrarily use some unrelated earlier del-pos. Admittedly, such lone (MARKER . OFFSET) entries should/will never happen, but I'd rather we catch them and emit a warning than silently do something arbitrary. IOW, I think we should only change the entry corresponding to a deletion such that it directly handles all the immediately following marker-adjustments (it can check them before doing the insertion, hence using del-pos without having to care about insert-after vs insert-before). Then the current handling of marker-adjustments can be changed to emit a warning. > @@ -2351,18 +2354,30 @@ If we find an element that crosses an edge of this region, > we stop and ignore all further elements." > (let ((undo-list-copy (undo-copy-list buffer-undo-list)) > (undo-list (list nil)) > - undo-adjusted-markers > + ;; The position of a deletion record (TEXT . POSITION) of the > + ;; current change group. > + ;; > + ;; This is used to check that marker adjustmenets are in the > + ;; region. Bug 16818 describes why the marker's position is > + ;; not suitable. > + del-pos > some-rejected > undo-elt temp-undo-list delta) > (while undo-list-copy > (setq undo-elt (car undo-list-copy)) > + ;; Update del-pos > + (if undo-elt > + (when (and (consp undo-elt) (stringp (car undo-elt))) > + (setq del-pos (cdr undo-elt))) > + ;; Undo boundary means new change group, so unset del-pos > + (setq del-pos nil)) > (let ((keep-this > (cond ((and (consp undo-elt) (eq (car undo-elt) t)) > ;; This is a "was unmodified" element. > ;; Keep it if we have kept everything thus far. > (not some-rejected)) > (t > - (undo-elt-in-region undo-elt start end))))) > + (undo-elt-in-region undo-elt start end del-pos))))) > (if keep-this > (progn > (setq end (+ end (cdr (undo-delta undo-elt)))) I'd have the same comment here, but if we emit a warning for sole marker-adjustments in the "non-region" code, we don't really have to worry about them here. But this is also affected by the issue of "move-after" markers where `del-pos' is not sufficient info since those markers won't be at del-pos any more if they were at del-pos before we inserted the deleted text. Stefan