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

Argument from caller macro is incorrectly passed through to callee macro? #478

Closed
jgerigmeyer opened this issue Jul 14, 2015 · 3 comments
Closed
Labels

Comments

@jgerigmeyer
Copy link
Contributor

If a macro is called (using {% call %}) with an argument, and the caller() is itself a macro that accepts an argument with the same name with a default of none, the argument from the caller macro is incorrectly passed through to the callee macro. That sounds ridiculously confusing, so here's the test case:

{% macro inner(class=none) %}
  <div class="{{ class }}"></div>
{% endmacro %}

{% macro outer(class=none) %}
  <div class="{{ class }}">
    {{ caller() }}
  </div>
{% endmacro %}

{% call outer(class='outer') %}
  {{ inner() }}
{% endcall %}

When rendered via jinja2, this is the output:

<div class="outer">
  <div class="None"></div>
</div>

However, when rendered via nunjucks, the output is:

<div class="outer">
  <div class="outer"></div>
</div>

This seems to only happen when the default for the inner arg is none; it doesn't occur when the default is '' or false.

@carljm
Copy link
Contributor

carljm commented Jul 17, 2015

Interesting! I'll look into this. I wonder if it's just arguments, or works for any variable of a matching name in the calling namespace...

@carljm
Copy link
Contributor

carljm commented Feb 17, 2016

This was partly fixed by #667 -- macro frames are now totally separated from the calling frame, so there's no longer any way for a macro to reach out and see or touch its calling frame. So you can no longer see this behavior with macro arguments with a default of none. Looking into whether it can still crop up in other situations...

@carljm carljm closed this as completed in ba5e514 Feb 17, 2016
carljm added a commit that referenced this issue Mar 14, 2016
* master: (88 commits)
  Bump versions for dev.
  Update maintenance docs.
  Revert accidental changes to mocha.js.
  Bump version to 2.4.0.
  Update changelog.
  Merge pull request #694 from mariusbuescher/master
  Update changelog.
  Add support for boolean globals
  Add note about include and blocks; update wrapping of templating docs.
  Merge pull request #688 from pra85/patch-1
  Add note about include and blocks; update wrapping of templating docs.
  Update api.md
  fixed bad character leading % in {% endraw %}
  Respect null/none as a value in its own right, distinct from undefined. Fixes #478.
  Use dot reporter for npm test.
  Don't bail on first failed test in 'npm test'.
  Add acknowledgement for #561 to changelog.
  Tighter scoping of vars in blocks, to match Jinja2. Fixes #561.
  Rename all test templates to use .j2 extension.
  Rename all test templates to use .j2 extension.
  ...
@jgerigmeyer
Copy link
Contributor Author

I'm still seeing this bug (or seeing it again?):

http://jsfiddle.net/jgerigmeyer/tro6jhy9/1/
http://jsfiddle.net/jgerigmeyer/6cL8wqd7/1/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants