code review 4837063: exp/template: url filter.

exp/template: url filter.

[dsymonds, 2011-08-08 05:56:13]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2011-08-08 06:19:57]

LGTM although i think some of this stuff will end up elsewhere.

http://codereview.appspot.com/4837063/diff/2001/src/pkg/exp/template/doc.go
File src/pkg/exp/template/doc.go (right):

http://codereview.appspot.com/4837063/diff/2001/src/pkg/exp/template/doc.go#n...
src/pkg/exp/template/doc.go:249: its arguments in a form suitable for use in a
URL.
s/use/embedding/

[dsymonds, 2011-08-08 06:29:45]

http://codereview.appspot.com/4837063/diff/2001/src/pkg/exp/template/doc.go
File src/pkg/exp/template/doc.go (right):

http://codereview.appspot.com/4837063/diff/2001/src/pkg/exp/template/doc.go#n...
src/pkg/exp/template/doc.go:249: its arguments in a form suitable for use in a
URL.
On 2011/08/08 06:19:57, r wrote:
> s/use/embedding/

Done.

[dsymonds, 2011-08-08 06:30:11]

*** Submitted as http://code.google.com/p/go/source/detail?r=095d46c5b963 ***

exp/template: url filter.

R=r
CC=golang-dev
http://codereview.appspot.com/4837063

[rsc, 2011-08-08 11:46:30]

Can we call this urlquery instead?
It does not apply to urls in general, just to query parameters.

Russ

[dsymonds, 2011-08-08 11:50:57]

On Mon, Aug 8, 2011 at 9:46 PM, Russ Cox <rsc@golang.org> wrote:

> Can we call this urlquery instead?
> It does not apply to urls in general, just to query parameters.

"url" matches the existing "html" and "js", which are used for
filtering a string to make it suitable for use *in* HTML or JS.
Also, this applies to any part of a URL path onwards, and you hardly
ever care to use arbitrary user data before the path.


Dave.

[rsc, 2011-08-08 11:57:51]

> "url" matches the existing "html" and "js", which are used for
> filtering a string to make it suitable for use *in* HTML or JS.

yes, and this makes it suitable for use in a url query.

> Also, this applies to any part of a URL path onwards, and you hardly
> ever care to use arbitrary user data before the path.

not true.  godoc http URLEscape
it translates ' ' to '+'.

[dsymonds, 2011-08-08 12:34:41]

On Mon, Aug 8, 2011 at 9:57 PM, Russ Cox <rsc@golang.org> wrote:

>> Also, this applies to any part of a URL path onwards, and you hardly
>> ever care to use arbitrary user data before the path.
>
> not true.  godoc http URLEscape
> it translates ' ' to '+'.

Okay, you've got me there. Still, most of it is still escaped
correctly, and my other point still stands: you hardly ever care to
have arbitrary user data in a position where this filter isn't
sufficient. I can't think of any other templating system I've used
where the equivalent to this filter mentions "query" in its name, and
that's because it hardly ever comes up.


Dave.

[dsymonds, 2011-08-08 12:38:21]

On Mon, Aug 8, 2011 at 9:57 PM, Russ Cox <rsc@golang.org> wrote:

>> "url" matches the existing "html" and "js", which are used for
>> filtering a string to make it suitable for use *in* HTML or JS.
>
> yes, and this makes it suitable for use in a url query.

Also, to be just as strict, we'd want to rename "js" to "jsstring",
because that's the only part of JavaScript where this kind of escaping
is relevant. But I think people can handle the slight lack of
precision.


Dave.

[rsc, 2011-08-08 12:44:02]

the reason to change the name is exactly
because you misunderstood what it does.

some day i will change the name in package
http too, but here i would like to start off on
the right foot.

[dsymonds, 2011-08-08 13:13:26]

On Aug 8, 2011 10:44 PM, "Russ Cox" <rsc@golang.org> wrote:

> the reason to change the name is exactly
> because you misunderstood what it does.

I didn't misunderstand what it does; I merely overreached its applicability.
My point still stands: in practice, you don't care about escaping any part
of the URL except for the query. Adding "query" to the name is just noise,
ans templates including URLs are noisy enough.

Dave.

[rsc, 2011-08-08 13:15:06]

>> the reason to change the name is exactly
>> because you misunderstood what it does.
>
> I didn't misunderstand what it does; I merely overreached its applicability.
> My point still stands: in practice, you don't care about escaping any part
> of the URL except for the query. Adding "query" to the name is just noise,
> ans templates including URLs are noisy enough.

i don't have the energy to argue.

[r2, 2011-08-08 13:16:15]

On 08/08/2011, at 11:14 PM, Russ Cox wrote:

>>> the reason to change the name is exactly
>>> because you misunderstood what it does.
>> 
>> I didn't misunderstand what it does; I merely overreached its applicability.
>> My point still stands: in practice, you don't care about escaping any part
>> of the URL except for the query. Adding "query" to the name is just noise,
>> ans templates including URLs are noisy enough.
> 
> i don't have the energy to argue.

you shouldn't have to. you're right.

-rob

[dsymonds, 2011-08-08 13:31:51]

Okay, if I'm the only one, I relent. Go for it.

[rsc, 2011-08-08 13:34:17]

As far as the name length arguments go, my hope is that the
new template/html stuff will get us to a point where no one
ever writes the |html or |js or |urlquery.

Russ

[rsc, 2011-08-15 15:19:20]

i am sad that this has not been fixed yet.
now we have actual template starting to
depend on the misnamed 'url'.

PLEASE RENAME THIS.

[bradfitz, 2011-08-15 15:35:13]

On Mon, Aug 15, 2011 at 8:19 AM, Russ Cox <rsc@golang.org> wrote:

> i am sad that this has not been fixed yet.
> now we have actual template starting to
> depend on the misnamed 'url'.
>
> PLEASE RENAME THIS.
>

http://code.google.com/p/go/source/detail?r=874c4905a815533f5288eb2ca9f58b0a9...
 ?

[rsc, 2011-08-15 15:41:04]

On Mon, Aug 15, 2011 at 11:19, Russ Cox <rsc@golang.org> wrote:
> i am sad that this has not been fixed yet.
> now we have actual template starting to
> depend on the misnamed 'url'.
>
> PLEASE RENAME THIS.

sorry, never mind.  that's what i get for
replying to mail without reading all
200 conversations that accumulated
over the weekend.