MessageFormat fixup

MessageFormat fixup
Committed: https://code.google.com/p/dart/source/detail?r=8413

[Emily Fortuna, 2012-06-06 22:05:46]

Hi Alan,
I realize this replicates a little bit of the work in your CL you've been
working on. I apologize for that -- I'm really anxious to make forward progress
on actual implementation of this, in which case we need our skeleton API to be
uniform between the two of us. 

This makes the changes we discussed on Wednesday's meeting so we can finally
start implementing the meat of this.

+Bob, if you have time, for dart style.

[Alan Knight, 2012-06-06 22:21:01]

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
File lib/i18n/intl_message.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:10: * '''I see ${intl.plural(NUM_PEOPLE,
Why are the triple quotes here needed?
Also, I think this would be clearer if put in context of how it's used. Right
now this looks like an expression that creates a string and then does nothing
with it.

[Bob Nystrom, 2012-06-06 22:22:48]

I always have time for style!

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.dart
File lib/i18n/date_format.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:2: * Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
The copyright block should just be line comments and not part of the dartdoc
comment. We don't want it in our generated docs. :)

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:10: * vary across locales include month name,
weekname, field, order, etc.
"weekname" -> "week name"
"field, order" -> "field order"

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:11: * //TODO(efortuna): Customized pattern system --
suggested by i18n needs
Just so you know, this TODO will get included in the docs. You could make it an
HTML comment, or move it outside the doc comment.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:49: * (Text): 4 or more pattern letters--use full
form,
You could use a markdown bullet list here, with emphasis for the first word:

* **Text** 4 or more patter...
    less than...
    In parsing...

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:112: /** Definition of this object formats dates. */
"formats" -> "format's"?

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:113: var formatDefinition;
Can this be typed? Should it be public and mutable?

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:116: * String indicating the locale code with which
the message is to be
Remove "String indicating".

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:123: * written this way so that they can be
discoverable via autocomplete. These
Yay autocomplete!

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:216: DateFormat([this.formatDefinition = fullDate,
this._locale]);
I think we try not to use private names as named parameters.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:245: * Formats a String indicating how long ago
(negative [duration]) or how far
I would either put "String" in [] or make it lowercase. Probably the latter.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
File lib/i18n/intl_message.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:10: * '''I see ${intl.plural(NUM_PEOPLE,
Do we expect the params to be SCREAMING_CAPS like this?

Also, if you add a blank line and indent the example 5 spaces (one to separate
it from the comment * and four for markdown), it will be a nice code block.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:15: * Calling format({'NUM_PEOPLE': 2, 'PLACE':
'Athens'}) would
Use [: :] or backticks (markdown is backticks) to format inline code:
`format(...)`.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:22: #library('IntlMessage');
Our naming convention for libraries is lowercase_with_underscores, and usually
the name of the file, so "intl_message".

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:47: * Constructor. Accepts a String [_message] that
contains the message (that
Remove "Constructor."

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:53: * a browser). The values of [desc] and [examples]
MUST be
Use emphasis instead of all caps: "MUST" -> "*must*".

https://chromiumcodereview.appspot.com/10538033/diff/7/tests/lib/i18n/message...
File tests/lib/i18n/message_format_test.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/tests/lib/i18n/message...
tests/lib/i18n/message_format_test.dart:30: //                   
msg_format(5));
With gram's new awesome API:

expect(msg_format, equals('5 emails will be sent.'));

[Emily Fortuna, 2012-06-06 22:51:43]

PTAL!

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.dart
File lib/i18n/date_format.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:2: * Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> The copyright block should just be line comments and not part of the dartdoc
> comment. We don't want it in our generated docs. :)

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:10: * vary across locales include month name,
weekname, field, order, etc.
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> "weekname" -> "week name"
> "field, order" -> "field order"

These were copied from the Closure docs. :-P Fixed!

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:11: * //TODO(efortuna): Customized pattern system --
suggested by i18n needs
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Just so you know, this TODO will get included in the docs. You could make it
an
> HTML comment, or move it outside the doc comment.

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:49: * (Text): 4 or more pattern letters--use full
form,
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> You could use a markdown bullet list here, with emphasis for the first word:
> 
> * **Text** 4 or more patter...
>     less than...
>     In parsing...

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:112: /** Definition of this object formats dates. */
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> "formats" -> "format's"?

(added "how") done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:113: var formatDefinition;
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Can this be typed? Should it be public and mutable?

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:116: * String indicating the locale code with which
the message is to be
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Remove "String indicating".

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:116: * String indicating the locale code with which
the message is to be
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Remove "String indicating".

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:216: DateFormat([this.formatDefinition = fullDate,
this._locale]);
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> I think we try not to use private names as named parameters.

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/date_format.d...
lib/i18n/date_format.dart:245: * Formats a String indicating how long ago
(negative [duration]) or how far
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> I would either put "String" in [] or make it lowercase. Probably the latter. 

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
File lib/i18n/intl_message.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:10: * '''I see ${intl.plural(NUM_PEOPLE,
On 2012/06/06 22:21:01, Alan Knight wrote:
> Why are the triple quotes here needed?
> Also, I think this would be clearer if put in context of how it's used. Right
> now this looks like an expression that creates a string and then does nothing
> with it.

Triple quotes so I don't have to quote and unquote each line, so the quote would
be multi-line. Technically this would add some extra whitespace in the message
but I figured for the example it was fine.

@Bob -- SCREAMING_CAPS borrowed from the examples in Closure and such and I just
hadn't changed them (see right hand side, which basically copies a lot of the
closure doc). No preference. I've changed it and made a code block.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:15: * Calling format({'NUM_PEOPLE': 2, 'PLACE':
'Athens'}) would
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Use [: :] or backticks (markdown is backticks) to format inline code:
> `format(...)`.

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:22: #library('IntlMessage');
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Our naming convention for libraries is lowercase_with_underscores, and usually
> the name of the file, so "intl_message".

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:47: * Constructor. Accepts a String [_message] that
contains the message (that
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Remove "Constructor."

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/lib/i18n/intl_message....
lib/i18n/intl_message.dart:53: * a browser). The values of [desc] and [examples]
MUST be
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> Use emphasis instead of all caps: "MUST" -> "*must*".

Done.

https://chromiumcodereview.appspot.com/10538033/diff/7/tests/lib/i18n/message...
File tests/lib/i18n/message_format_test.dart (right):

https://chromiumcodereview.appspot.com/10538033/diff/7/tests/lib/i18n/message...
tests/lib/i18n/message_format_test.dart:30: //                   
msg_format(5));
On 2012/06/06 22:22:48, Bob Nystrom wrote:
> With gram's new awesome API:
> 
> expect(msg_format, equals('5 emails will be sent.'));

Done.

[Emily Fortuna, 2012-06-07 16:48:55]

please check if ready to go?

[Bob Nystrom, 2012-06-07 17:38:48]

LGTM, but you should get an LGTM from Alan too first.

[Alan Knight, 2012-06-07 21:24:43]

lgtm