search

tc39 / proposal-intl-duration-format

https://tc39.es/proposal-intl-duration-format
MIT License
163 stars 18 forks source link

Refactor numeric styles #185

Closed ben-allen closed 7 months ago

ben-allen commented 7 months ago

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 specify the style in all cases, and "fractional" is converted back to "numeric" in resolvedOptions.

Some unrelated wording clarifications: regularized table iteration language, replaced "options bag" with "Object" when describing parameters of GetDurationUnitOptions.

ben-allen commented 7 months ago

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 (_prevStyle, _nextStyle_) carried around when formatting units with numeric styles.

FrankYFTang commented 4 months ago

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

FrankYFTang commented 4 months ago

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.

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" » and what got passed into

1. Let style be ? GetOption(options, unit, string, stylesList, undefined).

in stylesList is « "long", "short", "narrow", "fractional" » and if user pass in {microseconds: "numeric"} it GetOption will throw RangeError.

ben-allen commented 4 months ago

Thanks for the catch! PR to fix it here: https://github.com/tc39/proposal-intl-duration-format/pull/197