mosra / m.css

A no-nonsense, no-JavaScript CSS framework, site and documentation theme for content-oriented websites
https://mcss.mosra.cz
Other
406 stars 92 forks source link

Duplicate detailed descriptions #104

Closed crisluengo closed 5 years ago

crisluengo commented 5 years ago

Here is a fix for two issues that, in hindsight, are not related, but I initially thought they were.

The first one is a duplication of detailed descriptions under both the namespace page and the group page, as previously discussed.

The second one is an issue where namespace members are ignored if they are added to a group using @ingroup. I think I might have been using @ingroup wrong, but Doxygen never complained about it, and produced the expected output. doxygen.py drops these members (ignores them with a warning).

The first commit is a test case that demonstrates these two issues. The second commit fixes doxygen.py. The first issue corresponds to the changes at lines 3364--3397. The second issue corresponds to the changes at lines 3181--3256.


All test cases fail on my computer, a Mac, due to differences in output files names (e.g. File vs _file). So I'm relying on the test results on Travis to see if all this goes well. I hope this doesn't break other tests.

crisluengo commented 5 years ago

Interesting... the undocumented test also shows the issue I'm fixing here. namespaceNamespace.html contains detailed descriptions that are not linked to, the links at the top of the file all point to other files.

I'll look into the rest of these test failures tomorrow, and will try to fix the tests so they run.

codecov[bot] commented 5 years ago

Codecov Report

Merging #104 into master will decrease coverage by 2.45%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
- Coverage   97.67%   95.22%   -2.46%     
==========================================
  Files          22       20       -2     
  Lines        4737     2115    -2622     
  Branches       40       40              
==========================================
- Hits         4627     2014    -2613     
+ Misses        110      101       -9
Impacted Files Coverage Δ
plugins/dot2svg.py 97.61% <0%> (-2.39%) :arrow_down:
documentation/python.py

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1dc7c2a...1ffd6b2. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #104 into master will decrease coverage by 0.07%. The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #104      +/-   ##
==========================================
- Coverage   97.67%   97.59%   -0.08%     
==========================================
  Files          22       22              
  Lines        4737     4748      +11     
  Branches       40       40              
==========================================
+ Hits         4627     4634       +7     
- Misses        110      114       +4
Impacted Files Coverage Δ
documentation/doxygen.py 99.43% <87.5%> (-0.19%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1dc7c2a...d29085a. Read the comment docs.

crisluengo commented 5 years ago

I've also found the Doxygen setting that controls output file names. Apparently the default is system-dependent. This means that by default, links will be different depending on what OS the docs were built on. I find this an odd choice...

I added a commit that fixes this setting to the default on Linux. However, the generated HTML is still not identical on my Mac. There are some additional empty lines and equations render differently as well (maybe a different font is used?). I have not put in the effort to try and fix these differences, they are likely irrelevant anyway.

mosra commented 5 years ago

Finally got to merge this ... actually turns out I only ever tested the Doxygen module support on plain files without any namespaces and so wrapping the modules in namespaces was a totally new thing the script didn't account for. After I understood the problem a bit more, I reworked your test cases, extended them to include also #defines (which exposed another "duplicate detailed docs" issue) and the fix was actually just checking for entry.base_url == compound.url for all entry types.

Besides that, your original patch (probably by mistake) removed the member groups without @name are not supported, ignoring warning -- that one is totally legit and not related to this problem. In your case that means you should use

/// @ingroup group1
/// @brief Foo
void foo();

/// @ingroup group1
/// @brief Bar
void bar();

instead of

/// @ingroup group1
/// @{

/// @brief Foo
void foo();

/// @brief Bar
void bar();

/// @}

-- the @{ adds an unnamed member group into the group1 module (which is not supported, as the warning says) and as a consequence everything inside that unnamed member group will go into the group1 module. If you would put @name inside the @{ you'd see a member group inside that module.

Important point is that the behavior of @ingroup is different from the @defgroup, which uses @{ for creating "a scope" for its members. I ... don't know why is it used so ambiguously, but, well. Doxygen.

Merged as eede7c0e563d183d95e5207de198cc30117a5ea7, d1961231589a1cdf7a742d1f55a8daaf4533dfd6, f705cea0e5106d43c4eba89cb0b03f59ec81598f, bd0de0bfe23ed85ff063039a56ed395d6779cc9e together with my 6669dfc93ade6767646f1336148e7592a259851c fixing the #defines and 18efda422e80688f04841ded7d987ef24a947d6d reverting a part of your fix after I realized the above difference with @{.

Let me know if there are any issues with what I did :)

crisluengo commented 5 years ago

You are right, I was using @ingroup where I should have been using @addtogroup, which puts all functions within the following @{...@} into an existing group. It seems that Doxygen is not very strict enforcing its own syntax. A lot of the things that I was doing wrong were producing the output I expected with plain Doxygen, so I didn't know I was doing them wrong. :)

The changes you made are correct, this is working properly for me. Many thanks!