Add dart_messages package.

Add dart_messages package.

R=johnniwinther@google.com, sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/6ecea2f559912ca6a04d7397e5995d21481c77b8

[floitsch, 2015-12-11 08:27:29]

floitsch@google.com changed reviewers:
+ sigmund@google.com

[Johnni Winther, 2015-12-11 11:27:28]

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

[Johnni Winther, 2015-12-11 11:27:29]

lgtm

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
File pkg/dart_messages/lib/parser.js (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:1: // Copyright (c) 2015, the Dart project
authors.  Please see the AUTHORS file
Why make a parser in JS? Who will use this?

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:72: // Multilines ignore the first new line.
Shouldn't you ignore whitespaces until the first new line as well?

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:73: if (cc() == NL) i++;
Remember our source files are converted to platform newlines on checkout so this
won't work on Windows unless we disable the auto-conversion for our messages
files.

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:76: for (; i < str.length; i++) {
A file ending with an unterminated string will just silently return `null` as
the result of parseString.

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:154: i++;  // The ":".
Assert this?

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
File pkg/dart_messages/test/parser_test.dart (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
pkg/dart_messages/test/parser_test.dart:5: String drt =
"../../../client/tests/drt/content_shell";
What is this? (I don't have a file at that location)

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
File pkg/dart_messages/test/smoke_messages.dart (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
pkg/dart_messages/test/smoke_messages.dart:1: // Copyright (c) 2015, the Dart
project authors.  Please see the AUTHORS file
Why is the file called smoke_messages?

[floitsch, 2015-12-14 18:56:40]

floitsch@google.com changed reviewers:
+ ricow@google.com

[floitsch, 2015-12-14 18:56:41]

+Rico: what's your suggestion on pkg/dart_messages/test/parser_test.dart ?
Should the parser_test.dart just run get_archive.py when the content_shell isn't
there?
Should it import 'tools/testing/dart/drt_updater.dart' and run the updater that
is in there?
Should it require the user to run a drt-test first? (would make it harder on the
build-bot).

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
File pkg/dart_messages/lib/parser.js (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:1: // Copyright (c) 2015, the Dart project
authors.  Please see the AUTHORS file
On 2015/12/11 11:27:29, Johnni Winther wrote:
> Why make a parser in JS? Who will use this?

See https://github.com/floitschG/messages and
http://floitschg.github.io/messages/

The idea is, that the webpage (documenting the messages), extracts as much
information as possible from the messages-file.
This way we don't need to have a build-step to update the dartmsg.org site
(assuming we get a nice domain name).

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:72: // Multilines ignore the first new line.
On 2015/12/11 11:27:29, Johnni Winther wrote:
> Shouldn't you ignore whitespaces until the first new line as well?

We don't need to follow the exact specification (it's a subset), but I added
support for leading spaces.
done.

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:73: if (cc() == NL) i++;
On 2015/12/11 11:27:29, Johnni Winther wrote:
> Remember our source files are converted to platform newlines on checkout so
this
> won't work on Windows unless we disable the auto-conversion for our messages
> files.

I'm now treating "CR" as whitespace. Should make it work.

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:76: for (; i < str.length; i++) {
On 2015/12/11 11:27:29, Johnni Winther wrote:
> A file ending with an unterminated string will just silently return `null` as
> the result of parseString.

Done.

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:154: i++;  // The ":".
On 2015/12/11 11:27:29, Johnni Winther wrote:
> Assert this?

Done.

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
File pkg/dart_messages/test/parser_test.dart (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
pkg/dart_messages/test/parser_test.dart:5: String drt =
"../../../client/tests/drt/content_shell";
On 2015/12/11 11:27:29, Johnni Winther wrote:
> What is this? (I don't have a file at that location)

The file is downloaded when running any test with -rdrt. But even then the name
is different depending on the platform.

I will need to figure out how to do this. I could just run the updater-script
when drt is not there. However, that's a 90MB download and I'm not sure we
should just do that for a test...

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
File pkg/dart_messages/test/smoke_messages.dart (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/test/...
pkg/dart_messages/test/smoke_messages.dart:1: // Copyright (c) 2015, the Dart
project authors.  Please see the AUTHORS file
On 2015/12/11 11:27:29, Johnni Winther wrote:
> Why is the file called smoke_messages?

Maybe "smoke test" is not the best name.
The messages in here are irrelevant, and are just here to keep the parser
somewhat tested, until the share_messages file has actually data in it.
I'm not actually testing a lot: just making sure that the parser can run
through, and that some of the entries in it can be found.

[Siggi Cherem (dart-lang), 2015-12-14 22:12:07]

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
File pkg/dart_messages/lib/parser.js (right):

https://codereview.chromium.org/1514333002/diff/20001/pkg/dart_messages/lib/p...
pkg/dart_messages/lib/parser.js:1: // Copyright (c) 2015, the Dart project
authors.  Please see the AUTHORS file
On 2015/12/14 18:56:40, floitsch wrote:
> On 2015/12/11 11:27:29, Johnni Winther wrote:
> > Why make a parser in JS? Who will use this?
> 
> See https://github.com/floitschG/messages and
> http://floitschg.github.io/messages/
> 
> The idea is, that the webpage (documenting the messages), extracts as much
> information as possible from the messages-file.
> This way we don't need to have a build-step to update the http://dartmsg.org
site
> (assuming we get a nice domain name).

I wonder whether people will forget that they are in a restricted Dart and may
have a hard time knowing what's allowed and what not. 

I mentioned this to Florian offline, but just to have it documented here -- An
alternative could be to have a small script that makes a JSON file out of the
shared-messages, and ask developers to follow a small protocol: run the script
every time they update shared_messages.dart and include the JSON file with all
commits. We can include in the test bots a test that ensures that the .json file
is up to date.

If we go down this route, we can accept any Dart within shared_messages.dart.

Because the JSON file will be submitted to the repo, you can fetch it in the
same way you fetched the Dart file below.

[floitsch, 2015-12-16 23:49:39]

floitsch@google.com changed reviewers:
- ricow@google.com

[floitsch, 2015-12-16 23:49:39]

Now requiring users to generate the json file at every update of the messages.

[Siggi Cherem (dart-lang), 2015-12-17 00:06:56]

lgtm!

https://codereview.chromium.org/1514333002/diff/60001/pkg/dart_messages/test/...
File pkg/dart_messages/test/dart_messages_test.dart (right):

https://codereview.chromium.org/1514333002/diff/60001/pkg/dart_messages/test/...
pkg/dart_messages/test/dart_messages_test.dart:20: throw "Content is not the
same";
Consider adding here instructions on how to regenerate it. 

You could even combine this and json_converter.dart, and make the test take a
flag `--regenerate-json` :)

[floitsch, 2015-12-17 07:27:17]

https://codereview.chromium.org/1514333002/diff/60001/pkg/dart_messages/test/...
File pkg/dart_messages/test/dart_messages_test.dart (right):

https://codereview.chromium.org/1514333002/diff/60001/pkg/dart_messages/test/...
pkg/dart_messages/test/dart_messages_test.dart:20: throw "Content is not the
same";
On 2015/12/17 00:06:56, Siggi Cherem (dart-lang) wrote:
> Consider adding here instructions on how to regenerate it. 
> 
> You could even combine this and json_converter.dart, and make the test take a
> flag `--regenerate-json` :)

I added instructions what to do when the test fails.
I prefer keeping the test and the generator separate. Test is in test/, and
converter is in bin/.

[Siggi Cherem (dart-lang), 2015-12-17 16:51:47]

lgtm

[floitsch, 2015-12-17 17:15:21]

Description was changed from

==========
Add dart_messages package.
==========

to

==========
Add dart_messages package.

R=johnniwinther@google.com, sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/6ecea2f559912ca6a04d7397e5995d21481c77b8
==========

[floitsch, 2015-12-17 17:15:23]

Committed patchset #7 (id:120001) manually as
6ecea2f559912ca6a04d7397e5995d21481c77b8 (presubmit successful).