The report in #17414 points out that if you have many many templates,
then this is an overwhelming list and just hurts the signal-to-noise ratio of the error.
Even the test of the old behavior also supports the idea that this is noise:
template: empty: "empty" is an incomplete or empty template; defined templates are: "secondary"
The chance that someone mistyped "secondary" as "empty" is slim at best.
Similarly, the compiler does not augment an error like 'unknown variable x'
by dumping the full list of all the known variables.
For all these reasons, drop the list.
Fixes#17414.
Change-Id: I78f92d2c591df7218385fe723a4abc497913acf8
Reviewed-on: https://go-review.googlesource.com/32116
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
All prior versions of Go have allowed redefining empty templates
to become non-empty. Unfortunately, that has never consistently
taken effect in html/template after the first execution:
// define and execute
t := template.New("root")
t.Parse(`{{define "T"}}{{end}}<a href="{{template "T"}}">`)
t.Execute(w, nil) // <a href="">
// redefine
t.Parse(`{{define "T"}}my.url{{end}}`) // succeeds, but ignored
t.Execute(w, nil) // <a href="">
When Go 1.6 added {{block...}} to text/template, that loosened the
redefinition rules to allow redefinition at any time. The loosening was
undone a bit in html/template, although inconsistently:
// define and execute
t := template.New("root")
t.Parse(`{{define "T"}}body{{end}}`)
t.Lookup("T").Execute(ioutil.Discard, nil)
// attempt to redefine
t.Parse(`{{define "T"}}body{{end}}`) // rejected in all Go versions
t.Lookup("T").Parse("body") // OK as of Go 1.6, likely unintentionally
Like in the empty->non-empty case, whether future execution takes
notice of a redefinition basically can't be explained without going into
the details of the template escape analysis.
Address both the original inconsistencies in whether a redefinition
would have any effect and the new inconsistencies about whether a
redefinition is allowed by adopting a new rule: no parsing or modifying
any templates after the first execution of any template in the same set.
Template analysis begins at first execution, and once template analysis
has begun, we simply don't have the right logic to update the analysis
for incremental modifications (and never have).
If this new rule breaks existing uses of templates that we decide need
to be supported, we can try to invalidate all escape analysis for the
entire set after any modifications. But let's wait on that until we know
we need to and why.
Also fix documentation of text/template redefinition policy
(redefinition is always OK).
Fixes#15761.
Change-Id: I7d58d7c08a7d9df2440ee0d651a5b2ecaff3006c
Reviewed-on: https://go-review.googlesource.com/31464
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
Add support for passing reflect.Values to and returning reflect.Values from
any registered functions in the FuncMap, much as if they were
interface{} values. Keeping the reflect.Value instead of round-tripping
to interface{} preserves addressability of the value, which is important
for method lookup.
Change index and a few other built-in functions to use reflect.Values,
making a loop using explicit indexing now match the semantics that
range has always had.
Fixes#14916.
Change-Id: Iae1a2fd9bb426886a7fcd9204f30a2d6ad4646ad
Reviewed-on: https://go-review.googlesource.com/31462
Reviewed-by: Rob Pike <r@golang.org>
Make two important points clearer:
- Giving a template definition containing
nothing but spaces has no effect.
- Giving a template definition containing
non-spaces can only be done once per template.
Fixes#16912.
Fixes#16913.
Fixes#17360.
Change-Id: Ie3971b83ab148b7c8bb800fe4a21579566378e3e
Reviewed-on: https://go-review.googlesource.com/31459
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The existing implementation calls l.next for each run up to the next
instance of the left delimiter ({{). For ascii text, this is multiple
function calls per byte. Change to use strings.Index to find the left
delimiter. The performace improvement ranges from 1:1 (no text outside
of {{}}'s) to multiple times faster (9:1 was seen on 8K of text with no
{{ }}'s).
Change-Id: I2f82bea63b78b6714f09a725f7b2bbb00a3448a3
Reviewed-on: https://go-review.googlesource.com/24863
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
This is reverting golang.org/cl/19622 and introducing "<input>"
as filename if no filename is specified.
Fixes#15813.
Change-Id: Iafc74b789fa33f48ee639c42d4aebc6f06435f95
Reviewed-on: https://go-review.googlesource.com/23402
Reviewed-by: Russ Cox <rsc@golang.org>
Return an error message instead of eating memory and eventually
triggering a stack overflow.
Fixes#15618
Change-Id: I3dcf1d669104690a17847a20fbfeb6d7e39e8751
Reviewed-on: https://go-review.googlesource.com/23091
Reviewed-by: Rob Pike <r@golang.org>
Document the subtle property that files with equivalent base names
will overwrite extant templates with those same names.
Fixesgolang/go#14320
Change-Id: Ie9ace1b08e6896ea599836e31582123169aa7a25
Reviewed-on: https://go-review.googlesource.com/21824
Reviewed-by: Rob Pike <r@golang.org>
When evaluating "{{.MissingField}}" on a nil *T, Exec returns
"can't evaluate field MissingField in type *T" instead of
"nil pointer evaluating *T.MissingField".
Fixesgolang/go#15125
Change-Id: I6e73f61b8a72c694179c1f8cdc808766c90b6f57
Reviewed-on: https://go-review.googlesource.com/21705
Reviewed-by: Rob Pike <r@golang.org>
This change removes a lot of dead code. Some of the code has never been
used, not even when it was first commited. The rest shouldn't have
survived refactors.
This change doesn't remove unused routines helpful for debugging, nor
does it remove code that's used in commented out blocks of code that are
only unused temporarily. Furthermore, unused constants weren't removed
when they were part of a set of constants from specifications.
One noteworthy omission from this CL are about 1000 lines of unused code
in cmd/fix, 700 lines of which are the typechecker, which hasn't been
used ever since the pre-Go 1 fixes have been removed. I wasn't sure if
this code should stick around for future uses of cmd/fix or be culled as
well.
Change-Id: Ib714bc7e487edc11ad23ba1c3222d1fd02e4a549
Reviewed-on: https://go-review.googlesource.com/20926
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Named returned values should only be used on public funcs and methods
when it contributes to the documentation.
Named return values should not be used if they're only saving the
programmer a few lines of code inside the body of the function,
especially if that means there's stutter in the documentation or it
was only there so the programmer could use a naked return
statement. (Naked returns should not be used except in very small
functions)
This change is a manual audit & cleanup of public func signatures.
Signatures were not changed if:
* the func was private (wouldn't be in public godoc)
* the documentation referenced it
* the named return value was an interesting name. (i.e. it wasn't
simply stutter, repeating the name of the type)
There should be no changes in behavior. (At least: none intended)
Change-Id: I3472ef49619678fe786e5e0994bdf2d9de76d109
Reviewed-on: https://go-review.googlesource.com/20024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
More clearly distinguish between tab-terminated cells
which are part of an (aligned) column, and non-tab terminated
cells which are not part of a column. Added additional examples.
For #14412.
Change-Id: If72607385752e221eaa2518238b11f48fbcb8a90
Reviewed-on: https://go-review.googlesource.com/19855
Reviewed-by: Alan Donovan <adonovan@google.com>
The header was in the wrong place, so the definition of a pipeline
was not in the section labeled "Pipelines".
Fixes#13972
Change-Id: Ibca791a4511ca112047b57091c391f6e959fdd78
Reviewed-on: https://go-review.googlesource.com/18775
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The prefix includes a semicolon.
Change-Id: I4bdb79aa9931e835e297f3ea2c46a001cd123d56
Reviewed-on: https://go-review.googlesource.com/17200
Reviewed-by: Andrew Gerrand <adg@golang.org>
Read what a non-empty interface points to.
The deleted lines were added in https://codereview.appspot.com/4810060/,
which attempted to break an infinite loop. That was a long time ago.
If I just delete these lines with current codebase, the test "bug1"
(added in that CL) does not fail.
All new tests fail without this fix.
Fixes#12924
Change-Id: I9370ca44facd6af3019850aa065b936e5a482d37
Reviewed-on: https://go-review.googlesource.com/15809
Reviewed-by: Andrew Gerrand <adg@golang.org>
This is a follow-up to a326c3e to avoid reflect being in the API.
Fixes#12801.
Change-Id: Ic4c2e592e2c35b5911f75d88f1d9c44787c80f30
Reviewed-on: https://go-review.googlesource.com/15240
Run-TryBot: David Symonds <dsymonds@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
This change adds a new "block" keyword that permits the definition
of templates inline inside existing templates, and loosens the
restriction on template redefinition. Templates may now be redefined,
but in the html/template package they may only be redefined before
the template is executed (and therefore escaped).
The intention is that such inline templates can be redefined by
subsequent template definitions, permitting a kind of template
"inheritance" or "overlay". (See the example for details.)
Fixes#3812
Change-Id: I733cb5332c1c201c235f759cc64333462e70dc27
Reviewed-on: https://go-review.googlesource.com/14005
Reviewed-by: Rob Pike <r@golang.org>
The definition of 'truth' used by if etc. is not trivial to compute, so publish
the implementation to allow custom template functions to have the
same definition as the template language itself.
Fixes#12033.
Change-Id: Icdfd6039722d7d3f984ba0905105eb3253e14831
Reviewed-on: https://go-review.googlesource.com/14593
Reviewed-by: Andrew Gerrand <adg@golang.org>
There was no verification in Funcs that the map had valid names,
which meant that the error could only be caught when parsing
the template that tried to use them. Fix this by validating the names
in Funcs and panicking before parsing if there is a bad name.
This is arguably an API change, since it didn't trigger a panic
before, but Funcs did already panic if the function itself was no
good, so I argue it's an acceptable change to add more sanity
checks.
Fixes#9685.
Change-Id: Iabf1d0602c49d830f3ed71ca1ccc7eb9a5521ff5
Reviewed-on: https://go-review.googlesource.com/14562
Reviewed-by: Andrew Gerrand <adg@golang.org>
Check reflect.Value.IsValid() before calling other reflect.Value methods
that panic on zero values.
Added tests for cases with untyped nils. They panicked without these fixes.
Removed a TODO.
Fixes#12356
Change-Id: I9b5cbed26db09a0a7c36d99a93f8b9729899d51e
Reviewed-on: https://go-review.googlesource.com/14340
Reviewed-by: Rob Pike <r@golang.org>
Borrowing a suggestion from the issue listed below, we modify the lexer to
trim spaces at the beginning (end) of a block of text if the action immediately
before (after) is marked with a minus sign. To avoid parsing/lexing ambiguity,
we require an ASCII space between the minus sign and the rest of the action.
Thus:
{{23 -}}
<
{{- 45}}
produces the output
23<45
All the work is done in the lexer. The modification is invisible to the parser
or any outside package (except I guess for noticing some gaps in the input
if one tracks error positions). Thus it slips in without worry in text/template
and html/template both.
Fixes long-requested issue #9969.
Change-Id: I3774be650bfa6370cb993d0899aa669c211de7b2
Reviewed-on: https://go-review.googlesource.com/14391
Reviewed-by: Andrew Gerrand <adg@golang.org>
Block comments appear after a block in the HTML documentation generated by
godoc. Words like "following" should be avoided.
Change-Id: Iedfad67f4b8b9c84f128b98b9b06fa76919af388
Reviewed-on: https://go-review.googlesource.com/14357
Reviewed-by: Rob Pike <r@golang.org>
This is follow-up to CL10607
- Refactor AddParseTree() to use t.associate()
- Refactor Parse() to use AddParseTree() to put entries into common structure
- Clone() should not put entry in t.tmpl for undefined template
- Clarify documentation for Templates()
- Clarify documentation for AddParseTree() to include the error case
Updates #10910
Uodates #10926
Includes test cases for most of the above changes
Change-Id: I25b2fce6f9651272866f881acf44e4dbca04a4a8
Reviewed-on: https://go-review.googlesource.com/10622
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Due to the requirements of parsing template definitions that mention
other templates that are not yet defined, a Template can be in two states:
defined and undefined. Thus, although one calls New, the resulting
template has no definition even though it exists as a data structure.
Thus, for example, will return nil for a template that is named but not
yet defined.
Fixes#10910Fixes#10926
Clarify the documentation a little to explain this,
Also tidy up the code a little and remove a spurious call to init.
Change-Id: I22cc083291500bca424e83dc12807e0de7b00b7a
Reviewed-on: https://go-review.googlesource.com/10641
Reviewed-by: Andrew Gerrand <adg@golang.org>
Executing a template involving variadic functions featuring
a []interface{} slice (such as printf) could result in a
panic in reflect.Value.Call, due to incorrect type checking.
The following expressions failed (with a panic):
{{true|printf}}
{{1|printf}}
{{1.1|printf}}
{{'x'|printf}}
{{1+2i|printf}}
Implemented proper type checks for the fixed parameters of the
variadic functions.
Fixes#10946
Change-Id: Ia75333f651f73b3d2e024cb0c47cc30d90cb6852
Reviewed-on: https://go-review.googlesource.com/10403
Reviewed-by: Rob Pike <r@golang.org>
The current escape code panics when an action involves chain nodes.
Such nodes can be seen in the following situation:
{{ . | AAA.B }} - AAA being a registered function
The above expression is actually valid, because AAA could return a
map containing a B key. The tests in text/template explicitly
demonstrate this case.
Fix allIdents to cover also chain nodes.
While I was investigating this issue, I realized that the tests
introduced in similar CL 9621 were incorrect. Parse errors were
caught as expected, but for the wrong reason. Fixed them as well.
No changes in text/template code itself.
Fixes#10801
Change-Id: Ic9fe43b63669298ca52c3f499e2725dd2bb818a8
Reviewed-on: https://go-review.googlesource.com/10340
Reviewed-by: Rob Pike <r@golang.org>
t.init() should be called at the time of template creation
i.e, template.New() and t.New() instead of later in the process.
- Removed calls of t.init() from t.Parse(), t.Execute(), t.Funcs()
- Also got rid of t.common != nil checks as it should never be nil
Fixes#10879
Change-Id: I1b7ac812f02c841ae80037babce7e2b0a2df13e8
Reviewed-on: https://go-review.googlesource.com/10240
Reviewed-by: Rob Pike <r@golang.org>
The Template objects are supposed to be goroutine-safe once they
have been parsed. This includes the text and html ones.
For html/template, the escape mechanism is triggered at execution
time. It may alter the internal structures of the template, so
a mutex protects them against concurrent accesses.
The text/template package is free of any synchronization primitive.
A race condition may occur when nested templates are escaped:
the escape algorithm alters the function maps of the associated
text templates, while a concurrent template execution may access
the function maps in read mode.
The less invasive fix I have found is to introduce a RWMutex in
text/template to protect the function maps. This is unfortunate
but it should be effective.
Fixes#9945
Change-Id: I1edb73c0ed0f1fcddd2f1516230b548b92ab1269
Reviewed-on: https://go-review.googlesource.com/10101
Reviewed-by: Rob Pike <r@golang.org>
Missed a case; just need to call validateType.
Fixes#10800.
Change-Id: I81997ca7a9feb1be31c8b47e631b32712d7ffb86
Reviewed-on: https://go-review.googlesource.com/10031
Reviewed-by: Andrew Gerrand <adg@golang.org>
This was added during testing but is unnecessary.
Thanks to gravis on GitHub for catching it.
See #10574.
Change-Id: I4a8f76d237e67f5a0ea189a0f3cadddbf426778a
Reviewed-on: https://go-review.googlesource.com/9841
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Russ Cox
Rob Pike
Brad Fitzpatrick
Josh Bleecher Snyder
Paul Borman
Jess Frazelle
Robert Griesemer
Andrew Gerrand
Johan Sageryd
Dominik Honnef
Martin Möhrmann
Nathan VanBenschoten
Nodir Turakulov
Alexander Morozov
David Symonds
Konstantin Shaposhnikov
Carlos C
Shenghou Ma
Aamir Khan
Didier Spezia