CL 95215 changed text/template so that untyped nil arguments were no
longer ignored, but were instead passed to functions as expected.
This had an unexpected effect on html/template, where all data is
implicitly passed to functions: originally untyped nil arguments were
not passed and were thus effectively ignored, but after CL 95215 they
were passed and were printed, typically as an escaped version of "<nil>".
This CL restores some of the behavior of html/template by ignoring
untyped nil arguments passed implicitly to escaper functions.
While eliminating one change to html/template relative to earlier
releases, this unfortunately introduces a different one: originally
values of interface type with the value nil were printed as an escaped
version of "<nil>". With this CL they are ignored as though they were
untyped nil values. My judgement is that this is a less common case.
We'll see.
This CL adds some tests of typed and untyped nil values to
html/template and text/template to capture the current behavior.
Updates #18716Fixes#25875
Change-Id: I5912983ca32b31ece29e929e72d503b54d7b0cac
Reviewed-on: https://go-review.googlesource.com/121815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
golang.org/cl/84480 altered the API for the parse package for
clarity and consistency. However, the changes also broke the
API for consumers of the package. This CL reverts the API
to the previous spelling, adding only a single new exported
symbol.
Fixes#25968
Change-Id: Ieb81054b61eeac7df3bc3864ef446df43c26b80f
Reviewed-on: https://go-review.googlesource.com/120355
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
These were added in CL 106979. I got them wrong.
They were fixed in CL 111643. They were still wrong.
Hopefully this change will be the last fix.
With this fix, CL 106979 is allocation-neutral for BenchmarkRagged.
The performance results for BenchmarkPyramid reported in CL 111643 stand.
Change-Id: Id6a522e6602e5df31f504adf5a3bec9969c18649
Reviewed-on: https://go-review.googlesource.com/116015
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
\f triggers a flush.
This is used (by gofmt, among others) to indicate that
the current aligned segment has ended.
When flushed, it is unlikely that the previous line is
in fact a good predictor of the upcoming line,
so stop treating it as such.
No performance impact on the existing benchmarks,
which do not perform any flushes.
Change-Id: Ifdf3e6d4600713c90db7b51a10e429d9260dc08c
Reviewed-on: https://go-review.googlesource.com/111644
Reviewed-by: Robert Griesemer <gri@golang.org>
The general policy for the current state of js/wasm is that it only
has to support tests that are also supported by nacl.
The test nilptr3.go makes assumptions about which nil checks can be
removed. Since WebAssembly does not signal on reading a null pointer,
all nil checks have to be explicit.
Updates #18892
Change-Id: I06a687860b8d22ae26b1c391499c0f5183e4c485
Reviewed-on: https://go-review.googlesource.com/110096
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Make comment example consistent with its description.
Fixes#24767
Change-Id: Icff54b489040b1ce66c644a21343c6576304cf75
Reviewed-on: https://go-review.googlesource.com/107663
Reviewed-by: Rob Pike <r@golang.org>
Noticed that we can simply use a []byte slice while investigating
a separate issue. Did the obvious simplification.
Change-Id: I921ebbb42135b5f1a10109236ceb9ae6e94ae7e2
Reviewed-on: https://go-review.googlesource.com/104757
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Variables can be declared and shadowing is supported, but modifying
existing variables via assignments was not available.
This meant that modifying a variable from a nested block was not
possible:
{{ $v := "init" }}
{{ if true }}
{{ $v := "changed" }}
{{ end }}
v: {{ $v }} {{/* "init" */}}
Introduce the "=" assignment token, such that one can now do:
{{ $v := "init" }}
{{ if true }}
{{ $v = "changed" }}
{{ end }}
v: {{ $v }} {{/* "changed" */}}
To avoid confusion, rename PipeNode.Decl to PipeNode.Vars, as the
variables may not always be declared after this change. Also change a
few other names to better reflect the added ambiguity of variables in
pipelines.
Modifying the text/template/parse package in a backwards incompatible
manner is acceptable, given that the package godoc clearly states that
it isn't intended for general use. It's the equivalent of an internal
package, back when internal packages didn't exist yet.
To make the changes to the parse package sit well with the cmd/api test,
update except.txt with the changes that we aren't worried about.
Fixes#10608.
Change-Id: I1f83a4297ee093fd45f9993cebb78fc9a9e81295
Reviewed-on: https://go-review.googlesource.com/84480
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Added examples for use of Mode, Whitespace, and IsIdentRune properties.
Fixes#23768
Change-Id: I2528e14fde63a4476f3c25510bf0c5b73f38ba5d
Reviewed-on: https://go-review.googlesource.com/93199
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
It's always nil, so simplify its signature. Found with unparam.
Change-Id: I45dd0f868ec2f5de98a970776be686417c8d73b6
Reviewed-on: https://go-review.googlesource.com/95235
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
reflect.Value is a struct and does not have a kind nor any flag for
untyped nils. As a result, it is tricky to differentiate when we're
missing a value, from when we have one but it is untyped nil.
We could start using *reflect.Value instead, to add one level of
indirection, using nil for missing values and new(reflect.Value) for
untyped nils. However, that is a fairly invasive change, and would also
mean unnecessary allocations.
Instead, use a special reflect.Value that depicts when a value is
missing. This is the case for the "final" reflect.Value in multiple
scenarios, such as the start of a pipeline. Give it a specific,
unexported type too, to make sure it cannot be mistaken for any other
valid value.
Finally, replace "final.IsValid()" with "final != missingVal", since
final.IsValid() will be false when final is an untyped nil.
Also add a few test cases, all different variants of the untyped nil
versus missing value scenario.
Fixes#18716.
Change-Id: Ia9257a84660ead5a7007fd1cced7782760b62d9d
Reviewed-on: https://go-review.googlesource.com/95215
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
It makes no sense to try to get the zero value of a nil type, hence the
panic. When we have a nil type, use reflect.ValueOf(nil) instead.
This was showing itself if one used a missing field on the data between
parentheses, when the data was a nil interface:
t := template.Must(template.New("test").Parse(`{{ (.).foo }}`))
var v interface{}
t.Execute(os.Stdout, v)
Resulting in:
panic: reflect: Zero(nil) [recovered]
panic: reflect: Zero(nil)
Fixes#21171.
Change-Id: Ifcc4a0c67e6df425b65bc9f82fde6fcf03828579
Reviewed-on: https://go-review.googlesource.com/84482
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The new break and continue actions do not work in html/template, and
fixing them requires thinking about security issues that seem too
tricky at this stage of the release. We will try again for 1.11.
Original CL description:
text/template: add break, continue actions in ranges
Adds the two range control actions "break" and "continue". They act the
same as the Go keywords break and continue, but are simplified in that
only the innermost range statement can be broken out of or continued.
Fixes#20531
Updates #20531
Updates #23683
Change-Id: Ia7fd3c409163e3bcb5dc42947ae90b15bdf89853
Reviewed-on: https://go-review.googlesource.com/92155
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
this change removes the state field from the lexer,
because it's only used by the run method and can be
replaced with a local variable
Change-Id: Ib7a90ab6e9a894716cba2c7d9ed71bf2ad1240c0
Reviewed-on: https://go-review.googlesource.com/47338
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The lastPos field used in the past to track the line number of a token.
it's irrelevant anymore, and we can remove it.
Change-Id: I42c0bf55e884b79574a7da4926489f2d77618cd0
Reviewed-on: https://go-review.googlesource.com/49591
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
All of these had a return or break in the else body, so flipping the
condition means we can unindent and simplify.
Change-Id: If93e97504480d18a0dac3f2c8ffe57ab8bcb929c
Reviewed-on: https://go-review.googlesource.com/74190
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Adds the two range control actions "break" and "continue". They act the
same as the Go keywords break and continue, but are simplified in that
only the innermost range statement can be broken out of or continued.
Fixes#20531
Change-Id: I4412b3bbfd4dadb0ab74ae718e308c1ac7a0a1e9
Reviewed-on: https://go-review.googlesource.com/66410
Reviewed-by: Rob Pike <r@golang.org>
Do the low-hanging fruit - tiny Less functions that are used exactly
once. This reduces the amount of code and puts the logic in a single
place.
Change-Id: I9d4544cd68de5a95e55019bdad1fca0a1dbfae9c
Reviewed-on: https://go-review.googlesource.com/63171
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Found with staticcheck. Not terribly important since the test would
likely fail anyway, but at least it will fail with a better explanation
now.
Change-Id: Ic3f9a94a2152404b7873cc8cd47b6db79d78c2e6
Reviewed-on: https://go-review.googlesource.com/62990
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It doesn't change the outcome. It might have been useful at some point
to avoid Replace from doing work or allocating. However, nowadays the
func returns early without doing any work if Count returns 0.
Change-Id: Id69dc74042a6e39672b405016484db8b50f43d58
Reviewed-on: https://go-review.googlesource.com/62991
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
The lexer keeps the byte offset and the line for the rune it's currently
on. This was simple enough up until whitespace trimming was introduced.
With whitespace trimming, we might skip over newlines. In that case, the
lexer wasn't properly updating the line counter. Fix it.
Also, TestPos now checks that the line is correct too, which it was
ignoring before. This was necessary to test this scenario in the lexer.
Fixes#21778.
Change-Id: I3880f3adf02662eac8f818d5caa6935cca9cb33b
Reviewed-on: https://go-review.googlesource.com/61870
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Found with mvdan.cc/unindent. It skipped the cases where parentheses
would need to be added, where comments would have to be moved elsewhere,
or where actions and simple logic would mix.
One of them was of the form "err != nil && err == io.EOF", so the first
part was removed.
Change-Id: Ie504c2b03a2c87d10ecbca1b9270069be1171b91
Reviewed-on: https://go-review.googlesource.com/57690
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Found with mvdan.cc/unindent. Prioritized the ones with the biggest wins
for now.
Change-Id: I2b032e45cdd559fc9ed5b1ee4c4de42c4c92e07b
Reviewed-on: https://go-review.googlesource.com/56470
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Done with grep & interactive search & replace, to double-check
replacements. Not many remained after CL 20022.
Fixes#18572
Change-Id: Idbe90ba3b584f9b9661d2bbd141607daaadfa41a
Reviewed-on: https://go-review.googlesource.com/45270
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
For #20292. (See discussion in that issue.)
Change-Id: I44cd69394fc47a01776905ec34305ba524c89883
Reviewed-on: https://go-review.googlesource.com/43452
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
While at it, unindent source text so column values are easier
to read, remove unnecessary text in output, and simplify the
loop.
Fixes#20346.
Change-Id: I0fde02b9e4242383da427f4cf4c6c13dd0ab3b47
Reviewed-on: https://go-review.googlesource.com/43450
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allow the predefined escapers "html", "urlquery", and "js" to be used
in pipelines when they have no potential to affect the correctness or
safety of the escaped pipeline output. Specifically:
- "urlquery" may be used if it is the last command in the pipeline.
- "html" may be used if it is the last command in the pipeline, and
the pipeline does not occur in an unquoted HTML attribute value
context.
- "js" may be used in any pipeline, since it does not affect the
merging of contextual escapers.
This change will loosens the restrictions on predefined escapers
introduced in golang.org/cl/37880, which will hopefully ease the
upgrade path for existing template users.
This change brings back the escaper-merging logic, and associated
unit tests, that were removed in golang.org/cl/37880. However, a
few notable changes have been made:
- "_html_template_nospaceescaper" is no longer considered
equivalent to "html", since the former escapes spaces, while
the latter does not (see #19345). This change should not silently
break any templates, since pipelines where this substituion will
happen will already trigger an explicit error.
- An "_eval_args_" internal directive has been added to
handle pipelines containing a single explicit call to a
predefined escaper, e.g. {{html .X}} (see #19353).
Also, the HTMLEscape function called by the predefined
text/template "html" function now escapes the NULL character as
well. This effectively makes it as secure as the internal
html/template HTML escapers (see #19345). While this change is
backward-incompatible, it will only affect illegitimate uses
of this escaper, since the NULL character is always illegal in
valid HTML.
Fixes#19952
Change-Id: I9b5570a80a3ea284b53901e6a1f842fc59b33d3a
Reviewed-on: https://go-review.googlesource.com/40936
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Execute incurs separate writes for each "step", e.g. each
variable that needs to be printed, and the final newline.
While it is correct to state that templates can be executed
concurrently, there is a more subtle nuance that is easily missed:
when writing to the same writer, the writes from concurrent execute
calls can be interleaved, leading to unexpected output.
Change-Id: I0abbd7960d8a8d15e109a8a3eeff3b43b852bbbf
Reviewed-on: https://go-review.googlesource.com/37444
Reviewed-by: Rob Pike <r@golang.org>
Mostly unnecessary *testing.T arguments.
Found with github.com/mvdan/unparam.
Change-Id: Ifb955cb88f2ce8784ee4172f4f94d860fa36ae9a
Reviewed-on: https://go-review.googlesource.com/41691
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Returns at the end of func bodies where the funcs have no return values
are pointless.
Change-Id: I0da5ea78671503e41a9f56dd770df8c919310ce5
Reviewed-on: https://go-review.googlesource.com/41093
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This was a subtle bug introduced in the previous release's fix for
issue 16156.
The definition of empty template was broken, causing the answer
to depend on the order of templates in the map.
Fixes#16156 (for real).
Fixes#19294.
Fixes#19204.
Change-Id: I1cd915c94534cad3116d83bd158cbc28700510b9
Reviewed-on: https://go-review.googlesource.com/38420
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Any method that affects the parse must happen before parsing.
This obvious point is clear, but it's not clear to some that the
set of defined functions affect the parse.
Fixes#18971
Change-Id: I8b7f8c8cf85b028c18e5ca3b9797de92ea910669
Reviewed-on: https://go-review.googlesource.com/38413
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It looks like this conditional may have been refactored at some point,
but the logic was still very confusing. The outer conditional checks if
the function is variadic, so there's no need to verify that in the
result. Additionally, since the function isn't variadic, there is no
reason to permit the function call if the number of input arguments is
less than the function signature requires.
Change-Id: Ia957cf83d1c900c08dd66384efcb74f0c368422e
Reviewed-on: https://go-review.googlesource.com/35491
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The existing implementation of text/template handles the option
"missingkey=error" in an inconsitent manner: If the provided data is
a nil-interface, no error is returned (despite the fact that no key
can be found in it).
This patch makes text/template return an error if "missingkey=error"
is set and the provided data is a not a valid reflect.Value.
Fixes#15356
Change-Id: Ia0a83da48652ecfaf31f18bdbd78cb21dbca1164
Reviewed-on: https://go-review.googlesource.com/31638
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of scanning the text to count newlines, which is n², keep track as we go
and store the line number in the token.
benchmark old ns/op new ns/op delta
BenchmarkParseLarge-4 1589721293 38783310 -97.56%
Fixes#17851
Change-Id: I231225c61e667535e2ce55cd2facea6d279cc59d
Reviewed-on: https://go-review.googlesource.com/33234
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 794fb71d9c.
Reason for revert: submitted without TryBots and it broke all three race builders.
Change-Id: I80a1e566616f0ee8fa3529d4eeee04268f8a713b
Reviewed-on: https://go-review.googlesource.com/33232
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead of scanning the text to count newlines, which is n², keep track as we go
and store the line number in the token.
benchmark old ns/op new ns/op delta
BenchmarkParseLarge-4 1589721293 38783310 -97.56%
Fixes#17851
Change-Id: Ieaf89a35e371b405ad92e38baa1e3fa98d18cfb4
Reviewed-on: https://go-review.googlesource.com/32923
Reviewed-by: Robert Griesemer <gri@golang.org>
CL 31462 made it possible to operate directly on reflect.Values
instead of always forcing a round trip to interface{} and back.
The round trip was losing addressability, which hurt users.
The round trip was also losing "interface-ness", which helped users.
That is, using reflect.ValueOf(v.Interface()) instead of v was doing
an implicit indirect any time v was itself an interface{} value: the result
was the reflect.Value for the underlying concrete value contained in the
interface, not the interface itself.
CL 31462 eliminated some "unnecessary" reflect.Value round trips
in order to preserve addressability, but in doing so it lost this implicit
indirection. This CL adds the indirection back.
It may help to compare the changes in this CL against funcs.go from CL 31462:
https://go-review.googlesource.com/#/c/31462/4/src/text/template/funcs.go
Everywhere CL 31462 changed 'v := reflect.ValueOf(x)' to 'v := x',
this CL changes 'v := x' to 'v := indirectInterface(x)'.
Fixes#17714.
Change-Id: I67cec4eb41fed1d56e1c19f12b0abbd0e59d35a2
Reviewed-on: https://go-review.googlesource.com/33139
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
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>
Catch some malformed pipelines at parsing time.
The current code accepts pipelines such as:
{{12|.}}
{{"hello"|print|false}}
{{.|"blah blah"}}
Such pipelines generate panic in html/template at execution time.
Add an extra check to verify all the commands of the pipeline are executable
(except for the first one).
Fixes#10610
Change-Id: Id72236ba8f76a59fa284fe3d4c2cb073e50b51f1
Reviewed-on: https://go-review.googlesource.com/9626
Reviewed-by: Rob Pike <r@golang.org>
When a parse error occurred, the lexing goroutine would lay idle.
It's not likely a problem but if the program is for some reason
accepting badly formed data repeatedly, it's wasteful.
The solution is easy: Just drain the input on error. We know this
will succeed because the input is always a string and is therefore
guaranteed finite.
With debugging prints in the package tests I've shown this is effective,
shutting down 79 goroutines that would otherwise linger, out of 123 total.
Fixes#10574.
Change-Id: I8aa536e327b219189a7e7f604a116fa562ae1c39
Reviewed-on: https://go-review.googlesource.com/9658
Reviewed-by: Russ Cox <rsc@golang.org>
At the end of lexInsideAction(), we return lexInsideAction: this is the default
behaviour when we are still parsing an action. But some switch branches return
lexInsideAction too.
So let's ensure code consistency by always reaching the end of the
lexInsideAction function when needed.
Change-Id: I7e9d8d6e51f29ecd6db6bdd63b36017845d95368
Reviewed-on: https://go-review.googlesource.com/9441
Reviewed-by: Rob Pike <r@golang.org>
Ian Lance Taylor
Andrew Braunstein
Daniel Theophanes
Josh Bleecher Snyder
Shengyu Zhang
Richard Musiol
Zheng Dayu
Robert Griesemer
Daniel Martí
Adam Woodbeck
Yury Smolsky
Kunpei Sakai
Ariel Mashraki
Tim Cooper
Emmanuel Odeke
Brad Fitzpatrick
Samuel Tan
Dieter Plaetinck
Rob Pike
Russ Cox
Seth Vargo
Özgür Kesim
Paul Borman
Jess Frazelle
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
Aymerick