-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Remove duplicated headings for docstrings nested in tabs/admonitions #610
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -231,17 +231,26 @@ def _process_block( | |
|
||
class _PostProcessor(Treeprocessor): | ||
def run(self, root: Element) -> None: | ||
self._remove_duplicated_headings(root) | ||
|
||
def _remove_duplicated_headings(self, parent: Element) -> bool: | ||
carry_text = "" | ||
for el in reversed(root): # Reversed mainly for the ability to mutate during iteration. | ||
found = False | ||
for el in reversed(parent): # Reversed mainly for the ability to mutate during iteration. | ||
if el.tag == "div" and el.get("class") == "mkdocstrings": | ||
# Delete the duplicated headings along with their container, but keep the text (i.e. the actual HTML). | ||
carry_text = (el.text or "") + carry_text | ||
root.remove(el) | ||
parent.remove(el) | ||
found = True | ||
elif carry_text: | ||
el.tail = (el.tail or "") + carry_text | ||
carry_text = "" | ||
elif self._remove_duplicated_headings(el): | ||
found = True | ||
break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has a wrong behavior: "If duplicated headings were found in a child element, stop scanning the rest of the document upwards". Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess there's no specific "why" and the assumption is that duplicated headings appear in a single child element. Do you recommend that we try and remove them from the whole subtree? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this was my assumption, that transformations would only nest the duplicated heading block without repeating it. Breaking early should improve the performance if the tree is large, but I have not idea by how much or if this was necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Results from the child element are being propagated to the parent loop and it gets interrupted. |
||
if carry_text: | ||
root.text = (root.text or "") + carry_text | ||
parent.text = (parent.text or "") + carry_text | ||
return found | ||
|
||
|
||
class MkdocstringsExtension(Extension): | ||
|
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.
This has a wrong behavior: "If text was carried, don't bother stepping into the current element". There is no connection between the two, so it shouldn't be an
else
.