code review 46370043: go.net/html: Tokenizer.Raw returns the original input w...

go.net/html: Tokenizer.Raw returns the original input when tokenizer errors occur.

Two tweaks enable this:
1) Updating the raw and data span pointers when Tokenizer.Next is called, even
if an error has occurred. This prevents duplicate data from being returned by
Raw in the common case of an EOF.

2) Treating '</>' as an empty comment token to expose the raw text as a
tokenization event. (This matches the semantics of other non-token events,
e.g., '</ >' is treated as '<!-- -->'.)

Fixes issue 7029.

[piatek, 2013-12-30 03:55:03]

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

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

[gobot, 2013-12-30 03:58:22]

Replacing golang-dev with golang-codereviews.

To the author of this CL: 

If you are using 'hg mail -r golang-dev' to mail the CL, use simply 'hg mail'
instead.

If you did not name golang-dev explicitly and it was still added to the CL,
it means your working copy of the repo has a stale codereview.cfg
(or lib/codereview/codereview.cfg).
Please run 'hg sync' to update your client to the most recent codereview.cfg.
If the most recent codereview.cfg has defaultcc set to golang-dev instead of
golang-codereviews, please send a CL correcting it.

Thanks very much.

[piatek, 2014-01-02 17:29:03]

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

Please take another look.

[piatek, 2014-01-02 17:30:38]

Rob, I've added you as a reviewer based on your reviews of previous changes.
Please reroute if needed.

On 2014/01/02 17:29:03, piatek wrote:
> Hello mailto:golang-codereviews@googlegroups.com, mailto:r@golang.org (cc:
> mailto:golang-codereviews@googlegroups.com),
> 
> Please take another look.

[bradfitz, 2014-01-02 18:07:47]

LGTM

Nigel can propose an alternate fix later if he disagrees, but seems fine to this
non-owner of the code.

See minor test comments.

https://codereview.appspot.com/46370043/diff/60001/html/token_test.go
File html/token_test.go (right):

https://codereview.appspot.com/46370043/diff/60001/html/token_test.go#newcode481
html/token_test.go:481: if _, err := parsed.Write(z.Raw()); err != nil {
no need checking for error from Write. a bytes.Buffer's Write is documented to
always succeed (or panic on out of memory)

https://codereview.appspot.com/46370043/diff/60001/html/token_test.go#newcode490
html/token_test.go:490: t.Errorf("%s: parsed output: got %q want %q", test.desc,
got, want)
it might be nicer to (also) check this inside the loop after each token (that
test.html has the prefix of parsed so far), so failures are easier to see where
it went wrong.  or at least here, put got and want on their lines, lined up so
it's easy to eyeball the diff:

"...:\n got: %q\nwant: %q\n"

(where the got/want colons are at the same column)

[piatek, 2014-01-02 18:38:15]

https://codereview.appspot.com/46370043/diff/60001/html/token_test.go
File html/token_test.go (right):

https://codereview.appspot.com/46370043/diff/60001/html/token_test.go#newcode481
html/token_test.go:481: if _, err := parsed.Write(z.Raw()); err != nil {
On 2014/01/02 18:07:47, bradfitz wrote:
> no need checking for error from Write. a bytes.Buffer's Write is documented to
> always succeed (or panic on out of memory)

Done.

https://codereview.appspot.com/46370043/diff/60001/html/token_test.go#newcode490
html/token_test.go:490: t.Errorf("%s: parsed output: got %q want %q", test.desc,
got, want)
On 2014/01/02 18:07:47, bradfitz wrote:
> it might be nicer to (also) check this inside the loop after each token (that
> test.html has the prefix of parsed so far), so failures are easier to see
where
> it went wrong.  or at least here, put got and want on their lines, lined up so
> it's easy to eyeball the diff:
> 
> "...:\n got: %q\nwant: %q\n"
> 
> (where the got/want colons are at the same column)

Done. (The token test strings are all tiny, so the diff should be clear from the
complete output.)

[bradfitz, 2014-01-02 18:39:50]

https://codereview.appspot.com/46370043/diff/80001/html/token_test.go
File html/token_test.go (right):

https://codereview.appspot.com/46370043/diff/80001/html/token_test.go#newcode486
html/token_test.go:486: t.Errorf("%s: parsed output:\ngot: %q\nwant: %q",
test.desc, got, want)
Add a space before "got" so the colons line up. 

I tried to say this twice in the previous comment.  :)

[piatek, 2014-01-02 18:43:16]

Thanks for the review. Will you submit? (I lack the permissions.)

https://codereview.appspot.com/46370043/diff/80001/html/token_test.go
File html/token_test.go (right):

https://codereview.appspot.com/46370043/diff/80001/html/token_test.go#newcode486
html/token_test.go:486: t.Errorf("%s: parsed output:\ngot: %q\nwant: %q",
test.desc, got, want)
On 2014/01/02 18:39:51, bradfitz wrote:
> Add a space before "got" so the colons line up. 
> 
> I tried to say this twice in the previous comment.  :)

Done.

[bradfitz, 2014-01-02 18:50:58]

*** Submitted as
https://code.google.com/p/go/source/detail?r=d4afe896f927&repo=net ***

go.net/html: Tokenizer.Raw returns the original input when tokenizer errors
occur.

Two tweaks enable this:
1) Updating the raw and data span pointers when Tokenizer.Next is called, even
if an error has occurred. This prevents duplicate data from being returned by
Raw in the common case of an EOF.

2) Treating '</>' as an empty comment token to expose the raw text as a
tokenization event. (This matches the semantics of other non-token events,
e.g., '</ >' is treated as '<!-- -->'.)

Fixes issue 7029.

R=golang-codereviews, r, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/46370043

Committer: Brad Fitzpatrick <bradfitz@golang.org>