mosra / m.css

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

Always set param.type to a valid string #171

Closed TheLartians closed 4 years ago

TheLartians commented 4 years ago

Hi and thanks for this great theme! We're currently working on using m.css with Doxygen in the ModernCppStarter project as it's a great improvement to the standard Doxygen output.

While adding support, I've noticed that the doxygen.py script can crash under certain conditions (specifically when M_SHOW_UNDOCUMENTED is set), as the script assumes param.type to be a valid string. However, as parse_type() returns either a string or None, this is not always the case.

This PR will fallback to an empty string when parse_type() returns None.

codecov[bot] commented 4 years ago

Codecov Report

Merging #171 into master will increase coverage by 1.74%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #171      +/-   ##
===========================================
+ Coverage   98.25%   100.00%   +1.74%     
===========================================
  Files          27         1      -26     
  Lines        6647       353    -6294     
  Branches       44        44              
===========================================
- Hits         6531       353    -6178     
+ Misses        116         0     -116     
Impacted Files Coverage Δ
plugins/m/gh.py
plugins/m/code.py
plugins/m/vk.py
plugins/m/link.py
documentation/python.py
plugins/ansilexer.py
plugins/m/alias.py
documentation/test/_search_test_metadata.py
plugins/m/qr.py
documentation/_search.py
... and 15 more

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 e6eff54...1bf162b. Read the comment docs.

mosra commented 4 years ago

Hi, first of all thank you for the appreciation and sorry for the extreme delay in replying to this, I'm very busy this year. :see_no_evil:

Could you give me a small snippet that triggers the crash? So I could include it as a regression test case. I tried to build the ModernCppStarter project but it didn't result in the crash and I'm not exactly sure what code could make this happen.

Thanks again.

TheLartians commented 4 years ago

Hey thanks for the repose! Interestingly, I also can't reproduce the crash anymore. Maybe the issue was caused by a different dependency that has been updated in the meantime. 🤔

I'll be doing some more testing, but it seems that the issue has resolved itself and this PR can be closed! If I remain unable to reproduce the error, I'll switch my starter to the official repo of m.css.

On a side note, would it be possible to create releases of m.css? This would allow us to refer to specific versions, as opposed to commit hashs.

mosra commented 4 years ago

Thanks. I even got the 1.8.18 version to test on that one, but no difference.

I'm maintaining releases for Magnum which is my main project, but even there, due to "too much to do too little time", there's only one or two huge releases a year. So due to time constraints I'm afraid I can't afford to do do proper versioning and packaging for this project, sorry.

TheLartians commented 4 years ago

So I've updated the starter to use the most recent commit here and the GitHub Workflow also completed without errors. So I guess the issue has been resolved somewhere else and we can close this PR!

I'm maintaining releases for Magnum which is my main project, but even there, due to "too much to do too little time", there's only one or two huge releases a year. So due to time constraints I'm afraid I can't afford to do do proper versioning and packaging for this project, sorry.

That I can relate to! We'll stick with commit hash for now then. Anyways, thanks for your time and great work on the magnum library! :)