[dart:io] Adds Platform.localeName

[dart:io] Adds Platform.localeName

related #29126

R=fschneider@google.com

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

[zra, 2017-03-30 17:19:48]

zra@google.com changed reviewers:
+ alanknight@google.com, fschneider@google.com

[zra, 2017-03-30 17:20:43]

Description was changed from

==========
[dart:io] Adds Platform.localeName
==========

to

==========
[dart:io] Adds Platform.localeName

related #29126
==========

[siva, 2017-03-30 17:35:35]

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

[siva, 2017-03-30 17:35:35]

DBC

https://codereview.chromium.org/2785073002/diff/90001/tests/standalone/io/loc...
File tests/standalone/io/locale_name_test.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/tests/standalone/io/loc...
tests/standalone/io/locale_name_test.dart:14: }
Need status file update for dart2js, analyzer etc. as this test could fail on
those configurations?

[vsm, 2017-03-30 17:38:54]

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

[vsm, 2017-03-30 17:38:55]

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/io_patch.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/io_patch.dart:257: }
DBC

Can you add the same here as well?

https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/...

[zra, 2017-03-30 17:48:39]

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/io_patch.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/io_patch.dart:257: }
On 2017/03/30 17:38:55, vsm wrote:
> DBC
> 
> Can you add the same here as well?
> 
>
https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/...

It doesn't look like that's the right thing to do. The input_sdk appears to be a
snapshot at a particular SDK version, not bleeding edge.

https://codereview.chromium.org/2785073002/diff/90001/tests/standalone/io/loc...
File tests/standalone/io/locale_name_test.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/tests/standalone/io/loc...
tests/standalone/io/locale_name_test.dart:14: }
On 2017/03/30 17:35:35, siva wrote:
> Need status file update for dart2js, analyzer etc. as this test could fail on
> those configurations?

I believe standalone/io tests are already handled correctly in the status files
for these configurations.

[vsm, 2017-03-30 18:00:14]

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/io_patch.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/io_patch.dart:257: }
On 2017/03/30 17:48:38, zra wrote:
> On 2017/03/30 17:38:55, vsm wrote:
> > DBC
> > 
> > Can you add the same here as well?
> > 
> >
>
https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/...
> 
> It doesn't look like that's the right thing to do. The input_sdk appears to be
a
> snapshot at a particular SDK version, not bleeding edge.

It used to be. It is now applied against the bleeding edge sdk.

[zra, 2017-03-30 18:08:20]

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/io_patch.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/io_patch.dart:257: }
On 2017/03/30 18:00:14, vsm wrote:
> On 2017/03/30 17:48:38, zra wrote:
> > On 2017/03/30 17:38:55, vsm wrote:
> > > DBC
> > > 
> > > Can you add the same here as well?
> > > 
> > >
> >
>
https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/...
> > 
> > It doesn't look like that's the right thing to do. The input_sdk appears to
be
> a
> > snapshot at a particular SDK version, not bleeding edge.
> 
> It used to be. It is now applied against the bleeding edge sdk. 

No, it doesn't look like it. For example, _Platform._ansiSupported is there but
no longer exists. It seems like a bad idea to have two copies of this patch
file, and I'm not sure this CL should be trying to patch everything up.

[Alan Knight, 2017-03-30 18:11:18]

https://codereview.chromium.org/2785073002/diff/110001/sdk/lib/io/platform.dart
File sdk/lib/io/platform.dart (right):

https://codereview.chromium.org/2785073002/diff/110001/sdk/lib/io/platform.da...
sdk/lib/io/platform.dart:74: static final _localeName = _Platform.localeName;
It's possible, although rare, for the OS locale to change while programs are
running. But it's probably not unreasonable that programs won't see it until
they're restarted.

https://codereview.chromium.org/2785073002/diff/110001/tests/standalone/io/lo...
File tests/standalone/io/locale_name_test.dart (right):

https://codereview.chromium.org/2785073002/diff/110001/tests/standalone/io/lo...
tests/standalone/io/locale_name_test.dart:17: print(Platform.localeName);
Is this leftover debugging code?

[zra, 2017-03-30 18:14:31]

https://codereview.chromium.org/2785073002/diff/110001/sdk/lib/io/platform.dart
File sdk/lib/io/platform.dart (right):

https://codereview.chromium.org/2785073002/diff/110001/sdk/lib/io/platform.da...
sdk/lib/io/platform.dart:74: static final _localeName = _Platform.localeName;
On 2017/03/30 18:11:18, Alan Knight wrote:
> It's possible, although rare, for the OS locale to change while programs are
> running. But it's probably not unreasonable that programs won't see it until
> they're restarted. 

Does package:intl handle that case? If so, I can change this to query the OS
again.

https://codereview.chromium.org/2785073002/diff/110001/tests/standalone/io/lo...
File tests/standalone/io/locale_name_test.dart (right):

https://codereview.chromium.org/2785073002/diff/110001/tests/standalone/io/lo...
tests/standalone/io/locale_name_test.dart:17: print(Platform.localeName);
On 2017/03/30 18:11:18, Alan Knight wrote:
> Is this leftover debugging code?

I left it in intentionally to make the test easier to debug if something goes
wrong. I'm not sure there's a downside to leaving it in.

[zra, 2017-03-30 19:09:47]

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/io_patch.dart (right):

https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/io_patch.dart:257: }
On 2017/03/30 18:08:19, zra wrote:
> On 2017/03/30 18:00:14, vsm wrote:
> > On 2017/03/30 17:48:38, zra wrote:
> > > On 2017/03/30 17:38:55, vsm wrote:
> > > > DBC
> > > > 
> > > > Can you add the same here as well?
> > > > 
> > > >
> > >
> >
>
https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/...
> > > 
> > > It doesn't look like that's the right thing to do. The input_sdk appears
to
> be
> > a
> > > snapshot at a particular SDK version, not bleeding edge.
> > 
> > It used to be. It is now applied against the bleeding edge sdk. 
> 
> No, it doesn't look like it. For example, _Platform._ansiSupported is there
but
> no longer exists. It seems like a bad idea to have two copies of this patch
> file, and I'm not sure this CL should be trying to patch everything up.

Anyhow, I removed _Platform._ansiSupported and added _Platform._localeName over
in the other file, but I'm not sure whether there are still other problems.

[Florian Schneider, 2017-03-30 19:28:08]

Lgtm

[vsm, 2017-03-30 19:34:44]

On 2017/03/30 19:09:47, zra wrote:
>
https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
> File sdk/lib/_internal/js_runtime/lib/io_patch.dart (right):
> 
>
https://codereview.chromium.org/2785073002/diff/90001/sdk/lib/_internal/js_ru...
> sdk/lib/_internal/js_runtime/lib/io_patch.dart:257: }
> On 2017/03/30 18:08:19, zra wrote:
> > On 2017/03/30 18:00:14, vsm wrote:
> > > On 2017/03/30 17:48:38, zra wrote:
> > > > On 2017/03/30 17:38:55, vsm wrote:
> > > > > DBC
> > > > > 
> > > > > Can you add the same here as well?
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://github.com/dart-lang/sdk/blob/master/pkg/dev_compiler/tool/input_sdk/...
> > > > 
> > > > It doesn't look like that's the right thing to do. The input_sdk appears
> to
> > be
> > > a
> > > > snapshot at a particular SDK version, not bleeding edge.
> > > 
> > > It used to be. It is now applied against the bleeding edge sdk. 
> > 
> > No, it doesn't look like it. For example, _Platform._ansiSupported is there
> but
> > no longer exists. It seems like a bad idea to have two copies of this patch
> > file, and I'm not sure this CL should be trying to patch everything up.
> 
> Anyhow, I removed _Platform._ansiSupported and added _Platform._localeName
over
> in the other file, but I'm not sure whether there are still other problems.

Thanks, Zach!  

I suspecting we're detecting when an expected patch is not there, but not when
an unexpected (unused) patch is there.  I'll file a bug.  

We do plan to unfork the patch files more, but dart2js and ddc occasionally
differ on the implementation side.

[zra, 2017-03-30 20:31:18]

Description was changed from

==========
[dart:io] Adds Platform.localeName

related #29126
==========

to

==========
[dart:io] Adds Platform.localeName

related #29126

R=fschneider@google.com

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

[zra, 2017-03-30 20:31:19]

Committed patchset #8 (id:130001) manually as
ae6d854ec6134bda2f64d2747b5cd9b534c5bb04 (presubmit successful).

[zra, 2017-03-30 20:35:47]

On 2017/03/30 20:31:19, zra wrote:
> Committed patchset #8 (id:130001) manually as
> ae6d854ec6134bda2f64d2747b5cd9b534c5bb04 (presubmit successful).

The build failed on the Mac buildbots, reverted here:
https://codereview.chromium.org/2791453002/

[zra, 2017-03-30 20:52:43]

On 2017/03/30 20:35:47, zra wrote:
> On 2017/03/30 20:31:19, zra wrote:
> > Committed patchset #8 (id:130001) manually as
> > ae6d854ec6134bda2f64d2747b5cd9b534c5bb04 (presubmit successful).
> 
> The build failed on the Mac buildbots, reverted here:
> https://codereview.chromium.org/2791453002/

Relanded here: https://codereview.chromium.org/2786183003/