PrettyPrinter: add an ability to prefix node fields, so instead of:

PrettyPrinter: add an ability to prefix node fields, so instead of:
<NodeList>
we will see:
<statements:NodeList>

(of course if that NodeList is statements for some function expression for example).




nextPrefix


BUG=
TEST=

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

[Anton Muhin, 2012-06-14 20:32:37]

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/tree/prettyprint.dart (right):

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:18: // Prefix for the next
openNode() invocation for that type.
nit: /** Prefix for the next [:openNode():] invocation for that type */

And what that type means?

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:66: if (nextPrefix != null) {
nit: !==

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:67: sb.add("$nextPrefix");
I think simple sb.add(nextPrefix) should work as well.

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:68: nextPrefix = null;
is it okay to reset it that early, do we want to have the matching closing tag?

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:252: openNode(type,
nit: somewhat weird formatting, I'd rather do it like:

openNode(type, {
  "isPrefix": ...,
  "isPostfix": ..
...

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:256: nextPrefix = "receiver:";
you may want to introduce withPrefix helper, something like:

withPrefix(prefix, action) {
  nextPrefix = prefix;
  action();
}

[Roman, 2012-06-14 20:47:32]

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/tree/prettyprint.dart (right):

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:18: // Prefix for the next
openNode() invocation for that type.
On 2012/06/14 20:32:37, antonmuhin wrote:
> nit: /** Prefix for the next [:openNode():] invocation for that type */
> 
> And what that type means?

Done. Reformulated. It's the type of the node to print out.

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:66: if (nextPrefix != null) {
On 2012/06/14 20:32:37, antonmuhin wrote:
> nit: !==

Done.

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:67: sb.add("$nextPrefix");
On 2012/06/14 20:32:37, antonmuhin wrote:
> I think simple sb.add(nextPrefix) should work as well.

Done.

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:68: nextPrefix = null;
On 2012/06/14 20:32:37, antonmuhin wrote:
> is it okay to reset it that early, do we want to have the matching closing
tag?

It may be nice, but not so necessary. And we would have to keep the stack of
current prefixes, I would like to avoid that :)

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:252: openNode(type,
On 2012/06/14 20:32:37, antonmuhin wrote:
> nit: somewhat weird formatting, I'd rather do it like:
> 
> openNode(type, {
>   "isPrefix": ...,
>   "isPostfix": ..
> ...

Done.

https://chromiumcodereview.appspot.com/10532150/diff/2001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:256: nextPrefix = "receiver:";
On 2012/06/14 20:32:37, antonmuhin wrote:
> you may want to introduce withPrefix helper, something like:
> 
> withPrefix(prefix, action) {
>   nextPrefix = prefix;
>   action();
> }

Done.

[Anton Muhin, 2012-06-14 20:52:07]

lgtm w/ comments

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/tree/prettyprint.dart (right):

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:18: /** Prefix for the type
passed to next openNode() call. */
nit: I think you'd better put openNode() into [: :]---[:openNode():]

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:19: String nextPrefix;
maybe something like nextTypePrefix would be a better name.

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:263: if (node.receiver !==
null) {
nit: single line?  here and below

[Roman, 2012-06-14 21:14:31]

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/tree/prettyprint.dart (right):

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:18: /** Prefix for the type
passed to next openNode() call. */
On 2012/06/14 20:52:07, antonmuhin wrote:
> nit: I think you'd better put openNode() into [: :]---[:openNode():]

Simple grep shows that in "lib" [::] is never used, so I just wrapped in []

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:19: String nextPrefix;
On 2012/06/14 20:52:07, antonmuhin wrote:
> maybe something like nextTypePrefix would be a better name.

Done.

https://chromiumcodereview.appspot.com/10532150/diff/5001/lib/compiler/implem...
lib/compiler/implementation/tree/prettyprint.dart:263: if (node.receiver !==
null) {
On 2012/06/14 20:52:07, antonmuhin wrote:
> nit: single line?  here and below

Done (where possible).