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

Docs: update doxygen support #1622

Closed
wants to merge 3 commits into from
Closed

Conversation

yadij
Copy link
Contributor

@yadij yadij commented Dec 17, 2023

Doxygen v1.7.5 changed HTML_HEADER requirements: We now need to add an
open div element to the header that Doxygen then closes for us. This
change also fixes the corresponding generated content on Squid website.

Also, disable IDL_PROPERTY_SUPPORT which has been causing some errors
with output getter/setter matching in the generated output.

Disable special IDL processing which mangles
some of our code badly, hiding getter/setter's
for some classes.

Enable Markdown .md files to be used instead
of our custom .dox files. This requires
doxygen 1.8.1 or later with Markdown enabled.
@yadij yadij changed the title Docs: update doxygen supprot Docs: update doxygen support Dec 17, 2023
kinkie
kinkie previously approved these changes Dec 17, 2023
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Please share a pointer to the resulting diff exposing changes in Doxygen-generated content.

squid.dox Outdated Show resolved Hide resolved
@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Dec 18, 2023
Co-authored-by: Alex Rousskov <[email protected]>
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Dec 28, 2023
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

Please share a pointer to the resulting diff exposing changes in Doxygen-generated content.

squid.dox Outdated Show resolved Hide resolved
@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Dec 28, 2023
@yadij yadij added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Dec 29, 2023
@yadij yadij requested a review from rousskov December 29, 2023 10:27
@yadij
Copy link
Contributor Author

yadij commented Dec 29, 2023

Please share a pointer to the resulting diff exposing changes in Doxygen-generated content.

The first change; to the doxygen.header.dyn file is 7154 chunks identical to the PR diff for doxygen.header.dyn - one for every .dyn produced in the output. These fix the display bug(s) you can see (for now) by visiting http://www.squid-cache.org/Doc/code/ :

  • the site menu is rendered outside the main content column area, and
  • the navmenu generator scripts abort with an error updating the page DOM. Leaving no tabbed menu for visitors to browse the code docs with.

The second change; to the squid.dox file is a bit subtle. The bug it fixes shows up in some of the images and reference lists for members, only for output produced by recent doxygen versions where the IDL hints are enabled by default.
It can be seen in the doxygen.log error/warnings list as entries like this:

/src/squid/yadij-git/src/auth/basic/Config.cc:70: warning: no uniquely matching class member found for
  const char * Auth::Basic::Config::type() const
Possible candidates:
  'int snmp_mib_tree::type' at line 56 of file /src/squid/yadij-git/include/parse.h
  'u_char variable_list::type' at line 48 of file /src/squid/yadij-git/include/snmp_vars.h
  'int32_t _ntlmhdr::type' at line 82 of file /src/squid/yadij-git/lib/ntlmauth/ntlmauth.h
  'int node::type' at line 109 of file /src/squid/yadij-git/lib/snmplib/parse.c

... where the candidates lists every class where the member name is used by a getter OR setter.

After this PR disabling the hints, doxygen correctly lists more of these candidate lists narrowing it down to only getter or setter. Am not sure why or what is going on - but we are not producing IDL hint files so disabling that unused feature entirely for now.

@kinkie
Copy link
Contributor

kinkie commented Jan 7, 2024

ok to test

@squid-anubis squid-anubis added M-failed-other https://github.com/measurement-factory/anubis#pull-request-labels and removed M-failed-other https://github.com/measurement-factory/anubis#pull-request-labels labels Jan 9, 2024
@yadij
Copy link
Contributor Author

yadij commented Jan 9, 2024

header regenerated using:

doxygen -w html header.out footer.out style.css squid.dox

@rousskov rousskov dismissed their stale review January 9, 2024 20:55

A meeting discussion addressed my concern.

@rousskov rousskov removed their request for review January 9, 2024 20:55
@rousskov rousskov removed the S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box label Jan 9, 2024
@rousskov
Copy link
Contributor

rousskov commented Jan 9, 2024

@yadij, I have adjusted PR description to (IMHO) better describe what happened in this PR, but I do not insist on those adjustments. Please feel free to merge with them, without them, or with some further corrections/edits. Thank you.

@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Jan 9, 2024
squid-anubis pushed a commit that referenced this pull request Jan 9, 2024
Doxygen v1.7.5 changed HTML_HEADER requirements: We now need to add an
open div element to the header that Doxygen then closes for us. This
change also fixes the corresponding generated content on Squid website.

Also, disable IDL_PROPERTY_SUPPORT which has been causing some errors
with output getter/setter matching in the generated output.
@squid-anubis squid-anubis added M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-passed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Jan 9, 2024
@yadij yadij added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Jan 11, 2024
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-passed-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants