samskivert / jmustache

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

Confusing behavior of nullValue #60

Closed LukasKnuth closed 9 years ago

LukasKnuth commented 9 years ago

When I specify a nullValue("something") to the compiler, it also changes the missingIsNull property to false. The default is true if you simply call Mustache.compiler().compile(template)....

That is confusing... I just spend 20 minutes figuring out why a previously fine working section that was checking for a non-existing value was suddenly throwing exceptions at me, when I specify a null-value.

The problem is, that the documentation states that it will "still throw an exception". Or it says:

By default, an exception will be thrown any time a variable cannot be resolved, or resolves to null.

which seems to not be true.

Maybe have missingIsNull be false by default? Or don't change the value when a null-value is set?

samskivert commented 9 years ago

Were you not aware of defaultValue? That is documented to return the provided value when a field is missing or null. nullValue specifically says that it only returns its value when the field is null, not when it's missing. The section on "Default Values" also goes into detail on the distinction between these two configurations. I'm happy to fix the documentation if you think it's confusing, but I just read it and it seems pretty clear to me.

LukasKnuth commented 9 years ago

The problem is, that the documentation there is wrong (or outdated?):

By default, an exception will be thrown any time a variable cannot be resolved, or resolves to null.

When I get a compiler by doing Mustache.compiler()..., missingIsNull is true and the nullValue is null (Source). The documentation says otherwise.

If you only wish to provide a default value for variables that resolve to null, and wish to preserve exceptions in cases where variables cannot be resolved, use nullValue()

This further indicates that the default behavior should have missingIsNull as false.

My problem was, that I had a section that was in one case not present and in the other it was an Object. Using a simple section worked fine with the default compiler, but as soon as I set a nullValue(), it didn't work anymore, which was hard to grasp.

samskivert commented 9 years ago

Ah, I think I know what the issue is. The default behavior is not quite as simple as it claims in the comments.

By default, a missing value will throw an exception, but a missing section will simply be omitted. If you configure nullValue() then it causes sections to now throw an exception on missing values.

I'm not entirely sure that was on purpose. I feel like I should either always have sections throw on missing values or never have sections throw on missing values. Since I'm already leaning toward strictness, maybe I'll have them always throw. Hrm... I'll have to think about it.

samskivert commented 9 years ago

Incidentally, this was all documented in the README:

Note that section behavior deviates from the above specification (for historical reasons and because it's kind of useful). By default, a section that is not resolvable or resolves to null will be omitted (and conversely, an inverse section that is not resolvable or resolves to null will be included). If you use defaultValue(), this behavior is preserved. If you use nullValue(), sections that refer to an unresolvable variable will now throw an exception (sections that refer to a resolvable, but null-valued variable, will behave as before).

Now I'm thinking I may just change sections to always accommodate null or missing unless you explicitly request them to fail.