code review 130620043: text/template: add back pointer to Nodes for better err...

text/template: add back pointer to Nodes for better error generation
ErrorContext now has all the information it needs from the Node,
rather than depending on the template that contains it. This makes
it easier for html/template to generate correct locations in its
error messages.

Updated html/template to use this ability where it is easy, which is
not everywhere, but more work can probably push it through.

Fixes issue 8577.

[r, 2014-08-26 22:44:30]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[adg, 2014-08-27 04:40:38]

LGTM

https://codereview.appspot.com/130620043/diff/80001/src/pkg/text/template/par...
File src/pkg/text/template/parse/node.go (right):

https://codereview.appspot.com/130620043/diff/80001/src/pkg/text/template/par...
src/pkg/text/template/parse/node.go:29: // parent returns the containing *Tree.
why "parent" and not "tree" ?

[r, 2014-08-27 05:21:40]

https://codereview.appspot.com/130620043/diff/80001/src/pkg/text/template/par...
File src/pkg/text/template/parse/node.go (right):

https://codereview.appspot.com/130620043/diff/80001/src/pkg/text/template/par...
src/pkg/text/template/parse/node.go:29: // parent returns the containing *Tree.
On 2014/08/27 04:40:37, adg wrote:
> why "parent" and not "tree" ?

i need a method and a field and can't use an initial cap to discriminate.
it is the parent.
i'll take a better suggestion.

[adg, 2014-08-27 05:23:06]

I didn't see the collision. It was "tr" where I saw it.
But "parent" is fine. My LGTM stands.

[r, 2014-08-29 16:54:05]

*** Submitted as https://code.google.com/p/go/source/detail?r=5f91e12d9798 ***

text/template: add back pointer to Nodes for better error generation
ErrorContext now has all the information it needs from the Node,
rather than depending on the template that contains it. This makes
it easier for html/template to generate correct locations in its
error messages.

Updated html/template to use this ability where it is easy, which is
not everywhere, but more work can probably push it through.

Fixes issue 8577.

LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://codereview.appspot.com/130620043

[gobot, 2014-08-29 16:57:40]

This CL appears to have broken the linux-amd64 builder.
See http://build.golang.org/log/344b2089ed1dabdb3c65b8f7937ff6b8b0f58a34

[josharian, 2014-08-29 17:06:14]

Real:

Error running API checker: exit status 1


On Fri, Aug 29, 2014 at 9:57 AM,  <gobot@golang.org> wrote:
> This CL appears to have broken the linux-amd64 builder.
> See http://build.golang.org/log/344b2089ed1dabdb3c65b8f7937ff6b8b0f58a34
>
>
> https://codereview.appspot.com/130620043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[bradfitz, 2014-08-29 17:08:27]

The api checker caught something!



On Fri, Aug 29, 2014 at 10:05 AM, Josh Bleecher Snyder <josharian@gmail.com>
wrote:

> Real:
>
> Error running API checker: exit status 1
>
>
> On Fri, Aug 29, 2014 at 9:57 AM,  <gobot@golang.org> wrote:
> > This CL appears to have broken the linux-amd64 builder.
> > See http://build.golang.org/log/344b2089ed1dabdb3c65b8f7937ff6b8b0f58a34
> >
> >
> > https://codereview.appspot.com/130620043/
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "golang-codereviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to golang-codereviews+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[r, 2014-08-29 17:23:56]

Cool.

It's an interesting one. I prefer the new code and it is actually an
API expansion since the receiver can now be a value. What do you
think? I think it's fine.

Nice catch regardless.

-rob


On Fri, Aug 29, 2014 at 10:08 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:
> The api checker caught something!
>
>
>
> On Fri, Aug 29, 2014 at 10:05 AM, Josh Bleecher Snyder <josharian@gmail.com>
> wrote:
>>
>> Real:
>>
>> Error running API checker: exit status 1
>>
>>
>> On Fri, Aug 29, 2014 at 9:57 AM,  <gobot@golang.org> wrote:
>> > This CL appears to have broken the linux-amd64 builder.
>> > See http://build.golang.org/log/344b2089ed1dabdb3c65b8f7937ff6b8b0f58a34
>> >
>> >
>> > https://codereview.appspot.com/130620043/
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "golang-codereviews" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an
>> > email to golang-codereviews+unsubscribe@googlegroups.com.
>> > For more options, visit https://groups.google.com/d/optout.
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "golang-codereviews" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-codereviews+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>

[bradfitz, 2014-08-29 17:39:16]

Why not both? Mailed https://codereview.appspot.com/136010043



On Fri, Aug 29, 2014 at 10:23 AM, Rob Pike <r@golang.org> wrote:

> Cool.
>
> It's an interesting one. I prefer the new code and it is actually an
> API expansion since the receiver can now be a value. What do you
> think? I think it's fine.
>
> Nice catch regardless.
>
> -rob
>
>
> On Fri, Aug 29, 2014 at 10:08 AM, Brad Fitzpatrick <bradfitz@golang.org>
> wrote:
> > The api checker caught something!
> >
> >
> >
> > On Fri, Aug 29, 2014 at 10:05 AM, Josh Bleecher Snyder <
> josharian@gmail.com>
> > wrote:
> >>
> >> Real:
> >>
> >> Error running API checker: exit status 1
> >>
> >>
> >> On Fri, Aug 29, 2014 at 9:57 AM,  <gobot@golang.org> wrote:
> >> > This CL appears to have broken the linux-amd64 builder.
> >> > See
> http://build.golang.org/log/344b2089ed1dabdb3c65b8f7937ff6b8b0f58a34
> >> >
> >> >
> >> > https://codereview.appspot.com/130620043/
> >> >
> >> > --
> >> > You received this message because you are subscribed to the Google
> >> > Groups
> >> > "golang-codereviews" group.
> >> > To unsubscribe from this group and stop receiving emails from it, send
> >> > an
> >> > email to golang-codereviews+unsubscribe@googlegroups.com.
> >> > For more options, visit https://groups.google.com/d/optout.
> >>
> >> --
> >> You received this message because you are subscribed to the Google
> Groups
> >> "golang-codereviews" group.
> >> To unsubscribe from this group and stop receiving emails from it, send
> an
> >> email to golang-codereviews+unsubscribe@googlegroups.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >
> >
>