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.

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

Support a custom index.rst.jinja #164

Merged
merged 3 commits into from
Jan 21, 2025

Conversation

rkent
Copy link
Contributor

@rkent rkent commented Jan 3, 2025

We use a standard index.rst.jinja to specify the main output of rosdoc2. This PR allows the user to specify a custom version of that.

@rkent rkent requested review from audrow and tfoote as code owners January 3, 2025 00:09
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

One suggested change, but otherwise this would be great to enable.

And could we add a quick reference to this capability in the documentation. I'm not sure exactly where would be best. But at least mentioning the capability if people search for index.rst.jinja they'll get a hit.

template_jinja = template_path.read_text()

# Did the user provide index.rst.jinja?
user_jinja_path = os.path.join(wrapped_sphinx_directory, 'index.rst.jinja')
Copy link
Member

Choose a reason for hiding this comment

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

Since we're using the more modern pathlib via resources below this could be symmetric and the read_text() method could be pulled outside the if block and avoid the with open ... read() Then the if block is just picking the template path and printing to say which it will use.

@rkent
Copy link
Contributor Author

rkent commented Jan 3, 2025

I added changes to address your issues.

Concerning documentation, I did a fairly comprehensive documentation patch for rosdoc2 in my development branch, but that is 14 commits ahead of where we are now. At one point I had hoped that I could get all of my work on rosdoc2 landed fairly quickly, and we could point package authors to the "new" rosdoc2 and its documentation, but that does not seem to be possible. Anyway I pulled out the index.rst documentation from that, updated it, and landed it on this PR's branch.

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks for pulling the docs out. I know it will cause extra effort to do the merge.

@tfoote tfoote merged commit 4e65262 into ros-infrastructure:main Jan 21, 2025
5 checks passed
@rkent rkent deleted the 2-allow-user-index-jinja branch February 5, 2025 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants