-
-
Notifications
You must be signed in to change notification settings - Fork 202
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
Doxygen dot to Sphinx graphviz #757
Doxygen dot to Sphinx graphviz #757
Conversation
The |
Found a bug in the |
@2bndy5 This seems very nice, thanks a lot. Just to make sure (even though I believe it says so in description), this does also include the inheritance diagrams etc found in Doxygen's own HTML output right? That really is a long-standing wish so fantastic to see this implemented if the case. Do you consider this ready for review/merging? |
Indeed, it does. This addition makes use of the dot executable though (as noted in the new docs' page dot_graphs.rst)
Yes, please. |
Its been so long since I opened this PR, that I figured I'd double check the proposed changes.... I think I messed up my fork's master branch by merging all changes from all my separate PRs (targeting upstream) into a branch on my fork that was supposed to be called "remix" (I think I mistakenly merged to my fork's master).
With that said, I found and removed changes that were specific to #760 and #759 . If I have to keep rebasing this branch, then I'll have to do it manually because git seems to be confusing my fork's master with upstream master. With all due respect, I want to mention that this lib's infrequent/sporadic dev activity is disheartening and tends to discourage contributions from those willing to do so. I mean no offense at all. It is amazing that the lib has gotten so old and popular considering the maintainers' limited time. |
@2bndy5 Specifically for this, I do agree it's unfortunate. Especially because the bigger changes like this that really give a lot of value usually end up sitting in queue longer as a result. Normally things progress more quickly but the past year (or maybe two at this point) have just been tough for me personally resulting in too little time for Breathe. I'll try my best to get some more time available for the pending PRs within a week and bump version again afterwards. |
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.
@2bndy5 Really many thanks for the very thorough patches, documentation looking very good too! Will do some rebasing to clean up the commits and to get things on top of master, then final local test and merge.
I know it took way to long to get these things properly reviewed and checked, will try to continue and clear the entire queue today.
Rebasing this properly and handling and the merge commits and conflicts, removing the does+undoes, merging fixups was a ton of work, but it's finally done now and clean enough for merging. @2bndy5 For a large part the convoluted history is a result of the delayed reviews, however for next time (at any project) I definitely recommend using Thanks again for this great addition! |
FYI #803 |
This resolves #45
The approach is to translate any
dot
anddotfile
doxygen commands into sphinx'sgraphviz
directive. This now encompasses the inheritance/include graphs that are generated by Doxygen (which are now translated to dot syntax and put in agraphviz
directive).This feature is my first comprehensive contribution to this library, so I'm very open to criticism about lib convention and testing.
Limitations
This feature depends on having the dot executable installed to work its magic.
As such, I've added a
allow-dot-graphs
option to the following directives:doxygenclass
doxygenstruct
doxygenfile
doxygenindex
autodoxygenfile
autodoxygenindex
Without this new option specified, breathe will behave like normal. If the dot executable is installed, then the user needs to specify this option to have graphs generated from XML.
\dot
&\dotfile
cmds as those already (required by doxygen) depend on having the dot tool accessible.Also, as stated in the added document dot_graphs.rst
The only
graphviz
directive option supported is thecaption
option. Graph captions are optionallyspecified using the
dot
ordotfile
command syntax. All othergraphviz
directive optionsfallback to their default behavior.
dot
ordotfile
commands are ignored.Using Doxygen's
@ref
command within any dot syntax is functionally ignored and treated as literal text.Sample (updated)
Attached is a sample I've been using for testing purposes:
graph_breathe.zip
Note: I don't use the
doxygenautoindex
directive in the provided sampleunit test CI change
@michaeljones The sphinx branch 4.2.x no longer exists (happened a few days ago). I've changed this in the CI to use the new 4.3.x branch. I hope this is ok.