samskivert / jmustache

A Java implementation of the Mustache templating language.
Other
834 stars 128 forks source link

Partials should be indented #101

Closed tompoch closed 9 months ago

tompoch commented 6 years ago

According to the specification https://github.com/mustache/spec/blob/72233f3ffda9e33915fd3022d0a9ebbcce265acd/specs/partials.yml#L82 each line of a partial should be indented.

JMustache does not seem to do that. Is this intention? (the partial tests are disabled)

I tried enable the corresponding test in my fork with the following result:

partials: Standalone Indentation(com.samskivert.mustache.specs.SpecTest)  Time elapsed: 0.007 sec  <<< FAILURE!
org.junit.ComparisonFailure: Template: '''\\n {{>partial}}\n/\n'''
Data: '{content=<\n->}'
 expected:<\\n |\n[ <\n->\n |]\n/\n> but was:<\\n |\n[<\n->\n|\n]\n/\n>
    at org.junit.Assert.assertEquals(Assert.java:115)
    at com.samskivert.mustache.specs.SpecTest.test(SpecTest.java:37)
samskivert commented 6 years ago

Hrm. I was not directly aware of this requirement of the specification. Partial spec tests are only disabled because most spec tests were disabled and then I enabled them one by one when JMustache was able to pass them. I guess partials never got over that hump, perhaps due to this indentation issue.

The way partials are implemented in JMustache, it would be difficult and expensive to guarantee that the output they generate has indentation properly applied to it, because we just hand off the Writer to the partial and let it write directly to the output. We would need to either inspect every character written to the writer and sneak in the indentation whenever we saw a newline go past, or somehow communicate the 'current indentation' to the partial machinery and ensure that it prepended that to every line prior to writing it.

Not impossible, but it's annoying that it has to either pessimize performance or complicate the code substantially for what seems like a debatable behavior. I'm sure there are some cases where it's nice, but in the vast majority of cases JMustache users probably don't care.

Maybe I'm just being pessimistic, I'll look at the code and see if perhaps it can be achieved without major performance or complexity costs.

spacether commented 3 years ago

This is definitely a feature that I care about. I am using jmustache to generate nested class definitions in python. They need to be nested so they do not have name conflicts with each other. Because we don't have this spec feature in jmustache, we have to add a String indentation variable in 4 classes and ensure that its value is alway set to the correct amount of whitespace and then include it in our templates.

Could this feature be prioritized and added? It would make

samskivert commented 3 years ago

If someone wants to send a pull request (preferably with the partials tests uncommented and passing), that would be great! But I have not been actively working on JMustache for many years. Future progress will have to come from civic minded users of the library. :)

gonozalviii commented 9 months ago

Here we go https://github.com/samskivert/jmustache/pull/142