search

atom / language-clojure

Clojure package for Atom
Other
49 stars 48 forks source link

Fix matching of numbers #90

Closed MrEbbinghaus closed 2 years ago

MrEbbinghaus commented 2 years ago

Description of the Change

This PR fixes several issues with number matching. (See https://github.com/atom/language-clojure/issues/89)

  1. Clojure allows for an explicit + prefix.
  2. Clojure has three symbolic Number values for +/- infinity and "Not a Number"
  3. Every integer number can have the BigInt suffix N, that includes octals, hexadecimals and arbitrary radix
    • I merged constant.numeric.bigint.clojure into constant.numeric.long.clojure as it doesn't make any sense any more, to have it separate.
    • I also merged constant.numeric.bigdecimal.clojure into constant.numeric.double.clojure for consistency.
  4. The decimal separator . is optional when followed by e, E, M
  5. "Arbitrary" radix is constraint to 2-36 (digit 0-9 & a-z). Also, \w also matches _ which is incorrect.
    • One could argue, that mismatching radix/value (e.g. 2rABZ) shouldn't be matched either, but I considered it overkill.
    • This change is debatable, because it
  6. Octal numbers can't contain 8 or 9

It also adds a bunch of tests to check for previously missed cases.

Alternate Designs

I considered constraining the changes to only eliminate false-negatives. (Change 5 and 6 are only fixing false-positives) I opted against that because a more strict highlighting is the fastest way of feedback a developer can get.

Benefits

More accurate highlighting of numbers.

Possible Drawbacks

The matching is more strict. Some false-positives aren't highlighted any more. (Although not really a drawback, it is a breaking change if someone relies on current bugs... But, how would?)

Applicable Issues

resolves https://github.com/atom/language-clojure/issues/89

MrEbbinghaus commented 2 years ago

These changes were merged into the Calva Clojure IDE. https://github.com/BetterThanTomorrow/calva/pull/1435

MrEbbinghaus commented 2 years ago

@kevinsawicki @50Wliu

Hey, is this repository still maintained or is it abandoned? If it is abandoned, where can I report and fix parsing/highlighting issues to?

winstliu commented 2 years ago

Hey! @darangi and @sadick254 would be the two best equipped to answer that, I think. Let me look at this PR though and see if I have any comments.

darangi commented 2 years ago

Thanks for the contribution 🙇🏾 @MrEbbinghaus