Skip to content
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 and improve annotation parsing #72979

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

dalexeev
Copy link
Member

@dalexeev dalexeev commented Feb 9, 2023

  • Fix multiline mode not working up to the first argument.
  • Disallow empty arguments.
  • Allow empty parentheses.

Before:


After:


Closes #54279.
Closes #66322.

@dalexeev dalexeev requested a review from a team as a code owner February 9, 2023 16:02
@YuriSizov YuriSizov added this to the 4.0 milestone Feb 9, 2023
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 17, 2023
@YuriSizov
Copy link
Contributor

Would be great to get a review from the GDScript team, but we can also cherrypick this in 4.0.x a bit later. Judging by the report, not a lot of people would be immediately affected.

@adamscott
Copy link
Member

Reviewed by the GDScript team. The PR seems right! This is alright.
@dalexeev Can you add some tests to make sure we don't end up with regressions afterwards? (positive and negative tests)

Thanks!

@dalexeev dalexeev force-pushed the gds-annotation-parsing branch from f8520f9 to 5038a33 Compare April 14, 2023 18:28
@dalexeev
Copy link
Member Author

Rebased and added tests.

@YuriSizov YuriSizov merged commit 6596a6c into godotengine:master Apr 17, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@dalexeev dalexeev deleted the gds-annotation-parsing branch April 17, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multi-line annotations won't parse parse error on annotation with empty arguments list
3 participants