-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
Fix single line comment lexing with CRLF line ending #964
Conversation
Codecov Report
@@ Coverage Diff @@
## master #964 +/- ##
==========================================
+ Coverage 59.29% 59.33% +0.03%
==========================================
Files 166 166
Lines 10689 10687 -2
==========================================
+ Hits 6338 6341 +3
+ Misses 4351 4346 -5
Continue to review full report at Codecov.
|
Do you also need to update the multi-line comment code? I think your first idea of making the lexer always return LF for the end of a line would be the best course as it makes lexing (very slightly) more simple (relevant if in future more tokens are added). |
I tried, but the lexer for single line comment uses boa/boa/src/syntax/lexer/comment.rs Lines 33 to 40 in 0c068cb
I did not see any issues in the lexer for multi-line comment. |
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.
This looks good :) thanks
Hmmm surprisingly, this made 6 tests fail, but maybe that's correct. Test262 conformance changes:
|
I have run the tests before and after the fix on Linux using WSL, and I have checked some of the newly failed tests. In fact, it seems that even when using Linux, some files from the test262 repository contains CRLF line endings. So, tests that were previously passed because they were considered empty are now failed because they contain not yet supported features (such as eval). |
This Pull Request fixes #963.
It changes the following: