-
-
Notifications
You must be signed in to change notification settings - Fork 21.7k
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: Fix multiline and trailing comma for assert #70655
Conversation
assert(x > 0,) | ||
assert(x > 0, 'message') | ||
assert(x > 0, 'message',) | ||
|
||
assert( | ||
x > 0 | ||
) | ||
assert( | ||
x > 0, | ||
) | ||
assert( | ||
x > 0, | ||
'message' | ||
) | ||
assert( | ||
x > 0, | ||
'message', | ||
) |
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.
Why are the trailing commas allowed? It doesn't make any sense to me, I'd expect an error when there's a trailing comma.
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 agree, the trailing comma doesn't really make sense.
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'm not sure I understand the surprise:
func method(a, b = 'c',) -> void:
print(a, b,)
func _ready() -> void:
method('a',)
method('a', 'b',)
print(['a',],)
Trailing commas and multi-line are supported by method calls and arrays. That's just make it behave consistent with those. What am I missing?
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.
@vnen Is this a bug or a feature?
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.
It is a feature. Search for "// Allow for trailing comma." comment in parser - parse_call
, parse_signal
, parse_enum
, parse_function_signature
, parse_array
, parse_dictionary
all have that comment for code that allows trailing comma.
The utility is visual consistency and smaller more relevant diffs/blame in git. So for example, you can write code like:
assert(
condition,
)
And then later in a separate commit add an error message, that would be 1 line addition instead of addition of 2 lines and removal of 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.
LGTM! :) Batch reviewing these smaller PRs to see if we get some momentum going, hehe :)
429dddb
to
7a9bb62
Compare
7a9bb62
to
71f7c8a
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.
Thanks! |
Fixed parsing of
assert
with multiline and/or trailing comma.Fixes #66535.