From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#16411: undo-only bugs Date: Wed, 14 May 2014 22:23:37 -0400 Message-ID: References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1400120692 27393 80.91.229.3 (15 May 2014 02:24:52 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 15 May 2014 02:24:52 +0000 (UTC) Cc: 16411 <16411@debbugs.gnu.org> To: Barry OReilly Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu May 15 04:24:45 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 1WklLh-0005cH-2d for geb-bug-gnu-emacs@m.gmane.org; Thu, 15 May 2014 04:24:45 +0200 Original-Received: from localhost ([::1]:55319 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WklLg-0000I2-K2 for geb-bug-gnu-emacs@m.gmane.org; Wed, 14 May 2014 22:24:44 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46961) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WklL7-000888-T4 for bug-gnu-emacs@gnu.org; Wed, 14 May 2014 22:24:17 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WklL0-0004iO-B3 for bug-gnu-emacs@gnu.org; Wed, 14 May 2014 22:24:09 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:41982) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WklL0-0004iB-7i for bug-gnu-emacs@gnu.org; Wed, 14 May 2014 22:24:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1WklKz-0007PU-QM for bug-gnu-emacs@gnu.org; Wed, 14 May 2014 22:24:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 15 May 2014 02:24:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 16411 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 16411-submit@debbugs.gnu.org id=B16411.140012062528450 (code B ref 16411); Thu, 15 May 2014 02:24:01 +0000 Original-Received: (at 16411) by debbugs.gnu.org; 15 May 2014 02:23:45 +0000 Original-Received: from localhost ([127.0.0.1]:35046 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WklKi-0007On-Kj for submit@debbugs.gnu.org; Wed, 14 May 2014 22:23:45 -0400 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:43044) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WklKf-0007OZ-Eb for 16411@debbugs.gnu.org; Wed, 14 May 2014 22:23:42 -0400 Original-Received: from pastel.home (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id s4F2NcaD027109; Wed, 14 May 2014 22:23:38 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 1CA2060366; Wed, 14 May 2014 22:23:38 -0400 (EDT) In-Reply-To: (Barry OReilly's message of "Wed, 14 May 2014 17:56:52 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux) X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 1 Rules triggered RV4942=0 X-NAI-Spam-Version: 2.3.0.9378 : core <4942> : inlines <870> : streams <1183088> : uri <1757034> 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:89114 Archived-At: >> You talk here about "undo element", but AFAICT this actually points >> to "a list of undo elements" (where the first element of that list >> is presumably the "undo element" you mean). Please clarify. > Yes, that's right. The first element is the "undo element" referred > to. The cons is put in the hash table to facilitate recursive lookup. Makes sense, but please change the docstring to make it clear. > Implementing the "Y undo-redos of X" optimization is a > mitigating benefit. I've never heard anyone bump into this "Y undo-redos of X" problem, so I don't think optimizing it is worth the trouble. If anything should be done with it, I think it'd be to *cut* the extra undo/redo pairs. >> [ Nitpick: the first line of the docstring should stand on its own. ] > I don't understand, I thought I formatted the docstring like others, > and did M-q it. "Stands on its own" basically mean "doesn't end in the middle of a sentence= ". Some commands only display the first line of a docstring. > I think you're saying to not pursue the use of a closure to generate > successive undo elements as needed? Actually, I didn't really mean to say that. I'm not completely convinced that this generator is worthwhile (i.e. my first reaction was to try and see how to get rid of it), but I then decided not to say anything about it yet ;-) > =E2=80=A2 I'm trying to prevent a performance regression of the undo-on= ly > command. Given the performance of undo in region with the default > undo limit, maybe that's not a big concern. One way to avoid the performance regression is to use undo-equiv-table for normal undo and only use undo-redo-table for undo-in-region (by "use" I mean "add elements to", since both tables need to be looked up when performing either undo). > =E2=80=A2 I'm concerned that undo-make-selective-list's O(N**2) algorit= hm is > unfriendly to those who want to increase the undo limit. The > generator allows for minimal processing of the history as needed. > Admittedly, I have not experimented with greater undo limits. The old code was no faster (neither constant-factor-wise nor algorithmically, IIUC), and it hasn't appeared as a problem so far, so maybe it's not worth worrying about it. I agree that doing it lazily sounds attractive, but let's keep it for later. I.e. the first patch should focus on fixing "undo-only of redo-in-region". > =E2=80=A2 I have to muck with the interfaces regardless -- > undo-make-selective-list still needs to deliver both the adjusted > element and the orig-tail to the higher undo code. Some change will be needed, indeed. Backward compatibility is important, but we should first come up with an "ideal" design, and only then can we try and figure out which parts need to be adjusted to better preserve compatibility. >> IIUC the crux of the matter is how to record the redo data for an >> undo-in-region. The way I see it, for such a "redo-in-region" group, >> we indeed need to remember which elements it undid (and which ones >> it skipped instead), but we could store that info as a single entry >> in an undo-redo/equiv-table. > I originally set out to do this, but making the weak references work > seemed overly tricky to me. The value stored in undo-redo-table would > need to be non weak with weak references to undo elements. I supposed > this would mean many one element weak hash tables. That seems dodgy. Hmm... that's a very good point. Worth mentioning in a comment. Stefan