Updated the schema of the yaml output.

Updated the schema of the yaml output.

R=rnystrom@google.com

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

[janicejl, 2013-07-15 22:03:02]

Also escapes new line characters.

[Bob Nystrom, 2013-07-15 22:16:03]

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/dart2yaml.dart
File pkg/docgen/lib/dart2yaml.dart (right):

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/dart2yaml.d...
pkg/docgen/lib/dart2yaml.dart:56: .replaceAll("\"", "\\\"").replaceAll("\n",
"\\n")}\"\n";
You can make this a little cleaner if you use raw strings and ':

return '"{element.toString()
    .replaceAll('\\', r'\\')
    .replaceAll('"', r'\"')
    .replaceAll('\n', r'\n')}"';

Also, I would probably move all of the replaceAll calls out of the interpolated
string, like:

var contents = element.toString().replaceAll(...).replaceAll(...);
return '"$contents"';

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:329: _getParameters(mirror.parameters),
_getAnnotations(mirror));
This constructor call is the same for every case, right? How about hoisting it
out:

var method = new Method(...);
if (mirror.isSetter) {
  setters[mirrorName] = method;
} else if (mirror.isGetter) {
  getters[mirrorName] = method;
...

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:351: var data = {'setters' : setters,
The variable isn't doing much for you here. May as well just do:

return {'setters': setters, ...};

[janicejl, 2013-07-15 22:38:43]

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/dart2yaml.dart
File pkg/docgen/lib/dart2yaml.dart (right):

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/dart2yaml.d...
pkg/docgen/lib/dart2yaml.dart:56: .replaceAll("\"", "\\\"").replaceAll("\n",
"\\n")}\"\n";
On 2013/07/15 22:16:03, Bob Nystrom wrote:
> You can make this a little cleaner if you use raw strings and ':
> 
> return '"{element.toString()
>     .replaceAll('\\', r'\\')
>     .replaceAll('"', r'\"')
>     .replaceAll('\n', r'\n')}"';
> 
> Also, I would probably move all of the replaceAll calls out of the
interpolated
> string, like:
> 
> var contents = element.toString().replaceAll(...).replaceAll(...);
> return '"$contents"';

Done.

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:329: _getParameters(mirror.parameters),
_getAnnotations(mirror));
On 2013/07/15 22:16:03, Bob Nystrom wrote:
> This constructor call is the same for every case, right? How about hoisting it
> out:
> 
> var method = new Method(...);
> if (mirror.isSetter) {
>   setters[mirrorName] = method;
> } else if (mirror.isGetter) {
>   getters[mirrorName] = method;
> ...

Done.

https://codereview.chromium.org/19220006/diff/2001/pkg/docgen/lib/docgen.dart...
pkg/docgen/lib/docgen.dart:351: var data = {'setters' : setters,
On 2013/07/15 22:16:03, Bob Nystrom wrote:
> The variable isn't doing much for you here. May as well just do:
> 
> return {'setters': setters, ...};

Done.

[Bob Nystrom, 2013-07-15 22:46:43]

https://codereview.chromium.org/19220006/diff/11001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/19220006/diff/11001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:326: var methodConstructor = new Method(mirrorName,
mirror.isStatic,
It's just a method, right? "methodConstructor" -> "method".

Also, move this inside the if (includePrivate || !mirror.isPrivate) { ... }
body. There's no point in creating methods just to discard them.

[janicejl, 2013-07-15 23:03:44]

https://codereview.chromium.org/19220006/diff/11001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/19220006/diff/11001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:326: var methodConstructor = new Method(mirrorName,
mirror.isStatic,
On 2013/07/15 22:46:43, Bob Nystrom wrote:
> It's just a method, right? "methodConstructor" -> "method".
> 
> Also, move this inside the if (includePrivate || !mirror.isPrivate) { ... }
> body. There's no point in creating methods just to discard them.

Done.

[Bob Nystrom, 2013-07-15 23:11:13]

LGTM!

https://codereview.chromium.org/19220006/diff/16001/pkg/docgen/lib/docgen.dart
File pkg/docgen/lib/docgen.dart (right):

https://codereview.chromium.org/19220006/diff/16001/pkg/docgen/lib/docgen.dar...
pkg/docgen/lib/docgen.dart:24: import 'package:pathos/path.dart' as path;
This should be "path" now. Is there a reason you needed to change it back?

[janicejl, 2013-07-15 23:38:41]

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