Stage 4 PR for proposal-intl-extend-timezonename#621
Conversation
Based on https://tc39.es/proposal-intl-extend-timezonename/ The <del> and some <ins> in the spec text is already in latest ECMA402
ryzokuken
left a comment
There was a problem hiding this comment.
LGTM, a few nits inline.
BTW, the diff at https://tc39.es/proposal-intl-extend-timezonename/ is outdated around FormatDateTimePattern pt 15.e.
| 1. Else if _property_ is *"timeZoneName"*, then | ||
| 1. If _optionsProp_ is *"short"* or *"shortGeneric"*, then | ||
| 1. If _formatProp_ is *"shortOffset"*, decrease _score_ by _offsetPenalty_. | ||
| 1. Else if _formatProp_ is *"longOffset"*, decrease _score_ by (_offsetPenalty_ + _shortMorePenalty_). |
There was a problem hiding this comment.
nit: are the brackets necessary here?
There was a problem hiding this comment.
I think that make it clearer
I know, I landed dbe5d72#diff-aae61752e383a82db0847140fc85c3f1718f6348407153200b6db8462f10a21a myself. So the diff in this PR is only the diff from the spec text. |
|
@FrankYFTang great, thanks. Consider my questions resolved. @gibson042 @leobalter do you folks want to take a quick look? Good luck for Stage 4! |
| 1. Else if _property_ is *"timeZoneName"*, then | ||
| 1. If _optionsProp_ is *"short"* or *"shortGeneric"*, then | ||
| 1. If _formatProp_ is *"shortOffset"*, decrease _score_ by _offsetPenalty_. | ||
| 1. Else if _formatProp_ is *"longOffset"*, decrease _score_ by (_offsetPenalty_ + _shortMorePenalty_). | ||
| 1. Else if _optionsProp_ is *"short"* and _formatProp_ is *"long"*, decrease _score_ by _shortMorePenalty_. | ||
| 1. Else if _optionsProp_ is *"shortGeneric"* and _formatProp_ is *"longGeneric"*, decrease _score_ by _shortMorePenalty_. | ||
| 1. Else if _optionsProp_ ≠ _formatProp_, decrease _score_ by _removalPenalty_. | ||
| 1. Else if _optionsProp_ is *"shortOffset"* and _formatProp_ is *"longOffset"*, decrease _score_ by _shortMorePenalty_. | ||
| 1. Else if _optionsProp_ is *"long"* or *"longGeneric"*, then | ||
| 1. If _formatProp_ is *"longOffset"*, decrease _score_ by _offsetPenalty_. | ||
| 1. Else if _formatProp_ is *"shortOffset"*, decrease _score_ by (_offsetPenalty_ + _longLessPenalty_). | ||
| 1. Else if _optionsProp_ is *"long"* and _formatProp_ is *"short"*, decrease _score_ by _longLessPenalty_. | ||
| 1. Else if _optionsProp_ is *"longGeneric"* and _formatProp_ is *"shortGeneric"*, decrease _score_ by _longLessPenalty_. | ||
| 1. Else if _optionsProp_ ≠ _formatProp_, decrease _score_ by _removalPenalty_. | ||
| 1. Else if _optionsProp_ is *"longOffset"* and _formatProp_ is *"shortOffset"*, decrease _score_ by _longLessPenalty_. | ||
| 1. Else if _optionsProp_ ≠ _formatProp_, decrease _score_ by _removalPenalty_. |
There was a problem hiding this comment.
@FrankYFTang this works, but I would love for the adjustments to be expressed more concisely. What would you think of this?
| 1. Else if _property_ is *"timeZoneName"*, then | |
| 1. If _optionsProp_ is *"short"* or *"shortGeneric"*, then | |
| 1. If _formatProp_ is *"shortOffset"*, decrease _score_ by _offsetPenalty_. | |
| 1. Else if _formatProp_ is *"longOffset"*, decrease _score_ by (_offsetPenalty_ + _shortMorePenalty_). | |
| 1. Else if _optionsProp_ is *"short"* and _formatProp_ is *"long"*, decrease _score_ by _shortMorePenalty_. | |
| 1. Else if _optionsProp_ is *"shortGeneric"* and _formatProp_ is *"longGeneric"*, decrease _score_ by _shortMorePenalty_. | |
| 1. Else if _optionsProp_ ≠ _formatProp_, decrease _score_ by _removalPenalty_. | |
| 1. Else if _optionsProp_ is *"shortOffset"* and _formatProp_ is *"longOffset"*, decrease _score_ by _shortMorePenalty_. | |
| 1. Else if _optionsProp_ is *"long"* or *"longGeneric"*, then | |
| 1. If _formatProp_ is *"longOffset"*, decrease _score_ by _offsetPenalty_. | |
| 1. Else if _formatProp_ is *"shortOffset"*, decrease _score_ by (_offsetPenalty_ + _longLessPenalty_). | |
| 1. Else if _optionsProp_ is *"long"* and _formatProp_ is *"short"*, decrease _score_ by _longLessPenalty_. | |
| 1. Else if _optionsProp_ is *"longGeneric"* and _formatProp_ is *"shortGeneric"*, decrease _score_ by _longLessPenalty_. | |
| 1. Else if _optionsProp_ ≠ _formatProp_, decrease _score_ by _removalPenalty_. | |
| 1. Else if _optionsProp_ is *"longOffset"* and _formatProp_ is *"shortOffset"*, decrease _score_ by _longLessPenalty_. | |
| 1. Else if _optionsProp_ ≠ _formatProp_, decrease _score_ by _removalPenalty_. | |
| 1. Else if _property_ is *"timeZoneName"*, then | |
| 1. Let _penalty_ be the value of the column with name matching _formatProp_ in the row of <emu-xref href="#table-timezonename-component-non-default-penalties"></emu-xref> for which the value of the “Requested Style” column is _optionsProp_. If that cell is empty, let _penalty_ be _removalPenalty_. | |
| 1. If _optionsProp_ = _formatProp_, then | |
| 1. Assert: _penalty_ is 0. | |
| 1. Else, | |
| 1. Decrease _score_ by _penalty_. |
<emu-table id="table-timezonename-component-non-default-penalties">
<emu-caption>*"timeZoneName"* component non-default penalties</emu-caption>| Requested Style | *"short"* | *"long"* | *"shortOffset"* | *"longOffset"* | *"shortGeneric"* | *"longGeneric"* |
|---|---|---|---|---|---|---|
| *"short"* | 0 | _shortMorePenalty_ | _offsetPenalty_ | _offsetPenalty_ + _shortMorePenalty_ | ||
| *"shortGeneric"* | _offsetPenalty_ | _offsetPenalty_ + _shortMorePenalty_ | 0 | _shortMorePenalty_ | ||
| *"shortOffset"* | 0 | _shortMorePenalty_ | ||||
| *"long"* | _longLessPenalty_ | 0 | _offsetPenalty_ + _longLessPenalty_ | _offsetPenalty_ | ||
| *"longGeneric"* | _offsetPenalty_ + _longLessPenalty_ | _offsetPenalty_ | _longLessPenalty_ | 0 | ||
| *"longOffset"* | _longLessPenalty_ | 0 |
There was a problem hiding this comment.
I am nervous about this suggestion. This is a much bigger (pure editorial I assume) changes. How about we merge it w/o such into ECMA402 and then later put that into a different Editorial PR?
There was a problem hiding this comment.
I don't object either way. But for clarity: would you support such a change, or do you prefer the algorithm steps as they are?
There was a problem hiding this comment.
Let me put down this way- I am supporting change it to a table form as an editorial change but I think we need more spend more time and review from more people (in particular @anba) for that PR to make sure we didn't change unintentionally. So I prefer not mix that into this Stage 4 PR.
Co-authored-by: Ujjwal Sharma <ryzokuken@disroot.org>
ryzokuken
left a comment
There was a problem hiding this comment.
LGTM. I like the change proposed by @gibson042 but we can do it as an editorial ECMA-402 PR after merging this, as @FrankYFTang proposed.
|
Since we've hit stage 4 and all editorial issues have been addressed, can we merge this? |
@ryzokuken Who are you asking this question to? Are you asking @gibson042 ? I do not have merge right since I am not editor so we need someone have merge right to click the button to merge. |
|
@FrankYFTang I have merge access, I just wanted to confirm that this PR is good to go before doing that. |
Are you asking @gibson042 ? or are you asking me? or someone else? |
|
@FrankYFTang I was asking both of you what you think. But I believe we're good to go, so since there aren't any objections, I'll merge this. |
|
thanks |
Based on https://tc39.es/proposal-intl-extend-timezonename/
The <del> and some <ins> in the spec text is already in latest ECMA402
pending on Stage 4 Advancement in 2021-Dec TC39