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

Add doxygen download action to indent #16024

Merged
merged 1 commit into from Oct 31, 2023

Conversation

jpthiele
Copy link
Contributor

This adds the action ssciwr/doxygen-install to download a more recent version of doxygen.
This change is necessary for the dark mode support of graphviz dot graphs proposed in #16013.

@jpthiele
Copy link
Contributor Author

Needs some #ifndef DOXYGEN and #endif encapsulations, I'm on it.
Navigation within the tutorial steps works, so I think the multiple use of section label warnings
could be safely ignored in the YAML file just like the too many nodes warnings.

@jpthiele jpthiele force-pushed the jpt-doxygen-action branch 2 times, most recently from 27b6676 to da5746a Compare September 22, 2023 14:24
@jpthiele
Copy link
Contributor Author

Some notes in the new warnings that are being generated and suppressed:

  • multiple use of section label '<label>' while adding anchor does not like all steps having a section named introduction and similar. It builds fine and they all link correctly in the respective steps. The only other way around it would be to have longer titles, i.e. Introduction to step-1, Results of step-1 but that would be somewhat tedious and it works anyways.
  • @copybrief/@copydetails or @copydoc target not found it warns that it has not found it but inserts the correct documentation anyways so this is somewhat spurious
  • has @param documentation sections but no arguments I just found out and check that this is a bug in doxygen 1.9.5, so we could bump the version in the indent.yml to 1.9.6 instead.
  • explicit link request to two were actual errors I just found. One referenced dealii::Vector but the namespace is removed for doxygen so that is completely correct. The other was in the changelogs and missing the VectorTools namespace as it was VectorTools::point_values()/::point_gradients() and so point_gradients() was flagged as unknown. The final error says it's located in deprecated:77 which is not a file I was able to find so I have no idea what the problem is there.
    Should I or someone else find that reference than it might make sense to remove that warning from the suppressed list.

@jpthiele
Copy link
Contributor Author

Update 1:
The multiple use of section label errors are relatively easy to fix by doing a search for <a name=" and replace it by <a name="XY for step-XY or with<a name="ABC-XYZ" for changes from version A.B.C to X.Y.Z.
I'd be happy to open a PR and change this in all existing files.
The question to me is whether the changes header files are auto generated. If yes I would also update the script to add these prefixes.

@jpthiele
Copy link
Contributor Author

Update 1: The multiple use of section label errors are relatively easy to fix by doing a search for <a name=" and replace it by <a name="XY for step-XY or with<a name="ABC-XYZ" for changes from version A.B.C to X.Y.Z. I'd be happy to open a PR and change this in all existing files. The question to me is whether the changes header files are auto generated. If yes I would also update the script to add these prefixes.

Also happy to add - or _ after the prefixes to increase readability. Just let me know what you think because I would prefer not to edit all these files multiple times.

@jpthiele
Copy link
Contributor Author

jpthiele commented Sep 22, 2023

Update 2:
Some of the @copydoc etc. warnings can be fixed e.g. by removing dealii:: or template parameters.
The big problem which seems to greatly confuse Doxygen is the large number of Triangulation classes.
I get a lot of those errors in the tria_base.h files. Adding more specialized names like parallel::Triangulation shifted the error from recursive declaration to not found.

Edit: This will be fixed by #16069. For tria_base.h the copydoc had to be changed to directly point to the header file of dealii::Triangulation

@jpthiele
Copy link
Contributor Author

jpthiele commented Sep 29, 2023

Due to #16051 the anchors warning should not have to be suppressed anymore.

Edit: Some were missed but will be fixed by #16067

@jpthiele
Copy link
Contributor Author

jpthiele commented Sep 29, 2023

As fixes to the warnings are now split between multiple self-contained pull requests I will rebase and check if all warnings except for the already suppressed graph size warnings are removed then.
-> Switching to draft for now

@bangerth
Copy link
Member

@jpthiele What's the status here?

@jpthiele
Copy link
Contributor Author

The question is still the chicken and egg problem with #16069 as it needs a newer doxygen version to
fix all copydoc reference warnings.
I'd propose to suppress those warnings here for now so it can be merged and then remove the suppression
in the other PR after merging the master branch with this PR in it.

If that is okay I'll add the suppressions at the latest on Thursday.

@jpthiele jpthiele marked this pull request as ready for review October 31, 2023 16:49
Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

This is ok with me, but I don't know the CI infrastructure well. Can someone else take a look at this?

@masterleinad masterleinad merged commit 92a1839 into dealii:master Oct 31, 2023
11 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants