-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Make support/python/mkdocstrings_handlers/cxx/__init__.py
PEP 8 compliant (1 of 2)
#4110
Make support/python/mkdocstrings_handlers/cxx/__init__.py
PEP 8 compliant (1 of 2)
#4110
Conversation
Thanks for the PR. Are you talking about PEP 8 and could you split indent/whitespace change into a separate commit/PR to make the more important changes easier to see in the diff? |
Yes! To be honest, I am just follow the recommendations (yellow flags) from CLion.
Sure, I'll do split it in 2. |
support/python/mkdocstrings_handlers/cxx/__init__.py
PEP compliantsupport/python/mkdocstrings_handlers/cxx/__init__.py
PEP 8 compliant
support/python/mkdocstrings_handlers/cxx/__init__.py
PEP 8 compliantsupport/python/mkdocstrings_handlers/cxx/__init__.py
PEP 8 compliant (1 of 2)
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.
There is one error:
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/home/runner/work/fmt/fmt/support/python/mkdocstrings_handlers/cxx/__init__.py", line 53, in <module>
def doxyxml2html(nodes: List[ElementTree.Element] | ElementTree.Element):
TypeError: unsupported operand type(s) for |: '_GenericAlias' and 'type'
… using operator '|'.
Ooops, Python 3.8 doesn't like that |
@@ -50,63 +62,71 @@ def doxyxml2html(nodes: List[et.Element]): | |||
out += '<code class="language-cpp">' if tag == 'pre' else '' | |||
if n.text: | |||
out += escape_html(n.text) | |||
out += doxyxml2html(n) | |||
out += doxyxml2html([n]) |
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.
Looks like this introduces infinite recursion: https://github.com/fmtlib/fmt/actions/runs/10303709064/job/28524343469?pr=4110
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.
Should probably be list(n)
instead of [n]
because what we need is a list of chile elements.
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.
Should probably be
list(n)
instead of[n]
because what we need is a list of chile elements.
Oh my! I had introduced the from __future__ import annotations
so that I could use the operator |
(I tested this in a python:3.8
docker), but then I also pushed this change (something I was trying on my machine). Sorry about that! I hope it works now.
…tor. Change convert_type return type to Optional[str].
Merged, thanks! |
Yey, thanks to you! |
Definition
class.convert_type
can returnstr | None
.et
is imported asElementTree
.doxyxml2html
can receive either aList[ElementTree.Element]
or anElementTree.Element
.type
variable is renamed totype_
(andtype_text
) to avoid conflicts with built-intype
.templateparamlist
is renamed totemplate_param_list
.