-
Notifications
You must be signed in to change notification settings - Fork 71
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
Proposition for a mechanism to add header/footer to sections. #147
base: master
Are you sure you want to change the base?
Conversation
Thanks @nminet! Before I go into a detailed review: where can I find the old discussion? It probably happened before my time. Once (if) we get beyond the point whether this feature should be in the spec, I suspect the devil will be in the details. I highly appreciate the elaborate rationale, though! |
Hi Julian
mustache/mustache#118 is one
Also interresting
https://stackoverflow.com/questions/11653764/mustache-how-to-detect-array-is-not-empty
It enumerates the main ad-hoc options
In my use cases I resort to data decoration (pre-prcessing before
rendering) as the host application does not allow executing unverified
incoming data.
Regards
Noel
…On Wed, Jun 7, 2023 at 5:18 AM Julian Gonggrijp ***@***.***> wrote:
Thanks @nminet <https://github.com/nminet>!
Before I go into a detailed review: where can I find the old discussion?
It probably happened before my time.
Once (if) we get beyond the point whether this feature should be in the
spec, I suspect the devil will be in the details. I highly appreciate the
elaborate rationale, though!
—
Reply to this email directly, view it on GitHub
<#147 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE7AN3TFEYXJP7CDKSBLHMDXJ6UD7ANCNFSM6AAAAAAY4W3IVQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi Again,
I definitely agree we need to dig in details.
At this time the really tricky area in my view is when a callable is in
section position, and the call has side effects. In that case repeating the
call is not trivial.
I believe it is ok to specify lambdas don't participate in detecting empty
sequences as the intended use of lambdas is to generate a modified template
and has only indirect effect on stack access.
For callables generating a context this is not so clear. Caching the call
result looks tempting but opens non-trivial questions on scoping rules
My current view is that we should recommend implementations provide a way
to activate the optional feature, but as the specification currently does
not address API (quite rightfully so) I am not sure it should be a
requirement here.
Anyway, thanks for your attention :-)
Regards
Noel
…On Wed, Jun 7, 2023 at 5:18 AM Julian Gonggrijp ***@***.***> wrote:
Thanks @nminet <https://github.com/nminet>!
Before I go into a detailed review: where can I find the old discussion?
It probably happened before my time.
Once (if) we get beyond the point whether this feature should be in the
spec, I suspect the devil will be in the details. I highly appreciate the
elaborate rationale, though!
—
Reply to this email directly, view it on GitHub
<#147 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE7AN3TFEYXJP7CDKSBLHMDXJ6UD7ANCNFSM6AAAAAAY4W3IVQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thanks for sharing the links. Regarding side effects, the problem is not really different from regular sections, is it? I mean, with regular sections, you might evaluate the same callable twice, too. It is still under control of the template author, either through template modifications or data preprocessing. Regarding frame-returning callables and template-returning lambdas, I think they can be united under the same rule, given that a template-returning lambda does not return a nonempty list by definition. I'm not too worried about this bit, either. The one thing that worries me, is whether this use case should be facilitated by the template language in the first place. Here is an attempt at a comprehensive list of ways to approach the use case, most of which appeared in the discussions you linked to: Edit: there used to be a table here, but I moved it to #157. Please use that ticket for further discussion about possible alternatives to the current proposal. |
Minor typo on the closing tag above (approach 5) |
No, that was intentional. This is one of the details that I thought would have the devil in it. In general, I interpret the spec as follows: take all text between the sigil and the end delimiter. Strip leading and trailing whitespace from this text. What remains is the tag's content. When a tag needs a matching End Section tag, the latter must have the exact same content. So I match tags as below, and this is how Wontache implements it (playground): {{#name}}{{/name}}
{{#nested.key}}{{/nested.key}}
{{<*nested.key}}{{/*nested.key}} Quote-sniping two parts of the spec that I think are specifically relevant: Lines 7 to 9 in 5d3b58e
Seems to confirm that the End Section tag matching a Parent tag must have the same content. Lines 166 to 173 in 5d3b58e
Seems to confirm that the If I extrapolate this to option 5, the |
ok - got it maybe we can add a test in ~dynamic-names or ~inheritance to clarify this?
This kind if conveys seperation between qualifier and name |
Sure, I'm open to suggestions. |
i'd interpret this the same way as #147 (comment)
that said, if this issue were raised with my implementation, i would point them towards "filters" instead. it doesn't suffer from the "use up a sigil for a single limited purpose" concern, can be composed with all sorts of other functionality, and is useful for much more than checking whether a list is empty! see #41 for where it originally was proposed, and specifically #41 (comment) which has a lot more context. as a concrete example, let's take a template like this: {{# list | ? }}
<h1>heading</j1>
{{# list }}
<p>{{ item }}</p>
{{/ list }}
{{/ list }}
{{^ list | ? }}
<h1>no heading!</h1>
{{/ list }} … and use it like this: $m = new Mustache_Engine([
'helpers' => [
'?' => fn($param) => !empty($param),
],
'pragmas' => [Mustache_Engine::PRAGMA_FILTERS],
]);
echo $m->render($tpl, ['list' => []]); we get there are very likely more useful names than |
IIRC there was a bit of discussion around whether this was a special case |
Not repeating the modifier in closing tag seemed like good way to clarify the modifier character it is not part of a 2-characters sigil. This is why I interpreted (admittedly wrongly) that the closing tag should not contain the modifier with parent tag in presence of a '*'. Anyway, that is a separate discussion and probably not something we want to consider here. |
@bobthecow Thank you for mentioning the filter option and the discussion in #41! It took me while to reply again, because I felt I needed to read that discussion in whole first. I understand the rationale for that syntax much better now, as well as how it compares to option 4, power lambdas (which were discussed in there as well). I have added the filter syntax as option 6 to my earlier post. In that post, I used @nminet, how do you feel about filters, compared to your current proposal? What really struck me in #41, is that @pvande has defended both option 6 and option 4, for different use cases. I think I subscribe to this vision, too: option 4 is more powerful than option 6, but in those cases where both are available, option 6 seems more elegant than option 4. I also like that @pvande proposed a semantic for option 4 which aligns exactly with the vision I ended up with after discussing with @bobthecow in #135. Both features should not be overused, but I agree with @bobthecow that a list nonemptiness check is a good use case for a filter. With filters, list nonemptiness could also be composed as I think my personal order of preference has now shifted to 1 > 6 > 4 > 2, with option 2 being only marginally acceptable. I no longer consider option 5 acceptable. However, I do not feel it is up to me alone to reject the pull request, so I will leave it open and I welcome further discussion. In the meanwhile, I would also welcome proposals for filters and for power lambdas. Also CC @spullara. |
The main issue in one of my use cases is that we are not allowed to push executable code to the target in production without a third party security review and approval. In such context option #1 is workable as it is possible to anticipate what lists will be allowed to render, but this is easy to miss in maintenance. This praticatly makes any option requiring executing code non-starters for that particular use case. Filters look like a workable option, but also open another 'Gliding scale' if some standard filter need to be specified. As matter of fact i would rather like '| empty?' |
@nminet Just for clarity: is executable code completely out of the question in your situation, or could a small set of curated functions work, if that set is approved once and then reused between renderings through some kind of side channel? |
Execution of code will be ok if and only if it has been reviewed before deployment. As the data allowed to render is also restricted this can be anticipated (adding the boolean flag when a list data is candidate for presentation), which is why option 1 works. A filter would also work as it is not hard to prove it does not affect data contents. In fact if an 'empty?' filters has a chance to be approved i can update the PR to use it. |
I don't think |
In addition to what @bobthecow wrote: as far as I'm concerned, you are welcome to submit a filters proposal. It does stand a chance of being approved. However, I suggest putting it on a separate branch (based on mustache:master) and opening a new pull request for it. That way, we can compare options, which further increases your chance of eventually having some sort of solution for your use case in the spec. If you go for it, I suggest especially taking inspiration from the following comments in #41: #41 (comment) (especially last paragraph) |
ok then - can close this one. Filters looks like a good way to go for me. |
As far as I'm concerned, this one can stay open until another solution has been merged. |
Check mustache.php's implementation and tests. It's probably a good starting point :) |
I'm not super comfortable with the filter solution. Seems like just moving code into the template. I would go with 1). It may be "inconvenient" but I don't think generally this is a huge problem in mustache and most languages / implementations have the ability to add a feature like this on a project wide basis. |
IMO filters, especially if they don't support parameters, are analogous to triple mustaches. They're a way to alter the value on its way into the rendered output. Because they only work with predefined code, the thing it's doing is in the view model where it belongs. They're equivalent in power but more mustachey to me than lambda sections, because at least they don't have the stringification first :) |
Sure the individual filter is defined in the view, but composing them would all be in the mustache template so I think you would immediately see filters like "ifTrue" and code would just move there.
|
@spullara I have not seen anyone suggest anything like an Regardless: is the fact that a feature can be abused enough reason to block legitimate use? I have seen people abuse Change Delimiter tags, but the possibility that this could happen never seems to have been a reason to exclude them from the spec. |
Yeah, maybe I wrote that too quickly (as you probably can't short circuit in a filter except through exceptions, or possibly returning a null or empty array?) but the point was that composition is being done in the template which is basically the same thing as changing the code. There have been plenty of proposals, many adopted by handlebars, for adding little languages to mustache to do various things like this and virtually all of them can be added as extensions to any of the mustache engines. Why does this rise to the level of changing the specification? |
Changing which code? Also, why shouldn't composition be done in the template? Isn't a list section a form of composition already?
I agree this is an important question. My take: there is a persistent use case that keeps coming up because template authors are unsatisfied with the available solutions. There is also an existing solution in some implementations that is general enough to address this use case and several others in an elegant way. Implementers (in this case at least @nminet, @bobthecow and myself) are looking for consensus in order to make templates more portable across implementations. While I agree that the template language should stay lean and logic-free, I think these are the three necessary conditions for a spec change in principle: (1) a widely shared use case, (2) a pre-existing solution that isn't in the spec yet and (3) a desire for consensus. Now, I have to say that I reacted allergically to filters when I first encountered the syntax. I didn't like the idea of adding more internal structure to tag contents at all (and I still don't, at least not in general). At the time, I wrote something similar to your objection, i.e., that I felt it would degrade Mustache's logic-less soul. I did recognize the use case, but I felt it should be addressed by making lambdas more powerful (#135), without any changes to the Mustache syntax (I'm still a proponent of power lambdas, by the way). Recently, I learned two things. Firstly, that the filter syntax employed in Mustache.php (and I suspect one or two other implementations) supports a large-ish subset of the same use cases as power lambdas, by deferring the filter implementation to the context. Secondly, that filter syntax is more suitable for that subset of use cases than power lambdas. Compare these three solutions for a conditional list wrapper: Option 1 (available in current spec) View {
list: [1, 2, 3],
listNotEmpty: true // <-- this does not scale well
} Template {{#listNotEmpty}}
<ul>
{{#list}}
<li>{{.}}</li>
{{/list}}
</ul>
{{/listNotEmpty}} Option 4 (power lambdas) View {
list: [1, 2, 3],
notEmpty: function(sectionText, implementationDefined) {
// somehow returns a new stack frame with each key
// in the current context mapped to a boolean
}
} Template {{#notEmpty.list}}
<ul>
{{#list}}
<li>{{.}}</li>
{{/list}}
</ul>
{{/notEmpty.list}} Option 6 (filters) View {
list: [1, 2, 3],
notEmpty: function(list) {
return list.length != 0;
}
} Template {{# list | notEmpty }}
<ul>
{{#list}}
<li>{{.}}</li>
{{/list}}
</ul>
{{/ list | notEmpty }} If we compare these options, I think all of them have the same amount of logic in the template. That is, you need to write some special notation in order to evaluate the nonemptiness of |
Are there a lot of examples of this in practice that I can look at? I have to say, in addition to not really being that excited about it, I do really hate this:
Rather than {{/list}} as I bet that can start to look pretty bad with a few filters (or even one). |
Mustache.php allows omitting the filter from the closing tag. |
My hesitation is that before the current React Native Web version of Twitter's front-end, the entire site from 2010 to about 2018 was rendered using mustache and we never thought we needed to extend it in this way (different from inheritance where everyone felt that pain). I think all their emails are currently rendered using it. Just never came up as even a feature request across a Ruby, Javascript and ultimately a Scala/Java implementation. Similarly at Bagcheck where I originally built mustache.java before being acquired by Twitter. The number of times that we needed "notEmptyList" were so small that we just did it in the view in a couple lines of code as demonstrated above. |
It has definitely come up before: #14 (ironically, option 3 was considered the best by most participants at the time) And possibly others that I missed. Fundamentally, Mustache needs a more powerful lambda mechanism in order to address advanced needs such as i18n. You illustrated this yourself in #24 (comment) back in 2011. I also know that I currently cannot implement anything like handlebars-i18next for Wontache, despite the fact that the latter passes the latest version of the Mustache spec 100%. This is one of the reasons why I opened #135. Power lambdas can do everything that filters can do and more, so in that sense, filters would be redundant. However, power lambdas are rather over-powered for most things that they enable compared to the current "weak" lambdas. Filters provide a nice middle ground, enabling many patterns that are currently not possible in a portable way, without elevating the privileges of the lambda involved all the way to the "power" level. Some examples that you might find more compelling than list emptyness:
{
fruits: ['apricot', 'banana', 'cherry'],
enumerate: function(list) {
wrappedList = [];
for (index in list) {
wrapped = {index: index, value: list[index]};
if (index == 0) wrapped.first = true;
if (index == list.length - 1) wrapped.last = true;
wrappedList.push(wrapped);
}
return wrappedList;
}
} Would you like:
{{# fruits | enumerate }}{{^first}}{{^last}},{{/last}}{{#last}} or{{/last}}{{/first}}
({{index}}) {{value}}{{#last}}?{{/last}}{{/ fruits | enumerate }}
{
fruits: ['apricot', 'banana', 'cherry'],
first: function(list) { return list[0]; },
last: function(list) { return list[list.length - 1]; }
} Our list of fruits starts with {{fruits | first}} and ends with {{fruits | last}}.
{
dictionary: {
mustache: 'facial hair between the nose and the upper lip',
sideburns: 'facial hair on the temple',
whiskers: 'long hairs on the cheek used by most mammals to sense air streams'
},
itemize: function(hash) {
wrappedList = [];
for (key in hash) {
wrapped = {key: key, value: hash[key]};
wrappedList.push(wrapped);
}
return wrappedList;
}
} <dl>
{{# dictionary | itemize }}
<dt>{{key}}</dt>
<dd>{{value}}</dd>
{{/ dictionary | itemize }}
</dl> (Compare to the Handlebars equivalent: <dl>
{{#each dictionary}}
<dt>{{@key}}</dt>
<dd>{{this}}</dd>
{{/each}}
</dl> ) |
I do all this with an easy to install decorator for collections that any implementation could do and standardizing that would be a nice addition rather than it working differently everywhere like it does today. Also happy to have more powerful lambdas as I do today but wish was standard. |
Do you mean that activating such a decorator from within the template should be standardized (which is basically what filters would amount to), or that the decorators themselves should be standardized? The latter seems unprecedented, as it would amount to standardizing the view instead of the template.
Great, that encourages me to implement those in Wontache. |
I'm sorry I am a little late to this thread but what I think what @spullara is doing with the decorator at an abstract level is somewhat similar to adding virtual keys like Handlebars does of I'm massively in favor of the virtual key approach:
That last one is really important because when you see this question popup on stackoverflow tons of people will respond with confusion to use JStachio (my implementation) follows the virtual key approach: https://jstach.io/jstachio/#mustache_virtual_keys I did this largely because my user base is familiar with JMustache (not mustache.java) and Handlebars.java (and javascript version). That being said I believe @jgonggrijp said that wontache has issues with that approach? @nminet and @spullara since both of you are I think more familiar with JVM technology I'm curious what your thoughts are on how JStachio does lambda: https://jstach.io/jstachio/io.jstach.jstache/io/jstach/jstache/JStacheLambda.html particularly the section reference of returned templates: Let me know if that javadoc needs more explanation. BTW @jgonggrijp I believe we were talking about searching for other Mustache implementations that have power(like) lambda support and JMustache does and JStachio was largely modeled after it with the exception of getting the whole stack (JStachio does only top of stack): https://www.javadoc.io/doc/com.samskivert/jmustache/latest/com/samskivert/mustache/Mustache.Lambda.html We have been using a forked version of JMustache for almost a decade. It's small yet fairly powerful. In fact we tried switching to JStachio is sadly not a user templating library. It is made for developers as it is compile time checked so it is not used by our consulting/off shore team but rather for non customizable UI (or doc generation). I bring this up because I believe there are largely two groups of users that are using Mustache:
The latter group is the one that really suffers without having virtual keys, or lambdas. What they end up doing is brittle hacks with CSS or Javascript or something else depending on output. I'm curious what group the Mustache users were at twitter @spullara or perhaps it was less binary than this? EDIT addendum on list is empty: Whoops I forgot to mention JMustache and JStachio do the empty list thing easily because Collection objects like lists have a method called I believe I realize that is not very spec compliant however like @spullara I only see 2 in our code ( |
Thanks for clarifying!
Sorry to disagree, but I think that filters are a much more elegant solution. They do not require special cases built into the engine and they do not introduce keywords that might conflict with pre-existing tag contents. Also, the filters themselves are generally really simple to implement. Filters can be further discussed in #153, if desired. I'm not sensitive to the "somebody else already did it" argument. There are lots of ideas around that I believe are better not copied. We are not standardizing
I don't remember what I said with regard to that. Right now, I cannot think of a reason why Wontache in particular would have trouble with virtual keywords, but see my above reason for not wanting to implement them.
Thanks for digging into this! I copied your comment to the new dedicated discussion for power lambdas: #155 (reply in thread).
Good point!
I don't see what's not spec-compliant about it? I don't think the spec claims anywhere that input data should only be JSON. In fact, that would completely preclude lambdas. Of course, the |
@agentgt I added your suggestion for virtual keywords as option 7 to the table in #157. Speaking for myself, it is my third most preferred solution, after filters and lambdas. I have also added your argument against context preprocessing to the table, and used your |
Let us focus any further discussion within this ticket on the proposal, i.e., the prefix |
@jgonggrijp Perhaps we close this issue soon so that there isn't a disconnect and two separate threads going on (this issue and the new discussion)? I responded in the new discussion of #157 |
@agentgt I didn't close it because it is not an issue but a pull request. 😉 I do not really expect this one to get merged, but I also see no reason to close it until some other solution is merged into the spec. Until that time, if anyone feels like reviewing or commenting on the current proposal, they can. |
Ha sorry about that! To be candid and to give context I have had Covid for the last week (mostly minor symptoms) and it might be impacting my cognitive ability more than I thought. That being said it is also why you might see me more active on these issue(s) as I finally have the cycles to put into it (not doing normal work at the moment). |
Hi
reopening a very old discussion :-)
Just to get feedback at this time
Implemented in https://github.com/nminet/kostache