Re: Get rid of verilog-no-change-functions

[Stefan Monnier]

> You're right of course, we use defvar everywhere else; not
> sure why the older code there did that.  Fixed all three.

Good, thanks.

>> But AFAIK as long you don't switch to "lexical-binding:t", no
>> byte-compiler will complain if you let-bind an unknown variable without
>> using it, so even that (defvar inhibit-modification-hooks) is
>> not indispensable.
> The default byte-compile warns.

Ha!  Indeed, the XEmacs byte compiler does warn about this!
Can't believe it took me so many years to find out.

>> As mentioned, here I'd have to understand why we need to prevent
>> *-change-functions from running.  After all, this is used in places
>> where we make very significant changes to the buffer and where font-lock
>> (for instance) would want to know about it.

> The child functions perform hundreds of thousands of minor
> changes, then call the hooks on everything.  Doing it this
> way makes an update maybe 10 seconds versus (as it once did)
> many minutes - which was almost unusable. At the end of
> everything fontification is correct.

Combining all the *-change-functions calls makes a lot of sense (that's
why we have combine-after-change-calls, for example).  But I don't see
where you "then call the hooks on everything".

I see the verilog-save-font-mods which re-font-locks the buffer
afterwards, but since it disables font-lock at the beginning it means
that verilog-save-no-change-functions is not needed to prevent
font-lock hooks.

And of course, disabling/enabling font-lock-mode doesn't take care of
other users of *-change-functions.

Also, I see that verilog-save-no-change-functions is wrapped inside
verilog-save-font-mods in verilog-auto, but not in verilog-delete-auto.

> -    (unless (boundp 'inhibit-point-motion-hooks)
> -      (defvar inhibit-point-motion-hooks nil))
> -    (unless (boundp 'deactivate-mark)
> -      (defvar deactivate-mark nil))
> +    (defvar inhibit-modification-hooks)
> +    (defvar inhibit-point-motion-hooks)
> +    (defvar deactivate-mark)

Those defvars should not be with an eval-when-compile (they happen to
also work if they are, currently, but that's a mere accident).

How 'bout the patch below?


        Stefan


diff --git a/lisp/progmodes/verilog-mode.el b/lisp/progmodes/verilog-mode.el
index f83c676..f2505e3 100644
--- a/lisp/progmodes/verilog-mode.el
+++ b/lisp/progmodes/verilog-mode.el
@@ -229,11 +229,6 @@ STRING should be given if the last search was by 
`string-match' on STRING."
       (defmacro customize-group (var &rest _args)
         `(customize ,var))
       )
-
-    (unless (boundp 'inhibit-point-motion-hooks)
-      (defvar inhibit-point-motion-hooks nil))
-    (unless (boundp 'deactivate-mark)
-      (defvar deactivate-mark nil))
     )
   ;;
   ;; OK, do this stuff if we are NOT XEmacs:
@@ -243,6 +238,10 @@ STRING should be given if the last search was by 
`string-match' on STRING."
        `(and transient-mark-mode mark-active))))
   )
 
+(defvar inhibit-point-motion-hooks)
+(defvar inhibit-modification-hooks)
+(defvar deactivate-mark)
+
 ;; Provide a regular expression optimization routine, using regexp-opt
 ;; if provided by the user's elisp libraries
 (eval-and-compile
@@ -3231,8 +3230,7 @@ user-visible changes to the buffer must not be within a
          (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
@@ -3240,16 +3238,23 @@ user-visible changes to the buffer must not be within a
         (progn ,@body)
        (and (not modified)
            (buffer-modified-p)
-           (set-buffer-modified-p nil)))))
+            (if (fboundp 'restore-buffer-modified-p)
+                (restore-buffer-modified-p nil)
+              (set-buffer-modified-p nil))))))
 
 (defmacro verilog-save-no-change-functions (&rest body)
   "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)
-     (progn ,@body)))
+  ;; For use when we're about to make many changes over the whole buffer,
+  ;; so we avoid running the *-change-functions too many times.
+  (let ((lensym (make-symbol "length")))
+    `(let* ((inhibit-point-motion-hooks t)
+            (verilog-no-change-functions t)
+            (inhibit-modification-hooks t)
+            (,lensym (- (point-max) (point-min))))
+       (run-hook-with-args 'before-change-functions (point-min) (point-max))
+       ,@body
+       (run-hook-with-args 'after-change-functions (point-min) (point-max) 
,lensym))))
 
 (defvar verilog-save-font-mod-hooked nil
   "Local variable when inside a `verilog-save-font-mods' block.")
@@ -9956,9 +9961,9 @@ Cache the output of function so next call may have faster 
access."
            (t
             ;; Read from file
             ;; Clear then restore any highlighting to make emacs19 happy
-            (let (func-returns)
-              (verilog-save-font-mods
-               (setq func-returns (funcall function)))
+            (let ((func-returns
+                    (verilog-save-font-mods ;FIXME: Is it still needed?
+                     (funcall function))))
               ;; Cache for next time
               (setq verilog-modi-cache-list
                     (cons (list (list modi function)
@@ -13438,7 +13443,7 @@ Enable with `verilog-auto-template-warn-unused'."
 ;;; Auto top level:
 ;;
 
-(defun verilog-auto (&optional inject)  ; Use verilog-inject-auto instead of 
passing an arg
+(defun verilog-auto (&optional inject) ; Use verilog-inject-auto instead of 
passing an arg
   "Expand AUTO statements.
 Look for any /*AUTO...*/ commands in the code, as used in
 instantiations or argument headers.  Update the list of signals
@@ -13514,7 +13514,6 @@
   (unless noninteractive (message "Updating AUTOs..."))
   (if (fboundp 'dinotrace-unannotate-all)
       (dinotrace-unannotate-all))
-  (verilog-save-font-mods
    (let ((oldbuf (if (not (buffer-modified-p))
                     (buffer-string)))
         (case-fold-search verilog-case-fold)
@@ -13524,7 +13523,6 @@
         (verilog-modi-cache-current-enable t)
         (verilog-modi-cache-current-max (point-min)) ; IE it's invalid
         verilog-modi-cache-current)
-     (unwind-protect
         ;; Disable change hooks for speed
         ;; This let can't be part of above let; must restore
         ;; after-change-functions before font-lock resumes
@@ -13625,9 +13623,7 @@
                   (t (unless noninteractive (message "Updating 
AUTOs...done"))))
             ;; End of after-change protection
             )))
-       ;; Unwind forms
-       ;; Currently handled in verilog-save-font-mods
-       ))))
+    ))
 
 ;;; Skeletons:
 ;;