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

wrap page/article content in blocks for easier extension of templates #141

Closed lpirl closed 4 years ago

lpirl commented 4 years ago

Dear @mosra,

thanks for this great theme.

This PR wraps, where useful, all [page|article].contents in Jinja blocks for easier (as in "less verbose") extension/customization/overriding of templates.

An example use case is to build a custom index page using the page.html template. Say we want to add some Jinja-powered content before the rendered page.content itself. In the current revision, we'd have to copy the whole main block. Over time, we'd have to maintain this copy in order to stay in sync with the theme's development. This use case could be achieved way less verbose if we could extend page.html and only override the part where the rendered content is inserted.

codecov[bot] commented 4 years ago

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #141      +/-   ##
===========================================
+ Coverage   98.01%   100.00%   +1.98%     
===========================================
  Files          27         1      -26     
  Lines        6459       353    -6106     
  Branches       44        44              
===========================================
- Hits         6331       353    -5978     
+ Misses        128         0     -128     
Impacted Files Coverage Δ
documentation/python.py
plugins/m/abbr.py
plugins/m/dox.py
plugins/m/link.py
plugins/m/images.py
plugins/m/sphinx.py
plugins/m/gh.py
plugins/m/vk.py
plugins/m/filesize.py
documentation/doxygen.py
... and 16 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 830352f...4d1a78e. Read the comment docs.

mosra commented 4 years ago

All tests passing, a completely non-intrusive change, what's not to like :)

lpirl commented 4 years ago

Absolutely fabulous. :) thx a lot @mosra

mosra commented 4 years ago

Wait, no, this definitely didn't pass tests. WTH, github, why do you show a green tick here when it all failed?!

jinja2.exceptions.TemplateAssertionError: block 'article_content' defined twice

mosra commented 4 years ago

Reverting the commit in 3cd5a21840066184c87c65901aad9b56322a0e39 because I currently don't have the time to dig into it. Plus apart from the test failures there's some unwanted whitespace change (stray empty line) that also shouldn't be there. Can you submit this again and ensure tests pass? ;) Not sure why Travis doesn't run the PR properly, I'm tired of their recurring bugs.

cd pelican-theme
python -m unittest

53 tests or so, it should all pass.

lpirl commented 4 years ago

Good spot! And thanks GH… Introducing a macro (which then expands to the content block multiple times) seems to fix things. Local tests are all green. Some more tests failed due to newlines introduced by this PR; removed those newlines now using Jinja whitespace control.

New PR opened as #142.

(Comments edited for cleanup)