From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: An idea: combine-change-calls Date: Mon, 26 Mar 2018 20:17:28 +0000 Message-ID: <20180326201728.GA28620@ACM> References: <20180324135024.GA6319@ACM> <20180325191424.GE6292@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1522096532 14225 195.159.176.226 (26 Mar 2018 20:35:32 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Mon, 26 Mar 2018 20:35:32 +0000 (UTC) User-Agent: Mutt/1.7.2 (2016-11-26) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Mar 26 22:35:28 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1f0Ypw-0003UI-IH for ged-emacs-devel@m.gmane.org; Mon, 26 Mar 2018 22:35:24 +0200 Original-Received: from localhost ([::1]:59035 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f0Yry-0004B6-F8 for ged-emacs-devel@m.gmane.org; Mon, 26 Mar 2018 16:37:30 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55664) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f0YrK-0004Ap-Tl for emacs-devel@gnu.org; Mon, 26 Mar 2018 16:36:52 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f0YrH-00028s-Gy for emacs-devel@gnu.org; Mon, 26 Mar 2018 16:36:50 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:28521 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1f0YrH-000222-6S for emacs-devel@gnu.org; Mon, 26 Mar 2018 16:36:47 -0400 Original-Received: (qmail 74915 invoked by uid 3782); 26 Mar 2018 20:36:43 -0000 Original-Received: from acm.muc.de (p5B146D82.dip0.t-ipconnect.de [91.20.109.130]) by colin.muc.de (tmda-ofmipd) with ESMTP; Mon, 26 Mar 2018 22:36:42 +0200 Original-Received: (qmail 28814 invoked by uid 1000); 26 Mar 2018 20:17:28 -0000 Content-Disposition: inline In-Reply-To: X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 9.x [fuzzy] X-Received-From: 193.149.48.1 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:224074 Archived-At: Hello, Stefan. On Sun, Mar 25, 2018 at 16:05:40 -0400, Stefan Monnier wrote: > > I've actually got a working implementation going. It is this: > > (defmacro combine-change-calls (beg end &rest form) > > `(if (not inhibit-modification-hooks) > > (let* ((-beg- ,beg) (-end- ,end) > > (end-marker (copy-marker -end-))) > > (run-hook-with-args 'before-change-functions beg end) > > (let ((inhibit-modification-hooks t)) > > ,@form) > > (run-hook-with-args 'after-change-functions > > beg (marker-position end-marker) > > (- -end- -beg-))) > > ,@form)) > You need to evaluate `beg` and `end` even if inhibit-modification-hooks > is set, otherwise someone will get bitten. > I recommend you move the `form` to a lambda so you don't have to > duplicate it: > `(let ((body (lambda () ,@form)) > (-beg- ,beg) > (-end- ,end)) > ...) > Another benefit is that by moving `form` outside of the `let*`, you > won't need to use gensym/make-symbol nor obfuscated names. > I'd also recommend you check that `beg` hasn't changed position and that > the distance between end-marker and point-max remained the same. > >> Maybe combine-change-calls should also combine all those changes on the > >> undo-list into a big "delete+insert" (of course, it could also try and > >> keep the undo granularity but mark those undo entries so that they're > >> undone within their own combine-change-calls). > > :-) Either of those would be quite a project, but possibly worth doing. > Replacing the entries with a pair of delete+insert should be > pretty easy. Something like > (let ((old-buffer-undo-list buffer-undo-list) > (orig-text (buffer-substring beg end))) > ... > (setq buffer-undo-list > `((,(marker-position end-marker) ,beg) > (,orig-text . ,beg) > . ,old-buffer-undo-list))) > modulo sanity checks (i.e. don't do it if undo is disabled and don't do > it if old-buffer-undo-list is not within buffer-undo-list any more). I'm experimenting with a different strategy: surrounding the mass of elements in buffer-undo-list with a `(combine-change-begin ,beg ,end) and a `(combine-change-end ,beg ,end). This is less violent to the undo mechanism, for example, still permitting programs to analyse the undo list. primitive-undo, when it meets the latter of these, calls before-change-functions, binds inhibit-modification-hooks to t and calls itself recursively. This recursive invocation is terminated by the combine-change-begin, after-change-functions being called immediately on return. The two arms inserted into the pcase form in primitive-undo look like: (`(combine-change-end ,(and beg (pred integerp)) ,(and end (pred integerp))) (save-excursion (run-hook-with-args 'before-change-functions beg end)) (setq old-len (- end beg)) (let ((inhibit-modification-hooks t)) (setq list (primitive-undo 1 list)))) (`(combine-change-begin ,(and beg (pred integerp)) ,(and end (pred integerp))) (if old-len ;; Non-nested invocation of `primitive-undo'. (save-excursion (run-hook-with-args 'after-change-functions beg end old-len) (setq old-len nil)) ;; Nested invocation of `primitive-undo'. Push the element back ;; on the list, and push nil to terminate this invocation. (push next list) (push nil list))) (where `old-len' is an extra local variable bound to nil in the surrounding `let' form). The current version of combine-change-calls, incorporating (at least most of) your suggestions now looks like: (defmacro combine-change-calls (beg end &rest form) `(let* ((-beg- ,beg) (-end- ,end) (body (lambda () ,@form)) (end-marker (copy-marker -end-))) (if inhibit-modification-hooks (funcall body) (run-hook-with-args 'before-change-functions -beg- -end-) (unless (eq buffer-undo-list t) (push `(combine-change-begin ,-beg- ,-end-) buffer-undo-list)) (unwind-protect (let ((inhibit-modification-hooks t)) (funcall body)) (unless (eq buffer-undo-list t) (push `(combine-change-end ,-beg- ,(marker-position end-marker)) buffer-undo-list))) (run-hook-with-args 'after-change-functions beg (marker-position end-marker) (- -end- -beg-))))) This makes undo blindingly fast after a large comment-region operation. It doesn't always leave point in the right place (I understand why - it's the C function record_point failing to record point because the top element of buffer-undo-list is no longer nil; it's a combine-change-begin list). Do you have any more helpful suggestions for this idea? Basically, the combine-change-calls idea works. Given enough encouragement, I will get my disorganised changes into a proper patch with documentation, with a view to pushing it to master. > Stefan -- Alan Mackenzie (Nuremberg, Germany).