Conversation
|
The rendered spec preview for this PR is available as a single page at https://tc39.es/ecma262/pr/3958 and as multiple pages at https://tc39.es/ecma262/pr/3958/multipage . |
826ec15 to
4e167f6
Compare
There was a problem hiding this comment.
I really like this direction and at a glance changes LGTM, though if you intend to have this merged unsquashed please avoid temporarily defining IndexableToLocaleString - having an AO in the commit history that never ended up being real is confusing, and if it was the end result of this PR I would have objected since Array and TypedArray toLocaleString are sufficiently different to warrant their own algorithms (one operates on arbitrary objects, the other on validated TAs - leading to different expectation wrt completions and element types and warranting TA-specific note steps). This concern goes away once the AO is broad enough to be used for join too.
4e167f6 to
d7cf89f
Compare
Depends upon JoinIndexedElements from tc39/ecma262#3958
linusg
left a comment
linusg
left a comment
There was a problem hiding this comment.
This commit progression works nicely IMO, thanks!
…}.prototype.join cf. SortIndexedProperties
…prototype.toLocaleString
d7cf89f to
c651b86
Compare
Depends upon JoinIndexedElements from tc39/ecma262#3958
…dArray%}.prototype.toLocaleString
…s own LocalizedListSeparator AO This is friendlier to ESMeta.
b68d8ea to
5579333
Compare
Depends upon JoinIndexedElements from tc39/ecma262#3958
| <h1>LocalizedListSeparator ( ): a String</h1> | ||
| <dl class="header"> | ||
| <dt>description</dt> | ||
| <dd>It returns an implementation-defined String appropriate for use as a list separator in the host environment's current locale (such as *", "*).</dd> |
There was a problem hiding this comment.
Should we clarify that it must return the same String every time it is called?
There was a problem hiding this comment.
what if the locale changes? or do we already guarantee it can't change without a pageload
There was a problem hiding this comment.
We don't make that guarantee, and there is no such constraint upon the toLocaleString methods, nor upon ECMA-402 DefaultLocale. I'd be open to exploring that, but not in the scope of this PR.
There was a problem hiding this comment.
Then we should clarify that there is no such constraint.
There was a problem hiding this comment.
I see no value in making Array/TypedArray localization special in that way. Tackling that should cover all toLocaleString methods, and doesn't belong here.
| <emu-note> | ||
| <p>If the ECMAScript implementation includes the ECMA-402 Internationalization API this method is based upon the algorithm for `Array.prototype.toLocaleString` that is in ECMA-402.</p> | ||
| </emu-note> | ||
| <p>An ECMAScript implementation that includes the ECMA-402 Internationalization API must implement this method as specified in ECMA-402. Otherwise, the following specification of this method is used.</p> |
There was a problem hiding this comment.
But Ecma-402 doesn't currently define this method, right? Are we planning to hold off until tc39/ecma402#1094 is fixed?
There was a problem hiding this comment.
The fix is tc39/ecma402#1095 which uses the AOs added here. I think it's fine to go ahead with this given the 402 PR is approved and likely to land shortly afterwards.
linusg
left a comment
linusg
left a comment
There was a problem hiding this comment.
Bunch of changes since I approved; still happy with the current state.
This branch has not been deployed
5 participants
I noticed while reviewing #3940 that %TypedArray%.prototype.toLocaleString gestures at Array.prototype.toLocaleString rather than defining its own <emu-alg>, and further that it requires analogous use of the superseding ECMA-402 Array.prototype.toLocaleString algorithm in relevant implementations while ECMA-402 does not mention TypedArrays at all.
This PR addresses the ECMA-262 issue by defining the %TypedArray%.prototype.toLocaleString algorithm using a new JoinIndexedProperties operation common to both it and Array.prototype.toLocaleString, and prepares for addressing the ECMA-402 issue by defining that operation such that it will be usable by superseding definitions (specifically, that it accepts arguments for forwarding to "toLocaleString" method invocations, to be provided by ECMA-402 algorithms but not by ECMA-262 ones).
As its name suggests, JoinIndexedProperties is also used by {Array,%TypedArray%}.prototype.join methods.