Skip to content

Add protobuf code comments as Elixir module documentation #352

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

Merged
merged 10 commits into from
Oct 20, 2024

Conversation

btkostner
Copy link
Contributor

@btkostner btkostner commented Jun 14, 2023

This PR takes code comments from the protobuf files and add them as @moduledoc blocks. Some of the code and all of the magic numbers are taken from protokit golang library. This is the same parser logic used by https://github.com/pseudomuto/protoc-gen-doc which is pretty popular.

Example generated code:

defmodule My.Test.Days do
  @moduledoc """
  This enum represents days of the week.
  """

  use Protobuf, enum: true, protoc_gen_elixir_version: "0.12.0", syntax: :proto2

  field :MONDAY, 1
  field :TUESDAY, 2
  field :LUNDI, 1
end

Todos:

  • Make the code cleaner. The recursive index based path for comments is pretty annoying to deal with.
  • Get documentation for messages working
  • Get documentation for services working
  • Documentation for fields? (This does nothing, but would be nice for future improvements)
  • Tests

@btkostner btkostner marked this pull request as ready for review June 14, 2023 23:39
@btkostner
Copy link
Contributor Author

btkostner commented Jun 14, 2023

Alright, this should be a good starting point. It generates the code comments as expected. Code is pretty cleaned up. I think adding some work for documenting individual fields will be the next step, but we should be good here so far.

Fixes #266

@Nezteb
Copy link

Nezteb commented Dec 22, 2023

@btkostner Regarding your comment:

Documentation for fields? (This does nothing, but would be nice for future improvements)

Could you elaborate on the "this does nothing" part? 😄

@btkostner
Copy link
Contributor Author

@btkostner Regarding your comment:

Documentation for fields? (This does nothing, but would be nice for future improvements)

Could you elaborate on the "this does nothing" part? 😄

Two ways to document the fields. Add a doc option for fields which I think would be preferred. Otherwise, just create a @typedoc block with each field documented. I decided not to implement the field documentation until the team decides on the way forward.

@Nezteb
Copy link

Nezteb commented Dec 22, 2023

until the team decides on the way forward

@whatyouhide @wingyplus Y'all show up often in the git history of the files Blake modified, so I feel like you are the most qualified to chime in. Do either of you have a preference on field docs?

I'd add you as PR reviewers but I don't have permissions. 😅

@v0idpwn
Copy link
Collaborator

v0idpwn commented Oct 16, 2024

@whatyouhide @ericmj I'd really like to merge this. The only potential blocker I see are warnings of this type for Elixir 1.12 / OTP 24:

The current heredoc line is indented too little
  nofile:6

warning: outdented heredoc line. The contents inside the heredoc should be indented at the same level as the closing """. The following is forbidden:

    def text do
      """
    contents
      """
    end

Instead make sure the contents are indented as much as the heredoc closing:

    def text do
      """
      contents
      """
    end

I'm not sure why this happens yet, trying to understand. They happen during generation, but not on actual generated code.

Edit: found it, fixed. Would you please take a brief look before I merge?

@v0idpwn v0idpwn changed the title feat: add protobuf code comments as Elixir module documentation Add protobuf code comments as Elixir module documentation Oct 16, 2024
@v0idpwn
Copy link
Collaborator

v0idpwn commented Oct 20, 2024

Well, I'm merging :)
If there's a post-merge review, I can fix any comments.

Thank you, @btkostner!
Over the next weeks, I'll try to get to your Google protos PR too.

@v0idpwn v0idpwn merged commit 18ad14f into elixir-protobuf:main Oct 20, 2024
3 checks passed
@btkostner btkostner deleted the documentation branch October 20, 2024 17:32
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.

3 participants