clojure-mode
clojure-mode copied to clipboard
Def highlighting is out of control
Expected behavior
Proper syntax highlighting
Actual behavior
Steps to reproduce the problem
Write a form containing a word starting with def
anywhere, e.g. (defface 'angry-face)
I get that the intent was to highlight defresource
etc the same way as defn
but this really isn't working as intended. All calls to functions like (default-user-settings)
are highlighted like a def now, no matter where in the source they appear.
I was pretty sure we had fixed this, at least for the word default. ...
It's not possible to fix this without enumerating proper defforms
or requiring defforms authors to tag their macros with some kind of metadata like ^{:defform true}
.
Proper defs can occur inside let forms, so highlighting top forms only is not a real fix.
One way to go about it is to highlight only defxyz
if it doesn't contain -
so that things like default-breakage
wont break.
Maybe we should remove this from clojure-mode and add the smarter behavior to CIDER, where we actually have the ability to understand the code?
I could swear we had fixed this ages ago, something related to not highlighting the symbol default
. I even remember some brief discussion of English words that start with "def".
I'm coming across this requiring a lib as [manifold.deferred :as deferred]
. default
is also still highlighted as a defform
. If whitelisting correct defforms
is a PITA, can we have some way to blacklist symbols?
I've been bitten by this too. The problem is the following line inside (defconst clojure-font-lock-keywords ...)
:
"\\(def[^ \r\n\t]*\\)"
See https://github.com/clojure-emacs/clojure-mode/blob/master/clojure-mode.el#L777 for context.
I've changed that to:
"\\("
(regexp-opt '("defn" "defmacro" "defmulti" "defmethod"))
"[^ \r\n\t]*\\)"
-- so only the calls we know for sure about are highlighted. I think it's much better to under-highlight than to over-highlight; the editor being "too smart" is simply annoying.
We should also probably have the stuff as we have for indentation, i.e.
-
define-clojure-font-lock
, analogous todefine-clojure-indent
- support for font-lock metadata in cider
While that's not done, I propose to go for the fixed set of highlighted def*s. Let me know if you want a pull request for that.
Also, we can add a defcustom for the list of symbols to highlight, so it'll actually be trivial to add other symbols to the list.
Sounds like a reasonable approach. I like it. It's clear that we can't have a one-regexp fits all solution here.
support for font-lock metadata in cider
Actually CIDER will highlight things differently even now based on their type - macros are font-locked as keywords, built-in as built-ins, function names as functions and so on.
Sounds like a reasonable approach.
Good! So do you want a pull request (defcustom list of symbols + the change of regex) or would you rather hack on it yourself?
Actually CIDER will highlight things differently even now based on their type
Right - but what I had in mind is some kind of :style/font-lock
metadata, so a library author could specify exactly how he wants his macro\fn to be highlighted.
Good! So do you want a pull request (defcustom list of symbols + the change of regex) or would you rather hack on it yourself?
PR would be great. The work on nREPL is eating all my spare time these days.
Right - but what I had in mind is some kind of :style/font-lock metadata, so a library author could specify exactly how he wants his macro\fn to be highlighted.
Yeah, I guess that makes sense if you want to override the defaults. It should also be relatively simple to do.
Before changing this at clojure-mode side, could we have :style/defform metadata implemented first? Before breaking long term behavior we should have a workaround. I often use defxyz
macros and occasional false positives don't really bother me that much.
Some suggestion from above are milder solutions than an explicit list of keywords:
- one can blacklist a list of common English words
- limit defxyz to a single word symbols (no - allowed)
- check if defxyz is a macro
Let's also consider the exclusion metadata :style/not-a-defform
. We could keep the current regex but maintain a list of known not-a-defform from popular libraries. I am pretty sure the number of false-positives right now are way fewer than the potential false-negatives if we restrict the regexp to a fixed list of basic keywords.
I love the :style/defform
suggestion!
I think we should be conservative because you can always opt-in your own code (though not library code) but you can't opt-out with a :style/not-a-defform
on library code that gives a false positive.
I am not sure I understand. You can modify other library var's metadata from your code. Or you could PR to that library and popularize :style/not-a-defform, or you could fix it on emacs side by adding keywords to the exclusion list. There are some possibilities.
In any case, for the completeness sake both :style/defform
and :style/not-a-defform
could be supported.
I think I'm running into a related issue. My problem is that if I have a def
form at the top level in my Clojure file, I lose fontification for the next few forms. I've attached a screenshot of the issue. The test file that reproduces the issue can be found here.
Removing the def
at the top of that file causes fontification to work correctly.
Good job! this was a long standing bug, one that you have to forcefully tell your self is not a big deal (but it is :smile: ). Thank you thank you thank you!