-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: add support for enums #374
Conversation
@@ -400,6 +400,7 @@ def _parse_docstring_summary(summary): | |||
summary_parts = [] | |||
attributes = [] | |||
attribute_type_token = ":type:" | |||
enum_type_token = "Values:" |
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 see there are other keywords defined as constants at the top of the file. Can this one also be a constant?
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.
If it was just values
docstring tokenized I would have kept it up there but since we're looking for the specific Values:
string I've kept it here.
if part.lstrip('\n').startswith('..'): | ||
if (potential_keyword := part.lstrip('\n')) and ( | ||
potential_keyword.startswith('..') or | ||
potential_keyword.startswith(enum_type_token) |
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.
Can this just be included in the extract_keyword
logic?
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 thought about it, but unfortunately the entire section didn't get tokenized:
Values:
name(val):
...
Other parts that do get tokenized and/or has proper format appear in this format:
.. note::
...
.. example::
...
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.
Thank you!
@@ -400,6 +400,7 @@ def _parse_docstring_summary(summary): | |||
summary_parts = [] | |||
attributes = [] | |||
attribute_type_token = ":type:" | |||
enum_type_token = "Values:" |
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.
If it was just values
docstring tokenized I would have kept it up there but since we're looking for the specific Values:
string I've kept it here.
if part.lstrip('\n').startswith('..'): | ||
if (potential_keyword := part.lstrip('\n')) and ( | ||
potential_keyword.startswith('..') or | ||
potential_keyword.startswith(enum_type_token) |
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 thought about it, but unfortunately the entire section didn't get tokenized:
Values:
name(val):
...
Other parts that do get tokenized and/or has proper format appear in this format:
.. note::
...
.. example::
...
Enums section was unformatted until now, adding a formatter for it.
I've considered stripping individual parts (name, value and description) to put into the table format, however we don't have any prior history of formatting enums this way. I've decided to just format the entire block in a codeblock, which is a significant improvement over what we have.
See b/195689267 for the staged example.
Mixed in a bit of lint/formatting fixes.
Fixes b/195689267.