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

Preserve comments in output _pb.jl files from original .proto #128

Closed
wants to merge 1 commit into from

Conversation

NHDaly
Copy link

@NHDaly NHDaly commented Jul 7, 2019

This commit adds code to generate_file to copy all the comments from
the original .proto file into the new generated _pb.jl file by
reading the comments fields off of the SourceCodeInfo proto.

The comments mechanism is a bit confusing in the proto descriptors, so I
separated out most of the newly added logic into a new file,
src/commetns.jl.

The changes in this commit were inspired by following the example of the
code here (which is under the MIT license):
https://github.com/pseudomuto/protokit/blob/7037620/comments.go


Before merging:

  • tests: I think we should probably add some tests, and some explanation to the docs/readme.
  • implementation: I think there are still some unhandled comments types, such as file-level comments and imports-level comments.

This commit adds code to `generate_file` to copy all the comments from
the original `.proto` file into the new generated `_pb.jl` file by
reading the comments fields off of the SourceCodeInfo proto.

The comments mechanism is a bit confusing in the proto descriptors, so I
separated out most of the newly added logic into a new file,
`src/commetns.jl`.

The changes in this commit were inspired by following the example of the
code here (which is under the MIT license):
https://github.com/pseudomuto/protokit/blob/7037620/comments.go
@NHDaly
Copy link
Author

NHDaly commented Aug 18, 2022

Closing this PR in favor of #208.

@NHDaly NHDaly closed this Aug 18, 2022
@NHDaly NHDaly deleted the nhdaly-comments branch August 18, 2022 18:08
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.

1 participant