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

Support for empty statement #260

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Support for empty statement #260

merged 5 commits into from
Nov 14, 2024

Conversation

riz0id
Copy link
Collaborator

@riz0id riz0id commented Nov 13, 2024

This PR adds support for parsing and discarding empty statements in enumerations, messages, and services. As well, this PR removes the "empty" constructors in DotProtoServicePart and DotProtoMessagePart as they're no longer used. This PR resolves issue #257.

@riz0id riz0id self-assigned this Nov 13, 2024
tests/TestCodeGen.hs Outdated Show resolved Hide resolved
Copy link
Collaborator

@j6carey j6carey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the specifics: are you planning to keep DPSEmpty? Having an explicit representation of an empty top-level statement shows a different style than the approach you are taking with this PR in service, message, and enum definitions.

Copy link
Collaborator

@j6carey j6carey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvement. I still wonder if we should do something about DPSEmpty being handled differently, but I don't want to block this PR for that.

@riz0id
Copy link
Collaborator Author

riz0id commented Nov 14, 2024

DPSEmpty

Thanks for the improvement. I still wonder if we should do something about DPSEmpty being handled differently, but I don't want to block this PR for that.

Typically the top-level needs to be handled differently. I think leaving it for now is fine, but if it can be removed then I think we should remove it in the future.

@j6carey j6carey merged commit 53ae1df into master Nov 14, 2024
84 checks passed
@j6carey j6carey deleted the riz0id/parse-empty-statement branch November 14, 2024 21:13
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.

2 participants