Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(22)

Issue 1593543004: Update the mojom lexer to emit comment tokens. (Closed)

Created:
4 years, 11 months ago by azani
Modified:
4 years, 11 months ago
Reviewers:
rudominer
CC:
mojo-reviews_chromium.org, gregsimon, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Update the mojom lexer to emit comment tokens. Also, add a filtered TokenStream so we can prevent the parser from seeing this tokens until it can handle them. R=rudominer@chromium.org, rudominer BUG= #592 Committed: https://chromium.googlesource.com/external/mojo/+/25d53bf53b4040f29db984d4774483c5de9d2dc5

Patch Set 1 #

Patch Set 2 : Fix pointer to interface bug. #

Patch Set 3 : Fix missing map type when defining map literal. #

Patch Set 4 : #

Total comments: 10

Patch Set 5 : #

Total comments: 6

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -3 lines) Patch
M mojom/mojom_parser/lexer/lexer.go View 1 2 3 4 5 3 chunks +13 lines, -3 lines 0 comments Download
M mojom/mojom_parser/lexer/lexer_test.go View 1 2 3 4 5 3 chunks +38 lines, -0 lines 0 comments Download
M mojom/mojom_parser/lexer/token_stream.go View 1 2 3 4 5 6 7 1 chunk +51 lines, -0 lines 0 comments Download
M mojom/mojom_parser/lexer/tokens.go View 1 2 3 4 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (1 generated)
azani
4 years, 11 months ago (2016-01-15 23:11:30 UTC) #1
azani
ptal Sorry for the premature send.
4 years, 11 months ago (2016-01-16 00:07:12 UTC) #2
rudominer
https://codereview.chromium.org/1593543004/diff/60001/mojom/mojom_parser/lexer/lexer.go File mojom/mojom_parser/lexer/lexer.go (right): https://codereview.chromium.org/1593543004/diff/60001/mojom/mojom_parser/lexer/lexer.go#newcode29 mojom/mojom_parser/lexer/lexer.go:29: // can be read from the returned TokenStream. You ...
4 years, 11 months ago (2016-01-16 01:08:58 UTC) #3
rudominer
lgtm lgtm with comment comments.
4 years, 11 months ago (2016-01-19 17:44:24 UTC) #4
azani
ptal I made some changes that I would like a second set of eyes on. ...
4 years, 11 months ago (2016-01-20 00:00:11 UTC) #5
rudominer
https://codereview.chromium.org/1593543004/diff/80001/mojom/mojom_parser/lexer/token_stream.go File mojom/mojom_parser/lexer/token_stream.go (right): https://codereview.chromium.org/1593543004/diff/80001/mojom/mojom_parser/lexer/token_stream.go#newcode78 mojom/mojom_parser/lexer/token_stream.go:78: s.tokenStream.ConsumeNext() I wonder if there should be a ConsumeNextInternal() ...
4 years, 11 months ago (2016-01-20 00:40:31 UTC) #6
azani
https://codereview.chromium.org/1593543004/diff/80001/mojom/mojom_parser/lexer/token_stream.go File mojom/mojom_parser/lexer/token_stream.go (right): https://codereview.chromium.org/1593543004/diff/80001/mojom/mojom_parser/lexer/token_stream.go#newcode78 mojom/mojom_parser/lexer/token_stream.go:78: s.tokenStream.ConsumeNext() On 2016/01/20 00:40:31, rudominer wrote: > I wonder ...
4 years, 11 months ago (2016-01-25 20:37:06 UTC) #7
rudominer
lgtm with one nit: I think the phrase "blacklisted" has the wrong connotation here. I ...
4 years, 11 months ago (2016-01-25 20:47:41 UTC) #8
azani
4 years, 11 months ago (2016-01-25 21:49:11 UTC) #10
Message was sent while issue was closed.
Committed patchset #8 (id:140001) manually as
25d53bf53b4040f29db984d4774483c5de9d2dc5 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698