From 7508450705d80a2e5b16b75da2c2467575f6a939 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Fri, 20 Aug 2021 16:40:02 -0500 Subject: [PATCH 1/5] WIP rough working concept --- src/text/template/exec.go | 17 +++++-- src/text/template/parent_test.go | 84 ++++++++++++++++++++++++++++++++ src/text/template/parse/lex.go | 2 + src/text/template/parse/node.go | 15 +++--- src/text/template/parse/parse.go | 27 ++++++++-- src/text/template/template.go | 48 +++++++++++++----- 6 files changed, 166 insertions(+), 27 deletions(-) create mode 100644 src/text/template/parent_test.go diff --git a/src/text/template/exec.go b/src/text/template/exec.go index 5ad3b4ec582c41..fd259248ab04b9 100644 --- a/src/text/template/exec.go +++ b/src/text/template/exec.go @@ -36,6 +36,7 @@ type state struct { node parse.Node // current node, for errors vars []variable // push-down stack of variable values. depth int // the height of the stack of executing templates. + scope map[string]int } // variable holds the dynamic value of a variable such as $, $x etc. @@ -207,9 +208,10 @@ func (t *Template) execute(wr io.Writer, data interface{}) (err error) { value = reflect.ValueOf(data) } state := &state{ - tmpl: t, - wr: wr, - vars: []variable{{"$", value}}, + tmpl: t, + wr: wr, + vars: []variable{{"$", value}}, + scope: make(map[string]int), } if t.Tree == nil || t.Root == nil { state.errorf("%q is an incomplete or empty template", t.Name()) @@ -230,6 +232,7 @@ func (t *Template) DefinedTemplates() string { t.muTmpl.RLock() defer t.muTmpl.RUnlock() for name, tmpl := range t.tmpl { + tmpl := tmpl[0] // TODO: invariant check - at least one if tmpl.Tree == nil || tmpl.Root == nil { continue } @@ -400,7 +403,12 @@ func (s *state) walkRange(dot reflect.Value, r *parse.RangeNode) { func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) { s.at(t) - tmpl := s.tmpl.Lookup(t.Name) + scope := s.scope[t.Name] + if t.Parent { + scope += 1 + } + s.scope[t.Name] = scope + tmpl := s.tmpl.LookupWithScope(t.Name, scope) if tmpl == nil { s.errorf("template %q not defined", t.Name) } @@ -415,6 +423,7 @@ func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) { // No dynamic scoping: template invocations inherit no variables. newState.vars = []variable{{"$", dot}} newState.walk(dot, tmpl.Root) + newState.scope[t.Name] = scope - 1 // TODO: unclear if this makes sense here } // Eval functions evaluate pipelines, commands, and their elements and extract diff --git a/src/text/template/parent_test.go b/src/text/template/parent_test.go new file mode 100644 index 00000000000000..cbfb6c2b6f2b9a --- /dev/null +++ b/src/text/template/parent_test.go @@ -0,0 +1,84 @@ +package template_test + +import ( + "bytes" + "testing" + "text/template" +) + +func TestParent(t *testing.T) { + parent, err := template.New("parent").Parse(`{{block "content" .}}parent{{end}}`) + if err != nil { + t.Fatalf("parsing parent template: %v", err) + } + + var b bytes.Buffer + if err := parent.Execute(&b, nil); err != nil { + t.Fatalf("executing parent template: %v", err) + } + if b.String() != "parent" { + t.Errorf("want %q, got %q", "parent", b.String()) + } + + var child *template.Template + { + clone, err := parent.Clone() + if err != nil { + t.Fatalf("cloning parent: %v", err) + } + + child, err = clone.Parse(`{{define "content"}}{{template parent}}child{{end}}`) + if err != nil { + t.Fatalf("parsing child template: %v", err) + } + + b.Reset() + if err := child.Execute(&b, nil); err != nil { + t.Fatalf("executing child template: %v", err) + } + if b.String() != "parentchild" { + t.Errorf("want %q, got %q", "child", b.String()) + } + } + + { + clone, err := parent.Clone() + if err != nil { + t.Fatalf("cloning parent: %v", err) + } + + child, err := clone.Parse(`{{define "content"}}{{template parent}}cloned child{{end}}`) + if err != nil { + t.Fatalf("parsing child template: %v", err) + } + + b.Reset() + if err := child.Execute(&b, nil); err != nil { + t.Fatalf("executing child template: %v", err) + } + if b.String() != "parentcloned child" { + t.Errorf("want %q, got %q", "parentcloned child", b.String()) + } + } + + { + clone, err := child.Clone() + if err != nil { + t.Fatalf("cloning child: %v", err) + } + + gc, err := clone.Parse(`{{define "content"}}{{template parent}}grandchild{{end}}`) + if err != nil { + t.Fatalf("parsing grandchild template: %v", err) + } + + b.Reset() + if err := gc.Execute(&b, nil); err != nil { + t.Fatalf("executing grandchild template: %v", err) + } + if b.String() != "parentchildgrandchild" { + t.Errorf("want %q, got %q", "parentchildgrandchild", b.String()) + } + } + +} diff --git a/src/text/template/parse/lex.go b/src/text/template/parse/lex.go index 6784071b1118d1..4a289ad1694b4e 100644 --- a/src/text/template/parse/lex.go +++ b/src/text/template/parse/lex.go @@ -68,6 +68,7 @@ const ( itemEnd // end keyword itemIf // if keyword itemNil // the untyped nil constant, easiest to treat as a keyword + itemParent // parent keyword itemRange // range keyword itemTemplate // template keyword itemWith // with keyword @@ -82,6 +83,7 @@ var key = map[string]itemType{ "if": itemIf, "range": itemRange, "nil": itemNil, + "parent": itemParent, "template": itemTemplate, "with": itemWith, } diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index 177482f9b26059..dd2d1b87d504e1 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -937,14 +937,15 @@ func (w *WithNode) Copy() Node { type TemplateNode struct { NodeType Pos - tr *Tree - Line int // The line number in the input. Deprecated: Kept for compatibility. - Name string // The name of the template (unquoted). - Pipe *PipeNode // The command to evaluate as dot for the template. + tr *Tree + Line int // The line number in the input. Deprecated: Kept for compatibility. + Name string // The name of the template (unquoted). + Parent bool // Whether to lookup template of this name in parent scope. + Pipe *PipeNode // The command to evaluate as dot for the template. } -func (t *Tree) newTemplate(pos Pos, line int, name string, pipe *PipeNode) *TemplateNode { - return &TemplateNode{tr: t, NodeType: NodeTemplate, Pos: pos, Line: line, Name: name, Pipe: pipe} +func (t *Tree) newTemplate(pos Pos, line int, name string, parent bool, pipe *PipeNode) *TemplateNode { + return &TemplateNode{tr: t, NodeType: NodeTemplate, Pos: pos, Line: line, Name: name, Parent: parent, Pipe: pipe} } func (t *TemplateNode) String() string { @@ -968,5 +969,5 @@ func (t *TemplateNode) tree() *Tree { } func (t *TemplateNode) Copy() Node { - return t.tr.newTemplate(t.Pos, t.Line, t.Name, t.Pipe.CopyPipe()) + return t.tr.newTemplate(t.Pos, t.Line, t.Name, t.Parent, t.Pipe.CopyPipe()) } diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go index 1a63961c13ecce..f2b1b759f14fa2 100644 --- a/src/text/template/parse/parse.go +++ b/src/text/template/parse/parse.go @@ -160,7 +160,7 @@ func (t *Tree) ErrorContext(n Node) (location, context string) { // errorf formats the error and terminates processing. func (t *Tree) errorf(format string, args ...interface{}) { t.Root = nil - format = fmt.Sprintf("template: %s:%d: %s", t.ParseName, t.token[0].line, format) + format = fmt.Sprintf("\x1b[1;33mPAULDEV\x1b[0m template: %s:%d: %s", t.ParseName, t.token[0].line, format) panic(fmt.Errorf(format, args...)) } @@ -580,7 +580,7 @@ func (t *Tree) blockControl() Node { block.add() block.stopParse() - return t.newTemplate(token.pos, token.line, name, pipe) + return t.newTemplate(token.pos, token.line, name, false, pipe) // TODO: support parent? } // Template: @@ -590,14 +590,33 @@ func (t *Tree) blockControl() Node { func (t *Tree) templateControl() Node { const context = "template clause" token := t.nextNonSpace() - name := t.parseTemplateName(token, context) + var ( + name string + parent bool + ) + if token.typ == itemParent { + // If we encounter the `parent` keyword, then we assume the current + // definition we're in the midst of is an overriding of a previously + // defined template, and the author's intent is to execute that + // "parent" template as part of the "child". + // + // We know the name of the template currently being defined + // (overridden), so we augment the name with a flag to indicate to the + // execution step that it should lookup the "parent" template with that + // name. + //fmt.Printf("\x1b[1;31mTEMPLATE NAME: %q\x1b[0m\n", t.Name) + name = t.Name + parent = true + } else { + name = t.parseTemplateName(token, context) + } var pipe *PipeNode if t.nextNonSpace().typ != itemRightDelim { t.backup() // Do not pop variables; they persist until "end". pipe = t.pipeline(context, itemRightDelim) } - return t.newTemplate(token.pos, token.line, name, pipe) + return t.newTemplate(token.pos, token.line, name, parent, pipe) } func (t *Tree) parseTemplateName(token item, context string) (name string) { diff --git a/src/text/template/template.go b/src/text/template/template.go index fd74d45e9b1c6d..6d9d5f2963c7be 100644 --- a/src/text/template/template.go +++ b/src/text/template/template.go @@ -5,6 +5,7 @@ package template import ( + "fmt" "reflect" "sync" "text/template/parse" @@ -12,8 +13,8 @@ import ( // common holds the information shared by related templates. type common struct { - tmpl map[string]*Template // Map from name to defined templates. - muTmpl sync.RWMutex // protects tmpl + tmpl map[string][]*Template // Map from name to defined templates. + muTmpl sync.RWMutex // protects tmpl option option // We use two maps, one for parsing and one for execution. // This separation makes the API cleaner since it doesn't @@ -70,7 +71,7 @@ func (t *Template) New(name string) *Template { func (t *Template) init() { if t.common == nil { c := new(common) - c.tmpl = make(map[string]*Template) + c.tmpl = make(map[string][]*Template) c.parseFuncs = make(FuncMap) c.execFuncs = make(map[string]reflect.Value) t.common = c @@ -91,14 +92,16 @@ func (t *Template) Clone() (*Template, error) { } t.muTmpl.RLock() defer t.muTmpl.RUnlock() - for k, v := range t.tmpl { + for k, stack := range t.tmpl { if k == t.name { - nt.tmpl[t.name] = nt + nt.tmpl[t.name] = append(nt.tmpl[t.name], nt) // TODO: double-check this - maybe should just be assigning new slice continue } // The associated templates share nt's common structure. - tmpl := v.copy(nt.common) - nt.tmpl[k] = tmpl + for i := range stack { + tmpl := stack[i].copy(nt.common) + nt.tmpl[k] = append(nt.tmpl[k], tmpl) + } } t.muFuncs.RLock() defer t.muFuncs.RUnlock() @@ -150,8 +153,8 @@ func (t *Template) Templates() []*Template { t.muTmpl.RLock() defer t.muTmpl.RUnlock() m := make([]*Template, 0, len(t.tmpl)) - for _, v := range t.tmpl { - m = append(m, v) + for _, stack := range t.tmpl { + m = append(m, stack[0]) // TODO: runtime assert there is at least one? invariant check } return m } @@ -191,7 +194,24 @@ func (t *Template) Lookup(name string) *Template { } t.muTmpl.RLock() defer t.muTmpl.RUnlock() - return t.tmpl[name] + return t.lookup(name, 0) +} + +func (t *Template) LookupWithScope(name string, scope int) *Template { + if t.common == nil { + return nil + } + t.muTmpl.RLock() + defer t.muTmpl.RUnlock() + return t.lookup(name, scope) +} + +func (t *Template) lookup(name string, scope int) *Template { + if scope >= len(t.tmpl[name]) { + return nil + } + fmt.Printf("\x1b[1;35mname: %q\tscope: %d\tlen: %d\x1b[0m\n", name, scope, len(t.tmpl[name])) + return t.tmpl[name][scope] // TODO: invariant check: at least one } // Parse parses text as a template body for t. @@ -228,11 +248,15 @@ func (t *Template) associate(new *Template, tree *parse.Tree) bool { if new.common != t.common { panic("internal error: associate not common") } - if old := t.tmpl[new.name]; old != nil && parse.IsEmptyTree(tree.Root) && old.Tree != nil { + if old := t.lookup(new.name, 0); old != nil && parse.IsEmptyTree(tree.Root) && old.Tree != nil { // If a template by that name exists, // don't replace it with an empty template. return false } - t.tmpl[new.name] = new + stack := t.tmpl[new.name] + stack = append(stack, nil) + copy(stack[1:], stack[0:]) + stack[0] = new + t.tmpl[new.name] = stack return true } From 079913ca1dcf817f62577dec161772630e53986e Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sat, 21 Aug 2021 08:28:20 -0500 Subject: [PATCH 2/5] WIP make tests pass --- src/text/template/exec.go | 8 ++++++-- src/text/template/multi_test.go | 8 ++++---- src/text/template/template.go | 3 +-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/text/template/exec.go b/src/text/template/exec.go index fd259248ab04b9..230000929ed124 100644 --- a/src/text/template/exec.go +++ b/src/text/template/exec.go @@ -405,7 +405,7 @@ func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) { s.at(t) scope := s.scope[t.Name] if t.Parent { - scope += 1 + scope++ } s.scope[t.Name] = scope tmpl := s.tmpl.LookupWithScope(t.Name, scope) @@ -423,7 +423,11 @@ func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) { // No dynamic scoping: template invocations inherit no variables. newState.vars = []variable{{"$", dot}} newState.walk(dot, tmpl.Root) - newState.scope[t.Name] = scope - 1 // TODO: unclear if this makes sense here + scope-- + if scope < 0 { // TODO logical bug + scope = 0 + } + newState.scope[t.Name] = scope // TODO: unclear if this makes sense here } // Eval functions evaluate pipelines, commands, and their elements and extract diff --git a/src/text/template/multi_test.go b/src/text/template/multi_test.go index b543ab5c47c6d3..f4d49cc0730589 100644 --- a/src/text/template/multi_test.go +++ b/src/text/template/multi_test.go @@ -71,12 +71,12 @@ func TestMultiParse(t *testing.T) { continue } for i, name := range test.names { - tmpl, ok := template.tmpl[name] + tmpl, ok := template.tmpl[name] // TODO making this pass tests for now, turn into an accessor method if !ok { t.Errorf("%s: can't find template %q", test.name, name) continue } - result := tmpl.Root.String() + result := tmpl[0].Root.String() // TODO if result != test.results[i] { t.Errorf("%s=(%q): got\n\t%v\nexpected\n\t%v", test.name, test.input, result, test.results[i]) } @@ -234,10 +234,10 @@ func TestClone(t *testing.T) { } // Verify that the clone is self-consistent. for k, v := range clone.tmpl { - if k == clone.name && v.tmpl[k] != clone { + if k == clone.name && v[0].tmpl[k][0] != clone { // TODO making this pass tests for now t.Error("clone does not contain root") } - if v != v.tmpl[v.name] { + if v[0] != v[0].tmpl[v[0].name][0] { // TODO making this pass tests for now t.Errorf("clone does not contain self for %q", k) } } diff --git a/src/text/template/template.go b/src/text/template/template.go index 6d9d5f2963c7be..16b7eb8a7ff7f3 100644 --- a/src/text/template/template.go +++ b/src/text/template/template.go @@ -5,7 +5,6 @@ package template import ( - "fmt" "reflect" "sync" "text/template/parse" @@ -210,7 +209,7 @@ func (t *Template) lookup(name string, scope int) *Template { if scope >= len(t.tmpl[name]) { return nil } - fmt.Printf("\x1b[1;35mname: %q\tscope: %d\tlen: %d\x1b[0m\n", name, scope, len(t.tmpl[name])) + //fmt.Printf("\x1b[1;35mname: %q\tscope: %d\tlen: %d\x1b[0m\n", name, scope, len(t.tmpl[name])) return t.tmpl[name][scope] // TODO: invariant check: at least one } From 21c2f7477408edf9778ce3af89d4f0621f5107e0 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Sun, 22 Aug 2021 12:44:19 -0500 Subject: [PATCH 3/5] Add parent example --- src/text/template/exampleparent_test.go | 78 +++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 src/text/template/exampleparent_test.go diff --git a/src/text/template/exampleparent_test.go b/src/text/template/exampleparent_test.go new file mode 100644 index 00000000000000..8900a027d4a903 --- /dev/null +++ b/src/text/template/exampleparent_test.go @@ -0,0 +1,78 @@ +package template_test + +import ( + "log" + "os" + "text/template" +) + +func ExampleTemplate_parent() { + const base_html = ` + + + {{block "title" .}}My website{{end}} + + +
+ {{- block "content" .}}{{end -}} +
+
+ {{- block "footer" .}} +

Thanks for visiting!

+ {{- end}} +
+ + + +` + base := template.Must(template.New("base.html").Parse(base_html)) + + const index_html = `{{define "content"}}

Welcome!

{{end}}` + index := template.Must(template.Must(base.Clone()).New("index.html").Parse(index_html)) + + { + err := index.ExecuteTemplate(os.Stdout, "base.html", nil) + if err != nil { + log.Println("executing template:", err) + } + } + + const about_html = `{{define "title"}}{{template parent .}} - About{{end}} +{{define "content"}}

About us

{{end}}` + about := template.Must(template.Must(base.Clone()).New("about.html").Parse(about_html)) + + { + err := about.ExecuteTemplate(os.Stdout, "base.html", nil) + if err != nil { + log.Println("executing template:", err) + } + } + + // Output: + // + // + // + // My website + // + // + //

Welcome!

+ //
+ //

Thanks for visiting!

+ //
+ // + // + // + // + // + // + // My website - About + // + // + //

About us

+ //
+ //

Thanks for visiting!

+ //
+ // + // + +} From 5519c273155e794f7ef3e0ce9d5eb7dcb880e498 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 8 Sep 2021 11:42:47 -0500 Subject: [PATCH 4/5] Add parse test --- src/text/template/exampleparent_test.go | 2 -- src/text/template/parse/node.go | 6 +++++- src/text/template/parse/parse.go | 21 ++++++++++----------- src/text/template/parse/parse_test.go | 2 ++ src/text/template/template.go | 1 - 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/text/template/exampleparent_test.go b/src/text/template/exampleparent_test.go index 8900a027d4a903..89b2491183377f 100644 --- a/src/text/template/exampleparent_test.go +++ b/src/text/template/exampleparent_test.go @@ -29,7 +29,6 @@ func ExampleTemplate_parent() { const index_html = `{{define "content"}}

Welcome!

{{end}}` index := template.Must(template.Must(base.Clone()).New("index.html").Parse(index_html)) - { err := index.ExecuteTemplate(os.Stdout, "base.html", nil) if err != nil { @@ -40,7 +39,6 @@ func ExampleTemplate_parent() { const about_html = `{{define "title"}}{{template parent .}} - About{{end}} {{define "content"}}

About us

{{end}}` about := template.Must(template.Must(base.Clone()).New("about.html").Parse(about_html)) - { err := about.ExecuteTemplate(os.Stdout, "base.html", nil) if err != nil { diff --git a/src/text/template/parse/node.go b/src/text/template/parse/node.go index dd2d1b87d504e1..558a3a7ac2a9da 100644 --- a/src/text/template/parse/node.go +++ b/src/text/template/parse/node.go @@ -956,7 +956,11 @@ func (t *TemplateNode) String() string { func (t *TemplateNode) writeTo(sb *strings.Builder) { sb.WriteString("{{template ") - sb.WriteString(strconv.Quote(t.Name)) + if t.Parent { + sb.WriteString("parent") + } else { + sb.WriteString(strconv.Quote(t.Name)) + } if t.Pipe != nil { sb.WriteByte(' ') t.Pipe.writeTo(sb) diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go index f2b1b759f14fa2..19a1e09b61fd06 100644 --- a/src/text/template/parse/parse.go +++ b/src/text/template/parse/parse.go @@ -160,7 +160,7 @@ func (t *Tree) ErrorContext(n Node) (location, context string) { // errorf formats the error and terminates processing. func (t *Tree) errorf(format string, args ...interface{}) { t.Root = nil - format = fmt.Sprintf("\x1b[1;33mPAULDEV\x1b[0m template: %s:%d: %s", t.ParseName, t.token[0].line, format) + format = fmt.Sprintf("template: %s:%d: %s", t.ParseName, t.token[0].line, format) panic(fmt.Errorf(format, args...)) } @@ -580,7 +580,7 @@ func (t *Tree) blockControl() Node { block.add() block.stopParse() - return t.newTemplate(token.pos, token.line, name, false, pipe) // TODO: support parent? + return t.newTemplate(token.pos, token.line, name, false, pipe) } // Template: @@ -595,16 +595,15 @@ func (t *Tree) templateControl() Node { parent bool ) if token.typ == itemParent { - // If we encounter the `parent` keyword, then we assume the current - // definition we're in the midst of is an overriding of a previously - // defined template, and the author's intent is to execute that - // "parent" template as part of the "child". + // If we encounter the `parent` keyword, then we assume the template we're + // in the midst currently defining is overriding a previously defined + // template, and that the template author's intent is to execute that + // "parent" template as part of this "child" template. // - // We know the name of the template currently being defined - // (overridden), so we augment the name with a flag to indicate to the - // execution step that it should lookup the "parent" template with that - // name. - //fmt.Printf("\x1b[1;31mTEMPLATE NAME: %q\x1b[0m\n", t.Name) + // We know the name of the template currently being defined (i.e., the one + // overriding a previous definition), so we set a flag to indicate to + // template execution that it should lookup the "parent" template with the + // same name. name = t.Name parent = true } else { diff --git a/src/text/template/parse/parse_test.go b/src/text/template/parse/parse_test.go index 9b1be272e573d9..2aaf9e4b58c317 100644 --- a/src/text/template/parse/parse_test.go +++ b/src/text/template/parse/parse_test.go @@ -236,6 +236,8 @@ var parseTests = []parseTest{ `{{template "x"}}`}, {"template with arg", "{{template `x` .Y}}", noError, `{{template "x" .Y}}`}, + {"template with parent", "{{template parent .}}", noError, + `{{template parent .}}`}, {"with", "{{with .X}}hello{{end}}", noError, `{{with .X}}"hello"{{end}}`}, {"with with else", "{{with .X}}hello{{else}}goodbye{{end}}", noError, diff --git a/src/text/template/template.go b/src/text/template/template.go index 16b7eb8a7ff7f3..aac8a628be1d5c 100644 --- a/src/text/template/template.go +++ b/src/text/template/template.go @@ -209,7 +209,6 @@ func (t *Template) lookup(name string, scope int) *Template { if scope >= len(t.tmpl[name]) { return nil } - //fmt.Printf("\x1b[1;35mname: %q\tscope: %d\tlen: %d\x1b[0m\n", name, scope, len(t.tmpl[name])) return t.tmpl[name][scope] // TODO: invariant check: at least one } From 1dd4a2209b95f580798b628cdd8cbe01ac5ddff5 Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Wed, 8 Sep 2021 12:09:01 -0500 Subject: [PATCH 5/5] Clean up some notes and refer to generations instead of stack --- src/text/template/exec.go | 26 ++++++++-------- src/text/template/parse/lex_test.go | 1 + src/text/template/template.go | 46 ++++++++++++++++++----------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/text/template/exec.go b/src/text/template/exec.go index 230000929ed124..40cb8e4669f1d2 100644 --- a/src/text/template/exec.go +++ b/src/text/template/exec.go @@ -33,10 +33,10 @@ func initMaxExecDepth() int { type state struct { tmpl *Template wr io.Writer - node parse.Node // current node, for errors - vars []variable // push-down stack of variable values. - depth int // the height of the stack of executing templates. - scope map[string]int + node parse.Node // current node, for errors + vars []variable // push-down stack of variable values. + depth int // the height of the stack of executing templates. + level map[string]int // map from template name to current level. } // variable holds the dynamic value of a variable such as $, $x etc. @@ -211,7 +211,7 @@ func (t *Template) execute(wr io.Writer, data interface{}) (err error) { tmpl: t, wr: wr, vars: []variable{{"$", value}}, - scope: make(map[string]int), + level: make(map[string]int), } if t.Tree == nil || t.Root == nil { state.errorf("%q is an incomplete or empty template", t.Name()) @@ -403,12 +403,12 @@ func (s *state) walkRange(dot reflect.Value, r *parse.RangeNode) { func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) { s.at(t) - scope := s.scope[t.Name] + level := s.level[t.Name] if t.Parent { - scope++ + level++ } - s.scope[t.Name] = scope - tmpl := s.tmpl.LookupWithScope(t.Name, scope) + s.level[t.Name] = level + tmpl := s.tmpl.LookupByLevel(t.Name, level) if tmpl == nil { s.errorf("template %q not defined", t.Name) } @@ -423,11 +423,11 @@ func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) { // No dynamic scoping: template invocations inherit no variables. newState.vars = []variable{{"$", dot}} newState.walk(dot, tmpl.Root) - scope-- - if scope < 0 { // TODO logical bug - scope = 0 + level-- + if level < 0 { + level = 0 // TODO: this could be masking a bug } - newState.scope[t.Name] = scope // TODO: unclear if this makes sense here + newState.level[t.Name] = level } // Eval functions evaluate pipelines, commands, and their elements and extract diff --git a/src/text/template/parse/lex_test.go b/src/text/template/parse/lex_test.go index 6510eed674dd9f..ae0e517ebf7bd0 100644 --- a/src/text/template/parse/lex_test.go +++ b/src/text/template/parse/lex_test.go @@ -40,6 +40,7 @@ var itemName = map[itemType]string{ itemIf: "if", itemEnd: "end", itemNil: "nil", + itemParent: "parent", itemRange: "range", itemTemplate: "template", itemWith: "with", diff --git a/src/text/template/template.go b/src/text/template/template.go index aac8a628be1d5c..59d49fb531c2bd 100644 --- a/src/text/template/template.go +++ b/src/text/template/template.go @@ -12,8 +12,11 @@ import ( // common holds the information shared by related templates. type common struct { - tmpl map[string][]*Template // Map from name to defined templates. - muTmpl sync.RWMutex // protects tmpl + // Map from name to defined templates. There is a slice of templates, + // because there are generations of previously defined templates with the + // same name. + tmpl map[string][]*Template + muTmpl sync.RWMutex // protects tmpl option option // We use two maps, one for parsing and one for execution. // This separation makes the API cleaner since it doesn't @@ -91,14 +94,14 @@ func (t *Template) Clone() (*Template, error) { } t.muTmpl.RLock() defer t.muTmpl.RUnlock() - for k, stack := range t.tmpl { + for k, generations := range t.tmpl { if k == t.name { - nt.tmpl[t.name] = append(nt.tmpl[t.name], nt) // TODO: double-check this - maybe should just be assigning new slice + nt.tmpl[t.name] = append(nt.tmpl[t.name], nt) continue } // The associated templates share nt's common structure. - for i := range stack { - tmpl := stack[i].copy(nt.common) + for i := range generations { + tmpl := generations[i].copy(nt.common) nt.tmpl[k] = append(nt.tmpl[k], tmpl) } } @@ -152,8 +155,8 @@ func (t *Template) Templates() []*Template { t.muTmpl.RLock() defer t.muTmpl.RUnlock() m := make([]*Template, 0, len(t.tmpl)) - for _, stack := range t.tmpl { - m = append(m, stack[0]) // TODO: runtime assert there is at least one? invariant check + for _, generations := range t.tmpl { + m = append(m, generations[0]) } return m } @@ -196,20 +199,20 @@ func (t *Template) Lookup(name string) *Template { return t.lookup(name, 0) } -func (t *Template) LookupWithScope(name string, scope int) *Template { +func (t *Template) LookupByLevel(name string, level int) *Template { if t.common == nil { return nil } t.muTmpl.RLock() defer t.muTmpl.RUnlock() - return t.lookup(name, scope) + return t.lookup(name, level) } -func (t *Template) lookup(name string, scope int) *Template { - if scope >= len(t.tmpl[name]) { +func (t *Template) lookup(name string, level int) *Template { + if level < 0 || level >= len(t.tmpl[name]) { return nil } - return t.tmpl[name][scope] // TODO: invariant check: at least one + return t.tmpl[name][level] } // Parse parses text as a template body for t. @@ -251,10 +254,17 @@ func (t *Template) associate(new *Template, tree *parse.Tree) bool { // don't replace it with an empty template. return false } - stack := t.tmpl[new.name] - stack = append(stack, nil) - copy(stack[1:], stack[0:]) - stack[0] = new - t.tmpl[new.name] = stack + + // associating a template with the name of an existing one retains a + // reference to the original, pushing the new one to the front of a slice + // of previously defined templates, so that previously defined templates by + // that name can be accessed by indexing into the slice, each successive + // level being an older generation. + generations := t.tmpl[new.name] + generations = append(generations, nil) + copy(generations[1:], generations[0:]) + generations[0] = new + t.tmpl[new.name] = generations + return true }