Use on_env event instead of on_page_read_source()

[timvink]

Hi!

I'm the developer of mkdocs-table-reader-plugin, and I got a PM from a user saying it doesn't work together with mkdocs-markdownextradata-plugin.

table-reader (and other plugins like mkdocs-git-authors-plugin) add jinja tags to the markdown file as well, for example {{ read_csv() }}. Enabling together with markdownextradata results in the following error:

jinja2.exceptions.UndefinedError: 'read_csv' is undefined

You can find a reproducable example in timvink/mkdocs-table-reader-plugin#7

I think the root cause are in these lines:

mkdocs-markdownextradata-plugin/markdownextradata/plugin.py

Lines 95 to 97 in 35e9128

	with open(page.file.abs_src_path, 'r', encoding='utf-8-sig', errors='strict') as f:
	md_template = Template(f.read())
	return md_template.render(**config.get("extra"))

Have you considered using the on_env() mkdocs event instead of on_page_read_source()? You can use it to alter the jinja2 environment, making all the info from extra: available. It might be cleaner than re-implementing core functionality from MkDocs, it might improve compatiblity with other plugins, and I suspect it will also solve #21.

On my side, I use on_page_markdown() because I initially had trouble getting the custom jinja filter (basically adding functions instead of variables to jinja) to work, but perhaps I should research again. Curious to hear your perspective on this :)

[rosscdh]

Yes I've been looking at the same code and came to the same conclusion. Thanks for getting involved. The change was introduced in a recent PR I thought it good as it went in at a lower level, however seems to be causing more problems than its worth. I'll reimplement using your suggestion and see how it goes!

…

On Wed, Jun 24, 2020 at 8:34 PM Tim Vink ***@***.***> wrote: Hi! I'm the developer of mkdocs-table-reader-plugin <https://github.com/timvink/mkdocs-table-reader-plugin>, and I got a PM from a user saying it doesn't work together with mkdocs-markdownextradata-plugin. table-reader (and other plugins like mkdocs-git-authors-plugin <https://github.com/timvink/mkdocs-git-authors-plugin>) add jinja tags to the markdown file as well, for example {{ read_csv() }}. Enabling together with markdownextradata results in the following error: jinja2.exceptions.UndefinedError: 'read_csv' is undefined You can find a reproducable example in timvink/mkdocs-table-reader-plugin#7 <timvink/mkdocs-table-reader-plugin#7> I think the root cause are in these lines: https://github.com/rosscdh/mkdocs-markdownextradata-plugin/blob/35e91288a97aee8b4302c4e06b140c21339c4ca0/markdownextradata/plugin.py#L95-L97 Have you considered using the on_env() mkdocs event <https://www.mkdocs.org/user-guide/plugins/#on_env> instead of on_page_read_source()? You can use it to alter the jinja2 environment, making all the info from extra: available. It might be cleaner than re-implementing core functionality from MkDocs, it might improve compatiblity with other plugins, and I suspect it will also solve #21 <#21>. On my side, I use on_page_markdown() because I initially had trouble getting the custom jinja filter (basically adding functions instead of variables to jinja) to work, but perhaps I should research again. Curious to hear your perspective on this :) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#24>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADA6MS44QO6CN5ADDSZLHDRYJBLBANCNFSM4OHBS6JQ> .

[rosscdh]

I originally implemented the plugin using on_page_markdown however some users wanted a earlier processing of the tokens. Naja live and learn!

…

On Wed, Jun 24, 2020 at 10:44 PM Ross ***@***.***> wrote: Yes I've been looking at the same code and came to the same conclusion. Thanks for getting involved. The change was introduced in a recent PR I thought it good as it went in at a lower level, however seems to be causing more problems than its worth. I'll reimplement using your suggestion and see how it goes! On Wed, Jun 24, 2020 at 8:34 PM Tim Vink ***@***.***> wrote: > Hi! > > I'm the developer of mkdocs-table-reader-plugin > <https://github.com/timvink/mkdocs-table-reader-plugin>, and I got a PM > from a user saying it doesn't work together with > mkdocs-markdownextradata-plugin. > > table-reader (and other plugins like mkdocs-git-authors-plugin > <https://github.com/timvink/mkdocs-git-authors-plugin>) add jinja tags > to the markdown file as well, for example {{ read_csv() }}. Enabling > together with markdownextradata results in the following error: > > jinja2.exceptions.UndefinedError: 'read_csv' is undefined > > You can find a reproducable example in > timvink/mkdocs-table-reader-plugin#7 > <timvink/mkdocs-table-reader-plugin#7> > > I think the root cause are in these lines: > > > https://github.com/rosscdh/mkdocs-markdownextradata-plugin/blob/35e91288a97aee8b4302c4e06b140c21339c4ca0/markdownextradata/plugin.py#L95-L97 > > Have you considered using the on_env() mkdocs event > <https://www.mkdocs.org/user-guide/plugins/#on_env> instead of > on_page_read_source()? You can use it to alter the jinja2 environment, > making all the info from extra: available. It might be cleaner than > re-implementing core functionality from MkDocs, it might improve > compatiblity with other plugins, and I suspect it will also solve #21 > <#21>. > > On my side, I use on_page_markdown() because I initially had trouble > getting the custom jinja filter (basically adding functions instead of > variables to jinja) to work, but perhaps I should research again. Curious > to hear your perspective on this :) > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#24>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AADA6MS44QO6CN5ADDSZLHDRYJBLBANCNFSM4OHBS6JQ> > . >

[rosscdh]

I've reverted and released 0.1.7 https://pypi.org/project/mkdocs-markdownextradata-plugin/
Thanks for getting involved! Happy to discuss the suggested option when I get more time

[timvink]

Wow, that was quick, thanks! And it solves the compatibility issue (unit tests now passing: https://github.com/timvink/mkdocs-table-reader-plugin/actions/runs/147141623)

On second thought, I'm not sure if on_env() is the best way to go, I can't get it to work. For reference, here's my attempt:

# Add a {{ foo }} tag to a markdown page
# And following methods to your plugin
def on_env(self, env, **kwargs):
    """
    Modify the jinja2 environment. 
    Docs: https://jinja.palletsprojects.com/en/2.11.x/api/#jinja2.Environment
    """
    
    env.globals['foo'] = "bar_from_on_env"
    return env

def on_template_context(self, context, **kwargs):
    
    context["foo"] = "bar_from_template_context"
    return context

def on_page_context(self, context, **kwargs):
    
    context["foo"] = "bar_from_page_context"
    return context

Feel free to close the issue.

[rosscdh]

I've not really had a lot of time to investigate on_env tbh (briefly last night but the tests failed so pass), but there is potential for a change, but there are so many things that can go wrong fiddling around at that level. In the end, like most things it can't be everything to everyone; and is not trying to be.. "inject variable values into markdown, and mkdocs much more than it was originally envisioned to be".. done :) Grand that you are involved appreciate the input!

…

On Thu, Jun 25, 2020 at 9:55 AM Tim Vink ***@***.***> wrote: Wow, that was quick, thanks! And it solves the compatibility issue (unit tests now passing: https://github.com/timvink/mkdocs-table-reader-plugin/actions/runs/147141623 ) On second thought, I'm not sure if on_env() is the best way to go, I can't get it to work. For reference, here's my attempt: # Add a {{ foo }} tag to a markdown page# And following methods to your plugindef on_env(self, env, **kwargs): """ Modify the jinja2 environment. Docs: https://jinja.palletsprojects.com/en/2.11.x/api/#jinja2.Environment """ env.globals['foo'] = "bar_from_on_env" return env def on_template_context(self, context, **kwargs): context["foo"] = "bar_from_template_context" return context def on_page_context(self, context, **kwargs): context["foo"] = "bar_from_page_context" return context Feel free to close the issue. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#24 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADA6MWAUX2FCILLO5Y4OZTRYL7FLANCNFSM4OHBS6JQ> .