Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(def s "a string") font locks "a string" as a docstring #445

Closed
oskarkv opened this issue Sep 20, 2017 · 9 comments
Closed

(def s "a string") font locks "a string" as a docstring #445

oskarkv opened this issue Sep 20, 2017 · 9 comments
Labels

Comments

@oskarkv
Copy link

oskarkv commented Sep 20, 2017

Expected behavior

Font lock the string "a string" as a string in (def s "a string").

Actual behavior

The string "a string" is font locked as a docstring in (def s "a string").

clojure-mode version information

clojure-mode (version 5.7.0-snapshot)

Proposed solution

I played around with this, and it can be solved by adding a check in this and https://github.com/clojure-emacs/clojure-mode/blob/master/clojure-mode.el#L898 in the function clojure-font-lock-syntactic-face-function. For example

(and ...
  (or (save-excursion
        (goto-char startpos)
        (not (= ?\) (char-after (+ startpos (length (sexp-at-point)) 2)))))
      (string= "defprotocol" firstsym)))

This, of course, checks for a closing paren after the string, and if so, it is not considered a docstring, except for defprotocol, which is the only kind of form that can have a docstring in the last position. Instead of checking for a paren, one could of course check for another element, but it is more involved.

Opinions, suggestions, etc?

@oskarkv oskarkv changed the title (def s "a string") font locks "a string" as a docstring (def s "a string") font locks "a string" as a docstring Sep 20, 2017
@Macroz
Copy link
Contributor

Macroz commented Oct 30, 2017

Your proposed fix works for me.

@vspinu
Copy link
Contributor

vspinu commented Oct 30, 2017

Relatedly in

(s/def :some/keyword (some-spec))

:some/keyword is highlighted with font-lock-function-name-face instead of the keyword-face.

@bbatsov
Copy link
Member

bbatsov commented Oct 31, 2017

@vspinu That one should be easier to fix. Just a minor regexp tweak.

@bbatsov
Copy link
Member

bbatsov commented Oct 31, 2017

@oskarkv File a PR and we can discuss the potential fix there. I'm a bit concerned about potential performance implications on larger buffers, but overall the approach seems reasonable, although it seems to be it won't work for ns for instance, so maybe it should be scope to just def or something like this.

@bbatsov
Copy link
Member

bbatsov commented Oct 31, 2017

e.g. (ns some.ns "some docstring").

@Macroz
Copy link
Contributor

Macroz commented Oct 31, 2017

Hmm this works for me in ns case also but it might be because I'm running a slightly customized version of clojure-mode with more specific faces for ns and defn as you can see from the screenshot.
works-for-me

@Macroz
Copy link
Contributor

Macroz commented Nov 2, 2017

ns works if there are requires etc, not without them.

@bbatsov
Copy link
Member

bbatsov commented Nov 4, 2017

That's what I meant. :-)

@bbatsov
Copy link
Member

bbatsov commented Nov 4, 2017

Perhaps this should just check for a list of specific forms (with semantics similar to def) and that would be both safe and solves the problem.

@bbatsov bbatsov added the bug label Sep 25, 2018
carlosgeos pushed a commit to carlosgeos/clojure-mode that referenced this issue Jan 20, 2019
…t can have...

strings and docstrings highlighted differently
carlosgeos pushed a commit to carlosgeos/clojure-mode that referenced this issue Jan 21, 2019
carlosgeos pushed a commit to carlosgeos/clojure-mode that referenced this issue Feb 20, 2019
…t can have...

strings and docstrings highlighted differently
carlosgeos pushed a commit to carlosgeos/clojure-mode that referenced this issue Feb 21, 2019
carlosgeos pushed a commit to carlosgeos/clojure-mode that referenced this issue Feb 25, 2019
cichli added a commit that referenced this issue Feb 27, 2019
[Fix comment in #445] Proper font lock in (s/def ::keyword)
cichli added a commit to cichli/clojure-mode that referenced this issue Feb 27, 2019
@cichli cichli closed this as completed in a9a0276 Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants