From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: phillip.lord@russet.org.uk (Phillip Lord) Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] fix/no-undo-boundary-on-secondary-buffer-change f59d1be: Move undo amalgamation to lisp. Date: Mon, 05 Oct 2015 17:24:51 +0100 Message-ID: <87h9m52sh8.fsf@russet.org.uk> References: <20151005134118.10933.50859@vcs.savannah.gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1444062327 25326 80.91.229.3 (5 Oct 2015 16:25:27 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 5 Oct 2015 16:25:27 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Oct 05 18:25:27 2015 Return-path: Envelope-to: ged-emacs-devel@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 1Zj8Zq-0003ow-LP for ged-emacs-devel@m.gmane.org; Mon, 05 Oct 2015 18:25:26 +0200 Original-Received: from localhost ([::1]:46564 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zj8Zq-00018G-0E for ged-emacs-devel@m.gmane.org; Mon, 05 Oct 2015 12:25:26 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:41721) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zj8ZS-00015c-6Y for emacs-devel@gnu.org; Mon, 05 Oct 2015 12:25:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zj8ZO-0005HO-PR for emacs-devel@gnu.org; Mon, 05 Oct 2015 12:25:02 -0400 Original-Received: from cheviot22.ncl.ac.uk ([128.240.234.22]:49759) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zj8ZO-0005Dh-J6 for emacs-devel@gnu.org; Mon, 05 Oct 2015 12:24:58 -0400 Original-Received: from smtpauth-vm.ncl.ac.uk ([10.8.233.129] helo=smtpauth.ncl.ac.uk) by cheviot22.ncl.ac.uk with esmtp (Exim 4.63) (envelope-from ) id 1Zj8ZI-0001Uj-Ew; Mon, 05 Oct 2015 17:24:52 +0100 Original-Received: from jangai.ncl.ac.uk ([10.66.67.223] helo=localhost) by smtpauth.ncl.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1Zj8ZH-0001sP-Hl; Mon, 05 Oct 2015 17:24:51 +0100 In-Reply-To: (Stefan Monnier's message of "Mon, 5 Oct 2015 11:15:34 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 128.240.234.22 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:190939 Archived-At: Stefan Monnier writes: >> +(defun undo-auto-pre-command-hook() >> + (when (and (eq last-command 'self-insert-command) >> + (eq this-command 'self-insert-command)) > > I think this code should be called from self-insert-command rather than > from pre-command-hook. On a hook? Or do I just directly call a function defined in lisp form C? Does the same argument apply to the post-command-hook and after-change-functions also? I've been trying to use already existing functionality where possible (as it stands, I've only reduced code in the C layer -- the one thing I have added -- undo-size -- isn't used anymore, and I may back out before I've finished). > And it should also be called from delete-char. Yes, next on my list. Wanted to do one thing first and check that it didn't crash first. >> + ;; As last-command was s-i-c, there should be "insert" cons just >> + ;; before this. We need to check that there have not been too many insertions >> + (let ((last-before-nil >> + (cadr buffer-undo-list))) >> + (when >> + (> 20 >> + (- (cdr last-before-nil) >> + (car last-before-nil))) > > We don't actually know that (cdr last-before-nil) and (car > last-before-nil) are numbers. The previous self-insert-command might > have performed all kinds of buffer modifications (via abbrev-expansion, > post-self-insert-hook, ...). Hmmm. That's unfortunate -- I was trying to avoid "global" state and just user buffer state; the undo-list seemed like a sensible place to get this knowledge from. Other ideas? A buffer-local "undo-auto-self-insert-counter" perhaps? Or I could just use the logic above when it works and do nothing when it doesn't which would be safe, but means a post-self-insert-hook might break the blocks-of-20 amalgamation. Phil