ros-infrastructure / rosdoc2

Command-line tool for generating documentation for ROS 2 packages.
Apache License 2.0
29 stars 9 forks source link

Add `rosdoc2` option to specify `primary_domain` #24

Open aprotyas opened 2 years ago

aprotyas commented 2 years ago

This option should enable sphinx to land on reasonable defaults for code highlighting Pygments lexers, as well as other markup behaviors, depending on the domain of a package (defaults to cpp).

Signed-off-by: Abrar Rahman Protyasha abrar@openrobotics.org

aprotyas commented 2 years ago

So this PR would allow a user to provide a rosdoc2.yaml file to override and get Python highlighting. If they did not do this, then they would get C++ highlighting, even on Python packages. Is that a correct assessment of the change?

That's right. There's an assumption here about the frequency of C++ packages over Python packages, which is not correct in hindsight. If the primary_domain/highlight_domain options are not set, the default values are py and default (basically Python3) respectively. I don't know if that's ideal either as a default case for rosdoc2. If they are set to None, then of course there is no highlighting.

aprotyas commented 2 years ago

In the case that the user specifies neither a primary_domain/highlight_language in their conf.py configuration nor a primary_domain in rosdoc2_settings (i.e. it is not the case that the user's setting will be overwritten), is there a reasonable default for the domain/language that rosdoc2 could/should configure? The sphinx default value for these options is py/default respectively, which is essentially a pseudo-Python3 syntax. I'm leaning towards a cpp default, but I can't back that up with a relative frequency of ament_cmake packages versus that of ament_python packages comparison

Thoughts?

aprotyas commented 2 years ago

With ac4a5ea, the current behavior is to not change the primary domain setting in any way unless explicitly specified. However, having extracted build type information in #28, it would make more sense to set the primary domain to cpp/py depending on whether the build type is ament_cmake/ament_python (suggestion in https://github.com/ros-infrastructure/rosdoc2/pull/24#discussion_r683977435). This information can be format mapped to rosdoc2_wrapping_conf_py_template through a template variable.

Thoughts?

wjwwood commented 2 years ago

However, having extracted build type information in #28, it would make more sense to set the primary domain to cpp/py depending on whether the build type is ament_cmake/ament_python (suggestion in #24 (comment)).

Yes I think so, I was imaging cpp for ament_cmake and not setting anything (using the default) for ament_python. But if the standard practice for Python projects is to manually set it to py then that would be ok for ament_python as well. Basically I think ament_python should do whatever is "normal" for a plain python package.