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: Unbalanced change hooks (part 2) Date: Sun, 31 Jul 2016 18:03:19 +0300 Message-ID: <83a8gxq288.fsf@gnu.org> References: <20160731121642.GB2205@acm.fritz.box> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit X-Trace: blaine.gmane.org 1469977468 24882 80.91.229.8 (31 Jul 2016 15:04:28 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 31 Jul 2016 15:04:28 +0000 (UTC) Cc: ofv@wanadoo.es, rcopley@gmail.com, emacs-devel@gnu.org To: Alan Mackenzie Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Jul 31 17:04:15 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 1bTsHl-0006PI-JN for ged-emacs-devel@m.gmane.org; Sun, 31 Jul 2016 17:04:13 +0200 Original-Received: from localhost ([::1]:39861 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bTsHh-00008d-Ra for ged-emacs-devel@m.gmane.org; Sun, 31 Jul 2016 11:04:09 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34018) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bTsHA-00008Y-VL for emacs-devel@gnu.org; Sun, 31 Jul 2016 11:03:38 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bTsH6-00068Q-2m for emacs-devel@gnu.org; Sun, 31 Jul 2016 11:03:36 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:58597) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bTsGv-00064w-Oh; Sun, 31 Jul 2016 11:03:21 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:3451 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bTsGu-000225-Re; Sun, 31 Jul 2016 11:03:21 -0400 In-reply-to: <20160731121642.GB2205@acm.fritz.box> (message from Alan Mackenzie on Sun, 31 Jul 2016 12:16:42 +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:206268 Archived-At: > Date: Sun, 31 Jul 2016 12:16:42 +0000 > From: Alan Mackenzie > Cc: Óscar Fuentes , > Richard Copley > > I propose that the call to signal_before_change should be removed from > prepare_to_modify_buffer, and that it should be called explicitly from > the places it is needed. This would allow bug #240[79]4 to be fixed. > > Comments? Some. First, I don't agree with your conclusion that calls to before-change-functions and after-change-functions must always be balanced. For starters, they aren't, and never have been. Neither is it written anywhere that they should, and your interpretation of the ELisp manual seems to take the desired world for the real one. My interpretation of these two variables is that they are two hooks provided for different kinds of features, because hooking into both should not generally be required. It should suffice for an application that wants to react to a change in a buffer to hook into after-change-functions only. But even if I agree, for the sake of argument, with your interpretation, let's put this issue into a proper perspective, okay? We have a single major mode (albeit a very important one, one that I use every day) that under very specific conditions apparently fails to discard its caches, because its design assumed calls to these two hooks will always be balanced. As a solution, you are proposing a change in a very low-level and delicate part of Emacs, which means significant changes to code that was basically unchanged for more than 20, if not 30 years. This code is involved in almost every operation on buffer text, and so changes you propose will necessarily cause ripples all over Emacs. Just look what the code you propose to change does: . call barf-if-buffer-read-only . set the buffer's redisplay flag . affect the undo recording . save the region when select-active-regions is non-nil . sets deactivate-mark non-nil Can you imagine how much Lisp code and basic features will be affected by calling this where we never did before? All that for the benefit of a single major mode that fails in a single, albeit important use case? I'm sorry, but that makes very little sense to me. So I suggest to take one or two steps back, and try to find a safer solution for the problem. Doing so will also greatly enhance the chances of Emacs users to see the solution in the next Emacs release (whereas doing what you suggest will push it back to Emacs 26 or maybe even later). Questions: 1) Why does CC Mode need both hooks? What does it do in each one of them, in the specific use case reported in the named bugs? And why isn't it enough to make only the change you proposed in part 1 of your report? 2) Can this problem be fixed in CC Mode itself, without touching either insert-file-contents or insdel.c functions? 3) If the answer to 2) above is a categorical NO (but please provide enough arguments so we could be very sure it is that), then I _might_ be convinced to consider changes to insert-file-contents only, and only for the case when both VISIT and REPLACE are non-nil. That's because I think this combination is only used by revert-buffer and similar functionality (if someone knows about other callers that do this, please holler), so the ripples will be not as wide as if we mess with insdel.c. (Still, even this possibility scares the living s**t out of me; it should do that to you as well.) To this end, please tell which calls to del_range_byte in insert-file-contents are involved with this use case and cause trouble to CC Mode, and maybe we could change only those, in addition to the 'if' clause that guards the call to before-change-functions.