Editorial Changes to Align with 262 #120
Conversation
…haracter. this change allows a clear separation between internal slot names and properties/fields used across the spec as in [[<name>]]. This change does not change any semantic or any logic, it only adds internal slot column in table 4 and table 2 to have a clear distintion between property and internal slot names.
…ith uppercase characters. This change does not change any semantic or any logic.
|
/cc @zbraniecki @ericf |
| 1. Set _collator_.[[locale]] to _r_.[[locale]]. | ||
| 1. Let _relevantExtensionKeys_ be %Collator%.[[RelevantExtensionKeys]]. | ||
| 1. Let _r_ be ResolveLocale(%Collator%.[[AvailableLocales]], _requestedLocales_, _opt_, _relevantExtensionKeys_, _localeData_). | ||
| 1. Set _collator_.[[Locale]] to _r_.[[locale]]. |
There was a problem hiding this comment.
Shouldn't the field on _r_ also be Locale?
There was a problem hiding this comment.
the problem with ResolveLocale is that it creates an arbitrary record with fields, and we use that to match the keys provided via options. We could attempt to do the same, but those are not internal slots, just fields in a record. I think we can keep them like that. I know it is confusing though.
|
👏 |
| 1. Let _value_ be _r_.[[<_key_>]]. | ||
| 1. If the name given in the Type column of the row is *"boolean"*, let _value_ be the result of comparing value with *"true"*. | ||
| 1. Set _collator_.[[<_property_>]] to _value_. |
There was a problem hiding this comment.
So happy to get rid of these.
There was a problem hiding this comment.
It took some time, and some big refactors, but we are getting there :). Still using it in a couple of places where a record with fields is created and matching some options keys, but it is at least narrowed down to one or two abstracts at the moment.
|
LGTM! |
|
If you're changing the internal names to upper case, should the |
|
@littledan I have created a new issue #124 to solve that in a different patch to eliminate the usage of |
| <td>numeric</td> | ||
| <td>"boolean"</td> | ||
| <td></td> | ||
| </tr> | ||
| <tr> | ||
| <td>kf</td> | ||
| <td>[[CaseFirst]]</td> | ||
| <td>caseFirst</td> |
There was a problem hiding this comment.
Editorial suggestion: now that this is just a property name and not also an internal slot, maybe it should be written as "caseFirst" rather than caseFirst.
There was a problem hiding this comment.
@littledan this is now fixed: e4eaa72
|
Sorry for the slow response, thanks for fixing up all these issues, retroactive LGTM. |
This PR does not modify any logic or semantic in any way, and hopefully it will go smooth. Please, take time to review all the details in case I have missed something.