Added optional flag for pre-pending Enum name to Enum values to Proto generation #828
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s with enum name during *.proto schema generation
mgravell
reviewed
Aug 21, 2021
src/protobuf-net/Meta/MetaType.cs
Outdated
| foreach (var member in enums) | ||
| { | ||
| var parsed = member.TryGetInt32(); | ||
| if (parsed.HasValue && parsed.Value == 0) | ||
| { | ||
| NewLine(builder, indent + 1).Append(member.Name).Append(" = 0;"); | ||
| NewLine(builder, indent + 1).Append(enumPrefix + member.Name).Append(" = 0;"); |
There was a problem hiding this comment.
Can this be two separate append calls, to prevent the extra alloc? Not really going to make a huge difference in real terms, but since we have the builder...
mgravell
reviewed
Aug 21, 2021
src/protobuf-net/Meta/MetaType.cs
Outdated
| haveWrittenZero = true; | ||
| } | ||
| } | ||
|
|
||
| if (syntax == ProtoSyntax.Proto3 && !haveWrittenZero) | ||
| { | ||
| NewLine(builder, indent + 1).Append("ZERO = 0; // proto3 requires a zero value as the first item (it can be named anything)"); | ||
| NewLine(builder, indent + 1) | ||
| .Append(enumPrefix + "ZERO = 0; // proto3 requires a zero value as the first item (it can be named anything)"); |
mgravell
reviewed
Aug 21, 2021
src/protobuf-net/Meta/MetaType.cs
Outdated
| @@ -2047,11 +2049,11 @@ public bool IsGroup | |||
| if (parsed.HasValue) | |||
| { | |||
| if (parsed.Value == 0) continue; | |||
| NewLine(builder, indent + 1).Append(member.Name).Append(" = ").Append(parsed.Value).Append(';'); | |||
| NewLine(builder, indent + 1).Append(enumPrefix + member.Name).Append(" = ").Append(parsed.Value).Append(';'); | |||
mgravell
reviewed
Aug 21, 2021
src/protobuf-net/Meta/MetaType.cs
Outdated
| } | ||
| else | ||
| { | ||
| NewLine(builder, indent + 1).Append("// ").Append(member.Name).Append(" = ").Append(member.Value).Append(';').Append(" // note: enums should be valid 32-bit integers"); | ||
| NewLine(builder, indent + 1).Append("// ").Append(enumPrefix + member.Name).Append(" = ").Append(member.Value).Append(';').Append(" // note: enums should be valid 32-bit integers"); |
mgravell
requested changes
Aug 21, 2021
mgravell
reviewed
Aug 24, 2021
| @@ -2025,19 +2024,21 @@ public bool IsGroup | |||
|
|
|||
| bool haveWrittenZero = false; | |||
| // write zero values **first** | |||
|
|
|||
| string enumPrefix = flags.HasFlag(SchemaGenerationFlags.PrefixEnumValuesWithEnumName) ? GetSchemaTypeName(callstack) + "_" : ""; | |||
There was a problem hiding this comment.
minor nit: boxing, but... I don't think it is going to break the world
mgravell
approved these changes
Aug 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added a new flag in
SchemaGenerationFlagsfor applying( EnumName_ + ValueName )to the generation of enum values.Google's Proto compiler requires globally unique enum field names. Adding this prefix fixes compilation issues with Google's proto library. Most generators (C# for sure) will strip the prefix because it is part of Protobuf style guidelines.