From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: wsnyder@wsnyder.org (Wilson Snyder) Newsgroups: gmane.emacs.devel Subject: Re: Get rid of verilog-no-change-functions Date: Sat, 12 Sep 2015 07:33:32 -0400 Message-ID: <7q8u8bj2ib.fsf@emma.svaha.wsnyder.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1442057638 13855 80.91.229.3 (12 Sep 2015 11:33:58 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 12 Sep 2015 11:33:58 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Sep 12 13:33:53 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 1Zaj45-0003f3-4W for ged-emacs-devel@m.gmane.org; Sat, 12 Sep 2015 13:33:53 +0200 Original-Received: from localhost ([::1]:60321 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zaj44-0006Uq-Cz for ged-emacs-devel@m.gmane.org; Sat, 12 Sep 2015 07:33:52 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zaj3q-0006Uk-HW for emacs-devel@gnu.org; Sat, 12 Sep 2015 07:33:39 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zaj3n-0003zg-9O for emacs-devel@gnu.org; Sat, 12 Sep 2015 07:33:38 -0400 Original-Received: from vpo3.wsnyder.org ([173.230.154.183]:52093 helo=wsnyder.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zaj3n-0003zV-33 for emacs-devel@gnu.org; Sat, 12 Sep 2015 07:33:35 -0400 X-ssh-sendmail: true User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 173.230.154.183 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:189859 Archived-At: Thanks for the proposed verilog-mode.el patch. 1. Minorly, the assertion code requires verilog-in-hooks remain in place, otherwise it fires inappropriately. 2. There is another block of similar code which presumably we would want to fix also. 3. This code was originally developed on Emacs 21 so it's odd that inhibit-modification-hooks wasn't sufficient. But the the verilog-mode.el code also works on ancient XEmacs. This is annoying for our code, but we still get bug reports occasionally so are reluctant to abandon it. At a minimum inhibit-modification-hooks needs to be defined to prevent XEmacs compile warnings. I'd propose the below which adds inhibit-modification-hooks, but retains the older code. If you feel that we should remove clearing before/after-change-functions we can do that, which will harm XEmacs and pre-Emacs 21 performance, but that does pass our regressions and I don't see XEmacs performance drop as a big concern. -Wilson diff --git a/verilog-mode.el b/verilog-mode.el index 57063b5..83246d6 100644 --- a/verilog-mode.el +++ b/verilog-mode.el @@ -230,6 +230,8 @@ STRING should be given if the last search was by `string-match' on STRING." `(customize ,var)) ) + (unless (boundp 'inhibit-modification-hooks) + (defvar inhibit-modification-hooks nil)) (unless (boundp 'inhibit-point-motion-hooks) (defvar inhibit-point-motion-hooks nil)) (unless (boundp 'deactivate-mark) @@ -3231,6 +3233,7 @@ user-visible changes to the buffer must not be within a (inhibit-read-only t) (inhibit-point-motion-hooks t) + (inhibit-modification-hooks t) ; Emacs 21+ (verilog-no-change-functions t) before-change-functions after-change-functions deactivate-mark @@ -3247,6 +3250,7 @@ user-visible changes to the buffer must not be within a For insignificant changes, see instead `verilog-save-buffer-state'." `(let* ((inhibit-point-motion-hooks t) + (inhibit-modification-hooks t) (verilog-no-change-functions t) before-change-functions after-change-functions) (progn ,@body))) Stefan Monnier writes: >I believe the patch below replaces a workaround with an actual fix. > >It's indeed unsafe to call syntax-ppss when before-change-functions is >let-bound, but the patch avoids the problem by not let-binding >before-change-functions, relying on the cleaner >inhibit-modification-hooks, which was introduced way back in Emacs-21 >for similar reasons. > >Any objection to my applying this to Emacs's version of verilog-mode.el? > > > Stefan > > >diff --git a/lisp/progmodes/verilog-mode.el b/lisp/progmodes/verilog-mode.el >index 5fcdba6..0f90d60 100644 >--- a/lisp/progmodes/verilog-mode.el >+++ b/lisp/progmodes/verilog-mode.el >@@ -408,10 +408,6 @@ This function may be removed when Emacs 21 is no longer supported." > ;; And GNU Emacs 22 has obsoleted last-command-char > last-command-event))) > >-(defvar verilog-no-change-functions nil >- "True if `after-change-functions' is disabled. >-Use of `syntax-ppss' may break, as ppss's cache may get corrupted.") >- > (defvar verilog-in-hooks nil > "True when within a `verilog-run-hooks' block.") > >@@ -422,14 +418,13 @@ Set `verilog-in-hooks' during this time, to assist AUTO caches." > (run-hooks ,@hooks))) > > (defun verilog-syntax-ppss (&optional pos) >- (when verilog-no-change-functions >- (if verilog-in-hooks >- (verilog-scan-cache-flush) >- ;; else don't let the AUTO code itself get away with flushing the cache, >- ;; as that'll make things very slow >- (backtrace) >- (error "%s: Internal problem; use of syntax-ppss when cache may be corrupt" >- (verilog-point-text)))) >+ (if verilog-in-hooks >+ (verilog-scan-cache-flush) >+ ;; else don't let the AUTO code itself get away with flushing the cache, >+ ;; as that'll make things very slow >+ (backtrace) >+ (error "%s: Internal problem; use of syntax-ppss when cache may be corrupt" >+ (verilog-point-text))) > (if (fboundp 'syntax-ppss) > (syntax-ppss pos) > (parse-partial-sexp (point-min) (or pos (point))))) >@@ -3230,9 +3225,7 @@ user-visible changes to the buffer must not be within a > (buffer-undo-list t) > (inhibit-read-only t) > (inhibit-point-motion-hooks t) >- (verilog-no-change-functions t) >- before-change-functions >- after-change-functions >+ (inhibit-modification-hooks t) > deactivate-mark > buffer-file-name ; Prevent primitives checking > buffer-file-truename) ; for file modification >@@ -3246,9 +3239,7 @@ user-visible changes to the buffer must not be within a > "Execute BODY forms, disabling all change hooks in BODY. > For insignificant changes, see instead `verilog-save-buffer-state'." > `(let* ((inhibit-point-motion-hooks t) >- (verilog-no-change-functions t) >- before-change-functions >- after-change-functions) >+ (inhibit-modification-hooks t)) > (progn ,@body))) > > (defvar verilog-save-font-mod-hooked nil