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 bool type value for the Option element in the Generator #25

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

davdai01
Copy link
Contributor

Not all option values are string type. Some can be bool type. For example: java_multiple_files.
We should be able to generate option java_multiple_files = false; instead of option java_multiple_files = "false";

@criccomini
Copy link
Owner

Thanks! Does it make sense to add support for integers and floats, too?

@davdai01
Copy link
Contributor Author

Thanks! Does it make sense to add support for integers and floats, too?

I'm not too sure. To be honest, I'm myself just a beginner in Profobuf.
I'm looking at the language guide. I'm not seeing any examples of Option values in integer or float type.

@criccomini
Copy link
Owner

Ok, so the language guide you link to has an options section that links to a complete list of supported options:

complete list of available options is defined in /google/protobuf/descriptor.proto.

That .proto seems to have three types of options: string, bool, and enums. I think we should punt on enums until someone actually has a use case for it and submits a PR.

@davdai01 can you rebase your PR so I can merge it?

@criccomini
Copy link
Owner

Oop, sorry, looks like it's not a merge conflict; it's a linting failure. Please fix the " / ' issue.

@criccomini criccomini merged commit 5694378 into criccomini:main Apr 18, 2024
1 check passed
@criccomini
Copy link
Owner

Thanks, again, for the PR! Merged and released as part of 1.3.3.

https://pypi.org/project/proto-schema-parser/1.3.3/

@davdai01 davdai01 deleted the fix-option-in-generator branch April 18, 2024 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants