text/template: allow callers to override IsTrue

[bep]

The IsTrue, which is used in templates to determine if if and when etc. conditionals should be invoked makes sense in most situations. Some examples:

package main

import (
    "fmt"
    "text/template"
    "time"
)

func main() {

    type s struct{}

    printIsTrue(1)         // true
    printIsTrue(0)         // false
    printIsTrue(-1)        // true
    printIsTrue(s{})       // true
    printIsTrue(&s{})      // true
    printIsTrue((*s)(nil)) // false
    printIsTrue(nil)       // false
    printIsTrue("")        // false
    printIsTrue("foo")     // true

    printIsTrue(time.Now())  // true
    printIsTrue(time.Time{}) // true

}

func printIsTrue(in interface{}) {
    b, _ := template.IsTrue(in)
    fmt.Println(b)
}

My main challenge with the above is that Struct values are always true-- even the zero time.Time value above.

It would be useful if the above method could check some additional truther interface:

type truther interface {
    IsTrue() bool
}

If the above is considered a breaking change, an alternative suggestion would be to make the template.IsTrue function settable.

[bcmills]

Please use https://play.golang.org for examples in issue reports when possible. That makes the details much easier for others to investigate.

https://play.golang.org/p/mvC2Cr0_guo

[bcmills]

My main challenge with the above is that Struct values are always true-- even the zero time.Time value above.

The fact that the zero time.Time reports as true is either a bug in the documentation or the implementation. The documentation for template.IsTrue says:

IsTrue reports whether the value is 'true', in the sense of not the zero of its type, and whether the value has a meaningful truth value.

The zero time.Time is the zero of its type, so according to the documentation it should be considered to be true, but the implementation clearly intends otherwise.

[bcmills]

CC @robpike and @mvdan to clarify whether the fix belongs in the documentation or the implementation. (At this point I suspect that it will have to be the documentation.)

[bcmills]

Actually, I think the documentation problem is a separate issue. I'll split it out.

[bcmills]

Ok, over to @robpike and @mvdan to decide: should we allow user code to override the meaning of IsTrue for template conditionals?

[mvdan]

In what context do you need a time.Time or any other struct to be false in a template?

For example, I imagine that you could do {{if not .Time.IsZero}}... instead. I believe most of the named structs with useful zero values should have a similar method you can call.

I realise that using a struct as a boolean inside a template doesn't make much sense, and I would intuitively think that would be an error like it is in plain Go. Either way, I haven't seen a need for that specific use case yet.

[bep]

For example, I imagine that you could do {{if not .Time.IsZero}}... instead. I believe most of the named structs with useful zero values should have a similar method you can call.

My particular use case is this:

Hugo have some template funcs that now typically return a map.

The caller would typically then do:

{{ $result := someFunc }}
{{ with $result }}
{{ else }}
// nothing found
{{ end }}

Now, since the map is false when len is 0, the above works.

But in the error case, since Go templates do not allow {{ $result, $err := someFunc }} ... let's say I need to improve how that is handled, and I would really like to do it without breaking stuff.

So, I would like to do something ala:

{{ $result := someFunc }}
{{ if $result }}
{{ else }}
{{ if $result.Error }} 
{{ end }}
{{ end }}

You may argue that I should create some Result struct or something, but again, that would be breaking.

But I can probably also make the {{if not .Time.IsZero} argument work:

Go's templates are ... very reflective. You have no type switches etc., so to require that the client has a deep understanding of the types it receives just does not work at scale. And what you then end up with is return values of slice, map and pointer types.

[bep]

Also, the .IsZero concept isn't a "universal" Go concept, so I would then need to make up my own scheme to make it work for my custom structs -- and one could argue that time.Time{} is just as falsy as 0.

[robpike]

Why not just write a method on the type or a function that returns a boolean?

[bep]

Why not just write a method on the type or a function that returns a boolean?

Go templates are on some fundamental levels very different from Go code. The template funcs are dynamic from the caller's standpoint. He/she often does not care if it is a struct or a map. And the IsTrue function isn't just boolean checking, it is a nil checker, a "not found checker", so a common pattern seen in templates are:

{{ $result := invokeFunc }}
{{ with $result }}
// do something
{{ end }}

• So, I could add an IsZero method to $result, but then I would need to add that method to all return types, even create custom string, map and slice types, so the caller could use the {{ if not $result.IsZero }} for all function and method invocations without having to look in the docs for everything.
• Also, with would not work with IsZero

 IsTrue reports whether the value is 'true', in the sense of not the zero of its type, and whether the value has a meaningful truth value.

The above is from the GoDoc comment on IsTrue. The current implementation is very subjective ("meaningful truth value"), and that is fine -- it works great for most situations. But I think it would be fair and well worth it to allow me and others to define our own meaning of truth.

[mvdan]

Thanks for the input. I understand that you need to keep backwards compatibility with previously written templates that interact with Hugo, so you can't replace a map with a struct. However, wouldn't you need these templates to be changed to make use of the added error value/string too? It seems to me like existing templates would be outdated one way or another.

Adding an IsTrue interface is definitely possible, but it smells a bit of operator overloading to me. One could end up with templates that behave in a very weird way, such as named booleans which are false being truthy. On the other hand, the current rules are simple (once the documentation is clarified).

So I'm trying to figure out if there's another way to help this use case without complicating template.IsTrue. Have you thought about other traditional ways to overcome these issues, such as adding another function with a different function and deprecating the previous one?

[bep]

It seems to me like existing templates would be outdated one way or another.

Sure, but the "another" way is estimated to be a lot less work. Which I like.

without complicating template.IsTrue

How would this complicate template.IsTrue? It can still live as it is today, but let me and others with a stricter "truth definition" to enforce that (options.TruthFunc or something).

And, reading the existing documentation, the current implementation is not correct in the "zero of its type" case (I assume we agree on that a struct also has a zero value):

"in the sense of not the zero of its type, and whether the value has a meaningful truth value."

but it smells a bit of operator overloading to me.

To each its own. It smells like a bug fix to me :-)

[mvdan]

It can still live as it is today, but let me and others with a stricter "truth definition" to enforce that (options.TruthFunc or something).

You mean a Template.Option, such as "truthfunc=enabled"?

I presume that in Hugo's case one would supply a template without ever seeing that option being set, so I'm not sure that it would help avoid confusion. In my previous comment, I meant complexity in terms of a template's behavior being predictable for users.

And, reading the existing documentation, the current implementation is not correct

Yes; that's been separated into #28394. I don't think that matters here; even if we changed the implementation to have structs behave like maps, you'd still have the same problem, I think.

I personally haven't made up my mind on this issue. The request seems reasonable, but I'd prefer if there was a solution that didn't involve running arbitrary code to determine truthiness. Let's see if @robpike or @rogpeppe have any thoughts.

[bep]

You mean a Template.Option, such as "truthfunc=enabled"?

No, I meant template.SetTruthFunc.

I presume that in Hugo's case one would supply a template without ever seeing that option being set, so I'm not sure that it would help avoid confusion.

The first part is correct. But I'm confident that If I could adjust the "truth" function, the net amount of confusion among Hugo users would be reduced by >= 40%. And I picked that number out of a hat. The point is that the real cost of explaining this confusing behavior to the users is mainly paid by the Hugo maintainers, not Go maintainers. The tens of thousands of Hugo users do not come to this issue tracker to complain. Consider that before rejecting this issue.

[rogpeppe]

Given that template.IsTrue already implements a fairly arbitrary definition of "truthiness" (it already allows at least one kind of value which isn't the zero value for its type - empty but non-nil slices), I think it might be reasonable to allow that behavior to be a bit more general. In other words, doing something here seems appropriate to me.

Setting a global variable is out, as that effects all clients of text/template in the whole program, not all of whom might want that behaviour.

A template option could work to enable some predefined behaviour such as recognizing interface {IsZero() bool}, or recursively checking IsZero in struct types, but doesn't allow arbitrary definitions of truthiness. I'm not that keen on recursively delving into struct values, as it doesn't really seem right for the template code to be delving into private struct fields

If there was a good use case for adding an arbitrary truthiness function, one could add:

func (*Template) SetIsTrue(func(interface{}) (bool, bool))

or maybe:

func (*Template) TruthFunc(func(interface{}) (bool, bool)) *Template

but if not (and I'm not really seeing it), then an option like this might seem better.

truth: control how "true" values are determined.
    truth=default
       The default behavior, as defined by IsTrue.
    truth=iszero
        If a non-nil value implements an IsZero() bool method,
        that method is called to determine whether a value is true, otherwise
        falling back to the default behaviour.

If you really wanted an arbitrary truth function later, you could always add

    truth=func
        Expects a function named isTrue of type func(interface{}) bool" to be defined as a template
        function, which will be called to determine whether values are true.

without loss of backward compatibility.

[mvdan]

The tens of thousands of Hugo users do not come to this issue tracker to complain. Consider that before rejecting this issue.

I'm not trying to reject this issue :) I'm trying to reason about it publicly so others can point out better ideas or whether there's a flaw in my thinking. If anything, I lean in favor of changing the API in some way, because I agree that currently it's somewhat arbitrary and stiff.

I agree with Roger that modifying the template package in a global way isn't a good idea. Unless you meant Template.SetTruthFunc instead of template.SetTruthFunc.

I also think that a Template.Option would do the trick here, either "truth=istrue" (I presume Roger made a typo by saying iszero above) or "truth=func" - or even both. @bep does that sound reasonable?

In any case, I'll mark this as needing a decision, as I'm not the primary owner of the package.

[bep]

Unless you meant Template.SetTruthFunc

It should obviously not be a global thing, so yes.

@bep does that sound reasonable?

I'm fine with both suggestions above (but I don't think iszero was a typo), and I suspect that the "iszero" option route would be the least drastic change (it prevents people like me from totally redefining the truth, but it makes it possible to get the behavior many people expects).

[mvdan]

Would iszero be enough for your use case, though? I can't imagine how you could have a struct value be both zero and contain an error value. Unless you considered a struct only containing an error to be zero.

[bep]

I can't imagine how you could have a struct value be both zero and contain an error value. Unless you considered a struct only containing an error to be zero.

That was my idea, but note that my example above isn't my only use for this change. And that particular example is a compromise -- I can define my own zero (just as time.Time has epoch as its zero). As I see it:

This (which does not work):

{{ $v, $err := invokeFunc }}
{{ if $err }}
...

In the above $v would typically be nil or zero if err is not.

If I define a zero to exclude any error value, the above could be rewritten as:

{{ $v := invokeFunc }}
{{ if $v.Error }}
...

But for people who don't care about the error value ($v, _ := ...), they can do:

{{ $v := invokeFunc }}
{{ with $v}}
...

And since the above is already a very common pattern, I can gradually introduce this "new pattern" without having to add a bunch of invokeFunc2 variants.

[mvdan]

Thanks for the explanation - then I agree that adding truth=default and truth=iszero is a good step forward. With truth=iszero, any named type that implements interface{IsZero() bool} will have a meaningful truth value, following that type's own definition of a zero value.

I'd like to hear Rob's thoughts on the revised idea, since he's the main owner of the package. I presume this change could still be done for 1.12, assuming we can reach a consensus in the following week.

[robpike]

I don't think you need an option, just a new template function iszero. The "option" is just whether you call it or not.

I'm not sure what "the revised idea" is, in any case. This is a long confusing discussion.

[bep]

I'm not sure what "the revised idea" is, in any case. This is a long confusing discussion.

Let me try to summarize the discussion above.

• There is a consensus above that the API docs for template.IsTrue is not entirely correct.
  • So, you should fix the documentation -- but I think it would also be fair to allow for the documented and established behavior if it is reasonably simple and low risk. Both suggestions above (1) Add option to check for an IsZero method, 2) add a SetTruthFunc on the template instance) are simple to implement.

If I could get IsTrue to check for IsZero I could do this without having to worry too much about the type of $v :

{{ $v := invokeFunc }}
{{ with $v}}
// print $v
{{ end }}

@robpike suggests that a new iszero template function would serve the same purpose. With that, the above example could be rewritten to something like this:

{{ $v := invokeFunc }}
{{ if and $v (not (iszero $v) }}
{{ with $v}}
// print $v
{{ end }}
{{ end }}

I have purposely made the above a little more clumsy than it could have been to make my point look better. But the iszero template func wouldn't be a practical solution to this. I suspect the reason why IsTrue was implemented in such a way in the first place, was to make it easier for the user to reason about types in a dynamic context without code completion etc. That was a good thought.

[rogpeppe]

I don't think you need an option, just a new template function iszero. The "option" is just whether you call it or not.

@robpike So I think you're suggesting that if there is a template function named "iszero" of the right type, it will be called to determine whether a value is true. This might change the behavior of existing templates that already have an "iszero" function. Would that be OK?

[robpike]

If the iszero function is built in to the template package, any user-added function with the same name would override it.

[bep]

@robpike If I read you correctly, you suggest that we instead

1. Add iszero as built-in template function.
2. Use that template function as an additional check-in template.IsTrue.

Not sure how that is an improvement of any of the two suggestions above, but if that what's needed to solve this issue, so be it.

[bep]

@rogpeppe @robpike OK, I take that silence as a confirmation and will prepare a PR for the last proposal.

[bep]

A quick note: I didn't really understand the suggested solution, so I found an alternative way to solve this particular issue for me. But I still think this is a good suggestion.