raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.27k stars 844 forks source link

Fix documentation issues when upgrading Doxygen to 1.9.8 #1564

Closed mudge closed 4 months ago

mudge commented 7 months ago

See https://github.com/raspberrypi/pico-sdk/issues/1563

Between Doxygen 1.8.17 and Doxygen 1.9.8, the top-level "modules" tab type in the Doxygen layout has changed such that our "API Documentation" section is no longer generated and is instead considered of the type "topics". Update our layout XML to reflect this change and restore this critical section of the documentation.

As of Doxygen 1.8.19 (specifically, https://github.com/doxygen/doxygen/commit/327423e217337bf4e64515aba6cf78b9877bc165), the first line of a comment is no longer automatically extracted as a brief description unless we enable the JAVADOC_AUTOBRIEF setting. See https://www.doxygen.nl/manual/config.html#cfg_javadoc_autobrief:

If the JAVADOC_AUTOBRIEF tag is set to YES then doxygen will interpret the first line (until the first dot) of a Javadoc-style comment as the brief description. If set to NO, the Javadoc-style will behave just like regular Qt-style comments (thus requiring an explicit @brief command for a brief description.)

mudge commented 5 months ago

This no longer seems to work as expected with the very latest version of Doxygen (1.10.0) and I've discovered more subtle bugs with brief description extraction even without relying on JAVADOC_AUTOBRIEF: e.g. the brief description for https://github.com/raspberrypi/pico-sdk/blob/6a7db34ff63345a7badec79ebea3aaef1712f374/src/rp2_common/hardware_spi/include/hardware/spi.h#L100-L103 is incorrectly extracted as:

<para>Initialise SPI instances</para>
<para>Puts the SPI into a known state, and enable it. Must be called before other functions. </para>

Note the first line of the detailed description being included here, presumably because there is no blank line after the \brief command.

aallan commented 4 months ago

@mudge Deprecated in favour of #1660?

mudge commented 4 months ago

Yes, I'll close this one though the DoxygenLayout.xml changes may still be necessary.

aallan commented 4 months ago

@nelliemckesson Do we still need the DoxygenLayout.xml changes in this PR?

nelliemckesson commented 4 months ago

@aallan @mudge we don't use doxygen's generated nav data in the web documentation toolchain, and as far as I can tell, it also isn't used in the doxygentoasciidoc conversion. So, I think DoxygenLayout.xml would only affect the original pico-sdk site (can't remember where that lives/lived), but if that still exists, then yeah, those changes are still required.

lurch commented 4 months ago

So, I think DoxygenLayout.xml would only affect the original pico-sdk site (can't remember where that lives/lived), but if that still exists, then yeah, those changes are still required.

I think the only place that's still relevant is Appendix E of https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf ? But if this is going to become dependent on the version of Doxygen the user has installed, maybe the Doxygen-version also needs to be mentioned in that Appendix?

aallan commented 4 months ago

@nathan-contino Can you throw a quick NOTE block into Appendix E of the Pico C SDK book ahead of the documentation release (which looks to be next week at this point). Thanks!