Additional inheritance specs for block reindentation #131
Conversation
| @@ -224,3 +224,92 @@ tests: | |||
| partials: | |||
| parent: "{{$foo}}default content{{/foo}}" | |||
| expected: default content | |||
|
|
|||
| - name: Standalone parent | |||
| desc: A parent's opening and closing tags need not be on separate lines in order to be standalone | |||
There was a problem hiding this comment.
That is, this pair of tags behaves exactly like a single standalone partial tag.
| two | ||
|
|
||
| - name: Standalone block | ||
| desc: A block's opening and closing tags need not be on separate lines in order to be standalone |
There was a problem hiding this comment.
This is however not exactly the same as a single standalone partial tag, because that would conflict with the existing "Inherit indentation" spec on lines 154-164. "Standalone" here mostly refers to whether the block as a whole will be indented. Contrary to the previous spec, if the end section tag is not standalone by itself, then the trailing newline will not be removed from the output if the block ends up being empty.
| template: | | ||
| {{<parent}}{{$block}} | ||
| one | ||
| two{{/block}} |
There was a problem hiding this comment.
Note this little tweak I had to make here. If I had put the {{/block}} at the start of the next line instead, I would have to include double newlines at the end of the expected output, for consistency with the pre-existing "Inherit indentation" spec.
| parent: | | ||
| Hi, | ||
| {{$block}} | ||
| {{/block}} |
There was a problem hiding this comment.
Here, I'm avoiding the double newline problem by making the block tag and the end section tag standalone by themselves, so their trailing newlines are removed from the output.
|
I am mostly against all this stuff. It requires you to reformat the output which means you have an additional layer of buffering in many cases. For performance reasons I have elided some of the whitespace constraints in my own implementation because of it. Imagine it for recursive includes and the like, as opposed to the simple case you could do at compile time. |
|
@spullara My intuition would be that the performance situation is no worse than with the existing rule on partials indentation: you have to track the current "line indentation" and process user-provided input (whenever the current indentation is non-empty) to add this indentation after newlines. This can be done in a zero-copy way by processing the input line-by-line, outputting indentation in between -- or indeed by using a temporary buffer that is output as a whole afterwards. |
|
@spullara I'm not doing any buffering (not even a single layer) and all deindentation happens at compile time. My implementation passes all the specs without doing anything new at rendering time, except that indentation is now not only applied to partials and parents but also to blocks. I think my implementation will also handle recursive includes transparently, though I haven't tested that yet. Could you give an example of a hypothetical spec that you think would be problematic? @gasche The partials spec actually states that only the template is indented, not the user input. This is also what I have implemented for parents and blocks. It's not as pretty as indenting the final output, but substantially easier and more efficient to implement. Lines 82 to 100 in bb63070 |
|
Update: in the meanwhile, I open-sourced my implementation, Wontache, so I can show you how I implemented the proposed spec (as well as 100% of the latest release of the actual spec, plus the proposed dynamic partials per #134). Blocks, like partials/parents, are represented as separate templates in Wontache. Compiling and rendering a template happens in four stages. I'm sure many Mustache implementations work like this, but I'll list them just to be clear:
In Wontache, the removal of intrinsic indentation from a block happens during code generation, over here: Indentation may be added (back) during template execution, depending on the context in which the block, partial or parent is expanded. In that case, it is recompiled with added indentation. The recompiled template is cached, so this needs to be done only once for each level of indentation encountered. It's a bit crude, but fast. This "magic" happens over here: @spullara While it may not be immediately obvious, the above code proves that the output is not reformatted and that no additional buffering is needed. Apart from infrequent template recompilation, it's just string concatenation, immediately followed by output. Feedback welcome! |
|
Hi folks. I help maintain handlebars.java and somewhat JMustache. I also have a Java static mustache implementation in the works which is a fork of another project someone else started (this point will become relevant). It passes the spec with the exception of #139 . I also think the block contents should be eagerly executed: Lines 228 to 237 in 5d3b58e (in the above apples should show and not bananas in my implementation). However I will save that rant for another time. Let me first say this spec provides invaluable test cases (that I didn't have to write :)) and I am much appreciative of it however: I have used Mustache and Handlebars in production settings for over 10 years with massive code bases. I have some idea of what works, what people care about, what they want and most importantly what they do not care about. Whitespace is something people do not care as much about as the spec would lead you to believe especially in the context of generating HTML. In fact over and over and over bugs have been filed for the opposite support. Folks already have a hard time understanding standalone tags. See because the time people care about it they want to manage it explicitly which most templating languages do by default!. Usually this is when they are generating something other than HTML. So there are two groups that I see using Mustache:
Thus given the way the spec is written currently I think block tags should be treated as explicit (that is whitespace between the ( The exception of them having standalone semantics in the parent just like the current semantics for sections I would also be OK with.... but this whole pairing of tags and special semantics for blocks while may only take 7 lines in javascript is absolutely nontrivial for my Java static mustache (particularly because I did not write the tokenizer) and I can't imagine the complexity as well as perf (to @spullara point) when dealing with dynamic lambdas. Instead of all this continued focus on whitespace:
Anyway I was hoping to be 100% compliant with my implementation but I don't feel it is worth the effort for whitespace given my experience and especially so if it keeps getting more complicated. |
|
@agentgt on my way out the door for a long weekend in the woods, so i haven't spent the time forming an opinion on your issue, but i do know that most implementations that diverge from the spec do it on this issue, so you'd be in good company :) |
|
Welcome to the club, @agentgt. Glad to hear that you appreciate the spec. Please excuse me for quote-sniping.
The right place for that future rant is in #125 (beginning of discussion) and #129 (remainder). Do keep in mind that the decision was already taken, though, and that multiple people agreed with it.
Could you provide some examples?
Do you mean users or implementers? I think the intent of the concept is that users do not need to think about it, because the template magically does the right thing. However, I can confirm from my own observations that implementers have a hard time wrapping their heads around how to implement it correctly. I suspect that especially incorrect implementations lead to bug reports.
Isn't that a conscious design choice in Mustache, though? Simplicity over bells and whistles? If I wanted lots of bells and whistles to fine-tune the whitespace in my templates, I would probably use ERB or Jinja instead of Mustache. (That said, you can gain explicit control over whitespace in Mustache if needed, by moving newlines inside tags.)
Agreed on this one. As I wrote before, though, the tests proposed here can be implemented without any performance penalty. In fact, Wontache, which implements all whitespace-related tests to the letter, is faster than Mustache.js, which completely ignores all of those tests and bluntly strips leading and trailing whitespace instead.
Here I really have to disagree. I am using Mustache templates to generate HTML and JavaScript code, where I'm using partials specifically for indentation (for example, see here and here; this is not the only example).
You are contradicting yourself here: you want to depart from what is currently in the spec, and at the same time, you claim that what is currently in the spec supports the changes you want to make. Please clarify what you mean by "the easiest addendum to the spec". Easiest for whom, to what end?
I would like to argue that these semantics are not "special" or "new"; I have just formulated explicitly, what I think was already the implicit idea behind the existing whitespace-related tests. See #130 for a more detailed discussion.
To be fair, I did not just write 7 new lines; I added, removed and edited a larger bunch of lines, and ended up with 7 lines more than I started with, in total. I had to partly redesign my implementation, so I'll readily acknowledge that there is some complexity here. At the same time, I think it made my implementation better overall, in the sense of being fundamentally more correct, and fundamentally closer to the original intent behind the Mustache language.
Based on my own implementation, I can reassure you that these whitespace rules do not need to interact with lambdas or with dynamic names (I'm not sure which of the two you are meaning to refer to here). Of course, it depends on your existing foundations how much work it will be to conform to the spec, but it can certainly be done, even without performance cost (regardless of whether you include the new tests in this branch or not). If you'd appreciate it, I'm willing to take a look at your code and try whether I can make any suggestions that might help you get closer to the spec. Alternatively, feel free to ignore some of the tests; you wouldn't be the first who takes the whitespace-related rules with a grain of salt, or even completely ignores them. (Edit: this was also mentioned by @bobthecow while I was writing this long comment.)
Please feel welcome to propose new tests!
Me too. Please feel welcome to join the discussion in #135.
I don't. Mustache was conceived as a simple, logic-free yet expressive template language. I appreciate that concept and would like to keep it that way, even if that means that I occasionally need to take some extra steps in data preparation on the side of the programming language. From earlier discussions in this issue tracker, I know that I am not the only one with that opinion.
The way I understand the spec (and implemented it), block scope is already lexical. However, you should not think of a block as a closure over the template in which it is defined, but as an independent template with its own root context. Consider this: if a block would close over the template of origin, the destination template would never be able to pass any values into it.
Again:
|
There is not a single Java implementation that implements the whitespace rules correctly (my implementation will be the closest). The whitespace rules are currently more complicated than the whitespace rules for XML normalization. If the developers can't keep the model in their head it is no wonder end users that care about whitespace have similar problems.
I have seen many more usually with code generation. More often than not the request is to remove whitespace handling (ie no standalone) once the user figures out what is going on.
I did.
Yet we have lambdas aka functions embedded in JSON, we have dynamic creation of templates produced by lambdas as well as recursive execution of templates as well as dynamic scoping. I wouldn't even be surprised if some implementations are turing complete. What people are asking for is context object + another parameter -> string. I have provided hosted CMS like systems for some time (e.g. user templates) and it is a lot easier to white list a couple of function calls then prevent all the other possibly behavior you can do with mustache (mustache.java even mentions this).
Because the end user often does not have a choice to pick their templating. Tons and tons of Java libraries have chosen Mustache or Handlebars to do code generation. Or in my case I'm coming from knowing 50 or so of offshore not-very-techy developers who use our products handlebars/mustache to generate HTML, CSS, etc. These guys do not care about whitespace but what I did get repeatedly asked is for basic function calls... otherwise they would dip into using Javascript. That is why we use a locked down Handlebars for a good portion of our services.
Maybe because it is Friday and I'm exhausted but I'm not sure I follow but I'm talking about parents changing the context.
The parent changing the context scope IMO is dynamic scoping or you could call it just plain macro expansion. I can implement it that way and I probably begrudgingly will but I think it is confusing for most. It is ripe for bugs but I suppose I can make that configurable and ditto for whitespace compliance.
Feel free to take a look: https://github.com/snaphop/static-mustache/blob/snaphop/static-mustache-apt/src/main/java/com/snaphop/staticmustache/apt/TemplateCompiler.java I will warn you the code is awful..ly complicated. I inherited the project and would have written the tokenizer different. It is a lot more complicated than most Mustache implementations because I have two trees to deal with: the mustache code, and the Java AST (via the annotation processor) as well of course generating Java code which mind you is far more complicated than Javascript because of types. However I did put the whitespace logic in as the original author did not have it in so I take fault that it is overly complicated. I was hoping not to read and buffer all tokens till new line but given what apparently I need to do I should have done that all along and is probably why it is complicated. Oh and maybe I'm just dumb but I still really do not understand why the newline remains at the end on #139 but I am trying as I said I hope to be one of the most compliant and typesafe to boot (well provided you don't just use maps aka hashes). |
|
Could you give another example? In this one, the user expects exactly what the spec prescribes (i.e., the newline should be preserved).
I meant syntactically simple. You are right to point out that there is an explosion of possibility at the semantic level. This is also what I meant by "expressive".
I cannot really comment on this part. Maybe Mustache is not the best tool for this particular situation; I'll readily concede that Mustache is not a catch-all solution.
Why not use Handlebars everywhere? Side note: Handlebars retains the whitespace rules of Mustache (if you don't use the added delimiter modifiers to adjust how whitespace is handled).
Yes. I think you need a different mental model. Nothing is "changing" the context. Rather, the parent template (i.e., not the parent tag pair) is providing the context to the block at the site of interpolation (you can think of "providing context" as passing an argument to a function). Equivalently, you can imagine that the content of the parent template is substituted for the content of the parent tag pair, overriding blocks as provided. To illustrate: parent.mustache {{#people}}
{{! site of interpolation}}
* {{$greeting}}Hello, {{name}}{{/greeting}}
{{/people}}template.mustache {{<parent}}
{{! site of definition}}
{{$greeting}}How do you do, {{name}}?{{/greeting}}
{{/parent}}Rendering {{#people}}
* How do you do, {{name}}?
{{/people}}Imagine passing the following chunk of data to either template. You'll find that the spec also predicts the same result in both cases. It's just lexical scoping. {
"people": [
{"name": "Fred"},
{"name": "George"}
]
}Try the inheritance-based version of the above example in the playground by pasting the following code: {"data":{"text":"{\n \"people\": [\n {\"name\": \"Fred\"},\n {\"name\": \"George\"}\n ]\n}"},"templates":[{"name":"parent","text":"{{#people}}\n {{! site of interpolation}}\n* {{$greeting}}Hello, {{name}}{{/greeting}}\n{{/people}}"},{"name":"template","text":"{{<parent}}\n {{! site of definition}}\n {{$greeting}}How do you do, {{name}}?{{/greeting}}\n{{/parent}}"}]}Try the single-template version of the above example in the playground by pasting the following code: {"data":{"text":"{\n \"people\": [\n {\"name\": \"Fred\"},\n {\"name\": \"George\"}\n ]\n}"},"templates":[{"name":"","text":"{{#people}}\n* How do you do, {{name}}?\n{{/people}}"}]}If I understood your previous comment correctly, you seemed to propose that the
Please don't! Such an implementation would be based on a misunderstanding. And yes, that would be confusing and a recipe for bugs as you wrote next. It is better to do something that you thoroughly understand, than to follow some authoritative rule that doesn't make sense to you.
Thanks! I'll study the code, this might take a while. I will not judge you or your code.
I'm not entirely sure what you mean by "buffering" here, but as I have written before, I don't think it should be necessary. My implementation keeps hold of the raw template text until tokenization and parsing are completed. Other than the raw text, the token stream and the parse tree, no intermediate strings, copies, lookups or whatever are used anywhere. Both code generation and rendering are just straight-to-output string concatenation.
I'll try whether I can clarify matters for you over there.
That's a noble goal. |
|
@jgonggrijp Alright after getting some sleep and playing with Wontache I understand how it is supposed to work. Block tags now are inherently special standalone (e.g. rule 1) in a parent template (ie not surrounded by I can implement it but it will require multiple passes (or replaying the tokens) which I was avoiding. What I meant before about where I said I was OK with blocks being standalone was for the original standalone behavior (you know where you said I was being contradictive). I know see the confusion. I'm mostly fine with this "special" standalone behavior but it complicates parsing. I also don't know what is the correct behavior for embedded block tags (ie block tags within block tags) but that is sort of tangental to the spacing problem. For example: Assuming no replacement would be: So b1 and b2 are standalone even though b1 has a tag immediately following it. I am not sure why I find that confusing but I do but if I think about it as a template expansion site it sort of makes more since. Oh btw I'm a massive hypocrite on the code generation not wanting automatic whitespace handling as I actually used JMustache over mustache.java to generate the spec test code. |
Hooray! Although I'm afraid there is a bug in Wontache which might still have confused you somewhat. Wontache mostly does the right thing, but you cleverly exposed a bug that goes unnoticed by the existing tests in the spec. By all means, do use Wontache as an example of how the spec might be implemented, but please keep in mind that it might sometimes still be wrong.
Yes, although that should not be the case if there is something preceding the opening tag, or following the closing tag.
In the parsing stage, I think it's just a matter of keeping track of clearance before and after the tag. If you store that information in the parsing tree, the clearance of matching tags is easy to retrieve when you need it. To be honest, I think the code generation stage is more complicated (as far as this feature is concerned), especially if you implement reindentation.
It should be but Wontache is doing it wrong. Just like in #139, you found a bug in Wontache. And Wontache is probably doing it wrong exactly because of the spacing rules, so I guess it's not entirely tangential. Although I still have to look into the exact reason. I will report back on that in #139 as I also announced there.
Because you are right and Wontache is wrong! There, you can hang that acknowledgement on your fridge or something. :-)
And I presume JMustache does more of that whitespace management than mustache.java? Let us just enjoy the irony of that. |
|
Like in #130, I have updated my posts to include links to and savestates for my newly developed Mustache playground. I hope this will reduce barriers for readers to experiment with the examples. The playground is powered by my own implementation, Wontache. Because of that, it fully implements the latest spec as well as the rules proposed here. It aims to be a general, more powerful alternative to the "demo" on the official Mustache website. Update from the future: that demo was replaced by the Wontache playground, so you can no longer see it on the website. Whitespace is handled correctly, lambdas are supported and it is possible to enter multiple named templates alongside each other, so that partial and parent tags can be used effectively. And as demonstrated above, it is possible to share sessions with other people by copy-pasting chunks of JSON code. Please feel welcome to use it. FYI @agentgt @spullara @bobthecow. The bug that @agentgt discovered (discussed above) has been fixed in the latest release of Wontache and it is not present in the playground. |
|
@gasche Have your thoughts on the matter evolved in the meanwhile? Would you approve? I decided to draw your attention again, because I got in touch with @spullara and he told me not to take his previous response as a full rejection. CC @bobthecow: what are your thoughts on this one? |
|
FWIW and in great irony given my previous comments on this thread I'm for these changes! The whitespace handling of Mustache has become my favorite features such that I have even implemented it in extensions (not in spec) like fragments which happens to be similar to parent/blocks: https://jstach.io/jstachio/#mustache_fragments (fragments are basically referencing a subset of a template as a partial and in this case we use section-like tag pairs as the subset... its actually a pretty cool feature I wish I had come up with but the HTMX guys did). |
|
At this point, if I'm right, all participants in this pull request are somewhere on the spectrum between neutral and in favor:
I will now rebase the branch, address merge conflicts and then merge it. I will check for other open PRs that could be merged quickly before tagging a new release. |
jgonggrijp
force-pushed
the
inheritance-indentation
branch
from
745f692
to
7834503
Compare
When we pass a block to a parent, we would ideally expect it to "magically" adjust its indentation.
template.mustache
parent.mustache
output
Try the above example in the playground by pasting the following code:
{"data":{"text":"null"},"templates":[{"name":"template","text":"{{<parent}}\n {{$block}}\n Hello\n {{/block}}\n{{/parent}}"},{"name":"parent","text":"I say,\n {{$block}}\n Hi there!\n {{/block}}"}]}I wrote an extremely long post in #130 on how this might be approached, inviting for a discussion. However, I realized that the post was so long that most people simply might not find the time (or patience or courage) to read it. I hope this PR will be an acceptable alternative to those people, being more concrete and containing substantially less text.
For a detailed rationale behind the specs that I'm proposing here, please refer to #130. The short version is that I applied the following rules. I was able to fully conform my own implementation to these specs. I needed to grow the code size by only seven lines on balance, though there were some subtleties that I'll mention in code comments below.
Merging this PR would close #130.
CC to all people I CC'ed in #130: @gasche @softmoth @adam-fowler @Danappelxx @pvande @splumhoff @bobthecow @sayrer @spullara .