-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
[GDScript] Prevent running String
number functions on invalid literal
#87941
Conversation
error.start_column = column; | ||
error.leftmost_column = column; | ||
error.end_column = column + 1; | ||
error.rightmost_column = column + 1; |
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.
Unsure if the full error info is required here, but added it for completeness, unsure what the difference here is to not using it like below
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 think it is right. Such errors should be reported as a cursor rather than a selection.
Prevents printing excessive errors.
579601a
to
66d55e6
Compare
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.
Looks good to me. push_error()
vs return error
can be improved later (like lines 844-845 and 848).
Thanks! |
Thank you! |
Cherry-picked for 4.2.2. |
Prevents printing excessive errors.
We could also just remove the error print in
hex_to_int
but it was added quite a while ago and is a good indication of what's wrong when parsing strings IMO, and at least I don't think it should be removed just to prevent an annoying print in the GDScript parser (I'd even argue we might want to add errors tobin_to_int
and the others)Literals containing decimal points will already stop consuming input before them so that won't trigger an error print on that part (other than the GDScript tokenizer error, but the annoying part is the print to console)