[Fix #377] Fix font locking for def forms

[OknoLombarda]

Fixes #377

Well, the solution is mostly described here, I went with a defcustom approach. Also I noticed that font locking is applied multiple times for the same match. I'm not sure if this was intentional, but I just removed it and made a single top level capture for all def forms and everything seems to work. There were tests that checked if custom def forms were properly highlighted, but now they're redundant, so I removed them

---

Before submitting a PR mark the checkboxes for the items you've done (if you
think a checkbox does not apply, then leave it unchecked):

• The commits are consistent with our contribution guidelines.
• You've added tests (if possible) to cover your change(s). Bugfix, indentation, and font-lock tests are extremely important!
• You've run M-x checkdoc and fixed any warnings in the code you've written.
• You've updated the changelog (if adding/changing user-visible functionality).
• You've updated the readme (if adding/changing user-visible functionality).

Thanks!

[vemv]

Personally I feel that there are a lot of def-like forms, and consequently the default experience will be worse than currently.

Therefore I'd suggest an alternative approach: default to the current approach, but blacklist a few words from the English dictionary: default, defer, etc. There cannot be so many of them.

wdyt?

Cheers - V

[bbatsov]

I actually prefer the proposed solution, but that's not a surprise as I think I had something similar in mind. I'll accept the PR when the CI is green. We'll also have to document in the README how people can add new def forms. It's not like we're not doing the same for everything else, so let's become more consistent internally.

[vemv]

Is this something that CIDER already enhaces anyway? (or that it could, after this PR)

[bbatsov]

Is this something that CIDER already enhaces anyway? (or that it could, after this PR)

It does, that's one more reason why I prefer the approach suggested in this PR. :-) It will pick up all definitions, macros, etc. (you can turn this off if you don't care about it) That's why it's better to have less assumptions in clojure-mode IMO. The smarts should be at the level of CIDER, clojure-lsp, not in clojure-mode.

[OknoLombarda]

I thought that I'll have at least a week to fix everything before you guys notice PR, but it's like you were waiting for it :D

Also, I think the problem with CI is because of the added variable. Regexp is evaluated at compile time, so it probably can't use a dynamic variable. I have no idea how to fix this at the moment. I'll surely make a research, but if you could give a direction, it would be really appreciated

[bbatsov]

Hmm, I thought that defcustoms should be available at complile time. Anyways, no rush.

I thought that I'll have at least a week to fix everything before you guys notice PR, but it's like you were waiting for it :D

That's how we roll! 😉

[OknoLombarda]

Managed to fix it by wrapping defcustom form in eval-and-compile, but def forms added after compilation are just ignored. Everything works fine if I remove eval-when-compile from clojure-font-lock-keywords, though

clojure-mode/clojure-mode.el

Lines 826 to 827 in 0af29f4

	(defconst clojure-font-lock-keywords
	(eval-when-compile

But I guess compiling this regexp is necessary

[OknoLombarda]

So I just excluded regexp responsible for def forms from eval-when-compile

[bbatsov]

Looking at the Emacs docs it seems that eval-when-compile is just a performance optimization here. Not sure how big of an effect it has - frankly, even though I've been using Emacs almost 20 years I keep forgetting things about byte-compilation optimizations.

[OknoLombarda]

From the docs:

The result of evaluation by the compiler becomes a constant which appears in the compiled program.

It seems it's kinda like ^:const in Clojure. Expression is evaluated and used as a constant.

If this list is evaluated frequently, maybe it's better to leave at least part of it evaluated during compilation

[OknoLombarda]

So, should I leave it like this, remove eval-when-compile or try to find some other solution?

[bbatsov]

I'm wondering about this myself. It'd be simplest to just add the defs to the big form, but that would make them harder to customize.

Another simple solution would to have two font-lock-add-keywords invocations in the mode setup. That's how clojure-mode-extra-font-locking works:

(font-lock-add-keywords 'clojure-mode
                        `((,(concat "(\\(?:\.*/\\)?"
                                    (regexp-opt clojure-built-in-vars t)
                                    "\\>")
                           1 font-lock-builtin-face)))

I'm leaning towards just inlining all the known forms in the existing regexp and adding some snippet in the README about how people can add more themselves.

[OknoLombarda]

Done

[bbatsov]

Looks good. Thanks!