-
Notifications
You must be signed in to change notification settings - Fork 224
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
Collapsible namespace in sphinxext #3441
Collapsible namespace in sphinxext #3441
Conversation
Note that this only works in Python3, because the extension used to create the <details> tag is only compatible for Sphinx>=2.0.
So far, the description for a single port was wrapped inside both a paragraph and a list item. This caused inconsistent vertical spacing when namespaces and single ports were mixed. To avoid this, the paragraph wrapping the content was removed. If the increased spacing is desired, this should probably be done in CSS on the list.
If you're wondering about the spacing after the namespace port list, that seems to be just how that particular theme shows nested bullet lists - I did fix am inconsistency between regular ports and port namespaces, though. |
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.
thanks @greschd!
Looks fine to me.
paragraph += nodes.emphasis(text=self.format_valid_types(port.valid_type)) | ||
paragraph += nodes.Text(', ') | ||
paragraph += nodes.Text('required' if port.required else 'optional') | ||
res = [] |
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 guess this switch from paragraph
was necessary to make the spacing consistent?
Perhaps a comment to explain this might be useful.
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.
Yeah, that's exactly why. I did document that in the commit message, but since the paragraph is no longer there in the code I'm not sure a code comment would really be understandable.
Because the sphinxcontrib-details-directive requires sphinx>=2.0, this dependency is also updated. Since that version of Sphinx is incompatible with Python2, a version selector is added, and for Python2 the previous version of Sphinx is kept.
The 'python' highlighting does not work in the visualising_graphs page, because an ipython ! command (shell command) is used. Changing the highlighting type to 'ipython' fixes this issue. The sphinxcontrib-contentui needs updating to be compatible with Sphinx version 2.2.0.
77abc6f
to
140891d
Compare
"sphinx==1.8.5", | ||
"sphinxcontrib-contentui==0.2.2" | ||
"sphinx==1.8.5; python_version<'3'", | ||
"sphinx==2.2.0; python_version>='3.0'", | ||
"sphinxcontrib-contentui==0.2.2; python_version<'3'", | ||
"sphinxcontrib-contentui==0.2.4; python_version>='3.0'", |
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.
@ltalirz this is the main change compared to before: For the Python3 version, Sphinx needed to be updated (to at least 2.0) to be compatible with sphinxcontrib-details-directive
, and that in turn required an update to sphinxcontrib-contentui
.
Very nice @greschd , thanks for this.
This would indeed be nice but if it comes at the risk of not working at all in some cases, I agree it is better to leave it for now.
I was wondering why you would use Edge, but then I remembered ... 😉 Final question: does this work with nested namespaces? Technically your example does contain a doubly nested one, since |
I wouldn't have noticed if it wasn't mentioned in the docs for the
Yeah good point -- it works, but I added a nested namespace to the test to make sure. |
Use the HTML
<details>
tag to create collapsible namespaces in the sphinx extension for process documentation.All collapsed:

All open:

We also discussed (#3300) directly making the heading clickable (without the extra "Namespace Ports" line), but I couldn't get this to work -- at least not without messing with the CSS. Although I'm not exactly an HTML wizard, so a more capable person might be more successful. Changing CSS is a bit of a risk IMO, it might break when the theme is updated, or people could decide to use a different theme for their plugin docs.
Note also: Edge doesn't support

<display>
, it will show like this:But that should probably be resolved soon, with the Chrome-based Edge.
I wouldn't consider #3300 resolved, this is just an incremental improvement.