-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add error message for illegal literals - Parser.scala:602 #1667
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
Conversation
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.
Except for a few nitpicks this looks great! 👍
| - Character Literals: 'a', '\u0041', '\\n' | ||
| - String Literals: "Hello, World!" | ||
| - null | ||
|""".stripMargin |
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.
Strip margin isn't necessary on hl
strings :)
| - Integer literals: 0, 21, 0xFFFFFFFF, -42L | ||
| - Floating Point Literals: 0.0, 1e30f, 3.14159f, 1.0e-100, .1 | ||
| - Boolean Literals: true, false | ||
| - Character Literals: 'a', '\u0041', '\\n' |
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.
There shouldn't be an extra \
in '\\n'
right? You don't need to escape things (except $
) in triple quoted strings.
val kind = "Syntax" | ||
val msg = "illegal literal" | ||
val explanation = | ||
hl"""|Expected a valid literal. |
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.
I would skip this line, the error message already says "illegal literal" right? If you think it should be kept, you could bake it into the sentence where you present the legal literals in Scala.
@felixmulder PTAL |
LGTM |
val kind = "Syntax" | ||
val msg = "illegal literal" | ||
val explanation = | ||
hl"""|Available literals can be divided into the several groups: |
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.
typo: "into the several groups" -> "into several groups"
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.
Fixed in: bffefe9
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.
Oh, snap! Sorry for this typo guys.
This pull request is related to the following issue