-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor numeric styles #185
Conversation
…" style for subsecond units. This is introduced to simplify the logic for handling "numeric" units in GetDurationUnitOptions and PartitionDurationFormatPattern, as this style means something different for subsecond units than it does for hours, minutes, and seconds. This change is non-observable: users still use "numeric" to specify the style in all cases, and "fractional" is converted back to "numeric" in resolvedOptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good although the outcome is not as concise as I had been hoping it would be. The steps to fix "numeric" vs "fractional" for internal/external uses adds as much code as using "fractional" removes.
I have work in progress for a PR to apply on top of this one which factors out handling for times in numeric and numeric-like styles into their own AOs, though less in the interest of reducing LOC and more in the interest of reducing the amount of state ( |
* Editorial: updated resolvedOptions and Table 3 to use new "fractional" style for subsecond units. This is introduced to simplify the logic for handling "numeric" units in GetDurationUnitOptions and PartitionDurationFormatPattern, as this style means something different for subsecond units than it does for hours, minutes, and seconds. This change is non-observable: users still use "numeric" to specify the style in all cases, and "fractional" is converted back to "numeric" in resolvedOptions.] Separate PartitionDurationFormatPattern's handling for "numeric" and "2-digit" into their own AOs
I believe intl402/DurationFormat/constructor-options-defaults.js in test262 need to be adjust after this PR but I have a hard time to figure out how to hange it. @ben-allen could you change that |
Wait. how could this change be non-observable? you change "numeric" to "fractional" in Table 3 and therefore what got passed into GetDurationUnitOptions() for milliseconds, microseconds, and nanoseconds are now « "long", "short", "narrow", "fractional" »
in stylesList is « "long", "short", "narrow", "fractional" » |
Thanks for the catch! PR to fix it here: #197 |
Editorial: updated
resolvedOptions
and Table 3 to use new"fractional"
style for subsecond units.This is introduced to simplify the logic for handling
"numeric"
units inGetDurationUnitOptions
and PartitionDurationFormatPattern, as this style means something different for subsecond units than it does for hours, minutes, and seconds.This change is non-observable: users still use
"numeric"
to specifythe style in all cases, and
"fractional"
is converted back to"numeric"
inresolvedOptions.
Some unrelated wording clarifications: regularized table iteration language, replaced "options bag" with "Object" when describing parameters of
GetDurationUnitOptions
.