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

Allow no-link option in doxygenpage #748

Closed
wants to merge 1 commit into from

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Sep 30, 2021

Precisely what the title says. Whenever using .. doxygenpage:: to pull a Doxygen page into its own rst document, the implicit link to the page may be redundant with that of the title of the document -- necessary for .. toctree:: inclusion. That redundancy may cause duplicate ID warnings.

This patch allows the user to disable such links.

@hidmic
Copy link
Contributor Author

hidmic commented Oct 25, 2021

@jakobandersen ping.

@hidmic
Copy link
Contributor Author

hidmic commented Nov 2, 2021

@michaeljones @jakobandersen any chance we can this moving? Same for the other, more recent PRs. Some folks and I would rather step up and help you guys maintain this repository and not fork or patch downstream. FYI @clalancette @nuclearsandwich.

@clalancette
Copy link

For some context; we're some of the core maintainers of https://github.com/ros2 , and we've been developing a tool to generate API documentation on our packages. The tool is based on doxygen, breathe, exhale, and sphinx, and you can see the implementation at https://github.com/ros-infrastructure/rosdoc2 . This change, along with #750 and #753 and some fixes in Exhale will allow us to generate the documentation with far fewer errors than before.

As @hidmic said, we'd prefer to help maintain this package rather than carry our own fork. Please let us know if you are interested in having maintenance help here. Thanks!

@jakobandersen
Copy link
Collaborator

Do you have a small example project (or can make one) where this PR makes a difference?
The change looks reasonable, but it would be nice for reference to see the concrete issue it resolves.

@jakobandersen
Copy link
Collaborator

As @hidmic said, we'd prefer to help maintain this package rather than carry our own fork. Please let us know if you are interested in having maintenance help here. Thanks!

PRs are definitely appreciated, and indeed I think having a single project instead of multiple forks is preferable. Though, do note that our maintenance resources are current rather limited, so it may sometimes take a while before we get to each PR.

@hidmic
Copy link
Contributor Author

hidmic commented Nov 9, 2021

Hmm, I was unable to put together a minimal example reproducing the issue, and surprisingly enough I cannot reproduce it when building documentation in our project anymore. Perhaps I initially misunderstood the interplay between Breathe, Exhale, and Sphinx when it comes to hyperlinks.

@jakobandersen I think we can safely drop this PR.

@hidmic
Copy link
Contributor Author

hidmic commented Dec 6, 2021

Closing.

@hidmic hidmic closed this Dec 6, 2021
@hidmic hidmic deleted the no-page-link branch December 6, 2021 18:58
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.

None yet

3 participants