Def highlighting is out of control

[expez]

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.

[Malabarba]

I was pretty sure we had fixed this, at least for the word default. ...

[expez]

[vspinu]

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.

[expez]

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?

[Malabarba]

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".

[sooheon]

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?

[j-cr]

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.

1. define-clojure-font-lock, analogous to define-clojure-indent
2. 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.

[bbatsov]

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.

[j-cr]

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.

[bbatsov]

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.

[vspinu]

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.

[expez]

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.

[vspinu]

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.