Better handling of large files.

Better handling of large files.

R=kasperl@google.com

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

[ahe, 2014-05-02 16:17:53]

FYI, not ready for review yet.

[ahe, 2014-05-05 19:40:13]

This is now ready to be reviewed.

[kasperl, 2014-05-06 04:51:05]

LGTM.

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

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:144: print('onMutation took
${sw.elapsedMilliseconds}ms');
Do you always want to do this?

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:248: node.appendText('\n\n');
A general comment that explains why you're adding newlines (sometimes two of
them) would be helpful.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:252: window.console.dir(node);
Use cascades?

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:289: print('Single line change:
${node.outerHtml}');
Do you want to keep the printing in here?

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:305: for (String line in
currentText.split(new RegExp('^', multiLine: true))) {
Long line. Consider adding a local for the result of calling split.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:308: tokenizeAndHighlight(line,
state, offset, trySelection, lineNodes);
Long line.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:887: // return false;
Remove this comment.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/select...
File dart/site/try/src/selection.dart (right):

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/select...
dart/site/try/src/selection.dart:53: print('selection.collapse took
${sw.elapsedMilliseconds}ms');
Sure you want to keep this in?

https://codereview.chromium.org/265063002/diff/40001/dart/tests/try/cursor_po...
File dart/tests/try/cursor_position_test.dart (right):

https://codereview.chromium.org/265063002/diff/40001/dart/tests/try/cursor_po...
dart/tests/try/cursor_position_test.dart:136: typedef void VoidFunction();
Is this really so much better than just using Function?

[ahe, 2014-05-06 12:54:02]

Thank you, Kasper!

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

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:144: print('onMutation took
${sw.elapsedMilliseconds}ms');
On 2014/05/06 04:51:06, kasperl wrote:
> Do you always want to do this?

Removed.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:248: node.appendText('\n\n');
On 2014/05/06 04:51:06, kasperl wrote:
> A general comment that explains why you're adding newlines (sometimes two of
> them) would be helpful.

Done.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:252: window.console.dir(node);
On 2014/05/06 04:51:06, kasperl wrote:
> Use cascades?

Done.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:289: print('Single line change:
${node.outerHtml}');
On 2014/05/06 04:51:06, kasperl wrote:
> Do you want to keep the printing in here?

For now, yes.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:305: for (String line in
currentText.split(new RegExp('^', multiLine: true))) {
On 2014/05/06 04:51:06, kasperl wrote:
> Long line. Consider adding a local for the result of calling split.

I created a method for splitting text into lines.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:308: tokenizeAndHighlight(line,
state, offset, trySelection, lineNodes);
On 2014/05/06 04:51:06, kasperl wrote:
> Long line.

Done.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/intera...
dart/site/try/src/interaction_manager.dart:887: // return false;
On 2014/05/06 04:51:06, kasperl wrote:
> Remove this comment.

Done.

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/select...
File dart/site/try/src/selection.dart (right):

https://codereview.chromium.org/265063002/diff/40001/dart/site/try/src/select...
dart/site/try/src/selection.dart:53: print('selection.collapse took
${sw.elapsedMilliseconds}ms');
On 2014/05/06 04:51:06, kasperl wrote:
> Sure you want to keep this in?

Removed.

https://codereview.chromium.org/265063002/diff/40001/dart/tests/try/cursor_po...
File dart/tests/try/cursor_position_test.dart (right):

https://codereview.chromium.org/265063002/diff/40001/dart/tests/try/cursor_po...
dart/tests/try/cursor_position_test.dart:136: typedef void VoidFunction();
On 2014/05/06 04:51:06, kasperl wrote:
> Is this really so much better than just using Function?

I don't think Function serves much purpose, and I could just as well have
written "final setup". But I wanted the types to document the relationship
between the definition of TestCase and how it is being run using runTests.

[ahe, 2014-05-06 12:58:40]

Committed patchset #4 manually as r35801 (presubmit successful).