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

(clojure) does not highlight defn- properly #2420

Closed
joshgoebel opened this issue Feb 22, 2020 · 6 comments · Fixed by #2438
Closed

(clojure) does not highlight defn- properly #2420

joshgoebel opened this issue Feb 22, 2020 · 6 comments · Fixed by #2438
Labels
bug help welcome Could use help from community language

Comments

@joshgoebel
Copy link
Member

Clojure does not highlight defn- as a keyword. The - is highlighted as part of the title instead.

You can find an example in markup global_definition.txt:

; private function
(defn- add [x y] (+ x y))

This might actually require a parser fix since the problem is word boundaries (which is how the parser handles beginKeywords) are funny in how they work with -. So there is not actually a word boundary AFTER the - which is where you'd need it to work properly.

@joshgoebel
Copy link
Member Author

joshgoebel commented Feb 22, 2020

I propose a look-ahead that finds a word boundary OR a space.

mode.begin = '\\b(' + mode.beginKeywords.split(' ').join('|') + ')(?=\\b|\\s)';

This would work for identifiers that include "non-word" characters and hence don't end up word boundaries but rather on the boundary between the last character and whitespace... This does nothing to fix keywords that BEGIN with strange characters (we'd need look-behind) but I think that's a less common occurrence, so I'm not super worried about it until we have a real life example.

It would make it hard to have keywords with spaces, but our whole keyword system already kind of makes that super difficult anyways.

Thoughts? CC @egor-rogov

@egor-rogov
Copy link
Collaborator

It works fine it seems.

@joshgoebel
Copy link
Member Author

You mean you agree that my idea would work?

@egor-rogov
Copy link
Collaborator

Yes, I even tried it and it really works.

@joshgoebel
Copy link
Member Author

Oh, I tried it before I proposed it, lol... I was floating it just to see if there was anything I wasn't considering that someone else might think of - ie a good reason not to fix it that way.

@egor-rogov
Copy link
Collaborator

The best way to think of something is to try it... I can't see any objection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help welcome Could use help from community language
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants