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 https://github.com/timvink/mkdocs-table-reader-plugin/pull/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 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 notifications@github.com 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 https://github.com/timvink/mkdocs-table-reader-plugin/pull/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 https://github.com/rosscdh/mkdocs-markdownextradata-plugin/issues/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 https://github.com/rosscdh/mkdocs-markdownextradata-plugin/issues/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 sendrossemail@gmail.com 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 notifications@github.com 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 https://github.com/timvink/mkdocs-table-reader-plugin/pull/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 https://github.com/rosscdh/mkdocs-markdownextradata-plugin/issues/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 https://github.com/rosscdh/mkdocs-markdownextradata-plugin/issues/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 notifications@github.com 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 https://github.com/rosscdh/mkdocs-markdownextradata-plugin/issues/24#issuecomment-649333617, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADA6MWAUX2FCILLO5Y4OZTRYL7FLANCNFSM4OHBS6JQ .