Tokenize one line at a time.

Tokenize one line at a time.

R=kasperl@google.com

Committed: https://code.google.com/p/dart/source/detail?r=35688

[ahe, 2014-04-04 14:19:14]

Work in progress.

[kasperl, 2014-04-07 04:54:24]

Comments in progress.

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
File dart/site/try/src/interaction_manager.dart (right):

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
dart/site/try/src/interaction_manager.dart:246: int lineNumber = 1;
If you have to keep an index anyway, maybe it would be cleaner to introduce a
local variable for the List that's the result of the split and run from zero to
its length?

At least you should be updating lineNumber...

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
dart/site/try/src/interaction_manager.dart:700:
trySelection.addNodeFromSubstring(start + offset, start + charOffset, nodes);
Long line.

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
dart/site/try/src/interaction_manager.dart:710:
trySelection.addNodeFromSubstring(start + offset, start + currentText.length,
nodes);
Long line.

[ahe, 2014-04-07 09:38:29]

Thanks for taking a look, Kasper!

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
File dart/site/try/src/interaction_manager.dart (right):

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
dart/site/try/src/interaction_manager.dart:246: int lineNumber = 1;
On 2014/04/07 04:54:25, kasperl wrote:
> If you have to keep an index anyway, maybe it would be cleaner to introduce a
> local variable for the List that's the result of the split and run from zero
to
> its length?
> 
> At least you should be updating lineNumber...

Variable not needed. Line numbers implemented using CSS only.

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
dart/site/try/src/interaction_manager.dart:700:
trySelection.addNodeFromSubstring(start + offset, start + charOffset, nodes);
On 2014/04/07 04:54:25, kasperl wrote:
> Long line.

Done.

https://codereview.chromium.org/225903003/diff/1/dart/site/try/src/interactio...
dart/site/try/src/interaction_manager.dart:710:
trySelection.addNodeFromSubstring(start + offset, start + currentText.length,
nodes);
On 2014/04/07 04:54:25, kasperl wrote:
> Long line.

Done.

[ahe, 2014-05-01 15:07:31]

It's ready for review now :-)

[ahe, 2014-05-01 15:40:23]

FYI: this will regress colorization of multiline tokens, see
https://code.google.com/p/dart/issues/detail?id=18580.

I feel it is more important to make progress on other things than fix the
colors.

[ahe, 2014-05-02 07:10:26]

Adding Johnni in case he is curious about how I use the new tokens.

[kasperl, 2014-05-02 08:01:05]

LGTM.

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/project_se...
File dart/site/try/project_server.dart (right):

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/project_se...
dart/site/try/project_server.dart:11: Stream;
Do you actually need this?

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
File dart/site/try/src/interaction_manager.dart (right):

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:719: String newState = '';
Consider using a StringBuffer or += to build the newState string?

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:758:
trySelection.addNodeFromSubstring(
Compue (start - state.length) only once?

[ahe, 2014-05-02 08:29:07]

One more regression: https://code.google.com/p/dart/issues/detail?id=18601

[ahe, 2014-05-02 10:01:45]

Thank you, Kasper!

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/project_se...
File dart/site/try/project_server.dart (right):

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/project_se...
dart/site/try/project_server.dart:11: Stream;
On 2014/05/02 08:01:06, kasperl wrote:
> Do you actually need this?

Yes. For a type annotations.

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
File dart/site/try/src/interaction_manager.dart (right):

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:719: String newState = '';
On 2014/05/02 08:01:06, kasperl wrote:
> Consider using a StringBuffer or += to build the newState string? 

I knew the other version would generate nice JS code on dart2js. But your
suggestion works equally great.

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:732: newState =
'$newState${token.start}';
Using += compiles to:

        newState += t1.get$start(token);

This is good, but not great (there shouldn't be a need to go through the
interceptor.

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:741: newState =
'$newState${follow.begin.value}';
Compiles to:

            newState += H.S(t1.get$value(t1));

Again, good but not great (interceptor problems).

https://codereview.chromium.org/225903003/diff/90001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:758:
trySelection.addNodeFromSubstring(
On 2014/05/02 08:01:06, kasperl wrote:
> Compue (start - state.length) only once?

Done.

[ahe, 2014-05-02 12:08:50]

Committed patchset #6 manually as r35688 (presubmit successful).