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

doxygen: rewrite set_canonical_doxygen.py in Perl #8736

Merged
merged 3 commits into from
Sep 30, 2019

Conversation

fvanmaele
Copy link
Contributor

@fvanmaele fvanmaele commented Sep 11, 2019

Fixes issue #8735


The Perl script ends up slightly longer than the Python version. Using Tie::File would add brevity, it however has issues with UTF-8 encoded files. Most doxygen files are ascii encoded, but on my system (Ubuntu 18.04) there were some utf8 encoded as well.

The Perl version also correctly appends the canonical link precisely once; the Python version would not do this, due to setting the canonical_link_added inside the loop to False at every iteration.

I wasn't sure on style guidelines for Perl scripts, and as far as I know clang-format only applies to C++ source files. As such, I tried following the style of the other Perl scripts in the doc/doxygen/scripts directory.


This is a draft pull request since I'm still building and comparing all doxygen created files between set_canonical_doxygen.py and set_canonical_doxygen.pl (and some remarks pointed out below).

@fvanmaele
Copy link
Contributor Author

fvanmaele commented Sep 11, 2019

I've compared two copies of the dealii directory, one with this PR applied, and another from master (dealii-orig).

Apart from header.html, the generated files are identical between set_canonical_doxygen.py and set_canonical_doxygen.pl. (Full diff with path changes.)

@fvanmaele fvanmaele marked this pull request as ready for review September 11, 2019 14:07
The script contrib/utilities/set_canonical_webpages.py is missing in
the dealii source tree.
@drwells
Copy link
Member

drwells commented Sep 30, 2019

I am not fluent in perl but the script looks reasonable and produces the correct output. Since this is just a doc change lets merge.

@drwells drwells merged commit fb64fdc into dealii:master Sep 30, 2019
@fvanmaele
Copy link
Contributor Author

fvanmaele commented Oct 3, 2019 via email

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

2 participants