tc39 / proposal-intl-duration-format

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

fractionalDigits, (||milli|micro|nano)secondsDisplay impact to number of fractional digits while style: "digital" #139

Closed FrankYFTang closed 1 year ago

FrankYFTang commented 1 year ago

So... we have the following options in the object creation all seems connect to the formating result of the "second". While the style is not "digital", these options are not interacted with each other so it is easy to understand, however, I am not sure what is the current intended result when the style is "digital"

fractionalDigits: 0...9 (default 0) secondsDisplay: 'auto' | 'always' (default auto) millisecondsDisplay: 'auto' | 'always' (default auto) microsecondsDisplay: 'auto' | 'always' (default auto) nanosecondsDisplay: 'auto' | 'always' (default auto)

consider the following data

let d1 = {minutes: 9, seconds: 1};
let d2 = {minutes: 9, milliseconds: 2};
let d3 = {minutes: 9, seconds: 1, milliseconds: 2};
let d4 = {minutes: 9, microseconds: 3};
let d5 = {minutes: 9, seconds: 1, miicroseconds: 3};
let d6 = {minutes: 9, nanoseconds: 4};
let d7 = {minutes: 9, seconds: 1, miicroseconds: 4};

Looking from the current spec, it seems the only option decide the number of formatted fractional digits is solely controlled by fractionalDigits and the output is not impacted by the present of secondsDisplay, millisecondsDisplay, microsecondsDisplay or nanosecondsDisplay . For example, if we have the following code

(new Intl.DurationFormat('en', {style: 'digital', millisecondsDisplay: 'always' })).format(d3)

it seems the current spec text will output "9:01" instead of "9:01.003"

but is that what we like to see? If we will ignore millisecondsDisplay, microsecondsDisplay and nanosecondsDisplay while the style is "digital", then should we throw in such condition during the constrcutor ?

If we will not ignore them, then what should we do if we also have the presence of fractionalDigits ?

@ryzokuken @sffc

sffc commented 1 year ago

It's a bit late to make changes, but reasonable behavior could be to change the default millisecondStyle, microsecondStyle, and nanosecondStyle in style: "digital" to be "short" if the Display for that unit is "always".

FrankYFTang commented 1 year ago

I think you didn't get my point. the issue is with style: "digital" (and some other cases that the fractional second units fold to be a higher unit because it is set to 'numeric'), any units below second is now part of second and never displayed by itself. (so the formatToParts will not have a unit with these unit when the style is digital) so how would the display for these units make sense when it is "always"? (since it cannot be "always" in that case)

sffc commented 1 year ago

I thought we were allowing { style: "digital", millisecondsStyle: "short" }, producing output like "0:00:00, 0 ms" but maybe we don't actually allow that.

FrankYFTang commented 1 year ago

I thought we were allowing { style: "digital", millisecondsStyle: "short" }, producing output like "0:00:00, 0 ms" but maybe we don't actually allow that. do you mean

{ style: "digital", milliseconds: "short" } ?

FrankYFTang commented 1 year ago

If we have { style: "digital", milliseconds: "short" } in the option, step 6-a-i of GetDurationUnitOptions will throw RangeError now because

under the iterator of the table, when the GetDurationUnitOptions got called with "seconds" as unit, the return style will be set to "numeric" by step 3-a-ii "ii. Set style to digitalBase." so the next iteration GetDurationUnitOptions will come with unit as "milliseconds", prevStyle: "numeric" and then in step 1

  1. Let style be ? GetOption(options, unit, "string", stylesList, undefined). will now get "short" as styel but then in step 6
  2. If prevStyle is "numeric" or "2-digit", then it will be true and 6-a. If style is not "numeric" or "2-digit", then is also trye (since we now have style: "short" and in 6-a-i we will i. Throw a RangeError exception.
ryzokuken commented 1 year ago

@FrankYFTang is correct, you cannot tweak the milliseconds style option to something that doesn't work for the overall style "digital" and in this situation, the display options aren't perfectly respected since they make most sense for non-digital styles too. This returns us to the general theme of digital being a difficult style to design around and trying to accommodate the style alongside the options for the non-digital style will always have some drawbacks, we just need to find out which of them we can live with.

One option I feel works best is to allow only one of the *secondsDisplay options or fractionalDigits for the digital style, since they counteract each other, or perhaps even going farther and allowing only fractionalDigits to be used for the digital style, disallowing setting *secondsDisplay to "always" completely for that style since it applies a different display strategy for these units anyway.

FrankYFTang commented 1 year ago

One option I feel works best is to allow only one of the secondsDisplay options or fractionalDigits for the digital style, the issue is .+secondsDisplay: 'always' (not secondsDisplay) is conflicting with style: 'digital' because under digital, we never output any units under 'milliseconds", "microseconds", or "nanoseconds" in the formatToParts unit, right?

ryzokuken commented 1 year ago

the issue is .+secondsDisplay: 'always' (not *secondsDisplay) is conflicting with style: 'digital' because under digital, we never output any units under 'milliseconds", "microseconds", or "nanoseconds" in the formatToParts unit, right?

you're right, syntax error from my side but basically this. Anything below seconds will be handled as a fraction in "digital" which means the +display doesn't make sense in that case.

sffc commented 1 year ago

TG2 approval: https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-04-06.md#fractionaldigits-millimicronanosecondsstyle-impact-to-number-of-fractional-digits-while-style-digital-139