preliminary feedback

Thanks for putting out a PR on this! Obviously, I've been neglecting this repo... as you probably discovered, there's a lot of technical debt here, a lot mine, and a lot not mine. I've fixed up most of the CI in #115 but need to switch over to GitHub Actions and ditch Travis. I'm moving soon but have some time to squeak out a release and get pages working. This one should have tests added, but I can help with that (the testing framework is a lot).

To be honest, I've never really used doxygen pages. I'll try and build your linked PR and see what's going on, initially I'm apprehensive to remove rootFileTitle as a required argument. See https://exhale.readthedocs.io/en/latest/usage.html#additional-usage-and-customization

1. Can you update that table (docs/reference/configs.rst, search begin_root_api_document_layout) to include where the pages go?
2. Linked https://exhale.readthedocs.io/en/latest/reference/configs.html#exhale.configs.rootFileTitle docs, sphinx uses the rootFileTitle wherever the .. toctree:: that includes it is. I'm not seeing why this can be relaxed in the presence of pages, but perhaps that will become more clear after I build it.

Ok quick update, CI was fixed, will be able to look more closely at the implementation here over the next couple of days / get acquainted with doxygen pages.

Ok I added dracula in with some doxygen page stuff, no actual testing done right now, there's some issues with the current implementation and the rst links.

$ cd /path/to/exhale
# (or however you manage py installs, get tox)
$ pip install tox

# Run just the tests associated with the project that has the doxygenpage examples
$ tox -e py -- -k cpp_nesting -s

# See the issues in current html output
$ find testing/projects/cpp_nesting -name index.html -exec xdg-open {} \;

# (alternatively open them manually)
$ find testing/projects/cpp_nesting -name index.html
testing/projects/cpp_nesting/docs_CPPNestingPages_test_hierarchies_with_pages_dracula_not_mainpage/_build/html/index.html
testing/projects/cpp_nesting/docs_CPPNestingPages_test_hierarchies_with_pages_dracula_is_mainpage/_build/html/index.html

Action items:

• Do not force the word Contents on people, that can be a configuration variable.
• Fix the rst links (note tree view is not enabled in those builds)
  • AFAICT the page kind of doxygen in xml will not have a def_in_file, we can omit those warnings and comment. That may be the break.
  • Edit maybe the back-link should go back to the library root document instead?
• Validate tree view manually (by inspection)
• Actually validate tests.

What I'm not understanding:

1. Why is there any parent relationship stuff going on? I don't understand how nested pages come in or how to create them.
2. Seems to be related to you removing the title. Why does the \mainpage version remove the Class / File hierarchy from the toctree? I don't like this behavior, can it be opt in?
3. Do you have a sample project I can look at (rather than just pasting Dracula in...) so I can actually see what you're trying to do? That'll probably make things clearer 🙂

Mostly just taking notes to myself here, but will get back at it soon.

From what I'm gathering here, if p.refid == "indexpage": you're wanting to remove the rootFileTitle because of this one. If you could orient me a bit on the tension points, and more generally how peoples use doxygen pages, I'm sure we can find a compromise or hack.

Right. The basic idea is that if your documentation is already organized in pages within Doxygen, there's no need for rootFileTitle. \mainpage will already have a title.

So leaning toward keep it required, and just explicitly say it will not get used if you are also using pages?

I don't have a strong opinion here but I will point out that Doxygen may have pages but no main page, in which case you do want a rootFileTitle. Only a user knows whether they need it or not. Thus why I made it optional.

Why is there any parent relationship stuff going on? I don't understand how nested pages come in or how to create them.

Doxygen supports page hierarchies via \subpage refs.

Seems to be related to you removing the title. Why does the \mainpage version remove the Class / File hierarchy from the toctree? I don't like this behavior, can it be opt in?

Hmm, I suspect this is related to writeOutHierarchy changes. I changed it so that it won't write out a hierarchy if it is empty (e.g. no class hierarchy if there's no API to show).

Do you have a sample project I can look at (rather than just pasting Dracula in...) so I can actually see what you're trying to do? That'll probably make things clearer

Sure. There's changes in flight so it's not super straightforward to reproduce. Check this bash gist.

Do not force the word Contents on people, that can be a configuration variable.

Agreed.

Fix the rst links (note tree view is not enabled in those builds)

Hmm, I've noticed Breathe has issues with links in pages. Is that what you mean?

Thanks for reviewing @svenevs! I agree with most of your points. I'll try to address your comments sooner than later.

I did a lot of reworking on the testing setup to get this going, it's going to be more straightforward to just generate page documentation directly. The skeleton is there for it now, but not actually anything done (I suppose I'll be removing Dracula xD).

Anyway, \subpage makes sense -- thanks for clarifying. I'm gonna keep steamrolling this one, but I have to go right now. The testing setup is more or less complete, but actual test cases for projects are what's next now that I understand what's going on. I think I have a good idea about the rootFileTitle thing but need to double check.

If I were you I'd hang on a bit before jumping back in on this, the testing machinery is not something you should have to deal with. To be honest, I was really impressed you got things working as well as you did given the chaos that is graph.py 🙂

Do not force the word Contents on people, that can be a configuration variable.

Done in 72b5c4d.

If I were you I'd hang on a bit before jumping back in on this, the testing machinery is not something you should have to deal with.

Got it. I'll stop here then.

@svenevs any updates on this front? Anything I can help with?

Hey @hidmic sorry...

I fixed the first problem where the rst links were broken, FYI the .. _link_name: has to appear right before a heading, the .. did not find the file comment being the cause of the breakage there (rst is very fickle). However, see this nonsense

https://github.com/hidmic/exhale/blob/f162aa9c9ae157c4b4cf26037cf5414d76c16281/testing/tests/cpp_nesting.py#L94-L120

I've been looking at the doxygen manual but it's really unclear to me how this command is supposed to be used. If you switch the \page second Second Page there to be \mainpage, there's more or less no change -- neither page_third.rst nor page_fourth.rst are getting generated. If you want to muddle around with that and produce a docstring that actually results in some kind of page hierarchy that'd be great.

$ cd </path/to/exhale>
$ tox -e py -- -k cpp_nesting -s

That'll drop you into a shell to pause the testing process before things get deleted, just after the html build has finished. You will be able to open the file testing/projects/cpp_nesting/docs_CPPNestingPages_test_hierarchies_without_pages/_build/html/index.html (and correspondingly look around testing/projects/cpp_nesting/docs_CPPNestingPages_test_hierarchies_without_pages/api/ for the generated pages). Ignore the name of the test... it'll also fail, that's ok too. Really it's just there because I am trying to follow what's going on, I'm not getting the right hierarchy.

That said, this one is kind of a significant change and currently has some bugs. I don't think I'm going to be able to finish this before moving (which is a lengthy / complicated process, I have to deconstruct this computer soon). I hadn't intended on doing anything with this repo until settling in (~beginning of november).

If you can give me a docstring that's going to produce valid results, I'm ok with merging things without passing tests, I just want to see things working myself 😉 Hope that makes sense, basically if it's not obvious then I will revisit this one when I have more time.

@svenevs any chance we can get this moving soon? Some folks and I would rather step up and help you maintain this repository and not fork or patch downstream. FYI @clalancette @nuclearsandwich.

@svenevs any chance we can get this moving soon? Some folks and I would rather step up and help you maintain this repository and not fork or patch downstream. FYI @clalancette @nuclearsandwich.

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 #117 and #118 and some fixes in Breathe 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!

Short answer: yes, sorry, and thank you, help would be most welcome -- you need things faster than I can get to right now. I couldn't rebuild the computer, I'm not able to find the box with the last part. I also can't work on this on my work computer. I will add you as collaborators tomorrow, and you can proceed forward. I can't seem to do it from my phone.

Edit: finally succeeded. Good phone.

@svenevs Thanks, appreciate the understanding. I've now accepted the invite. I'll take a look at some of the open issues in a bit here.

Ok, I was able to fix the things that weren't working afaict. Please verify that this still does what you want with doxygen pages and let me know. I just need to update some docs on the required vs optional rootFileTitle config now.

8eae7ff

changes in graph.py there are the main ones to pay attention to, like i was saying nested pages weren't being generated. they should all be generated now. self.pages only includes top level pages, so generatePageDocuments() needs to build out the list of nested kids.

@2bndy5 thanks for reiterating the doxygen example i see how the doxygen nesting system works, or at least I think lol. any page can declare on itself a subpage and then doxygen does its thing? otherwise we're out of scope of what doxygen supports if there is a max level of subpages allowed (see testing/tests/cpp_nesting.py and associated testing/projects/cpp_nesting/page_town_rock*.hpp files)

hope this is coherent

From comment in your last commit about

find out why everything breaks in Doxygen 1.9.2

I've come across "unmatched XML tags" in the doxygen output when using v1.9.2 with breathe. I found that the closing XML tags are missing from the doxygen output.

any page can declare on itself a subpage and then doxygen does its thing?

Yes, I'm under the same impression. I have never really used subpages in Doxygen though. And I don't recall the Doxygen manual declaring a limit for nesting pages.

otherwise we're out of scope of what doxygen supports if there is a max level of subpages allowed (see testing/tests/cpp_nesting.py and associated testing/projects/cpp_nesting/page_town_rock*.hpp files)

This seems very specific to the PR, and I'm not sure what to make of the cpp_nesting.py code, but the page_town_rock.hpp looks right.

I've come across "unmatched XML tags" in the doxygen output when using v1.9.2 with breathe. I found that the closing XML tags are missing from the doxygen output.

Oh my! 😱 That's good to know about, definitely not going to do anything about it in this PR then...

Thanks so much for all your work here and on breathe 🙂

Yes, I'm under the same impression. I have never really used subpages in Doxygen though. And I don't recall the Doxygen manual declaring a limit for nesting pages.

Haha me neither. See attached images and zip of html output if helpful below, I think we're more or less on par with doxygen html output. In some senses, a little better because when there is a use of \mainpage nothing actually shows up on Related Pages section whereas in exhale it will still be in the page hierarchy 🎉

I'm pretty sure this is the original functionality @hidmic implemented (specifically, root page absorbs \mainpage contents and title -- testing framework cheats to achieve it), but it would be good to test on some of the projects you all are after before saying it is good to go.

with \mainpage

docs_CPPNestingPages_test_hierarchies_primary_mainpage.zip

without \mainpage

docs_CPPNestingPages_test_hierarchies_primary_no_mainpage.zip

ref for myself: #121

@svenevs Thanks for the demonstration! I didn't know that "Related Pages" could be used to show a nested hierarchy. That makes the \subpage cmd make a lot more sense. Originally, I thought the hierarchy only showed in the TREEVIEW (when enabled in doxygen).

Ok this is rebased and staged to be in the changelog, ready for me to squash merge pending a third party corroborating that they are able to use this branch to build a doxygen project with pages as they expect it to.

I've done all I can for this weekend, it turns out pip install exhale is broken right now requiring people to pip install sphinx first. I'd like to merge this and do a release.

There aren't many low hanging fruit left in the issues, so after this release next weekend I'm going to look into the changes you all have been stuffing into breathe -- I want to just depend on the next version to come out (incl. dropping python 2 🙃), and ideally find a way to get whatever needs to happen for #106 fixed. Whether that's fix exhale or update the interface to allow refid lookup on .. doxygenfunction or whatever.

Tested this PR with the RF24 project I've been working to convert to sphinx docs...
The pages (which are generated by doxygen from MD files) are re-generated well with exhale. ☑️
The RF24 project does not use \subpage cmds, so it isn't a full coverage field test.

Coming back from some time off. Thank you both for pushing this!