stencilproject / Stencil

Stencil is a simple and powerful template language for Swift.
https://stencil.fuller.li
BSD 2-Clause "Simplified" License
2.34k stars 223 forks source link

extended context to enable implementation of swiftlike scoping rules … #198

Closed kaygro closed 4 years ago

kaygro commented 6 years ago

…(pushLocals function)

added a testcase for this new behavior modified ForTag to make use of this new mechanism

kylef commented 6 years ago

Hi @kaygro, I'm having a hard time trying to understand what you are trying to solve that is different from the push method on Context which already takes a closure. Could you perhaps elaborate and perhaps provided an example use-case where the regular push method does not suffice?

kaygro commented 6 years ago

If you want to to implement an extension, which has a set tag (as StencilSwiftKit for example), you want to be able to do mutations inside a for loop, which are visible outside of it. A resonable implementation of a set tag would do something like context[variableName] = value, but if that gets executed while rendering tags inside a for tag, it gets popped off afterwards when using regular push.

kaygro commented 6 years ago

Exampe Stencil code for this scenario would look like:

{% set foundElem %}false{% endset %}
{% for elem in array %}
  {% if satisfiesInterestingProperty elem %}
    {% set foundElem %}true{% endset %}
  {% endif %}
{% endfor %}
{# When normal push/pop is used, foundElem will always be "false" at this point #}
{% if foundElem == "true" %}
...
{% endif %}

I'd like to stress, that this modification does not implement a set tag, but merely enables implementation of a set tag, that works like this.

ilyapuchka commented 6 years ago

Not sure about this change, feels too invasive, also I think it can be implemented in a simpler way, but I didn't explore it enough to be sure. Also I think current behaviour is the same as in Jinja. Speaking of example shared, I believe it can be achieved by using filter on array and then checking its count.

{{ if array|yourCustomFilterThatChecksForPredicate|count > 0 }}

Of course you'll need to implement this filter and register it in environment, so there is no way to do it just with templates. Another way would be to use filter that I've implemented in #189

kaygro commented 6 years ago

Well, I think it can be implemented entriely from within the set node, but that would mean, the implementation of the set node would need to make copies at the various heights of the context stack, where that variable is visible, (for every variable change that is). It would be less cubersome, if we could observe/override the subscript operators of the Context class, but that is not possible due to access beeing public, not open and Context not being a protocol.

djbe commented 6 years ago

@kaygro I get what this is trying to achieve (seeing the issues with set), but I agree with the others that this might not be the best way to achieve this.

It might be better to have a mechanism in Context to directly modify the stack, so that set (for example) could modify a variable outside it's scope. For example modify the subscript operator here: https://github.com/stencilproject/Stencil/blob/master/Sources/Context.swift#L17 To accept a parameter to, instead of modifying the current level, search through the levels and modify the topmost one it finds (or throw an error if it doesn't find it?).

kaygro commented 6 years ago

I like that idea. I didn't realize, you could have multiple parameters in subscript operators.

ilyapuchka commented 4 years ago

Closing this due to inactivity.