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

Some valid floats are highlighted incorrectly #1378

Closed
MrEbbinghaus opened this issue Nov 10, 2021 · 3 comments
Closed

Some valid floats are highlighted incorrectly #1378

MrEbbinghaus opened this issue Nov 10, 2021 · 3 comments

Comments

@MrEbbinghaus
Copy link
Contributor

MrEbbinghaus commented Nov 10, 2021

While working on improved number highlighting for highlight.js, I noticed that there are some valid Clojure numbers that Calva does not correctly highlight.

I don't know how Calva / VS Code does its highlighting, if you could point me towards the right place that would be great.

image

They are indeed valid Clojure numbers.
image

Here are some regex form cljs.tools.reader

(def int-pattern #"^([-+]?)(?:(0)|([1-9][0-9]*)|0[xX]([0-9A-Fa-f]+)|0([0-7]+)|([1-9][0-9]?)[rR]([0-9A-Za-z]+)|0[0-9]+)(N)?$")
(def ratio-pattern #"([-+]?[0-9]+)/([0-9]+)")
(def float-pattern #"([-+]?[0-9]+(\.[0-9]*)?([eE][-+]?[0-9]+)?)(M)?")

I have written the following monolithic (JavaScript)-regex for this if you need it:

/[-+]?(((0[xX][0-9a-fA-F]+|0[0-7]+|[1-9][0-9]?[rR][0-9a-zA-Z]+)N?)|[0-9]+(\/[0-9]+|N|((\.[0-9]*)?([eE][+-]?[0-9]+)?M?)))/

I had them split at first, but it wouldn't match them correctly (42 can be matched as an int and as a float with the rules above), I am currently working on this to pull this apart again.
highlightjs/highlight.js@e1fda58#diff-acf0030489ef9b080c066ef3ba1df651db3c94c56b5da44cbf6319b0689559c0R48

This is the full list I tested against:
(GitHub makes even more mistakes, I will submit a PR to them as well.)

; integer ; BigInt  ; octal   ; hex
00          42N     052       0x2a
42          0N      00N       0x0N

; radix.  ; radix BigInt
2r101010  2r101010N
8r52      8r52N
16r2a     16r2aN
36r16     36r16N

;; ratios  ;; floats
1/2        42.0
-1/2       -42.0
           42.

; BigDecimal ; with Exponent
42.0M        42.0E2
-42M         -42.0E+9
42.M         42E-0
42M          42.0E2M
             42E+9M
@PEZ
Copy link
Collaborator

PEZ commented Nov 10, 2021

Hello. Thanks for reporting!

This probably goes wrong in the tmLanguage config. See https://github.com/BetterThanTomorrow/calva/wiki/How-to-Hack-on-Calva#syntax-grammar for how to hack on that.

As mentioned on that wiki-page Calva has two grammar definition. tmLanguage, and then a custom one defined in src/cursor-doc/clojure-lexer.ts. That one is used for all navigation and also for some extra highlight. But for floats all highlighting is performed by VS Code based on Calva's tmLanguage definition.

Let me know if the wiki is not helpful enough.

@MrEbbinghaus
Copy link
Contributor Author

@PEZ I submitted a PR to https://github.com/atom/language-clojure

atom/language-clojure#90

I don't know if you have any workflow for updating the tmLanguage config in Calva through that repository.
If you don't, I can submit the same PR to this repository as well if you like.

@PEZ
Copy link
Collaborator

PEZ commented Dec 12, 2021

A PR is welcome. There's no linking back to the atom grammar. We have diverged quite a lot. Also, my last PR to that repo took half a year to get reviewed...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants