-
Notifications
You must be signed in to change notification settings - Fork 168
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
Fix missing detached leading comments #539
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.
Doh! Thanks for finding and fixing this. I clearly don't have good enough test coverage in here.
Is this an easy issue to reproduce? If so, it would be great to add a test that reproduces the bug, and then confirm that this change fixes it.
Yes, it is easy to reproduce. It happens every time I trigger I think I can modify |
You can manually modify it, but it might impact other test cases across the repo that use that file. I think it may be better to define a small, minimal file that reproduces the issue and then inline it into the test case. You can compile the source into descriptors using the |
@@ -455,6 +456,109 @@ func TestBuildersFromDescriptors_PreserveComments(t *testing.T) { | |||
testutil.Eq(t, count, descCount) | |||
} | |||
|
|||
func TestBuilder_PreserveAllCommentsAfterBuild(t *testing.T) { | |||
files := map[string]string{"test.proto": ` | |||
syntax = "proto3"; |
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.
I think this is a much bigger example than necessary. Maybe just one enum and value and one message and field?
Thanks for the contribution! |
There's a bug causing the
LeadingDetachedComments
not copied properly to the source info.