Improve compatibility with other MkDocs plugins that use jinja2 templating

[timvink]

I propose to change the default behaviour of the jinja2 environment .render() in PR https://github.com/fralau/mkdocs_macros_plugin/pull/116 :

The current behaviour of mkdocs_macros_plugin is to (silently) replace an unknown jinja2 variable with an empty string.

For example {{ foobar }} in a markdown document will be replaced by ""

This PR changes that behaviour and leaves the jinja string ({{ foobar }} --> {{ foobar }})

Context is that other plugins might also define custom jinja2 variables. See for example timvink/mkdocs-git-authors-plugin#60

According to @fralau :

There are a few general questions we need to solve, and there might be more than one update.

Let's discuss!

[github-actions[bot]]

Welcome to this project and thank you!' first issue

[fralau]

Yes, that's likely a good step in the direction of solving a key problem: the interference between mkdocs-macros and another plugin X that uses jinja2 statements. @timvink , let me know if what follows makes sense to you?

It has been a catch-22:

• If mkdocs-macros is declared later in the list of plugins,the default behavior of plug-in X is that it will replace the non-existent variables with blanks (and therefore mkdocs-plugin will have nothing to work with); and plug-in X will crash if it finds undefined functions or filters. Either way, it's bad.
• if mkdocs-macros is declared earlier, it will replace the variables expected by the plugin X with blanks, and plug-in X will do nothing.

Some projects have decided to use a different syntax for mkdocs-macros statements (eg. replacing '{}' with '[]'), to avoid those interferences with another plugin. It's a solution that works, but it's a touch decision. Another possible solution (which I would of course encourage), would be to rewrite the plugin as an mkdocs-macros module or, even better, as as distributable mkdocs-macros pluglet: that could be quickly and easily done. But there are many plugins already in use and tested, so we need to take care of the ecosystem.

At least, if we defined a sensible default behavior (that mkdocs-macros will leave in place the Jinja2 statements that contain unknown variables), it would leave a chance for plugin X (if declared later) to work as expected.

There would be an additional, positive side-effect: if someone mis-typed so variable in the context of mkdocs-macros, mkdocs would now react by printing the HTML with the jinja2 statement as-is, instead of a blank. That seems a much better behavior, because now anyone who reads the page would notice this artifact, and know that something has gone wrong.

Caveat: The one thing I would like to make sure, is that this evolution will not create issues for some existing projects -- who knows: they might count on this feature that unknown variables are ignored. Nothing really bad would happen to them: mkdocs-macros would likely not crash, but users would start complaining that strange stuff (uninterpretd Jinja2 statements) appeared in some places! I guess it would be good to at least let know the biggests projects on github that use mkdocs-macros, to give them a chance to express their view on this (if they have one). In the worst case, we could add a parameter in the config file, that would keep the old behavior?

[eyllanesc]

@fralau I think that a possible solution is to establish a parameter in config (for example strict: false) that by default keeps things as they are, there would be no complaints but to point out in the changelog that in future versions it will be changed so that it is true by default (or it will simply remove that configuration by making the new feature whatever it is by default). This will give you a warning and the change will not be abrupt.

[fralau]

@eyllanesc Yes, that might be a good solution that would be acceptable, for both the needs of @timvink (for the authors plugin), and for the needs of other projects?

Maybe we should find a name for the parameter that is more expressive 🤔 ("strict" might be ambiguous?). Perhaps something like skip_unknown: true?

[fralau]

For the record, here is an explanation (at least for me) of what the PR #116 is doing in case of misinterpreted code.

If it works as I understand it, that would make mkdocs-macros more robust in case of misinterpreted code (e.g. something that looks like jinja2 but is not) , since mkdocs-macros would not bomb and the page would still be rendered? 🤔

[timvink]

I agree on the catch-22 described, however it's the responsibility of each plugin to make sure it plays well with others. In some of my plugins, I raise warnings or errors when plugin order is incorrect. (and as an aside, you can also replace jinja2 variables via regex replacement, which is what I do in some of my plugins).

Changing the jinja2 syntax, macros modules and pluglets, using search & replace, are indeed all valid solutions, but I prefer leaving the unknown tags as-is for the reasons you describe: users get visual feedback & other plugins will keep working.

I don't agree with hiding the 'leave-as-is instead of silently replacing by blank' feature behind an option you need to enable, or waiting a release cycle before enabling it. Adds cognitive overhead for users, and enabling it by default should be the sensible setting for 99.99% of users (it doesn't break any sites). Actually, I don't see a scenario (yet) where silently replacing unknown jinja2 variables with blanks would be preferred.

Perhaps we can add a variable warn_if_unknown that defaults to false (leaves jinja2 variable as is) and when set to true, will also print a mkdocs warning in the console.

Do you know of any other plugins that jinja2 environments that might conflict with mkdocs-macros ? Like I said, mine use search-and-replace, but maybe I can provide upstream PRs to those plugins as well (once we agree on the best approach here).

[fralau]

@timvink Noted about your advice that we should activate this feature. It's good that we are making a survey of the viewpoints.

For compatibility with other plugins, I admit that I worked so far with a "whitelist" notion of saying which one were known to work. Of course, if mkdoc-macros starts become really more tolerant, we could perhaps turn around a make a list of those that are not compatible... and perhaps try to influence them, if possible and sensible.

[fralau]

Giving the heads up for that change of behavior to contributors of some popular projects that use mkdocs-macros, just in case they might have some view on it: @owenrumney @liamg @pdeligia @jaypipes @mattmoor @efekarakus ?

[pdeligia]

Giving the heads up for that change of behavior to contributors of some popular projects that use mkdocs-macros, just in case they might have some view on it: @owenrumney @liamg @pdeligia @jaypipes @mattmoor @efekarakus ?

+@lovettchris

[liamg]

We're fine with this for tfsec etc. - @itaysk Do you know if this is an issue for any other Aqua projects?

[itaysk]

@itaysk Do you know if this is an issue for any other Aqua projects?

AFAIK it won't affect us, thanks for checking. and generally it sounds like preferable behavior to me

[fralau]

Since there is consensus that compatibility with other plugins is key and I have not seen a vigorous objection to the new behavior (i.e. thinking that it will impair an existing website), I am considering to implement the new behavior as the default. That would mean passing PR #116.

For the question of a strict option that will cause an error (for testing, or to reduce cognitive overhead): is there still a request for it?

[timvink]

A possible improvement on the strict parameter could be to call it something like unknown_jinja_tag with the possible values of "ignore" (default), "warn" or "error".

For me, it's not needed..tbh I don't use mkdocs-macros-plugin for any projects but I do want it to be compatible with my plugins :)

[fralau]

@timvink Both the motivaton for your interest and your solution make sense!

[timvink]

Closing as #116 has been merged.

@fralau do you have any plans for when to make a new release to PyPi ? ;)

[fralau]

Thanks. That's technically solved but I have been working on it to release a new version with the arguments. It requires some work to get it right but I made some good progress.

[fralau]

New version was pushed. For all who are following up, let me know if it works for you and I will deploy it on pypi?

[fralau]

New version v.0.6.4 pushed on pypi