Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Fix matching of numbers #90

Merged
merged 2 commits into from
Jan 19, 2022
Merged

Fix matching of numbers #90

merged 2 commits into from
Jan 19, 2022

Conversation

MrEbbinghaus
Copy link
Contributor

@MrEbbinghaus MrEbbinghaus commented Dec 12, 2021

Description of the Change

This PR fixes several issues with number matching. (See #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 #89

@MrEbbinghaus
Copy link
Contributor Author

These changes were merged into the Calva Clojure IDE. BetterThanTomorrow/calva#1435

@MrEbbinghaus
Copy link
Contributor Author

@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?

@50Wliu
Copy link
Contributor

50Wliu commented Jan 19, 2022

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.

Copy link
Contributor

@50Wliu 50Wliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks about right.

@darangi
Copy link
Contributor

darangi commented Jan 19, 2022

Thanks for the contribution 🙇🏾 @MrEbbinghaus

@darangi darangi merged commit 8983208 into atom:master Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some valid numbers are highlighted incorrectly
3 participants