WIP: Fix a code generation edge case where a oneof is named "result"#368
Draft
okkero wants to merge 1 commit intoanmolitor:masterfrom
Draft
WIP: Fix a code generation edge case where a oneof is named "result"#368okkero wants to merge 1 commit intoanmolitor:masterfrom
okkero wants to merge 1 commit intoanmolitor:masterfrom
Conversation
When a oneof is named "result" (or possibly any other Elm prelude name), the code generator tries to add underscores at the end of the name to avoid conflicts. However, sometimes in longer path names in Internals_.elm the underscore is added after a period, not before as it should (e.g. Foo.Result._Code vs Foo.Result_.Code).
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
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.
When a oneof is named "result" (or possibly any other Elm prelude name), the code generator tries to add underscores at the end of the name to avoid conflicts. However, sometimes in longer path names in Internals_.elm the underscore is added after a period, not before as it should (e.g.
Foo.Result._CodevsFoo.Result_.Code).This PR is not a complete fix yet. I have only made a test to demonstrate the issue. The fact that
Mapper.Name.escapeTypeappends one underscore andMapper.Name.internalizeintersperses double underscores makes it a bit problematic to restore the original values inMapper.Name.externalize. You end up with internal names likeProto__OneofResult__Foo__Result___Code(notice the triple underscore at the end), and sinceString.splitworks eagerly,externalizeends up outputtingProto.OneofResult.Foo.Result._Codeinstead ofProto.OneofResult.Foo.Result_.Code.The reason I haven't implemented a complete fix yet is that I was unsure about which approach I should take, and I would like some input before I proceed. I considered changing
externalizeto the following, but that felt a bit too hacky:Another alternative would be to change the reserved names suffix from
_to something like0, but I'm not sure how that would interact with everything else. The best approach might just be to use an alternative split implementation so thatsplit "__" "A___B"splits into["A_", "B"]instead of["A", "_B"].Thoughts? @anmolitor