Async initialization for DateFormat

Async initialization for DateFormat
Committed: https://code.google.com/p/dart/source/detail?r=11901

[Alan Knight, 2012-09-03 23:48:36]

This looks like considerably more changes than it is because it includes
re-arranging some files and generating quite a few data files.

- There is a new directory "tools" with generate_locale_data_file.dart in it.
This will generate the files in data/dates/ when run based on
date_symbol_data_local.dart and date_time_patterns.dart. Almost all of the files
in that directory are JSON and can pretty much be ignored. There is one Dart
file localeList.dart which is sourced in order to have a list of locales
immediately available.

- Various files are re-organized into _local_ and _file_ versions, and sometimes
an _core_ version. For example, date_time_format_test.dart changes into
date_time_format_core_test.dart which is almost unchanged, but now expects to be
used from an external shell. The date_time_format_local_test shell sets it up to
use local data and date_time_format_file_test sets it up to use data read from
files. Next CL will provide an additional shell that reads from an HTTP server.

- Usage is now that you import one of date_symbol_data_local.dart or
date_symbol_data_file.dart. Whichever one you use, you must call its async
initialization function before using any locales. If you don't, you should get
an informative error message from the UninitializedLocaleData object defined in
date_format_internal.dart

- The main new functionality is in LazyLocaleData which can be configured to
fetch data from a reasonably arbitrary source. This is hidden from the end user
via a combination of separate packages in lib that are indirectly imported and
initialization function calls that configure it while keeping the internals
fairly isolated.

- There's a very trivial serialize/deserialize facility added for DateSymbols
objects. It's hard-coded because it can't be done reflectively on dart2js yet.
I'd hope this could be replaced with a more general facility later on.

- Note that reading these dynamically made it so that the DateSymbols couldn't
be const, which required small changes to the generation of the data files.
Those files also now define a function that returns the data rather than a
global constant. The variable they defined is now in date_format_internal.dart
and initialized via the setup functions mentioned above.

 - One open question is if the file reading code will work on Windows or if it
needs to be set up to think about backslashes as directory separators. I'm
hoping we abstract that in Dart and that it doesn't.

[Emily Fortuna, 2012-09-04 20:14:42]

looking good. mostly minor details.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
File pkg/intl/data/dates/symbols/af.json (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
pkg/intl/data/dates/symbols/af.json:1: {"DATEFORMATS":["EEEE dd MMMM y","dd MMMM
y","dd MMM
y","yyyy-MM-dd"],"NARROWWEEKDAYS":["S","M","D","W","D","V","S"],"STANDALONESHORTMONTHS":["Jan","Feb","Mar","Apr","Mei","Jun","Jul","Aug","Sep","Okt","Nov","Des"],"ERANAMES":["voor
Christus","na
Christus"],"FIRSTWEEKCUTOFFDAY":5,"NARROWMONTHS":["J","F","M","A","M","J","J","A","S","O","N","D"],"STANDALONESHORTWEEKDAYS":["So","Ma","Di","Wo","Do","Vr","Sa"],"FIRSTDAYOFWEEK":6,"STANDALONEWEEKDAYS":["Sondag","Maandag","Dinsdag","Woensdag","Donderdag","Vrydag","Saterdag"],"WEEKDAYS":["Sondag","Maandag","Dinsdag","Woensdag","Donderdag","Vrydag","Saterdag"],"QUARTERS":["1ste
kwartaal","2de kwartaal","3de kwartaal","4de
kwartaal"],"ERAS":["v.C.","n.C."],"MONTHS":["Januarie","Februarie","Maart","April","Mei","Junie","Julie","Augustus","September","Oktober","November","Desember"],"TIMEFORMATS":["h:mm:ss
a zzzz","h:mm:ss a z","h:mm:ss a","h:mm
a"],"SHORTWEEKDAYS":["So","Ma","Di","Wo","Do","Vr","Sa"],"SHORTQUARTERS":["K1","K2","K3","K4"],"WEEKENDRANGE":[5,6],"STANDALONENARROWWEEKDAYS":["S","M","D","W","D","V","S"],"AVAILABLEFORMATS":null,"NAME":"af","AMPMS":["vm.","nm."],"STANDALONEMONTHS":["Januarie","Februarie","Maart","April","Mei","Junie","Julie","Augustus","September","Oktober","November","Desember"],"SHORTMONTHS":["Jan","Feb","Mar","Apr","Mei","Jun","Jul","Aug","Sep","Okt","Nov","Des"],"STANDALONENARROWMONTHS":["J","F","M","A","M","J","J","A","S","O","N","D"]}
Can we either have a set of comments at the beginning explaining how the files
in  "symbols" vs "patterns" are used, or at least some sort of README in each
directory explaining about what you wrote in the message you sent with this CL?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
File pkg/intl/data/dates/symbols/ar.json (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
pkg/intl/data/dates/symbols/ar.json:1: {"DATEFORMATS":["EEEE، d MMMM، y","d
MMMM،
y","dd‏/MM‏/yyyy","d‏/M‏/yyyy"],"NARROWWEEKDAYS":["ح","ن","ث","ر","خ","ج","س"],"STANDALONESHORTMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"ERANAMES":["قبل
الميلاد","ميلادي"],"FIRSTWEEKCUTOFFDAY":4,"NARROWMONTHS":["ي","ف","م","أ","و","ن","ل","غ","س","ك","ب","د"],"STANDALONESHORTWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"FIRSTDAYOFWEEK":5,"STANDALONEWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"WEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"QUARTERS":["الربع
الأول","الربع الثاني","الربع الثالث","الربع
الرابع"],"ERAS":["ق.م","م"],"MONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"TIMEFORMATS":["zzzz
h:mm:ss a","z h:mm:ss a","h:mm:ss a","h:mm
a"],"SHORTWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"SHORTQUARTERS":["الربع
الأول","الربع الثاني","الربع الثالث","الربع
الرابع"],"WEEKENDRANGE":[4,5],"STANDALONENARROWWEEKDAYS":["ح","ن","ث","ر","خ","ج","س"],"AVAILABLEFORMATS":null,"NAME":"ar","AMPMS":["ص","م"],"STANDALONEMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"SHORTMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"STANDALONENARROWMONTHS":["ي","ف","م","أ","و","ن","ل","غ","س","ك","ب","د"]}
nit: can we format these guys more nicely with line breaks so it's not one big
line of text? like:
{
    "DATEFORMATS": ["EEE", "dd/MM/YYYY", etc],
    "NARROWWEEKDAYS":
         ["more weekday names", etc]
}

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_forma...
File pkg/intl/date_format.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_forma...
pkg/intl/date_format.dart:257: * Returns a list of all locales for which we have
date formatting information.
80 char. :-P

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_file.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_file.dart:23: * For some implementations this may be a
no-op.
Explain briefly when/why would this be a no-op.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_file.dart:26: var reader = new
FileDataReader('${path}symbols/');
You can use the Path separator from the Platform class:
http://api.dartlang.org/docs/continuous/dart_io/Platform.html#pathSeparator to
have OS-independent code here.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_local.dart (left):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_local.dart:54: AMPMS: const ['AM', 'PM'],
was this supposed to be removed?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_local.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_local.dart:29: Map dateTimeSymbolMap() => {
Since this is a function now, let's add a little line of documentation saying
how it works.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbols.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbols.dart:5: 
warning warning extra newline insertion detected

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbols.dart:53: DateSymbols.deserializeFromMap(Map map) {
Add a TODO saying replace with the proper Dart serialize utility when it comes
(and file a bug about it, if we don't have one already)

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_time_...
File pkg/intl/date_time_patterns.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_time_...
pkg/intl/date_time_patterns.dart:968: 'en_ISO' : const {
why wasn't this here before?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
File pkg/intl/lib/date_format_internal.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:7: * which are split out into a separate
library because they require public
"split out into separate libraries"?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:36: * Initialize the symbols dictionary.
Can you explain here how "symbols" is expected to work as a function?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:44: /// Initialize the patterns
dictionary.
For sake of consistency, make this comment in the /** */ format like the
function above.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:51: /**
Either add a comment here or remove the comment notation: /** */

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/intl_h...
File pkg/intl/lib/intl_helpers.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/intl_h...
pkg/intl/lib/intl_helpers.dart:38: /// An abstract superclass for data readers
to keep the type system happy.
Style nit: I'd say have consistent commenting style throughout a particular
file. So either all /** */ or ///. Check with Bob if you disagree. Also, check
with Bob, but last I heard (which, granted, was quite a while ago) apidoc just
used /** */ to detect comments, not ///...

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/lazy_l...
File pkg/intl/lib/lazy_locale_data.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/lazy_l...
pkg/intl/lib/lazy_locale_data.dart:54: Set _availableLocaleSet;
add a newline below, and perhaps move this declaration up with the other fields
before the constructor?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/lazy_l...
pkg/intl/lib/lazy_locale_data.dart:69: /// Return the list of keys/locale names.
same on consistent commenting format.  You can use
/** Return the list of keys/locale names. */ all on one line

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/data_...
File pkg/intl/test/data_directory.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/data_...
pkg/intl/test/data_directory.dart:16: if (new
RegExp('.*/test').hasMatch(current)) return '../data/datess/';
is "data/datess" what you intended?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/data_...
pkg/intl/test/data_directory.dart:17: if (new
RegExp('.*/intl').hasMatch(current)) return 'data/dates/';
Use Path separator from Platform library here?

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/date_...
File pkg/intl/test/date_time_format_test_core.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/date_...
pkg/intl/test/date_time_format_test_core.dart:5: 
Error error extra whitespace

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/tools/gene...
File pkg/intl/tools/generate_locale_data_files.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/tools/gene...
pkg/intl/tools/generate_locale_data_files.dart:12: * a file called
localeList.dart, which will be imported by the
Comment is incomplete here. Try rephrasing this so it's clearer.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/pkg.status
File pkg/pkg.status (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/pkg.status#newc...
pkg/pkg.status:19: intl/test/date_time_format_file_test: skip
Add a small comment as to why this is skipped so someone doesn't try to fix this
as a "bug" -- this is because it's for the wrong platform (browser vs local),
right?

[Alan Knight, 2012-09-04 23:39:03]

PTAL

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
File pkg/intl/data/dates/symbols/af.json (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
pkg/intl/data/dates/symbols/af.json:1: {"DATEFORMATS":["EEEE dd MMMM y","dd MMMM
y","dd MMM
y","yyyy-MM-dd"],"NARROWWEEKDAYS":["S","M","D","W","D","V","S"],"STANDALONESHORTMONTHS":["Jan","Feb","Mar","Apr","Mei","Jun","Jul","Aug","Sep","Okt","Nov","Des"],"ERANAMES":["voor
Christus","na
Christus"],"FIRSTWEEKCUTOFFDAY":5,"NARROWMONTHS":["J","F","M","A","M","J","J","A","S","O","N","D"],"STANDALONESHORTWEEKDAYS":["So","Ma","Di","Wo","Do","Vr","Sa"],"FIRSTDAYOFWEEK":6,"STANDALONEWEEKDAYS":["Sondag","Maandag","Dinsdag","Woensdag","Donderdag","Vrydag","Saterdag"],"WEEKDAYS":["Sondag","Maandag","Dinsdag","Woensdag","Donderdag","Vrydag","Saterdag"],"QUARTERS":["1ste
kwartaal","2de kwartaal","3de kwartaal","4de
kwartaal"],"ERAS":["v.C.","n.C."],"MONTHS":["Januarie","Februarie","Maart","April","Mei","Junie","Julie","Augustus","September","Oktober","November","Desember"],"TIMEFORMATS":["h:mm:ss
a zzzz","h:mm:ss a z","h:mm:ss a","h:mm
a"],"SHORTWEEKDAYS":["So","Ma","Di","Wo","Do","Vr","Sa"],"SHORTQUARTERS":["K1","K2","K3","K4"],"WEEKENDRANGE":[5,6],"STANDALONENARROWWEEKDAYS":["S","M","D","W","D","V","S"],"AVAILABLEFORMATS":null,"NAME":"af","AMPMS":["vm.","nm."],"STANDALONEMONTHS":["Januarie","Februarie","Maart","April","Mei","Junie","Julie","Augustus","September","Oktober","November","Desember"],"SHORTMONTHS":["Jan","Feb","Mar","Apr","Mei","Jun","Jul","Aug","Sep","Okt","Nov","Des"],"STANDALONENARROWMONTHS":["J","F","M","A","M","J","J","A","S","O","N","D"]}
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Can we either have a set of comments at the beginning explaining how the files
> in  "symbols" vs "patterns" are used, or at least some sort of README in each
> directory explaining about what you wrote in the message you sent with this
CL?

Added a README at the data/dates level, which is explains both. I thought that
would be more visible than a README inside the patterns and symbols directories.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
File pkg/intl/data/dates/symbols/ar.json (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
pkg/intl/data/dates/symbols/ar.json:1: {"DATEFORMATS":["EEEE، d MMMM، y","d
MMMM،
y","dd‏/MM‏/yyyy","d‏/M‏/yyyy"],"NARROWWEEKDAYS":["ح","ن","ث","ر","خ","ج","س"],"STANDALONESHORTMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"ERANAMES":["قبل
الميلاد","ميلادي"],"FIRSTWEEKCUTOFFDAY":4,"NARROWMONTHS":["ي","ف","م","أ","و","ن","ل","غ","س","ك","ب","د"],"STANDALONESHORTWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"FIRSTDAYOFWEEK":5,"STANDALONEWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"WEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"QUARTERS":["الربع
الأول","الربع الثاني","الربع الثالث","الربع
الرابع"],"ERAS":["ق.م","م"],"MONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"TIMEFORMATS":["zzzz
h:mm:ss a","z h:mm:ss a","h:mm:ss a","h:mm
a"],"SHORTWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"SHORTQUARTERS":["الربع
الأول","الربع الثاني","الربع الثالث","الربع
الرابع"],"WEEKENDRANGE":[4,5],"STANDALONENARROWWEEKDAYS":["ح","ن","ث","ر","خ","ج","س"],"AVAILABLEFORMATS":null,"NAME":"ar","AMPMS":["ص","م"],"STANDALONEMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"SHORTMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"STANDALONENARROWMONTHS":["ي","ف","م","أ","و","ن","ل","غ","س","ك","ب","د"]}
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> nit: can we format these guys more nicely with line breaks so it's not one big
> line of text? like:
> {
>     "DATEFORMATS": ["EEE", "dd/MM/YYYY", etc],
>     "NARROWWEEKDAYS":
>          ["more weekday names", etc]
> }

This is the way our JSON library writes things.  I don't really want to format
them manually just to have it overwritten when the data is regenerated. I was
thinking it was sufficient to indicate that the data is exactly the same as that
found in the files this is generated from, where it is formatted more nicely.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_forma...
File pkg/intl/date_format.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_forma...
pkg/intl/date_format.dart:257: * Returns a list of all locales for which we have
date formatting information.
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> 80 char. :-P

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_file.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_file.dart:23: * For some implementations this may be a
no-op.
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Explain briefly when/why would this be a no-op.
Actually, it's not true, because even for the local implementation if you don't
call the initialization function it will fail. Removed.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_file.dart:26: var reader = new
FileDataReader('${path}symbols/');
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> You can use the Path separator from the Platform class:
> http://api.dartlang.org/docs/continuous/dart_io/Platform.html#pathSeparator to
> have OS-independent code here.

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_local.dart (left):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_local.dart:54: AMPMS: const ['AM', 'PM'],
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> was this supposed to be removed?

No, it wasn't. Thanks. Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_local.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_local.dart:29: Map dateTimeSymbolMap() => {
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Since this is a function now, let's add a little line of documentation saying
> how it works.

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
File pkg/intl/date_symbols.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbols.dart:5: 
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> warning warning extra newline insertion detected

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_symbo...
pkg/intl/date_symbols.dart:53: DateSymbols.deserializeFromMap(Map map) {
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Add a TODO saying replace with the proper Dart serialize utility when it comes
> (and file a bug about it, if we don't have one already)

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_time_...
File pkg/intl/date_time_patterns.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/date_time_...
pkg/intl/date_time_patterns.dart:968: 'en_ISO' : const {
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> why wasn't this here before?

It should have been here before. The en_ISO locale isn't in CLDR, but the python
script hard-codes it. But then it's missing from this file. As a result, the
previous version would actually crash on an en_ISO date format because it
thought it had the locale, but then failed to find the patterns. That being
said, I'm not sure this data is correct. Right now it's just a copy of "en".
Added a TODO to verify the patterns, and another one to either use or remove the
patterns that are defined in the DateSymbols that seem to be redundant with
these patterns.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
File pkg/intl/lib/date_format_internal.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:7: * which are split out into a separate
library because they require public
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> "split out into separate libraries"?

I don't think so. The methods (plural) are split out into this library
(singular). But obviously it's not very clear. Rephrased.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:36: * Initialize the symbols dictionary.
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Can you explain here how "symbols" is expected to work as a function?

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:44: /// Initialize the patterns
dictionary.
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> For sake of consistency, make this comment in the /** */ format like the
> function above.

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/date_f...
pkg/intl/lib/date_format_internal.dart:51: /**
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Either add a comment here or remove the comment notation: /** */

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/intl_h...
File pkg/intl/lib/intl_helpers.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/intl_h...
pkg/intl/lib/intl_helpers.dart:38: /// An abstract superclass for data readers
to keep the type system happy.
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Style nit: I'd say have consistent commenting style throughout a particular
> file. So either all /** */ or ///. Check with Bob if you disagree. Also, check
> with Bob, but last I heard (which, granted, was quite a while ago) apidoc just
> used /** */ to detect comments, not ///...

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/lazy_l...
File pkg/intl/lib/lazy_locale_data.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/lazy_l...
pkg/intl/lib/lazy_locale_data.dart:54: Set _availableLocaleSet;
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> add a newline below, and perhaps move this declaration up with the other
fields
> before the constructor?

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/lib/lazy_l...
pkg/intl/lib/lazy_locale_data.dart:69: /// Return the list of keys/locale names.
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> same on consistent commenting format.  You can use
> /** Return the list of keys/locale names. */ all on one line

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/data_...
File pkg/intl/test/data_directory.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/data_...
pkg/intl/test/data_directory.dart:16: if (new
RegExp('.*/test').hasMatch(current)) return '../data/datess/';
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> is "data/datess" what you intended?

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/data_...
pkg/intl/test/data_directory.dart:17: if (new
RegExp('.*/intl').hasMatch(current)) return 'data/dates/';
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Use Path separator from Platform library here?

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/date_...
File pkg/intl/test/date_time_format_test_core.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/test/date_...
pkg/intl/test/date_time_format_test_core.dart:5: 
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Error error extra whitespace

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/tools/gene...
File pkg/intl/tools/generate_locale_data_files.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/tools/gene...
pkg/intl/tools/generate_locale_data_files.dart:12: * a file called
localeList.dart, which will be imported by the
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Comment is incomplete here. Try rephrasing this so it's clearer.

Done.

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/pkg.status
File pkg/pkg.status (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/pkg.status#newc...
pkg/pkg.status:19: intl/test/date_time_format_file_test: skip
On 2012/09/04 20:14:42, Emily Fortuna wrote:
> Add a small comment as to why this is skipped so someone doesn't try to fix
this
> as a "bug" -- this is because it's for the wrong platform (browser vs local),
> right?

Done.

[Emily Fortuna, 2012-09-05 01:06:20]

lgtm with a few minor comments!

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
File pkg/intl/data/dates/symbols/ar.json (right):

https://chromiumcodereview.appspot.com/10917069/diff/6001/pkg/intl/data/dates...
pkg/intl/data/dates/symbols/ar.json:1: {"DATEFORMATS":["EEEE، d MMMM، y","d
MMMM،
y","dd‏/MM‏/yyyy","d‏/M‏/yyyy"],"NARROWWEEKDAYS":["ح","ن","ث","ر","خ","ج","س"],"STANDALONESHORTMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"ERANAMES":["قبل
الميلاد","ميلادي"],"FIRSTWEEKCUTOFFDAY":4,"NARROWMONTHS":["ي","ف","م","أ","و","ن","ل","غ","س","ك","ب","د"],"STANDALONESHORTWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"FIRSTDAYOFWEEK":5,"STANDALONEWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"WEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"QUARTERS":["الربع
الأول","الربع الثاني","الربع الثالث","الربع
الرابع"],"ERAS":["ق.م","م"],"MONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"TIMEFORMATS":["zzzz
h:mm:ss a","z h:mm:ss a","h:mm:ss a","h:mm
a"],"SHORTWEEKDAYS":["الأحد","الاثنين","الثلاثاء","الأربعاء","الخميس","الجمعة","السبت"],"SHORTQUARTERS":["الربع
الأول","الربع الثاني","الربع الثالث","الربع
الرابع"],"WEEKENDRANGE":[4,5],"STANDALONENARROWWEEKDAYS":["ح","ن","ث","ر","خ","ج","س"],"AVAILABLEFORMATS":null,"NAME":"ar","AMPMS":["ص","م"],"STANDALONEMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"SHORTMONTHS":["يناير","فبراير","مارس","أبريل","مايو","يونيو","يوليو","أغسطس","سبتمبر","أكتوبر","نوفمبر","ديسمبر"],"STANDALONENARROWMONTHS":["ي","ف","م","أ","و","ن","ل","غ","س","ك","ب","د"]}
On 2012/09/04 23:39:03, Alan Knight wrote:
> On 2012/09/04 20:14:42, Emily Fortuna wrote:
> > nit: can we format these guys more nicely with line breaks so it's not one
big
> > line of text? like:
> > {
> >     "DATEFORMATS": ["EEE", "dd/MM/YYYY", etc],
> >     "NARROWWEEKDAYS":
> >          ["more weekday names", etc]
> > }
> 
> This is the way our JSON library writes things.  I don't really want to format
> them manually just to have it overwritten when the data is regenerated. I was
> thinking it was sufficient to indicate that the data is exactly the same as
that
> found in the files this is generated from, where it is formatted more nicely.

sgtm.

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/data/dates...
File pkg/intl/data/dates/README.txt (right):

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/data/dates...
pkg/intl/data/dates/README.txt:2: from JSON files on disk or over the network.
There are two directories
:-) thanks for including this!

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_file.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_file.dart:23: * The [path] parameter should end with a
directory separator: '/' or '\' as
'\' should be => '\\', right? Or better -- just get rid of the '/' or '\' and
just say as appropriate for the platform.

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_local.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_local.dart:1852: ERAS: const [ 'BC', 'AD'],
was this supposed to change? O.o

[Alan Knight, 2012-09-05 19:56:02]

Committed as 11901

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_file.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_file.dart:23: * The [path] parameter should end with a
directory separator: '/' or '\' as
On 2012/09/05 01:06:21, Emily Fortuna wrote:
> '\' should be => '\\', right? Or better -- just get rid of the '/' or '\' and
> just say as appropriate for the platform.

Done.

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
File pkg/intl/date_symbol_data_local.dart (right):

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/intl/date_symbo...
pkg/intl/date_symbol_data_local.dart:1852: ERAS: const [ 'BC', 'AD'],
On 2012/09/05 01:06:21, Emily Fortuna wrote:
> was this supposed to change? O.o

Hmm. I didn't do anything to change that, I think it must reflect an update to
the CLDR data since these were last generated. Regenerating for Closure gives
the same info, so I think it's correct.

[vsm, 2012-09-05 21:09:35]

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/pkg.status
File pkg/pkg.status (right):

https://chromiumcodereview.appspot.com/10917069/diff/3182/pkg/pkg.status#newc...
pkg/pkg.status:19: [ $runtime == opera || $runtime == ff || $runtime == ie ||
$runtime == dartium || $runtime == chrome || $runtime == safari || $runtime ==
dartium ]
I think one of these runtimes should be 'drt'.  It's failed on the bot now.