Skip to content
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

tpl: Get rid of the custom template truth logic #6631

Merged
merged 1 commit into from
Dec 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions hugolib/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,34 @@ Partial cached3: {{ partialCached "p1" "input3" $key2 }}
`)
}

// https://github.com/gohugoio/hugo/issues/6615
func TestTemplateTruth(t *testing.T) {
b := newTestSitesBuilder(t)
b.WithTemplatesAdded("index.html", `
{{ $p := index site.RegularPages 0 }}
{{ $zero := $p.ExpiryDate }}
{{ $notZero := time.Now }}

if: Zero: {{ if $zero }}FAIL{{ else }}OK{{ end }}
if: Not Zero: {{ if $notZero }}OK{{ else }}Fail{{ end }}
not: Zero: {{ if not $zero }}OK{{ else }}FAIL{{ end }}
not: Not Zero: {{ if not $notZero }}FAIL{{ else }}OK{{ end }}

with: Zero {{ with $zero }}FAIL{{ else }}OK{{ end }}

`)

b.Build(BuildCfg{})

b.AssertFileContent("public/index.html", `
if: Zero: OK
if: Not Zero: OK
not: Zero: OK
not: Not Zero: OK
with: Zero OK
`)
}

func TestTemplateDependencies(t *testing.T) {
b := newTestSitesBuilder(t).Running()

Expand Down
1 change: 1 addition & 0 deletions scripts/fork_go_templates/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ var (
"func (s *state) evalFunction", "func (s *state) evalFunctionOld",
"func (s *state) evalField(", "func (s *state) evalFieldOld(",
"func (s *state) evalCall(", "func (s *state) evalCallOld(",
"func isTrue(val reflect.Value) (truth, ok bool) {", "func isTrueOld(val reflect.Value) (truth, ok bool) {",
)

htmlTemplateReplacers = strings.NewReplacer(
Expand Down
21 changes: 0 additions & 21 deletions tpl/compare/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,27 +71,6 @@ func init() {
[][2]string{},
)

ns.AddMethodMapping(ctx.And,
[]string{"and"},
[][2]string{},
)

ns.AddMethodMapping(ctx.Or,
[]string{"or"},
[][2]string{},
)

// getif is used internally by Hugo. Do not document.
ns.AddMethodMapping(ctx.getIf,
[]string{"getif"},
[][2]string{},
)

ns.AddMethodMapping(ctx.Not,
[]string{"not"},
[][2]string{},
)

ns.AddMethodMapping(ctx.Conditional,
[]string{"cond"},
[][2]string{
Expand Down
73 changes: 0 additions & 73 deletions tpl/compare/truth.go

This file was deleted.

60 changes: 0 additions & 60 deletions tpl/compare/truth_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion tpl/internal/go_templates/texttemplate/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func IsTrue(val interface{}) (truth, ok bool) {
return isTrue(reflect.ValueOf(val))
}

func isTrue(val reflect.Value) (truth, ok bool) {
func isTrueOld(val reflect.Value) (truth, ok bool) {
if !val.IsValid() {
// Something like var x interface{}, never set. It's a form of nil.
return false, true
Expand Down
6 changes: 6 additions & 0 deletions tpl/internal/go_templates/texttemplate/hugo_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"io"
"reflect"

"github.com/gohugoio/hugo/common/hreflect"

"github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate/parse"
)

Expand Down Expand Up @@ -301,3 +303,7 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a
}
return v
}

func isTrue(val reflect.Value) (truth, ok bool) {
return hreflect.IsTruthfulValue(val), true
}
35 changes: 3 additions & 32 deletions tpl/tplimpl/template_ast_transformers.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,36 +195,9 @@ func (c *templateContext) wrapInPartialReturnWrapper(n *parse.ListNode) *parse.L

}

// The truth logic in Go's template package is broken for certain values
// for the if and with keywords. This works around that problem by wrapping
// the node passed to if/with in a getif conditional.
// getif works slightly different than the Go built-in in that it also
// considers any IsZero methods on the values (as in time.Time).
// See https://github.com/gohugoio/hugo/issues/5738
// TODO(bep) get rid of this.
func (c *templateContext) wrapWithGetIf(p *parse.PipeNode) {
if len(p.Cmds) == 0 {
return
}

// getif will return an empty string if not evaluated as truthful,
// which is when we need the value in the with clause.
firstArg := parse.NewIdentifier("getif")
secondArg := p.CopyPipe()
newCmd := p.Cmds[0].Copy().(*parse.CommandNode)

// secondArg is a PipeNode and will behave as it was wrapped in parens, e.g:
// {{ getif (len .Params | eq 2) }}
newCmd.Args = []parse.Node{firstArg, secondArg}

p.Cmds = []*parse.CommandNode{newCmd}

}

// applyTransformations do 3 things:
// 1) Wraps every with and if pipe in getif
// 2) Parses partial return statement.
// 3) Tracks template (partial) dependencies and some other info.
// applyTransformations do 2 things:
// 1) Parses partial return statement.
// 2) Tracks template (partial) dependencies and some other info.
func (c *templateContext) applyTransformations(n parse.Node) (bool, error) {
switch x := n.(type) {
case *parse.ListNode:
Expand All @@ -235,10 +208,8 @@ func (c *templateContext) applyTransformations(n parse.Node) (bool, error) {
c.applyTransformationsToNodes(x.Pipe)
case *parse.IfNode:
c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList)
c.wrapWithGetIf(x.Pipe)
case *parse.WithNode:
c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList)
c.wrapWithGetIf(x.Pipe)
case *parse.RangeNode:
c.applyTransformationsToNodes(x.Pipe, x.List, x.ElseList)
case *parse.TemplateNode:
Expand Down
71 changes: 0 additions & 71 deletions tpl/tplimpl/template_ast_transformers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@
package tplimpl

import (
"strings"

"github.com/gohugoio/hugo/hugofs/files"

"testing"
"time"

template "github.com/gohugoio/hugo/tpl/internal/go_templates/htmltemplate"
"github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate/parse"
Expand Down Expand Up @@ -74,74 +71,6 @@ type T struct {
func (T) Method0() {
}

func TestInsertIsZeroFunc(t *testing.T) {
t.Parallel()

c := qt.New(t)

var (
ctx = map[string]interface{}{
"True": true,
"Now": time.Now(),
"TimeZero": time.Time{},
"T": &T{NonEmptyInterfaceTypedNil: (*T)(nil)},
}

templ1 = `
{{ if .True }}.True: TRUE{{ else }}.True: FALSE{{ end }}
{{ if .TimeZero }}.TimeZero1: TRUE{{ else }}.TimeZero1: FALSE{{ end }}
{{ if (.TimeZero) }}.TimeZero2: TRUE{{ else }}.TimeZero2: FALSE{{ end }}
{{ if not .TimeZero }}.TimeZero3: TRUE{{ else }}.TimeZero3: FALSE{{ end }}
{{ if .Now }}.Now: TRUE{{ else }}.Now: FALSE{{ end }}
{{ with .TimeZero }}.TimeZero1 with: {{ . }}{{ else }}.TimeZero1 with: FALSE{{ end }}
{{ template "mytemplate" . }}
{{ if .T.NonEmptyInterfaceTypedNil }}.NonEmptyInterfaceTypedNil: TRUE{{ else }}.NonEmptyInterfaceTypedNil: FALSE{{ end }}
{{ template "other-file-template" . }}
{{ define "mytemplate" }}
{{ if .TimeZero }}.TimeZero1: mytemplate: TRUE{{ else }}.TimeZero1: mytemplate: FALSE{{ end }}
{{ end }}
`

// https://github.com/gohugoio/hugo/issues/5865
templ2 = `{{ define "other-file-template" }}
{{ if .TimeZero }}.TimeZero1: other-file-template: TRUE{{ else }}.TimeZero1: other-file-template: FALSE{{ end }}
{{ end }}
`
)

d := newD(c)
h := d.Tmpl.(*templateHandler)

// HTML templates
c.Assert(h.AddTemplate("mytemplate.html", templ1), qt.IsNil)
c.Assert(h.AddTemplate("othertemplate.html", templ2), qt.IsNil)

// Text templates
c.Assert(h.AddTemplate("_text/mytexttemplate.txt", templ1), qt.IsNil)
c.Assert(h.AddTemplate("_text/myothertexttemplate.txt", templ2), qt.IsNil)

c.Assert(h.markReady(), qt.IsNil)

for _, name := range []string{"mytemplate.html", "mytexttemplate.txt"} {
var sb strings.Builder
tt, _ := d.Tmpl.Lookup(name)
err := h.Execute(tt, &sb, ctx)
c.Assert(err, qt.IsNil)
result := sb.String()

c.Assert(result, qt.Contains, ".True: TRUE")
c.Assert(result, qt.Contains, ".TimeZero1: FALSE")
c.Assert(result, qt.Contains, ".TimeZero2: FALSE")
c.Assert(result, qt.Contains, ".TimeZero3: TRUE")
c.Assert(result, qt.Contains, ".Now: TRUE")
c.Assert(result, qt.Contains, "TimeZero1 with: FALSE")
c.Assert(result, qt.Contains, ".TimeZero1: mytemplate: FALSE")
c.Assert(result, qt.Contains, ".TimeZero1: other-file-template: FALSE")
c.Assert(result, qt.Contains, ".NonEmptyInterfaceTypedNil: FALSE")
}

}

func TestCollectInfo(t *testing.T) {

configStr := `{ "version": 42 }`
Expand Down