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

Enable access to local variables in extensions #860

Closed
OddBloke opened this issue Jun 1, 2018 · 3 comments · Fixed by #861
Closed

Enable access to local variables in extensions #860

OddBloke opened this issue Jun 1, 2018 · 3 comments · Fixed by #861
Milestone

Comments

@OddBloke
Copy link
Contributor

@OddBloke OddBloke commented Jun 1, 2018

As far as I can tell, there is no way for extensions to access local variables from their call site, they can only access the template context (which, for example, excludes loop-local variables).

Template Code

Using the extension at https://review.openstack.org/#/c/568021/5/jenkins_jobs/jinja2_extension.py, and

{% set y = 'foo' %}
{% for x in [1, 2] %}
    {% set z = 'bar' %}
    {% for a in [1, 2] %}
        {% jjb_include_raw 'jinja_jjb_include_raw001.sh.inc' %}
    {% endfor %}
{% endfor %}

the context passed by a ContextReference in _render_template looks like:

>>> context
<Context {'range': <class 'range'>, 'dict': <class 'dict'>, 'lipsum': <function generate_lorem_ipsum at 0x7fb8d23a0840>, 'cycler': <class 'jinja2.utils.Cycler'>, 'joiner': <class 'jinja2.utils.Joiner'>, 'namespace': <class 'jinja2.utils.Namespace'>, 'name': 'test-include_jjb', 'param': 'value', 'template-name': 'test-job-{param}', '': '', 'builders': [OrderedDict([('shell', <jenkins_jobs.local_yaml.Jinja2Loader object at 0x7fb8d5e09198>)])], 'y': 'foo'} of None>
>>> pprint.pprint(dict(context))
{'': '',
 'builders': [OrderedDict([('shell',
                            <jenkins_jobs.local_yaml.Jinja2Loader object at 0x7fb8d5e09198>)])],
 'cycler': <class 'jinja2.utils.Cycler'>,
 'dict': <class 'dict'>,
 'joiner': <class 'jinja2.utils.Joiner'>,
 'lipsum': <function generate_lorem_ipsum at 0x7fb8d23a0840>,
 'name': 'test-include_jjb',
 'namespace': <class 'jinja2.utils.Namespace'>,
 'param': 'value',
 'range': <class 'range'>,
 'template-name': 'test-job-{param}',
 'y': 'foo'}

Note that y is in the context, but none of x, z or a are.

Your Environment

  • Python version: 3.6
  • Jinja version: trunk
@OddBloke

This comment has been minimized.

Copy link
Contributor Author

@OddBloke OddBloke commented Jun 1, 2018

For including of templates, the local variables are available because the return value of self.dump_local_context(frame) is passed in to root_render_func when with_context is True (around https://github.com/pallets/jinja/blob/master/jinja2/compiler.py#L918).

@OddBloke

This comment has been minimized.

Copy link
Contributor Author

@OddBloke OddBloke commented Jun 1, 2018

One solution would be to extend ContextReference to allow locals to be included (whilst still defaulting to the existing behaviour):

diff --git a/jinja2/compiler.py b/jinja2/compiler.py
index 5135a77..6cfe750 100644
--- a/jinja2/compiler.py
+++ b/jinja2/compiler.py
@@ -1670,7 +1670,10 @@ class CodeGenerator(NodeVisitor):
         self.write(node.name)
 
     def visit_ContextReference(self, node, frame):
-        self.write('context')
+        if node.include_locals:
+            self.write(self.derive_context(frame))
+        else:
+            self.write('context')
 
     def visit_Continue(self, node, frame):
         self.writeline('continue', node)
diff --git a/jinja2/nodes.py b/jinja2/nodes.py
index 5ab2b31..61f2dd6 100644
--- a/jinja2/nodes.py
+++ b/jinja2/nodes.py
@@ -943,6 +943,7 @@ class ContextReference(Expr):
         Assign(Name('foo', ctx='store'),
                Getattr(ContextReference(), 'name'))
     """
+    attributes = ('include_locals',)
 
 
 class Continue(Stmt):
@OddBloke

This comment has been minimized.

Copy link
Contributor Author

@OddBloke OddBloke commented Jun 1, 2018

An alternative would be to use a separate node type to provide the locals, but playing with that locally I just ended up doing the context.derive(locals) call myself; I don't know if there are many cases where extension authors would want the locals without the rest of the context, which is (I think) the only use case that approach would enable that the above diff doesn't.

OddBloke added a commit to OddBloke/jinja that referenced this issue Jun 1, 2018
This allows extensions to access locals in the scope from which they
were called, fixing pallets#860.
OddBloke added a commit to OddBloke/jinja that referenced this issue Sep 24, 2018
This allows extensions to access locals in the scope from which they
were called, fixing pallets#860.
@davidism davidism added this to the 2.11.0 milestone Dec 2, 2019
@davidism davidism closed this in #861 Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.