code review 4243055: fmt: make ScanState.Token more general.

fmt: make ScanState.Token more general.
When writing custom scanners, I found that
Token itself was rarely useful, as I did not always
want to stop at white space. This change makes
it possible to stop at any class of characters
while reusing the buffer within State.
(also fix a bug in Token)

[rog, 2011-03-04 17:17:32]

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

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

[rog, 2011-03-04 17:21:30]

On 4 March 2011 17:17,  <rogpeppe@gmail.com> wrote:
> +       // Token skips space in the input if skipSpace is true, then
> +       // returns the run of Unicode code points c satisfying f(c).
> +       // If f is nil, !unicode.IsSpace(c) is used; that is, the token
> +       // will hold non-space characters.
> +       Token(skipSpace bool, f func(int) bool) (token string, err os.Error)

i wasn't sure whether the make the predicate function
match the characters in the returned string, or
the characters to stop at.

i chose the former as it seems easier to reason about
and the definition naturally covers EOF, but YMMV.

[r, 2011-03-04 17:43:11]

is there more? i'd rather not make this a replacement for lex.

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go#newcode38
src/pkg/fmt/scan.go:38: // Token skips space in the input if skipSpace is true,
then
define space. in particular, what about newline?

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go#newcode41
src/pkg/fmt/scan.go:41: // will hold non-space characters.
again, newline gets tricky

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go#newcode392
src/pkg/fmt/scan.go:392: func notSpace(r int) bool {
move this up nearer Token and say 
// notSpace is the default scanning function used in Token.

[rog, 2011-03-04 18:06:02]

On 4 March 2011 17:43,  <r@golang.org> wrote:
> is there more? i'd rather not make this a replacement for lex.

no, that's it. it all just arose from issues i encountered when
writing a rather simple custom scanner.
(something to scan "attr=value")

the only other change i was contemplating, for efficiency's
sake, was to make the return from token a []byte rather
than string. that way we can avoid allocating each time.

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go#newcode38
src/pkg/fmt/scan.go:38: // Token skips space in the input if skipSpace is true,
then
On 2011/03/04 17:43:11, r wrote:
> define space. in particular, what about newline?

space is defined just as it was in the previous version
of the comment. i thought that the docs for the
individual Scan functions was sufficient e.g. "Fscan scans text read from r,
storing successive space-separated values into successive arguments.
Newlines count as space."

perhaps it might be best to reiterate that, but i'm not sure what phrasing to
use.

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go#newcode41
src/pkg/fmt/scan.go:41: // will hold non-space characters.
On 2011/03/04 17:43:11, r wrote:
> again, newline gets tricky

it just falls out naturally from the definition of the operators.
Scanln stops at newline. otherwise newline is treated as space.

http://codereview.appspot.com/4243055/diff/2002/src/pkg/fmt/scan.go#newcode392
src/pkg/fmt/scan.go:392: func notSpace(r int) bool {
On 2011/03/04 17:43:11, r wrote:
> move this up nearer Token and say 
> // notSpace is the default scanning function used in Token.

Done.

[r2, 2011-03-04 18:14:46]

you're right about the return value from Token. let's do that now and be done.

-rob

[rog, 2011-03-04 18:26:04]

PTAL

i'm sure my phrasing of the Token comment could be better.

On 4 March 2011 18:14, Rob 'Commander' Pike <r@google.com> wrote:
> you're right about the return value from Token. let's do that now and be done.
>
> -rob
>
>

[r, 2011-03-04 18:31:10]

http://codereview.appspot.com/4243055/diff/12001/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

http://codereview.appspot.com/4243055/diff/12001/src/pkg/fmt/scan.go#newcode46
src/pkg/fmt/scan.go:46: // or when the calling Scan method returns.
The returned slice points to shared data that may be overwritten by...

also please deragged the lines.

[rog, 2011-03-04 19:02:13]

PTAL
thanks, by the way.

http://codereview.appspot.com/4243055/diff/12001/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

http://codereview.appspot.com/4243055/diff/12001/src/pkg/fmt/scan.go#newcode46
src/pkg/fmt/scan.go:46: // or when the calling Scan method returns.
On 2011/03/04 18:31:10, r wrote:
> The returned slice points to shared data that may be overwritten by...
> 
> also please deragged the lines.

Done.

[rog, 2011-03-08 10:07:20]

ping

[r, 2011-03-08 19:42:58]

http://codereview.appspot.com/4243055/diff/14001/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

http://codereview.appspot.com/4243055/diff/14001/src/pkg/fmt/scan.go#newcode44
src/pkg/fmt/scan.go:44: // by next call to Token, a call to a Scan function
using the ScanState
s/next/the next/

[rog, 2011-03-09 12:06:16]

http://codereview.appspot.com/4243055/diff/14001/src/pkg/fmt/scan.go
File src/pkg/fmt/scan.go (right):

http://codereview.appspot.com/4243055/diff/14001/src/pkg/fmt/scan.go#newcode44
src/pkg/fmt/scan.go:44: // by next call to Token, a call to a Scan function
using the ScanState
On 2011/03/08 19:42:58, r wrote:
> s/next/the next/

Done.

[r, 2011-03-09 17:53:47]

LGTM

[r, 2011-03-09 18:01:53]

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

fmt: make ScanState.Token more general.
When writing custom scanners, I found that
Token itself was rarely useful, as I did not always
want to stop at white space. This change makes
it possible to stop at any class of characters
while reusing the buffer within State.
(also fix a bug in Token)

R=r, r2
CC=golang-dev
http://codereview.appspot.com/4243055

Committer: Rob Pike <r@golang.org>