Find the system locale from the browser or OS

Find the system locale from the browser or OS
Committed: https://code.google.com/p/dart/source/detail?r=12461

[Emily Fortuna, 2012-09-17 21:26:38]

looks good! Just a few questions.

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl.dart
File pkg/intl/intl.dart (right):

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl.dart#newcode185
pkg/intl/intl.dart:185: static String systemLocale = 'en_US';
in that case, should by default we make systemLocale = '' or null, to ensure
that the user ends up with the locale they want?

Also, if we're making another class field, let's move it up with the other
fields at the top of this file

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl.dart#newcode200
pkg/intl/intl.dart:200: if (_defaultLocale == null) _defaultLocale =
systemLocale;
if you add this systemLocale variable, the necessity for the _getDefaultLocale()
function goes away. Can we just get rid of it?

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl_standalone.dart
File pkg/intl/intl_standalone.dart (right):

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl_standalone.dart...
pkg/intl/intl_standalone.dart:27: * command on Windows, and use "defaults read
-g AppleLocale" on the Mac. This
rephrase the mac section to say you fall back to AppleLocale if LANG isn't set
(I see that in the code, but I didn't get that from reading this)

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl_standalone.dart...
pkg/intl/intl_standalone.dart:83: RegExp _sysInfoRegex = const RegExp(@"System
Locale:\t(\w\w-\w+);");
I could be persuaded otherwise, but I prefer having all of our constants/fields
up at the top of the file.

[Alan Knight, 2012-09-17 22:16:18]

PTAL

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl.dart
File pkg/intl/intl.dart (right):

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl.dart#newcode185
pkg/intl/intl.dart:185: static String systemLocale = 'en_US';
On 2012/09/17 21:26:38, Emily Fortuna wrote:
> in that case, should by default we make systemLocale = '' or null, to ensure
> that the user ends up with the locale they want?
> 
> Also, if we're making another class field, let's move it up with the other
> fields at the top of this file

Default: The user may not call either of these mechanisms, so I wanted to give
it a default that would at least do something rather than just failing. If we
can make this happen transparently, then we wouldn't need a default value.

Moving up: Done.

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl.dart#newcode200
pkg/intl/intl.dart:200: if (_defaultLocale == null) _defaultLocale =
systemLocale;
On 2012/09/17 21:26:38, Emily Fortuna wrote:
> if you add this systemLocale variable, the necessity for the
_getDefaultLocale()
> function goes away. Can we just get rid of it?

Yes. I think I'd thought that but then forgot. Done.

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl_standalone.dart
File pkg/intl/intl_standalone.dart (right):

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl_standalone.dart...
pkg/intl/intl_standalone.dart:27: * command on Windows, and use "defaults read
-g AppleLocale" on the Mac. This
On 2012/09/17 21:26:38, Emily Fortuna wrote:
> rephrase the mac section to say you fall back to AppleLocale if LANG isn't set
> (I see that in the code, but I didn't get that from reading this)

Done.

https://codereview.chromium.org/10907266/diff/1/pkg/intl/intl_standalone.dart...
pkg/intl/intl_standalone.dart:83: RegExp _sysInfoRegex = const RegExp(@"System
Locale:\t(\w\w-\w+);");
On 2012/09/17 21:26:38, Emily Fortuna wrote:
> I could be persuaded otherwise, but I prefer having all of our
constants/fields
> up at the top of the file.

I prefer having the important bit at the top, which in this case is the only
public function. But put the constants right below it.

[Emily Fortuna, 2012-09-17 22:17:41]

lgtm!