Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(299)

Issue 1998045: code review 1998045: scanner: change package comment to be clearer about its... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 7 months ago by r
Modified:
13 years, 7 months ago
Reviewers:
CC:
gri, rsc1, golang-dev
Visibility:
Public.

Description

scanner: change package comment to be clearer about its functionality. No semantic changes.

Patch Set 1 #

Total comments: 2

Patch Set 2 : code review 1998045: scanner: change package comment to be clearer about its... #

Patch Set 3 : code review 1998045: scanner: change package comment to be clearer about its... #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -8 lines) Patch
M src/pkg/scanner/scanner.go View 1 2 1 chunk +8 lines, -8 lines 0 comments Download

Messages

Total messages: 8
r
Hello gri (cc: golang-dev@googlegroups.com), I'd like you to review this change.
13 years, 7 months ago (2010-08-25 00:45:28 UTC) #1
gri
LGTM http://codereview.appspot.com/1998045/diff/1/2 File src/pkg/scanner/scanner.go (right): http://codereview.appspot.com/1998045/diff/1/2#newcode5 src/pkg/scanner/scanner.go:5: // A scanner for UTF-8 encoded text containing ...
13 years, 7 months ago (2010-08-25 01:42:01 UTC) #2
gri
PS: Note that it "recognizes" special chars (e.g. ':') and returns them with the token ...
13 years, 7 months ago (2010-08-25 01:43:58 UTC) #3
r2
PTAL
13 years, 7 months ago (2010-08-25 01:54:04 UTC) #4
gri
LGTM On Tue, Aug 24, 2010 at 6:53 PM, Rob 'Commander' Pike <r@google.com> wrote: > ...
13 years, 7 months ago (2010-08-25 02:02:00 UTC) #5
rsc1
LGTM might be worth changing the first line s/scanner/tokenizer/ gri has mentioned wanting to make ...
13 years, 7 months ago (2010-08-25 02:47:05 UTC) #6
r
*** Submitted as http://code.google.com/p/go/source/detail?r=19168be456e4 *** scanner: change package comment to be clearer about its functionality. ...
13 years, 7 months ago (2010-08-25 03:07:13 UTC) #7
gri
13 years, 7 months ago (2010-08-25 05:24:43 UTC) #8
On Tue, Aug 24, 2010 at 7:47 PM, <rsc@google.com> wrote:

> LGTM
>
> might be worth changing the first line s/scanner/tokenizer/
>
> gri has mentioned wanting to make this work well enough
> to use it instead of go/scanner.  I wonder if the right long
>

...instead of go/scanner for places where I currently use go/scanner but
really don't want (all) the go operators and delimiters.


> term plan is to do that and then move it to go/scanner.
> It's really a scanner only for "Go-like" tokens, for various
> values of Go-like.


for that to work well, the special character scanning would have to be more
customizable - it's really meant for smaller languages where there's only a
few single-char special tokens

- gri

>
>
>
> http://codereview.appspot.com/1998045/
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b