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 c7a6601 1/5: undo-size can count number of boundaries. Date: Thu, 17 Sep 2015 09:08:58 +0100 Message-ID: <8761395uxx.fsf@russet.org.uk> References: <20150915152129.671.61929@vcs.savannah.gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1442477384 14330 80.91.229.3 (17 Sep 2015 08:09:44 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 17 Sep 2015 08:09:44 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Sep 17 10:09:28 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 1ZcUFy-0006EM-70 for ged-emacs-devel@m.gmane.org; Thu, 17 Sep 2015 10:09:26 +0200 Original-Received: from localhost ([::1]:56768 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcUFx-0005Jz-NP for ged-emacs-devel@m.gmane.org; Thu, 17 Sep 2015 04:09:25 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:53284) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcUFk-0005JZ-Nd for emacs-devel@gnu.org; Thu, 17 Sep 2015 04:09:13 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZcUFh-000397-Gb for emacs-devel@gnu.org; Thu, 17 Sep 2015 04:09:12 -0400 Original-Received: from cheviot12.ncl.ac.uk ([128.240.234.12]:42221) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcUFh-00037U-A9 for emacs-devel@gnu.org; Thu, 17 Sep 2015 04:09:09 -0400 Original-Received: from smtpauth-vm.ncl.ac.uk ([10.8.233.129] helo=smtpauth.ncl.ac.uk) by cheviot12.ncl.ac.uk with esmtp (Exim 4.63) (envelope-from ) id 1ZcUFZ-00016T-BV; Thu, 17 Sep 2015 09:09:01 +0100 Original-Received: from cpc6-benw10-2-0-cust45.gate.cable.virginm.net ([92.238.179.46] helo=localhost) by smtpauth.ncl.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1ZcUFY-0007xN-D4; Thu, 17 Sep 2015 09:09:00 +0100 In-Reply-To: (Stefan Monnier's message of "Tue, 15 Sep 2015 12:57:37 -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.12 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:190042 Archived-At: Stefan Monnier writes: > Here are some nitpicks and suggestions on your code. > >> + if(n){ > > Please try to reproduce the code layout you see around. > >> + // we have a boundary, so check we do not have too many > > Please capitalize and punctuate your comments. > >> + if (XCAR (elt) == Qnil) > > Don't compare Lisp_Objects with == but with "EQ (x, y)" or in this case > with "NILP (elt)". While hacking on the C code I recommend you > > ./configure --enable-checking > (and/or (re)read the GNU Coding Standards which describe the coding > style to use). > > In the above: > - Add a space before the open paren. > - Please the open brace on its own line. --enable-check-lisp-object-type > > which will help you catch those problems and a few others. All those should be fixed now. > >> + if(NILP (Vundo_buffer_undoably_changed)){ >> + Fset (Qundo_buffer_undoably_changed,Qt); >> + safe_run_hooks (Qundo_first_undoable_change_hook); >> + } > > Why do you need Vundo_buffer_undoably_changed? > Doesn't (car-safe buffer-undo-list) give you the same information? No. The point is that this can be reset when ever I choose, and so it may well occur *not* after a boundary. Still your question tells me that my name is not very good. "first-recent-change" hook might be better, and "buffer-recently-changed", perhaps. You should be able to see how this all fits together now. Part of my purpose is that the meaning and semantics of "recent" are now in the hands of the lisp developer, since they can reset "buffer-unably-changed" as they want, which will result in a new call to the hook. I have chosen not to add a "undoable-change" hook, because I didn't need it. The first change hook calls rarely, so the performance issues with handling this change in lisp are, I think, unimportant. >> + doc: /* Normal hook run when a buffer has its first recent undo-able change. > > Rather than "recent undo-able change", I'd talk about the "first undo > record after a boundary". I would have, iff it did this. >> +This hook will be run with `current-buffer' as the buffer that >> +has changed. Recent means since the value of `undo-buffer-undoably-changed' > ^^^ > Please try and follow the sentence-end-double-space convention. Fixed now, hopefully. Phil