fralau / mkdocs-macros-plugin

Create richer and more beautiful pages in MkDocs, by using variables and calls to macros in the markdown code.
https://mkdocs-macros-plugin.readthedocs.io
Other
318 stars 50 forks source link

fix for #215: Fix title and nav parsing #217

Closed AnvithLobo closed 3 months ago

AnvithLobo commented 5 months ago
fralau commented 5 months ago

Thank you for solving the conflicts.

I looked at your solution and I see elegance in it (modifications are well-localized). That is definitely an advantage.

What will matter, in final analysis, is whether it solves the issue or not, without creating new ones.

I have two objections at this stage:

  1. I see only processing of variables, in the rendering. What about expressions containing Jinja2 macros and filters?
  2. Do you have a theory of why it would be sufficient to intervene at the stage of on_nav, and it is no longer necessary at on_page_markdown? What about the processing of page variables (e.g. defined in the page's YAML front-matter)?
AnvithLobo commented 5 months ago

2. Do you have a theory of why it would be sufficient to intervene at the stage of on_nav, and it is no longer necessary at on_page_markdown? What about the processing of page variables (e.g. defined in the page's YAML front-matter)?

This is only for the nav items. Trying to render nav items from on_page_markdown is not ideal. As we would have to get the page and it's current nav then walk up the tree to find the first parent and then again walk down the tree all the way down to render the nav items. First this wouldn't be elegant nor efficient. As we would need to do this for every page. But If we only consider the last parent and try to render it's child. We will miss rendering any empty sections with no pages currently linked to that section.

The only disadvantage of rendering Nav items on on_nav instead of on_page_markdown is that we will miss out on any meta variables defined in the page. But this is more trouble than it is worth solving IMO.