Appending working for library_list.json

Appending working for library_list.json

R=rnystrom@google.com

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

[janicejl, 2013-08-16 20:23:26]

Gets the append option working again since we changed from library_list.txt to
library_list.json. 

Also updated the readme. 

Thank you.

[Bob Nystrom, 2013-08-16 20:51:10]

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

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:254: if (docsDir.contains('docs/library_list.json')){
Nit: space before "{".

Also, I would flip the condition and do the throw first, then you don't need an
else:

if (!docsDir.contains(...) {
  throw new StateError(...);
}

libraryMap = ...

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:260: e is Library).map((e) =>
e.previewMap).toList());
Nit, how about wrapping this on ".", like:

libraryMap['libraries'].addAll(filteredEntities
    .where((e) => e is Library)
    .map((e) => e.previewMap));

Also, you don't need the .toList().

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:264: intro = intro + '<br/><br/>' +
markdown.markdownToHtml(
intro += '<br/><br/>'...

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:264: intro = intro + '<br/><br/>' +
markdown.markdownToHtml(
You can get rid of some duplicate code:

var intro = libraryMap['introduction'];
if (intro.isNotEmpty) intro += '<br/><br/>';
intro += markdown.markdownToHtml(...);
libraryMap['introduction'] = intro;

[janicejl, 2013-08-16 21:33:45]

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

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:254: if (docsDir.contains('docs/library_list.json')){
On 2013/08/16 20:51:10, Bob Nystrom wrote:
> Nit: space before "{".
> 
> Also, I would flip the condition and do the throw first, then you don't need
an
> else:
> 
> if (!docsDir.contains(...) {
>   throw new StateError(...);
> }
> 
> libraryMap = ...

Done.

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:260: e is Library).map((e) =>
e.previewMap).toList());
On 2013/08/16 20:51:10, Bob Nystrom wrote:
> Nit, how about wrapping this on ".", like:
> 
> libraryMap['libraries'].addAll(filteredEntities
>     .where((e) => e is Library)
>     .map((e) => e.previewMap));
> 
> Also, you don't need the .toList().

Done.

https://codereview.chromium.org/23003010/diff/3/pkg/docgen/lib/docgen.dart#ne...
pkg/docgen/lib/docgen.dart:264: intro = intro + '<br/><br/>' +
markdown.markdownToHtml(
On 2013/08/16 20:51:10, Bob Nystrom wrote:
> You can get rid of some duplicate code:
> 
> var intro = libraryMap['introduction'];
> if (intro.isNotEmpty) intro += '<br/><br/>';
> intro += markdown.markdownToHtml(...);
> libraryMap['introduction'] = intro;

Done.

[Bob Nystrom, 2013-08-16 21:36:18]

LGTM!

[janicejl, 2013-08-16 21:39:07]

Committed patchset #5 manually as r26294 (presubmit successful).