Suppress bogus "Mismatched parentheses" at end of comment

[mattiase]

Partially fixes #269.

[Chris00]

Thanks. I use (show-paren-mode t) which needs fixing as well. ;)

[mattiase]

Yes, I too use show-paren-mode. I'll investigate it on the Emacs side if I get some spare time.

[Chris00]

On 4 September 2021 at 02:30 -07, mattiase @.***> wrote:

Yes, I too use show-paren-mode. I'll investigate it on the Emacs side if I get some spare time.

I fixed it. 😀

[mattiase]

I fixed it.

So you did! From there, it's easy to make show-paren-mode show matching comment delimiters. Did you find that too gaudy, or would you like me to write a patch?

[monnier]

BTW, rather than hard code other package's internal functions, we should probably be using add-function as in the patch below. Any objection?

diff --git a/Makefile b/Makefile
index 3313665d1..7993c72f8 100644
--- a/Makefile
+++ b/Makefile
@@ -85,7 +85,7 @@ check:
 indent-test: indent-test.ml.test

 tuareg-site-file.el: $(SOURCES)
-   (echo ";;; $@ --- Automatically extracted autoloads.";\
+   (echo ";;; $@ --- Automatically extracted autoloads.  -*- lexical-binding: t; -*-";\
     echo ";;; Code:";\
     echo "(add-to-list 'load-path";\
     echo "             (or (file-name-directory load-file-name) (car load-path)))";\
diff --git a/tuareg.el b/tuareg.el
index d041c5b6d..a7217f0bc 100644
--- a/tuareg.el
+++ b/tuareg.el
@@ -3087,19 +3087,19 @@ expansion at run-time, if the run-time version of Emacs does know this macro."
                 (forward-comment 1)
                 (eq (point) pt))))))

-(defun tuareg--blink-matching-check (start end)
+(defun tuareg--blink-matching-check (orig-fun &rest args)
+  ;; FIXME: Should we merge this with `tuareg--show-paren'?
   (if (tuareg--point-after-comment-p)
       ;; Immediately after a comment-ending "*)" -- no mismatch error.
       nil
-    (smie-blink-matching-check start end)))
+    (apply orig-fun args)))

 (defvar show-paren-data-function); Silence the byte-compiler
-(declare-function show-paren--default "paren" ())

-(defun tuareg--show-paren ()
+(defun tuareg--show-paren (orig-fun)
   (if (or (tuareg--point-before-comment-p) (tuareg--point-after-comment-p))
       nil
-    (show-paren--default)))
+    (funcall orig-fun)))

 (defun tuareg--common-mode-setup ()
   (setq-local syntax-propertize-function #'tuareg-syntax-propertize)
@@ -3107,7 +3107,8 @@ expansion at run-time, if the run-time version of Emacs does know this macro."
   (smie-setup tuareg-smie-grammar #'tuareg-smie-rules
               :forward-token #'tuareg-smie-forward-token
               :backward-token #'tuareg-smie-backward-token)
-  (setq-local blink-matching-check-function #'tuareg--blink-matching-check)
+  (add-function :around (local 'blink-matching-check-function)
+                #'tuareg--blink-matching-check)
   (tuareg--eval-when-macrop add-function
     (when (boundp 'smie--hanging-eolp-function)
       ;; FIXME: As its name implies, smie--hanging-eolp-function
@@ -3118,7 +3119,8 @@ expansion at run-time, if the run-time version of Emacs does know this macro."
   (add-hook 'smie-indent-functions #'tuareg-smie--args nil t)
   (add-hook 'smie-indent-functions #'tuareg-smie--inside-string nil t)
   (setq-local add-log-current-defun-function #'tuareg-current-fun-name)
-  (setq-local show-paren-data-function #'tuareg--show-paren)
+  (add-function :around (local 'show-paren-data-function)
+                #'tuareg--show-paren)
   (setq prettify-symbols-alist
         (if tuareg-prettify-symbols-full
             (append tuareg-prettify-symbols-basic-alist
@@ -3202,7 +3204,7 @@ Short cuts for interactions with the REPL:
     (setq indent-tabs-mode nil)
     (setq ff-search-directories '(".")
           ff-other-file-alist tuareg-other-file-alist)
-    (add-hook 'ff-file-created-hook #'tuareg--ff-file-created-hook)
+    (add-hook 'ff-file-created-hook #'tuareg--ff-file-created-hook nil t)
     (tuareg--common-mode-setup)
     (tuareg--install-font-lock)
     (setq-local beginning-of-defun-function #'tuareg-beginning-of-defun)

[mattiase]

-  (setq-local blink-matching-check-function #'tuareg--blink-matching-check)
+  (add-function :around (local 'blink-matching-check-function)
+                #'tuareg--blink-matching-check)

Am I a bad programmer for thinking that it would be clearer to use something like

  (let ((orig-fn blink-matching-check-function))
    (setq-local blink-matching-check-function
                (lambda (start end)
                  (tuareg--blink-matching-check orig-fn start end))))

instead?

[monnier]

Am I a bad programmer for thinking that it would be clearer to use something like

  (let ((orig-fn blink-matching-check-function))
    (setq-local blink-matching-check-function
                (lambda (start end)
                  (tuareg--blink-matching-check orig-fn start end))))

instead?

I think it's a normal reaction if you're not yet familiar with add-function.

add-function does indeed basically the same, but with bells and whistles.

E.g. using the above for show-paren-function will burp if paren.el is not yet loaded, whereas add-function should handle it correctly (because it delays looking up the default value until the function is called).

Or if you also use a minor mode which also fiddles with blink-matching-check-function (whether buffer-locally or not), the adds and removes don't need to nest properly to get the correct result.

    Stefan

[mattiase]

Fair enough. I do understand what add-function does, but just wondered if it weren't overkill for this particular usage (as well as being somewhat opaque for the reader). The point about nesting is correct and interesting, but Tuareg isn't going to remove the wrapping so it's a bit moot. You are right about requiring paren.el to be loaded, of course.

[monnier]

Fair enough. I do understand what add-function does, but just wondered if it weren't overkill for this particular usage (as well as being somewhat opaque for the reader).

The more it's used, the less opaque it becomes ;-)

The point about nesting is correct and interesting, but Tuareg isn't going to remove the wrapping so it's a bit moot.

Using add-function allows me to stop worrying about when that might be a problem and when not.

E.g. if a buffer-local minor mode needs to use this hook but needs to operate between tuareg's function and the original function, add-function lets it do just that (by specifying the desired "depth"), but if tuareg doesn't use add-function that minor mode will either not be able to do what it wants, or will have to do dig into the details of how tuareg-mode sets up that function, breaking modularity.

Similarly if some global minor mode redefines the global definition of the blink paren function your manual code will fail to notice such a change, so it will sometimes keep calling the original definition because the minor mode was enabled later, or it will sometimes keep calling the redefined function even though the minor has been disabled.

Most of the benefits in the use of add-function compared to the kind of "manual" code you suggest are "special cases" that most authors fail to foresee (no surprise: they're special cases) and which are painful to handle correctly with manual code: the only way to handle it correctly is to use a "protocol" that the various parties obey.

add-function is this protocol. Use it and everyone benefits in those rare cases where they hit the corner case.

    Stefan

[mattiase]

Good, add-function it is then!

[Chris00]

@monnier I've applied your patch.

[monnier]

Christophe Troestler [2021-09-09 03:24:27] wrote:

@monnier I've applied your patch.

Thanks.