Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If two different handlers are used in one document, the jinja env is not updated #502

Closed
jdpatt opened this issue Jan 4, 2023 · 3 comments · Fixed by #507
Closed

If two different handlers are used in one document, the jinja env is not updated #502

jdpatt opened this issue Jan 4, 2023 · 3 comments · Fixed by #507
Labels
bug Something isn't working

Comments

@jdpatt
Copy link
Contributor

jdpatt commented Jan 4, 2023

Found another issue/question with what I was trying to do with a custom handler.

If one document has two identifiers that use different handlers; only the first one's env is updated. My templates error out because the default mkdocstring filters are missing (highlight, heading or convert_markdown) don't exist in the second handler yet and instead I only have the built-in jinja filters and the any filter that is added when the env is initialized.

::: mkdocstrings_handlers.custom_handler.other_module.AccessTypes
    handler: python

::: demo.custom_identifier.stuff
    handler: custom_handler

It looks like these few lines are the offenders and I'm curious if there is a drawback to updating it every time, updating the env on init or make the _updated_env a part of the handler and not the extension? Commenting out self._updated_env = True is enough to resolve this issue.

        if not self._updated_env:
            log.debug("Updating renderer's env")
            handler._update_env(self.md, self._config)  # noqa: WPS437 (protected member OK)
            self._updated_env = True
@jdpatt jdpatt added the unconfirmed This bug was not reproduced yet label Jan 4, 2023
@pawamoy
Copy link
Member

pawamoy commented Jan 5, 2023

Hey, nice catch! Indeed it does make sense to hold that _updated_env boolean at the processor level. Instead, each handler should have its own (or none, but I recall adding this check to avoid updating Jinja envs again and again). Do you want to open a PR?

@pawamoy pawamoy added bug Something isn't working and removed unconfirmed This bug was not reproduced yet labels Jan 5, 2023
@jdpatt
Copy link
Contributor Author

jdpatt commented Jan 5, 2023

Yup, I can do that!

@jdpatt
Copy link
Contributor Author

jdpatt commented Jan 6, 2023

Introduced in #201

pawamoy pushed a commit that referenced this issue Jan 11, 2023
Instead of using a single boolean,
record which handler's env has been update with a set.

PR #201: #201
Issue #502: #502
PR #507: #507
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants