text/template: Not possible to "define" two inline templates with the same name

[bep]

This is the behaviour on Go 1.9. I have not tested other Go versions.

The following:

package main

import (
	"bytes"
	"fmt"
	"log"
	"text/template"
)

func main() {
	var (
		tpl1 = `{{define "T1"}}T1_1{{end}}TPL1:{{template "T1"}}`
		tpl2 = `{{define "T1"}}T1_2{{end}}TPL2:{{template "T1"}}`
	)

	var buf bytes.Buffer
	tmpl := template.New("")

	tmpl1, err := tmpl.New("tpl1").Parse(tpl1)
	if err != nil {
		log.Fatal(err)
	}
	tmpl2, err := tmpl.New("tpl2").Parse(tpl2)
	if err != nil {
		log.Fatal(err)
	}

	if err := tmpl1.Execute(&buf, nil); err != nil {
		log.Fatal(err)
	}

	fmt.Println(buf.String())

	buf.Reset()
	if err := tmpl2.Execute(&buf, nil); err != nil {
		log.Fatal(err)
	}

	fmt.Println(buf.String())

}

Prints:

TPL1:T1_2
TPL2:T1_2

Expected:

TPL1:T1_1
TPL2:T1_2

It looks like they are both put into the same namespace and the last one wins. I have read the documentation at https://golang.org/pkg/text/template/ 3 times and I cannot see that what's described there is what I get. And what I get is really confusing.

gohugoio/hugo#3996

[tschundler]

I've noticed this as well. {{define }} is handled at parse time, not at execution time. So when a {{define}} is found while parsing, it overrides whatever is in the shared named template list.

To work around your problem as expressed, you need something like:

tmpl2, err := tmpl.Clone().New("tpl2").Parse(tpl2)

But that sort of solution is not viable when using template.ParseGlob(...) collect files.

I suspect the solution (to not break compatibility) is to create an alternative to {{define}} that only defines templates at execution time, not parse time. {{deferred "tpl1"}} or {{redefine "tpl1"}} maybe?

This (IMO non-intuitive) behavior of {{define}} is really inhibiting my use of {{block}}s for template inheritance - https://play.golang.org/p/cClqo9wGhr

[bep]

I suspect the solution (to not break compatibility) is to create an alternative to {{define}} that only

What we have now is broken. So adding another keyword will not "unbreak" it without further work.

And since this is often so subtle (no warnings), people may live with their broken web apps without knowing it. Until someone adds a template named zzz.tmpl (will be parsed last because of its name) with some really visible flash content in some duplicate define.

There are (to my knowledge) two different uses of define today:

1. The local macro usage. The only way to do recursive loops in a template. This is (in most cases) obviously a template defined for just that template.
2. The master/overlay usage with the new block keyword introduced in Go 1.6. Here the documentation introduced a careful dance of cloning the overlay and parsing the child template.

And while 2) is a special case, and I guess only uses the define keyword for implementation simplicity, one could argue that redefinitions could make sense for descendants in the parse tree, but not for siblings/ancestors.

I know @natefinch is working on a gotfmt package that may be useful for this ...

[tschundler]

It would be nice to have the templates in the standard library be usable instead of relying on another package. The suggestion of an alternate keyword for execution-time redefine is that the standard library can't break existing customers. For the macro case, it's possible someone defines a bunch of common macros in a separate file and doesn't "include" the macro file with template because there is no need. If the way define works changes, that will suddenly break them.
That's why I think it makes sense to unfortunately have a new keyword. Although I wish that weren't needed.

Another possible solution could be having a loader function that is called when a template references something unknown. Then templates are only parsed as needed. But I don't think that's a good solution.

#3812 seems to be the original discussion of this years ago, with @adg submitting the current implementation. It seems the expectation seems be only parsing the templates you need for a page, and implications of using Glob weren't really considered(?).

[natefinch]

I agree that the original implementation is extremely surprising from a user's point of view. However, I don't think it's something that can be changed. I'm sure there are people out there relying on this behavior to override sections of templates in place of using blocks.

That means it has to be new functionality (and I think it's extremely valuable, given, as Bep said, using subtemplates as macros is very common, and is the only way to express some behaviors).

How about {{local "foo"}}some content{{end}} to define a subtemplate that is namespaced to this template? Thus, any use of {{template "foo"}} in this file would always use the local version if it exists, and no other templates would see "foo" as a template in the global namespace.

Note, I don't like "deferred" because that assumes way too much knowledge about how templates are parsed and executed, and that's part of why the current behavior is surprising.

[bep]

However, I don't think it's something that can be changed.

I'm not jumping to conclusions here, I just point to a fact that something should be done. And to the argument that changing the behaviour of define would break things, my argument is that it is already way more broken than the breakage that a fix would introduce.

So, for the sake of this discussion, we say that there are 100K defines in the wild. 99K of them are local. By fixing define you would fix way more than you break.

Which isn't to say that we should break those, but one could possibly think about creating the fix where it triggers the least amount of changes.

[rsc]

Go 1.5 and probably earlier would have rejected tpl2 saying redefinition of template "T1". Go 1.6 intentionally introduced redefinition of earlier templates, to make the new {{block}} feature more useful (see https://golang.org/doc/go1.6#template).

When you call t := template.New(""), you are creating a template group with a single name space. Future calls to t.New add to that single name space. This is all documented, except that the docs are very dense. There is a long-standing bug to improve those, but that's a separate issue and not going to happen for this release.

If you ever do want to create a separate name space, t.Clone does exactly that. If the model you want to present to Hugo users is that there is a set of global declarations in one place and then multiple independent, confined extensions to that global set in other files, then you could do that by parsing the global set and then using t.Clone as the base for each of the local additions.

I don't believe we should change anything in text/template itself.

[tschundler]

The issue isn't whether things are working as intended. The issue is that as designed, it is both confusing and not very useful. But with just a small change in behavior, it could be less confusing and much more useful.
This is particularly an issue if you want separate front end developers and also want to reduce per-page bespoke Go code to glue together your templates.

Right now, it's necessary to jump through hoops in Go code to fork off different name spaces, or write templates that look like they're from the 90s including header & footer separately instead of inheriting from a parent. (which also makes adding extra header CSS & footer JS awkward.)

Even if I load things in two tiers - a set of master templates / stuff to include, and then clone that on every page load and load the single page's template, I'm now stuck with just two levels in the page hierarchy. In real-world web development, often there are naturally more levels - there's a site-wide base, a template for one type of page that extends upon that, and then a final level for the specific page itself.