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: Sun, 1 Apr 2018 14:24:10 +0000 Message-ID: <20180401142410.GA9027@ACM> References: <20180328204254.GC6592@ACM> <20180329151033.GA5213@ACM> <20180329171101.GB5213@ACM> <20180330114636.GA5432@ACM> <20180331210052.GB5003@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1522592559 20783 195.159.176.226 (1 Apr 2018 14:22:39 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 1 Apr 2018 14:22:39 +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 Sun Apr 01 16:22:35 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 1f2dsQ-0005JQ-FD for ged-emacs-devel@m.gmane.org; Sun, 01 Apr 2018 16:22:34 +0200 Original-Received: from localhost ([::1]:38237 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f2duU-00020L-8A for ged-emacs-devel@m.gmane.org; Sun, 01 Apr 2018 10:24:42 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60620) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f2duJ-0001zL-IU for emacs-devel@gnu.org; Sun, 01 Apr 2018 10:24:32 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f2duG-0001yM-06 for emacs-devel@gnu.org; Sun, 01 Apr 2018 10:24:31 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:17752 helo=mail.muc.de) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1f2duF-0001wl-KR for emacs-devel@gnu.org; Sun, 01 Apr 2018 10:24:27 -0400 Original-Received: (qmail 53672 invoked by uid 3782); 1 Apr 2018 14:24:26 -0000 Original-Received: from acm.muc.de (p5B1465AC.dip0.t-ipconnect.de [91.20.101.172]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 01 Apr 2018 16:24:25 +0200 Original-Received: (qmail 9103 invoked by uid 1000); 1 Apr 2018 14:24:10 -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:224218 Archived-At: Hello, Stefan. On Sat, Mar 31, 2018 at 19:38:03 -0400, Stefan Monnier wrote: > > > (defun wrap-and-run-primitive-undo (beg end list) > > > (combine-change-calls > > That's a refinement I haven't managed to get around to, yet. > Any difficulty there? > Naively, I'd expect that it's just a matter of: > (defun undo--wrap-and-run-primitive-undo (beg end list) > (combine-change-calls > (while list > (setq list (primitive-undo 1 list))))) > [ and of course, making sure this function is defined after the > combine-change-calls macro. ] It was exactly that. No difficulties. [ .... ] > > So, I think this exercise is worthwhile. I have added some doc strings > > to the functions. I have introduced a variable to hinder a "recursive" > > undo--wrap-and-run-primitive-undo appearing in the undo list. This > > happened with uncomment-region calling comment-region. > That's weird: shouldn't the inhibit-modification-hooks already play this role? I don't think so. i-m-h is purely to do with whether the hooks should be run now. That has nothing to do with how items should be added to the undo list. > Oh, wait, I see you don't test inhibit-modification-hooks any more but > you test undo--combining-change-calls instead. Any particular reason > for this change? inhibit-modification-hooks is tested around the run-hooks-with-args calls. If i-m-h is set to non-nil by some function, and comment-region is run, we still want the undo records to be enclosed by an (apply ...) form. > > Sometimes, point ends up at the wrong place after an undo (probably > > because of primitive-undo removing POSITION elements from undo-lists, as > > we've already discussed.) > The code that removes those "position" elements is in `undo`, not in > `primitive-undo`, so what you're describing must come from something > else (unless you're seeing this when *redoing* or *reundoing*). I'm not sure. I think it's in re-undoing. As you've already guessed, I would be in favour of removing that code from `undo' that we don't see what it's for. Anyhow, here's another version of the code (below), with the following changes: 1/- undo--w-a-r-p-undo is now a macro call of c-c-c. 2/- end-marker is now of type t, so that it is relocated when text is inserted at the postion it points to. (DELTA was coming out wrong.) 3/- end-marker is now set to point nowhere at the end of the function. 4/- A whole lot of `prog1's have been replaced by the variable RESULT. 5/- c-c-c-1 no longer incorporates a timestamp undo record ((t . ...)) into the (apply ...) form. This just seemed wrong. I think it's almost time to commit this to master. I propose that combine-change-calls{-1,} go into subr.el, beside combine-after-change-calls, and undo--wrap-and-run-primitive-undo go into simple.el, beside the rest of the undo stuff. I will introduce the variable comment-combine-change-calls in newcomment.el, and modify {,un}comment-region to test it, and if set, to use c-c-c. I will document c-c-c in the Elisp manual on page "Change Hooks". For (almost) the last time, here's the current version of the code: (defvar undo--combining-change-calls nil "Non-nil when `combine-change-calls' is running.") (defun combine-change-calls-1 (beg end body) "Evaluate BODY, running the change hooks just once, for region \(BEG END). Firstly, `before-change-functions' is invoked for the region \(BEG END), then BODY (a function) is evaluated with `inhibit-modification-hooks' non-nil, then finally `after-change-functions' is invoked on the updated region (BEG NEW-END) with a calculated OLD-LEN argument. If `inhibit-modification-hooks' is initially non-nil, the change hooks are not run. The result of `comebine-change-calls-1' is the value returned by BODY. BODY must not make a different buffer current, except temporarily. It must not make any changes to the buffer outside the specified region. It must not change `before-change-functions' or `after-change-functions'. Additionally, the buffer modifications of BODY are recorded on the buffer's undo list as a single \(apply ...) entry containing the function `undo--wrap-and-run-primitive-undo'." (let ((old-bul buffer-undo-list) (end-marker (copy-marker end t)) result) (if undo--combining-change-calls (setq result (funcall body)) (let ((undo--combining-change-calls t)) (if (not inhibit-modification-hooks) (run-hook-with-args 'before-change-functions beg end)) (if (eq buffer-undo-list t) (setq result (funcall body)) (let ((inhibit-modification-hooks t)) (setq result (funcall body))) (let ((ap-elt (list 'apply (- end end-marker) beg (marker-position end-marker) #'undo--wrap-and-run-primitive-undo beg (marker-position end-marker) buffer-undo-list)) (ptr buffer-undo-list)) (if (not (eq buffer-undo-list old-bul)) (progn (while (and (not (eq (cdr ptr) old-bul)) ;; In case garbage collection has removed OLD-BUL. (cdr ptr) ;; Don't include a timestamp entry. (not (and (consp (cdr ptr)) (consp (cadr ptr)) (eq (caadr ptr) t) (setq old-bul (cdr ptr))))) (setq ptr (cdr ptr))) (unless (cdr ptr) (message "combine-change-calls: buffer-undo-list broken")) (setcdr ptr nil) (push ap-elt buffer-undo-list) (setcdr buffer-undo-list old-bul))))) (if (not inhibit-modification-hooks) (run-hook-with-args 'after-change-functions beg (marker-position end-marker) (- end beg))))) (set-marker end-marker nil) result)) (defmacro combine-change-calls (beg end &rest forms) "Evaluate FORMS, running the change hooks just once. Firstly, `before-change-functions' is invoked for the region \(BEG END), then the FORMS are evaluated with `inhibit-modification-hooks' non-nil, and finally `after-change-functions' is invoked on the updated region. The change hooks are not run if `inhibit-modification-hooks' is non-nil. The result of `combine-change-calls' is the value returned by the last of the FORMS to be evaluated. FORMS may not make a different buffer current, except temporarily. FORMS may not change the buffer outside the specified region. It must not change `before-change-functions' or `after-change-functions'. Additionally, the buffer modifications of BODY are recorded on the buffer's undo list as a single \(apply ...) entry containing the function `undo--wrap-and-run-primitive-undo'. " `(combine-change-calls-1 ,beg ,end (lambda () ,@forms))) (defun undo--wrap-and-run-primitive-undo (beg end list) "Call `primitive-undo' on the undo elements in LIST. This function is intended to be called purely by `undo' as the function in an \(apply DELTA BEG END FUNNAME . ARGS) undo element. It invokes `before-change-functions' and `after-change-functions' once each for the entire region \(BEG END) rather than once for each individual change. Additionally the fresh \"redo\" elements which are generated on `buffer-undo-list' will themselves be \"enclosed\" in `undo--wrap-and-run-primitive-undo'. Undo elements of this form are generated by the macro `combine-change-calls'." (combine-change-calls beg end (while list (setq list (primitive-undo 1 list))))) > Stefan -- Alan Mackenzie (Nuremberg, Germany).