Render docstrings and expose service descriptor#305
Conversation
Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
| f.print( | ||
| "return next(s for s in ", | ||
| service.file, | ||
| ".desc().services if s.type_name == '", |
There was a problem hiding this comment.
I considered threading in the index within the descriptor we see here, but while it should be quite rare, it could be possible for the pb to change especially ordering of items without regenerating connect for some reason. Didn't seem worth avoiding this more flexible matching.
There was a problem hiding this comment.
I think we're maybe missing the /**/* in our own .gitattributes to properly mark these files as generated (think this approach in protobuf-py should work).
|
|
||
|
|
||
| class ServerReflection(Protocol): | ||
| """""" |
There was a problem hiding this comment.
maybe should skip completely empty docstrings?
stefanvanburen
left a comment
stefanvanburen
left a comment
There was a problem hiding this comment.
looks good, I think we ought to just skip empty docstrings though(?) (and would be nice to fix the .gitattributes as well)
| for line in text.splitlines(): | ||
| # Comments in protobuf often start with a space, this | ||
| # leads to weird indentation in the generated docstring, so we remove it. | ||
| f.print(line.removeprefix(" ")) |
There was a problem hiding this comment.
seems reasonable to me as a first pass — I think both buf format and some of the other plugins have had to make tweaks over time to make this right for the ecosystem. Not too concerned with a little churn in the comments if we need to revisit (and not coming up with a better suggestion now).
2 participants
This renders docstrings for service and method very similar to how protobuf-py does for other types. It also adds a
desc()classmethod for accessing the descriptor also following protobuf-py's model. I debated against a module-level constant since there isn't anything async or sync about the descriptor, but it felt cleanest going with this consistency instead.