From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Barry OReilly Newsgroups: gmane.emacs.bugs Subject: bug#16411: undo-only bugs Date: Fri, 10 Jan 2014 17:33:37 -0500 Message-ID: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=047d7b3a976034d74c04efa551a5 X-Trace: ger.gmane.org 1389393243 30018 80.91.229.3 (10 Jan 2014 22:34:03 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 10 Jan 2014 22:34:03 +0000 (UTC) To: 16411@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jan 10 23:34:10 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 1W1keY-0002dD-2r for geb-bug-gnu-emacs@m.gmane.org; Fri, 10 Jan 2014 23:34:10 +0100 Original-Received: from localhost ([::1]:59348 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1keX-0002RW-J2 for geb-bug-gnu-emacs@m.gmane.org; Fri, 10 Jan 2014 17:34:09 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60900) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1keT-0002RR-04 for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:34:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1keR-0002vM-68 for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:34:04 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:59887) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1keR-0002vI-2H for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:34:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1W1keQ-0000ah-MM for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:34:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Barry OReilly Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 10 Jan 2014 22:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 16411 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.13893932292240 (code B ref -1); Fri, 10 Jan 2014 22:34:02 +0000 Original-Received: (at submit) by debbugs.gnu.org; 10 Jan 2014 22:33:49 +0000 Original-Received: from localhost ([127.0.0.1]:45673 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1W1keC-0000a3-A8 for submit@debbugs.gnu.org; Fri, 10 Jan 2014 17:33:49 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:59105) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1W1ke9-0000Zt-9S for submit@debbugs.gnu.org; Fri, 10 Jan 2014 17:33:46 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1ke7-0002tP-9q for submit@debbugs.gnu.org; Fri, 10 Jan 2014 17:33:44 -0500 Original-Received: from lists.gnu.org ([2001:4830:134:3::11]:46313) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1ke7-0002tL-6C for submit@debbugs.gnu.org; Fri, 10 Jan 2014 17:33:43 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1ke5-0002Q5-J1 for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:33:43 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1ke3-0002sK-26 for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:33:41 -0500 Original-Received: from mail-ob0-x22f.google.com ([2607:f8b0:4003:c01::22f]:33031) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1ke2-0002sD-Qb for bug-gnu-emacs@gnu.org; Fri, 10 Jan 2014 17:33:38 -0500 Original-Received: by mail-ob0-f175.google.com with SMTP id uz6so5438152obc.20 for ; Fri, 10 Jan 2014 14:33:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type; bh=jaH0NL2Ou5f5oZmLGuOpkNtr7dChkYlj+rOA378PQjs=; b=Cp+/L378ZMomW31J7MsaWpxqpf69VD6odEgePJx1NiJOa1ydvjmpp67PNMkfvqzNB0 MlRuNTpv2dG6pGxisJU9UuzJNKDpN/521eg1RT2h1P32jp6EDoR3EUgS+mO3zgnbws9h VhtVGsPzz7NFZYg2T0KPyXD6hkqMgz7G3C488CURIHsHyVwIh+B6FGk2Ci9OMsNyXZXX 5h1Rw9U2rvntEh53fHJ+pQF4rsz8yRH0IFQ2SObWO03Mc8m+tFoxcMgtmYJ9JwryjaTM pGSRQ0zAqtyOfD0AVZCAAmy9U049J+qpWMMCg/loB2kx/+YNEgMc7mj0LoHvFZYsdBS9 FK4w== X-Received: by 10.60.118.167 with SMTP id kn7mr10181358oeb.22.1389393217901; Fri, 10 Jan 2014 14:33:37 -0800 (PST) Original-Received: by 10.76.114.74 with HTTP; Fri, 10 Jan 2014 14:33:37 -0800 (PST) X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). 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:83256 Archived-At: --047d7b3a976034d74c04efa551a5 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Looking over the code for the undo system, I found two bugs and constructed recipes to demonstrate. For these, each "insert" should be exactly one change group or the recipes may not work. I use Evil so it's easy to arrange for that. Recipe 1: =95 Insert "aaa" =95 Insert "bbb" =95 Undo =95 Insert "ccc" =95 Insert "ddd" =95 Undo =95 Undo-only with prefix arg 2 =95 Expected: none of the above insertions are in the buffer =95 Actual: buffer has aaa and bbb insertions Reason is that undo-only skips "redo records" prior to calling undo-more through recursive lookup in undo-equiv-table. However, it doesn't reference undo-equiv-table again as it iterates prefix-arg times, so it may undo redo records after the first correct undo. In this case, it redoes bbb. I think the solution entails moving this sort of thing into a loop in undo-more: (when (and (consp equiv) undo-no-redo) ;; The equiv entry might point to another redo record if we have done ;; undo-redo-undo-redo-... so skip to the very last equiv. (while (let ((next (gethash equiv undo-equiv-table))) (if next (setq equiv next)))) (setq pending-undo-list equiv)) Recipe 2: =95 Insert "aaa" =95 Insert "bbb" =95 Mark region around "aaa" but not "bbb" =95 Undo (in region) =95 Mark region around "bbb" and where "aaa" used to be =95 Undo-only =95 Expected: none of the above insertions are in the buffer =95 Actual: buffer has aaa and bbb insertions The code appears to simply punt on undo-only in region and behaves like ordinary undo. Thus it undoes the redo record to bring back bbb. One idea I have to solve bug 2 is to extend undo-equiv-table or create another redo-record-table weak hash table with the same key type as the undo-equiv-table: a "redo record" as a cons of buffer-undo-list. The value would have prefix-arg number of "undone records" which that redo record undid. When undo-only in region using the redo-record-table the algorithm would go roughly like: While pending-undo-list non nil: Pop undo-i from pending-undo-list: If undo-i is a redo record (exists in the table): Remove undo-i's undone records from pending-undo-list (or otherwise arrange to skip it) Else: Undo undo-i If "undo undo-i" was done prefix-arg times: Break (finished) Reached end of undo history As a non rigorous example, if there is an undo A of an undo B of an edit C in the region: =95 When undo-i is A, B is removed from consideration =95 With B gone, undo-i never becomes B, so C remains in pending-undo-list. A and B effectively cancelled each other out =95 C is correctly picked for the undo-only The reason undo-equiv-table is insufficient is because its value is the change group *after* the undone record. That change group may have been filtered out of pending-undo-list. OTOH, replacing existing usage of undo-equiv-table with the proposed redo-record-table is straight forward: go one change group forward to get the same value as the undo-equiv-table. Thus undo-equiv-table could be phased out in favor of using the redo-record-table. Another issue is that for both recipes, Emacs echos "Undo!". For bug 2, it does not match what Emacs actually did. For bug 1, it is partly incorrect because Emacs did an undo and a redo. Perhaps when (< 1 prefix-arg), the echo message should convey more. Possible messages might be "Undo, redo!" and "Redo, undo, undo! No further undo information" The reason I'm looking at this code at all is that I am investigating Stefan's guidance [1] about better integrating undo-tree with the builtin undo system. I think redo-record-table may help to this end, but I'll elaborate on that at later time. No later than the time I would submit a patch for bug 2, if welcome. [1] http://lists.gnu.org/archive/html/gnu-emacs-sources/2009-11/msg00010.html --047d7b3a976034d74c04efa551a5 Content-Type: text/html; charset=windows-1252 Content-Transfer-Encoding: quoted-printable
Looking over the code for the undo system, I found tw= o bugs and
constructed recipes to demonstrate. For these, each "ins= ert" should be
exactly one change group or the recipes may not work= . I use Evil so
it's easy to arrange for that.

Recipe 1:
=A0 =95 Insert "= ;aaa"
=A0 =95 Insert "bbb"
=A0 =95 Undo
=A0 =95 Ins= ert "ccc"
=A0 =95 Insert "ddd"
=A0 =95 Undo
= =A0 =95 Undo-only with prefix arg 2
=A0 =95 Expected: none of the above insertions are in the buffer
=A0 =95= Actual: buffer has aaa and bbb insertions

Reason is that undo-only = skips "redo records" prior to calling
undo-more through recurs= ive lookup in undo-equiv-table. However, it
doesn't reference undo-equiv-table again as it iterates prefix-arg
t= imes, so it may undo redo records after the first correct undo. In
this = case, it redoes bbb.

I think the solution entails moving this sort o= f thing into a loop in
undo-more:

=A0 (when (and (consp equiv) undo-no-redo)
=A0=A0=A0 ;= ; The equiv entry might point to another redo record if we have done
=A0= =A0=A0 ;; undo-redo-undo-redo-... so skip to the very last equiv.
=A0=A0= =A0 (while (let ((next (gethash equiv undo-equiv-table)))
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 (if next (setq equiv next))))
=A0= =A0=A0 (setq pending-undo-list equiv))

Recipe 2:
=A0 =95 Insert &= quot;aaa"
=A0 =95 Insert "bbb"
=A0 =95 Mark region aro= und "aaa" but not "bbb"
=A0 =95 Undo (in region)
=A0 =95 Mark region around "bbb" and = where "aaa" used to be
=A0 =95 Undo-only
=A0 =95 Expected: = none of the above insertions are in the buffer
=A0 =95 Actual: buffer ha= s aaa and bbb insertions

The code appears to simply punt on undo-only in region and behaves
l= ike ordinary undo. Thus it undoes the redo record to bring back bbb.
One idea I have to solve bug 2 is to extend undo-equiv-table or create
another redo-record-table weak hash table with the same key type as
the = undo-equiv-table: a "redo record" as a cons of buffer-undo-list.<= br>The value would have prefix-arg number of "undone records" whi= ch that
redo record undid.

When undo-only in region using the redo-record-ta= ble the algorithm
would go roughly like:

=A0 While pending-undo-l= ist non nil:
=A0=A0=A0 Pop undo-i from pending-undo-list:
=A0=A0=A0 I= f undo-i is a redo record (exists in the table):
=A0=A0=A0=A0=A0 Remove undo-i's undone records from pending-undo-list (= or
=A0=A0=A0=A0=A0 otherwise arrange to skip it)
=A0=A0=A0 Else:
= =A0=A0=A0=A0=A0 Undo undo-i
=A0=A0=A0=A0=A0 If "undo undo-i" w= as done prefix-arg times:
=A0=A0=A0=A0=A0=A0=A0 Break (finished)
=A0 Reached end of undo history

As a non rigorous example, if there = is an undo A of an undo B of an
edit C in the region:
=A0 =95 When un= do-i is A, B is removed from consideration
=A0 =95 With B gone, undo-i n= ever becomes B, so C remains in
=A0=A0=A0 pending-undo-list. A and B effectively cancelled each other out=A0 =95 C is correctly picked for the undo-only

The rea= son undo-equiv-table is insufficient is because its value is
the change = group *after* the undone record. That change group may have
been filtered out of pending-undo-list. OTOH, replacing existing usage
o= f undo-equiv-table with the proposed redo-record-table is straight
forwa= rd: go one change group forward to get the same value as the
undo-equiv-= table. Thus undo-equiv-table could be phased out in favor
of using the redo-record-table.

Another issue is that for both recip= es, Emacs echos "Undo!". For bug
2, it does not match what Ema= cs actually did. For bug 1, it is partly
incorrect because Emacs did an = undo and a redo. Perhaps when (< 1
prefix-arg), the echo message should convey more. Possible messages
migh= t be "Undo, redo!" and "Redo, undo, undo! No further undoinformation"

The reason I'm looking at this code at all is= that I am investigating
Stefan's guidance [1] about better integrating undo-tree with the
bu= iltin undo system. I think redo-record-table may help to this end,
but I= 'll elaborate on that at later time. No later than the time I
would = submit a patch for bug 2, if welcome.

[1] http://lists.gnu.org/archive/html/gnu-emacs-sources/2009= -11/msg00010.html

--047d7b3a976034d74c04efa551a5--