Do not resolve up when setting a variable in macro. #653
Conversation
| @@ -7,10 +7,11 @@ var Obj = require('./object'); | |||
| // we know how to access variables. Block tags can introduce special | |||
| // variables, for example. | |||
| var Frame = Obj.extend({ | |||
| init: function(parent) { | |||
| init: function(parent, inMacro) { | |||
There was a problem hiding this comment.
What do you think about using a more generic name for this new parameter; something like isolate? It seems likely to me that this same parameter might prove useful in solving the other portions of #561, in which case the name inMacro might no longer apply.
There was a problem hiding this comment.
Though since it's internal, we can also leave it as-is for now and rename it later if necessary; I'm fine with that if you prefer.
|
This is great!! Thank you so much for all your work on nunjucks. I made a few small comments above. Also, while you're updating the PR anyway, maybe add an entry to changelog? Thanks! |
|
(Also, please comment here if you push another commit, GitHub doesn't notify when new commits are pushed to a PR.) |
|
Hi, @carljm. Thanks for your detailed comments which help a lot! :-) And I have add another test to make the test case more clear. Maybe you can add me as a collaborator and I would like to help to improve Chinese docs(merge some PRs or make some PRs by myself). Simply ignore it if you don't think that's appropriate . |
|
@oyyd Sure, I'd be happy to add you as a collaborator to work on Chinese docs. I'd prefer if you get a review from someone before merging code that touches |
|
Looks great, thank you! |
Do not resolve up when setting a variable in macro.
Do not resolve up when setting a variable in macro.
|
@carljm Thank you. And I will continue to do PRs in other situations. |
This PR should fix #577 and the "macro" part of issue #561.