Use the great new Devtools API for custom formatters. Now Dart objects are now generally as easy to…

Use the great new Devtools API for custom formatters. Now Dart objects are now generally as easy to view in the debugger in Chrome using dev_compiler as they are using Dartium. There are still some edge cases that need to be polished but most of the general property display and type information is done.


Checkpoint in devtools debugger support.

BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/b27092cfedec59ac59c3984455c0016b42a5532f

[Jacob, 2016-01-22 19:27:37]

jacobr@google.com changed reviewers:
+ jmesserly@google.com

[Jacob, 2016-01-22 19:27:37]

Use the great new Devtools API for custom formatters.
Now Dart objects are now generally as easy to view in the debugger in
Chrome using dev_compiler as they are using Dartium.
There are still some edge cases that need to be polished but most of the
general property display and type information is done.

Brian this code may look pretty familiar to you. :)

[Jennifer Messerly, 2016-01-22 19:52:17]

LGTM

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/_interna...
File tool/input_sdk/lib/_internal/libraries.dart (right):

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/_interna...
tool/input_sdk/lib/_internal/libraries.dart:219: "debugger/debugger.dart",
I think it would be a bit more consistent to put it under the
"input_sdk/private" directory.

(path would be "_internal/compiler/js_lib/debugger.dart")

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
File tool/input_sdk/lib/debugger/debugger.dart (right):

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:16: const SKIP_DART_CONFIG = const
Object();
nit, these should be lowerCamelCase in new code
https://www.dartlang.org/effective-dart/style/#prefer-using-lowercamelcase-fo...

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:86: return false;
Just curious, when does this throw? Do we have a bug somewhere?

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:93: this.skipDart = skipDart == true;
btw, this can be an initializer:

: skipDart = skipDart == true;

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:96: String name;
can these be final?

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:97: var value;
Object value?

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:169: ///
https://docs.google.com/document/d/1FTascZXT9cxfetuPRT2eXPQKXui4nWFivUnS_335T3U
I couldn't access this document. Wrong link?

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:202:
..setStyle('list-style-type:none;'
trivial, noticed there is no space after ":" here but there is below.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:304: // it is safe to expand untrusted
objects without side effects.
Ah, the eternal question in debuggers!

Usually, just calling things ends up working out best for users, as most
properties and toStrings are safe. I think we had an option to turn off eager
evaluation in case of debugging a heisenbug.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:520: return '${clause.name}
${clause.types.map((type) => getTypeName(type)).join(", ")}';
long line

[Jacob, 2016-01-22 21:28:32]

Description was changed from

==========
Use the great new Devtools API for custom formatters. Now Dart objects are now
generally as easy to view in the debugger in Chrome using dev_compiler as they
are using Dartium. There are still some edge cases that need to be polished but
most of the general property display and type information is done.


Checkpoint in devtools debugger support.

BUG=
==========

to

==========
Use the great new Devtools API for custom formatters. Now Dart objects are now
generally as easy to view in the debugger in Chrome using dev_compiler as they
are using Dartium. There are still some edge cases that need to be polished but
most of the general property display and type information is done.


Checkpoint in devtools debugger support.

BUG=
R=jmesserly@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/b27092cfedec59ac59c3984455c0...
==========

[Jacob, 2016-01-22 21:28:32]

Committed patchset #1 (id:1) manually as
b27092cfedec59ac59c3984455c0016b42a5532f (presubmit successful).

[Jacob, 2016-01-22 21:28:53]

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/_interna...
File tool/input_sdk/lib/_internal/libraries.dart (right):

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/_interna...
tool/input_sdk/lib/_internal/libraries.dart:219: "debugger/debugger.dart",
On 2016/01/22 19:52:16, John Messerly wrote:
> I think it would be a bit more consistent to put it under the
> "input_sdk/private" directory.
> 
> (path would be "_internal/compiler/js_lib/debugger.dart")

Done.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
File tool/input_sdk/lib/debugger/debugger.dart (right):

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:16: const SKIP_DART_CONFIG = const
Object();
On 2016/01/22 19:52:16, John Messerly wrote:
> nit, these should be lowerCamelCase in new code
>
https://www.dartlang.org/effective-dart/style/#prefer-using-lowercamelcase-fo...

Done.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:86: return false;
On 2016/01/22 19:52:17, John Messerly wrote:
> Just curious, when does this throw? Do we have a bug somewhere?

as I recall it throws if you ask whether 'constructor' is a method but it might
throw other times as well.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:93: this.skipDart = skipDart == true;
On 2016/01/22 19:52:17, John Messerly wrote:
> btw, this can be an initializer:
> 
> : skipDart = skipDart == true;

Done.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:96: String name;
On 2016/01/22 19:52:16, John Messerly wrote:
> can these be final?

yep. heads up this code was ported from JS with a minimal attempt to make it
Darty.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:97: var value;
On 2016/01/22 19:52:17, John Messerly wrote:
> Object value?

Done.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:169: ///
https://docs.google.com/document/d/1FTascZXT9cxfetuPRT2eXPQKXui4nWFivUnS_335T3U
On 2016/01/22 19:52:17, John Messerly wrote:
> I couldn't access this document. Wrong link?

I assume you accidentally had spaces due to 80 char line length wrapping in your
url.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:202:
..setStyle('list-style-type:none;'
On 2016/01/22 19:52:16, John Messerly wrote:
> trivial, noticed there is no space after ":" here but there is below.

why doesn't dart formatter fix my CSS style???? :)

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:304: // it is safe to expand untrusted
objects without side effects.
On 2016/01/22 19:52:16, John Messerly wrote:
> Ah, the eternal question in debuggers!
> 
> Usually, just calling things ends up working out best for users, as most
> properties and toStrings are safe. I think we had an option to turn off eager
> evaluation in case of debugging a heisenbug.

I agree with you. Added "button to turn off eager evaluation" to the list of
items for a future Dart Dev Compiler devtools extension.

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:520: return '${clause.name}
${clause.types.map((type) => getTypeName(type)).join(", ")}';
On 2016/01/22 19:52:17, John Messerly wrote:
> long line

wish ... dartfmt doesn't handle long lines due to expressions.
Fixed this and filed a low priority bug.
https://github.com/dart-lang/dart_style/issues/486

[skybrian, 2016-01-22 21:45:20]

skybrian@google.com changed reviewers:
+ skybrian@google.com

[skybrian, 2016-01-22 21:45:21]

Excellent!

https://codereview.chromium.org/1625563002/diff/1/lib/runtime/dart/_debugger.js
File lib/runtime/dart/_debugger.js (right):

https://codereview.chromium.org/1625563002/diff/1/lib/runtime/dart/_debugger....
lib/runtime/dart/_debugger.js:1: dart_library.library('dart/_debugger', null, /*
Imports */[
Readability: it's kind of a large chunk of code with no blank lines and very few
comments.
(Maybe we need a way to strip comments for javascript?)

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
File tool/input_sdk/lib/debugger/debugger.dart (right):

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:12: /// formatted by the Dart
formatter. This is used to specify that an Object
style nit: there should be blank line after "formatter.".

https://www.dartlang.org/effective-dart/documentation/#do-make-the-first-sent...

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:90: /// [JsonMLFormatter] consumes
[NameValuePair] objects and
looks like you're missing something after "and"

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:446: // This class does double duting
displaying metadata for
s/duting/duty/

[Jennifer Messerly, 2016-01-22 22:11:27]

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
File tool/input_sdk/lib/debugger/debugger.dart (right):

https://codereview.chromium.org/1625563002/diff/1/tool/input_sdk/lib/debugger...
tool/input_sdk/lib/debugger/debugger.dart:169: ///
https://docs.google.com/document/d/1FTascZXT9cxfetuPRT2eXPQKXui4nWFivUnS_335T3U
On 2016/01/22 21:28:53, Jacob wrote:
> On 2016/01/22 19:52:17, John Messerly wrote:
> > I couldn't access this document. Wrong link?
> 
> I assume you accidentally had spaces due to 80 char line length wrapping in
your
> url.

Oh, yes :). Thanks, Rietveld.