From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Preview: portable dumper Date: Sun, 04 Dec 2016 17:53:43 +0200 Message-ID: <83h96jlmgo.fsf@gnu.org> References: <83bmwuogfb.fsf@gnu.org> <878trydrbo.fsf@red-bean.com> <83a8cem1eq.fsf@gnu.org> <83zikdl7oo.fsf@gnu.org> <83y3zxkwms.fsf@gnu.org> <20161203143603.GA6921@acm.fritz.box> <83wpfhkpy2.fsf@gnu.org> <20161204122033.GA2791@acm.fritz.box> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1480866826 9112 195.159.176.226 (4 Dec 2016 15:53:46 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 4 Dec 2016 15:53:46 +0000 (UTC) Cc: kfogel@red-bean.com, dancol@dancol.org, emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Dec 04 16:53:38 2016 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 1cDZ6g-00017X-CC for ged-emacs-devel@m.gmane.org; Sun, 04 Dec 2016 16:53:38 +0100 Original-Received: from localhost ([::1]:34856 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cDZ6i-00014R-6H for ged-emacs-devel@m.gmane.org; Sun, 04 Dec 2016 10:53:40 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:50555) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cDZ6a-00014B-NC for emacs-devel@gnu.org; Sun, 04 Dec 2016 10:53:34 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cDZ6W-0002MM-Qi for emacs-devel@gnu.org; Sun, 04 Dec 2016 10:53:32 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:59250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cDZ6W-0002MI-Mt; Sun, 04 Dec 2016 10:53:28 -0500 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:1760 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1cDZ6V-0004Jr-Lt; Sun, 04 Dec 2016 10:53:28 -0500 In-reply-to: <20161204122033.GA2791@acm.fritz.box> (message from Alan Mackenzie on Sun, 4 Dec 2016 12:20:33 +0000) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e 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:210024 Archived-At: > Date: Sun, 4 Dec 2016 12:20:33 +0000 > Cc: kfogel@red-bean.com, dancol@dancol.org, emacs-devel@gnu.org > From: Alan Mackenzie > > I can't really estimate it accurately (or it would take far too much > time to do so). It could be as high as 1 in 3. It could be as low as 1 > in 10. I hope it's the latter. > > > (i) Changing the method of syntax.c scanning backwards over comments. My > > > changes found their way into branch comment-cache in 2016-03. Despite > > > this change having been extensively discussed in emacs-devel, and sort of > > > "approved", the final patch was never considered on its merits. The > > > ostensible reason was that it used a cache which wasn't the syntax-ppss > > > cache. > > > I don't think I participated in that discussion or reviewed that > > patch, so I cannot say anything about that. > > I've found that thread, and I misremembered it. The preceding > discussion was not extensive. It was in the thread with Subject: > "bug#22884: 25.0.92; C/l mode editing takes waaaayy too long" and > essentially consisted of just three emails: > (i) Alan -> Eli: Thu, 3 Mar 2016 23:18:23 +0000 > (ii) Eli -> Alan: Fri, 04 Mar 2016 10:32:56 +0200 > (iii) Alan -> Eli: Fri, 4 Mar 2016 09:37:07 +0000 > . In (i), I proposed using text properties to cache the string/comment > state of positions, and asked you whether that might overwhelm the text > property mechanism. In (ii) you replied that it shouldn't. In (iii) I > announced my intention to hack it, which I then did. > > I don't remember you reviewing the patch, or expressing any further > views on it afterwards, so we agree on that. The discussion after I > submitted my patch happened in the threads Subject: "Re: [Emacs-diffs] > comment-cache 223d16f 2/3: Apply `comment-depth' text properties when > calling `back_comment'." and Subject: "Permission requested to merge > branch comment-cache into master.". This discussion was long and (to > me) unexpectedly aggressive, and didn't concern itself with the > correctness of the patch. I've re-read those threads and found my participation in them to be almost non-existent. In the first one it was limited to presenting benchmarks of the performance that was discussed. In the second one I barely said anything at all, and when I did, it was to encourage you to install your patch on the release branch rather than master. > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21869#23 > > > It indicates that we already had a patch for those problems, which was > > already tested quite a lot by that time. I think it's only natural to > > prefer a well tested and discussed patch to a new one trying to fix > > the same problem. Reading this message further down: > > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21869#37 > > > I see that you agreed that the problem was fixed by the patch we had, > > which allowed me to close the bug. > > I've read through that debbugs bug archive. It was a long time ago. I > remember feeling discouraged that my patch was not considered. I think > I felt that my patch fixed the fundamental problem in xdisp.c, whereas > the patch actually applied was more of a workaround. Or something like > that. But I didn't push the point at the time. Once again, a patch that was discussed at length and tested by several people was committed that fixed the problem. I can understand your frustration, but we really cannot install more than a single patch for each problem. > > > (iii) Earlier this year, we were having problems because some primitives > > > were not calling before-change-functions and after-change-functions the > > > way we might wish. My offer to analyse the code and amend it so that all > > > primitives would call b-c-f and a-c-f predictably was declined, the > > > proviso being (if I remember correctly) "unless somebody writes a solid > > > suite of unit tests". At the time of this rejection, I'd already > > > invested some time on the analysis. > > > That's not exactly my recollection. The analysis was not rejected, I > > simply already did it when you proposed that, so it wasn't necessary. > > OK. But the notion of acutally fixing the primitives so that they would > each call b-c-f and a-c-f was strongly discouraged, if I remember > correctly. Yes, because those touched very fundamental functionalities in manipulating buffer text, which I didn't want to destabilize without having a test suite with good coverage. So in sum, I feel that these incidents don't have a lot in common with the issues discussed here. > > > In short, I feel discouraged from working at the C level because of the > > > above. > > > Please don't be discouraged. There's no policy of preventing changes > > on the C level. However, for some changes which affect important > > functions, I think it's prudent to require a reasonable coverage by > > tests, so that we know we didn't break anything. > > Perhaps it is that last point (that C functions are more likely to be > critical to Emacs's working than lisp functions, and therefore the > testing is more important) that is the thing. I think treating code in C as more delicate is prudent: changes there will almost always have wider implications than changes in Lisp, and the code is frequently harder to understand, so better testing is IMO justified.