-
Notifications
You must be signed in to change notification settings - Fork 205
entproto: Add proto3 optional label support #605
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
base: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds support for proto3 optional field labels as an alternative to using google/protobuf/wrappers.proto for optional fields, resulting in more concise generated code.
- Introduces
EnableOptionalLabel()MessageOption to enable proto3 optional labels per message - Updates field descriptor generation to use proto3 optional labels when enabled
- Maintains backward compatibility by keeping the feature opt-in
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| entproto/message.go | Adds EnableOptionalLabel() MessageOption and corresponding struct field |
| entproto/adapter.go | Implements optional label logic in field descriptor generation, adds optionalFieldLabel constant, and updates function signatures |
| entproto/service.go | Updates function call to match new toProtoFieldDescriptor signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func extractProtoTypeDetails(f *gen.Field, enableOptionalLabel bool) (fieldType, error) { | ||
| if f.Type.Type == field.TypeJSON { | ||
| return extractJSONDetails(f) | ||
| return extractJSONDetails(f) // repeat fields are not required for optional label. |
Copilot
AI
Nov 19, 2025
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.
Corrected 'repeat' to 'repeated' for grammatical accuracy.
| return extractJSONDetails(f) // repeat fields are not required for optional label. | |
| return extractJSONDetails(f) // repeated fields are not required for optional label. |
| func extractProtoTypeDetails(f *gen.Field, enableOptionalLabel bool) (fieldType, error) { | ||
| if f.Type.Type == field.TypeJSON { | ||
| return extractJSONDetails(f) | ||
| return extractJSONDetails(f) // repeat fields are not required for optional label. |
Copilot
AI
Nov 19, 2025
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.
The comment is unclear about why the enableOptionalLabel parameter is not passed to extractJSONDetails. Consider clarifying: 'JSON fields only support repeated (array) types, not optional labels.' This better explains that extractJSONDetails doesn't need the enableOptionalLabel parameter because it only handles repeated fields.
| return extractJSONDetails(f) // repeat fields are not required for optional label. | |
| return extractJSONDetails(f) // JSON fields only support repeated (array) types, not optional labels. |
proto3 supports using optional fields to define optional fields, so
google/protobuf/wrappers.proto does not need to be used. This will make the generated code more concise.The
EnableOptionLabelcan enable this feature for a specific Message individually.