clojure-mode icon indicating copy to clipboard operation
clojure-mode copied to clipboard

#_ comments are not marked as comments (e.g. via `syntax-table` text property)

Open dottedmag opened this issue 8 years ago • 11 comments

Expected behavior

#_ comments should mark the following expr as a comment (e.g. via syntax-table text property).

Actual behavior

It does not happen.

Steps to reproduce the problem

Enable rainbow-delimiters. See that the rainbow delimiters are not disabled in the form following #_:

8ae998ca-f94b-11e6-8dad-373385272955

See the corresponding (invalid) issue in rainbow-delimiters: https://github.com/Fanael/rainbow-delimiters/issues/29

Environment & Version information

clojure-mode version information

clojure-mode (version 5.7.0-snapshot)

Emacs version

GNU Emacs 25.1.90.1 (x86_64-apple-darwin15.6.0, NS appkit-1404.47 Version 10.11.6 (Build 15G1217))
 of 2017-02-06

Operating system

OS X 10.11

dottedmag avatar Feb 23 '17 21:02 dottedmag

Maybe this regex needs tuning?

arichiardi avatar Jun 14 '17 03:06 arichiardi

Actually, I think the regex is fine. My guess would be that the code that uses that regex is just not setting the syntax-table text property. Should be easy to fix.

Malabarba avatar Jun 19 '17 14:06 Malabarba

Is this diagnosis still correct? That is, does it still appear to be an easy fix? Any further hints or pointers on how this might be fixed?

sogaiu avatar Apr 04 '19 12:04 sogaiu

Yes, yes, and see the comment from @Malabarba.

dottedmag avatar Apr 04 '19 18:04 dottedmag

I saw the comment but didn't succeed in getting enough from it to investigate in a meaningful way :)

It wasn't clear here what "the regex" refers to as, IIUC, the original link no longer points to a regex, and consequently I'm not succeeding in interpreting what "the code that uses that regex" points to.

Any hints / clarifications on these things?

sogaiu avatar Apr 04 '19 23:04 sogaiu

I checked what might have been or was close to the master branch around June of 2017. Assuming what turned up was not off by much, the regex in question may be:

(defvar clojure--comment-macro-regexp
  (rx "#_" (* " ") (group-n 1 (not (any " "))))
  "Regexp matching the start of a comment sexp.
The beginning of match-group 1 should be before the sexp to be
marked as a comment.  The end of sexp is found with
`clojure-forward-logical-sexp'.

and the code that uses it (indirectly via clojure-comment-regexp) may be the function clojure--search-comment-macro-internal (and may be its friend clojure--search-comment-macro).

Perhaps an appropriate call or calls to put-text-property would be appropriate? Any help / hints on the precise argument values and where such a call or calls ought to go?

sogaiu avatar Apr 05 '19 09:04 sogaiu

Does putting something like:

        (put-text-property start (1+ start)
                           'syntax-table (cons 11 nil))
        (put-text-property (point) (1+ (point))
                           'syntax-table (cons 12 nil))

right before the t at the end of clojure--search-comment-macro-internal function seem close?

Will (setq parse-sexp-lookup-properties t) be necessary too?

Also, do those text properties need to be removed if the leading #_ is removed / disabled?

~~On a side note, to use edebug to investigate, it seems doing (setq font-lock-support-mode nil) first seems to help a lot...~~

sogaiu avatar Apr 05 '19 11:04 sogaiu

Sorry I can't be of more help here, but it's been a while since I've dealt with syntax tables and my memory is not helping.

What I can say is that it is possible to specify certain text properties that should be cleared when font-lock runs on a region. So, in theory, you don't have to manually clear the value if the #_ is removed. Look into font-lock-extra-managed-props.

On a side note, to use edebug to investigate, it seems doing (setq font-lock-support-mode nil) first seems to help a lot...

Oh my. I've had to edebug font-locking functions before and I can say I do not envy you. ;-)

Malabarba avatar Apr 05 '19 15:04 Malabarba

Thanks for the hints, font-lock-extra-managed-props sounds promising.

As to debugging font-lock-related functionality, it would be much worse if I had not found font-lock-studio ;)

sogaiu avatar Apr 05 '19 20:04 sogaiu

The following appears to help here:

diff --git a/clojure-mode.el b/clojure-mode.el
index 5d06272..150b6b0 100644
--- a/clojure-mode.el
+++ b/clojure-mode.el
@@ -532,6 +532,7 @@ replacement for `cljr-expand-let`."
   (setq-local lisp-doc-string-elt-property 'clojure-doc-string-elt)
   (setq-local clojure-expected-ns-function #'clojure-expected-ns)
   (setq-local parse-sexp-ignore-comments t)
+  (setq-local parse-sexp-lookup-properties t)
   (setq-local prettify-symbols-alist clojure--prettify-symbols-alist)
   (setq-local open-paren-in-column-0-is-defun-start nil)
   (setq-local beginning-of-defun-function #'clojure-beginning-of-defun-function))
@@ -688,6 +689,12 @@ what is considered a comment (affecting font locking).
         ;; Data for (match-end 1).
         (setf (elt md 3) (point))
         (set-match-data md)
+        (put-text-property start (1+ start)
+                           'syntax-table (cons 11 nil))
+        (put-text-property (1+ start) (point)
+                           'syntax-table (cons 14 nil))
+        (put-text-property (point) (1+ (point))
+                           'syntax-table (cons 12 nil))
         t))))
 
 (defun clojure--search-comment-macro (limit)
@@ -971,6 +978,8 @@ highlighted region)."
 (defun clojure-font-lock-setup ()
   "Configures font-lock for editing Clojure code."
   (setq-local font-lock-multiline t)
+  (setq-local font-lock-extra-managed-props
+              (cons 'syntax-table font-lock-extra-managed-props))
   (add-to-list 'font-lock-extend-region-functions
                #'clojure-font-lock-extend-region-def t)
   (setq font-lock-defaults

@dottedmag @arichiardi @Malabarba any time / interest in giving it a review / try?

sogaiu avatar Apr 06 '19 00:04 sogaiu

Can confirm that the patch above works:

screenshots with rainbow delimiters

before patch: image

with patch: image

Although it introduces another bug, which results in wrong matching of ignored parentheses with show-smartparens-mode:

image image

On the other hand, this doesn't happen with show-paren-mode, so maybe this is a bug in smartparens

andreyorst avatar Dec 23 '20 19:12 andreyorst