-
Notifications
You must be signed in to change notification settings - Fork 187
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
feat/support underscore in numberic #701
feat/support underscore in numberic #701
Conversation
Codecov Report
@@ Coverage Diff @@
## master #701 +/- ##
==========================================
- Coverage 82.03% 81.99% -0.04%
==========================================
Files 125 125
Lines 10891 10937 +46
==========================================
+ Hits 8934 8968 +34
- Misses 1957 1969 +12
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Could you also add another test case here to prove that something like 100_000_
or _1_0
is still rejected.
fe/crates/parser/tests/cases/errors.rs
Line 84 in c6edbe8
test_parse_err! { expr_dotted_number, expressions::parse_expr, "3.14" } |
Lastly, can you add a file newsfragments/149.feature.md
with the contents being something like
Support for underscores in numbers to improve readability e.g. `100_000`
This is to ensure the feature will be included in the upcoming release notes.
I have problem is when I write "fail tests" case error but snap file body is empty. How I can handle it?
I don't know format of the |
I guess you probably figured this out, but the snapshot testing stuff uses insta. You can just add a new test and run it. The test will initially fail, because the new snapshot won't match the "current" (non-existent) snapshot. To accept the snapshot, it's easiest to use
Usually there are some examples in there, but they're removed on every release and added to the changelog. What you have is good, just drop the leading |
Thank you very much for your work @vuvoth |
Proposed Changes
Support underscore for readable in numberic type.
Close #149
Implementation
Please review my PR! Thanks!
To-Do
OPTIONAL: Update Spec if applicable
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history