-
Notifications
You must be signed in to change notification settings - Fork 91
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
Generate documentation using sphinx-rtd-theme. #306
base: master
Are you sure you want to change the base?
Conversation
Earlier tests used Sphinx from PyPI rather than apt on Focal and thus had newer features. To run on Sphinx from apt on Focal we can't use the --extensions argument and so need to add sphinx_rtd_theme using a script.
@@ -7,6 +7,8 @@ html: apidoc | |||
|
|||
apidoc: | |||
$(SPHINXAPIDOC) -F -o _build ../src/catkin_pkg | |||
sed -i "/extensions =\[/a 'sphinx_rtd_theme'," _build/conf.py | |||
sed -i "s|html_theme =.*$$|html_theme = 'sphinx_rtd_theme'|" _build/conf.py |
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.
Hmm, this works but seems pretty fragile. If the conf.py
that is generated changes it might break this, no?
Two other options are:
- the template dir option https://www.sphinx-doc.org/en/master/man/sphinx-apidoc.html
- make a
conf.py
and addsphinx-autodoc
as an extension, e.g. https://eikonomega.medium.com/getting-started-with-sphinx-autodoc-part-1-2cebbbca5365
The benefit of using it how it is now is that you don't need the (mostly) boilerplate conf file and index file. But now we're trying to get that benefit while also changing something about the conf, which I think is tenuous.
That all being said, I'm sure this works right now.
What do you think?
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've been testing this variously with sphinx 1.8.3 and 4.1.2 and in all that time the conf.py
hasn't changed enough that the above two scripts don't apply. I agree that it's more brittle than being able to pass theme and extension data through sphinx-apidoc and sphinx-apidoc in sphinx 4 has a generic --extensions
option that eliminates one of these two modifications but that is not yet available in the apt repositories and I'm more interested in keeping things stable.
with that in mind, I think that the above will work reliably until we jump platforms again. I could put a circuit breaker in and print something to stderr and exit if sphinx >= 1 is used which would give us a reason to come back and review these changes.
* the template dir option [sphinx-doc.org/en/master/man/sphinx-apidoc.html](https://www.sphinx-doc.org/en/master/man/sphinx-apidoc.html)
I looked at that option but I don't know if there's any difference between "templates" and "themes" in Sphinx parlance and I thought that trying to find the template directory for the appropriate sphinx_rtd_theme installation would be even less robust than sed scripting the config file.
* make a `conf.py` and add `sphinx-autodoc` as an extension, e.g. [eikonomega.medium.com/getting-started-with-sphinx-autodoc-part-1-2cebbbca5365](https://eikonomega.medium.com/getting-started-with-sphinx-autodoc-part-1-2cebbbca5365)
This isn't something I've looked too closely since my goal with this PR was to change only the things that need to be changed.
Is the creation of a conf.py and index the only difference between sphinx-apidoc and sphinx with the autodoc extension? If so this could be the preferred solution.
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.
Sorry, I didn't get back sooner.
To be clear, I'm fine with the current approach, we just might have to do something with it later, but that's ok.
I think the idea of the templates directory option is that you provide templates for things like the conf.py
which sphinx-apidoc
will use instead of the standard boilerplate. I think it's completely separate from the idea of a theme or templates used to say generate html headers and footers. It might be equally fragile thought, to be honest.
Is the creation of a conf.py and index the only difference between sphinx-apidoc and sphinx with the autodoc extension?
Indeed, if you look at what sphinx-apidoc
does, it just generates a conf.py
and index.rst
and runs sphinx-build
over it (more or less).
The link I provided for this option basically walks you through creating your own conf.py
and index.rst
to accomplish this yourself.
These two commands, together with forthcoming changes for the doc-independent job configuration will switch the catkin_pkg API documentation to the rtd theme.
So we're not as consistent currently as I thought in the offline discussion earlier today.
rospkg and rosdep currently use the ros-theme while rosinstall and vcstools currently use the haiku theme. catkin-pkg and rosdistro use the default sphinx theme since both generate their sphinx conf.py files with sphinx-apidoc.
Trying to apply retroactive logic to the above, it seems like ros utility packages use the ROS theme and vcstools packages (including rosinstall) use the haiku theme, while catkin_pkg and rosdistro, which have a conf.py generated by sphinx-apidoc, use the default theme.
The scripts here could easily be modified to target the ROS theme rather than RTD although if I compare the two I personally find rtd to be more readable. Once this is settled I'll open a sibling PR on rosdistro so that the two of them are using the same theme.