Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add rosdoc2 option to specify primary_domain #24

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add rosdoc2 option to specify primary_domain #24

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3ebfb6
Add `rosdoc2` option to specify `primary_domain`
Jul 29, 2021
ac4a5ea
Fix default `rosdoc2_settings.primary_domain` val
Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions rosdoc2/verbs/build/builders/sphinx_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ def ensure_global(name, default):
# TODO(aprotyas): Migrate files under the `include` tag in the project's Doxygen
# configuration into the Sphinx project tree used to run the Sphinx builder in.
extensions.append('myst_parser')

# if no `primary_domain` option provided, defaults to None
if rosdoc2_settings.get('primary_domain'):
print(f"[rosdoc2] setting primary domain to '{{rosdoc2_settings.get('primary_domain')}}'",
file=sys.stderr)
# Tell sphinx what the primary language being documented is.
primary_domain = rosdoc2_settings.get('primary_domain')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the user already set primary_domain or highlight_language? We'd be overwriting them. I think it would be better to only set these if primary_domain is explicitly in rosdoc2_settings, or if the user didn't specify primary_domain or highlight_language.

Copy link
Contributor Author

@aprotyas aprotyas Aug 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the changes in ac4a5ea, the last talking point is about a reasonable default for the domain/highlighting - check #24 (comment). The following snippet should then do the trick:

ensure_global('primary_domain', {default_language})
ensure_global('highlight_language', {default_language})

if rosdoc2_settings.get('primary_domain'):
    ...

Of course, the ensure_global calls can be skipped altogether if we settle that the sphinx defaults of py/default for primary_domain/highlight_language are reasonable defaults for rosdoc2's use.

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wouldn't default to cpp I think. The user can always explicitly label the language of code in their doc blocks right? Or does this cover a case where that doesn't work?

If we do pick a default other than the sphinx default, I'd say we should do cpp if it is an ament_cmake package and py or do nothing if it is an ament_python package.

Copy link
Contributor Author

@aprotyas aprotyas Aug 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or does this cover a case where that doesn't work?

Off the top of my head, I don't think this would cover any case where the user couldn't explicitly label the code block.

I'd say we should do cpp if it is an ament_cmake package and py or do nothing if it is an ament_python package.

Alright, I will keep this in mind as I'm working on resolving #19.


# Tell sphinx what the pygments highlight language should be.
highlight_language = rosdoc2_settings.get('primary_domain')
"""

default_conf_py_template = """\
Expand Down Expand Up @@ -239,6 +249,13 @@ def ensure_global(name, default):

## Support markdown
# 'support_markdown': True,

## This setting, if set, will attempt to set the `sphinx` options
## `primary_domain` and `highlight_language` equal to this setting. These
## options allow `sphinx` to choose reasonable defaults for source
## code highlighting, among other things.
## Possible values (without extensions): 'c', 'cpp', 'js', 'py', None
# 'primary_domain': None,
}}
"""

Expand Down