tc39 / proposal-intl-duration-format

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

In digital format, what should we do with grouping separators in hours, minutes and seconds field when the value is > 60 or > 999? #192

Closed FrankYFTang closed 3 weeks ago

FrankYFTang commented 4 months ago

What should be the expected output of the following?

const df = new Intl.DurmationFormat("en", {style: "digital"})
df.format({hours: 300, minutes: 60, seconds: 120})

should it output "300:60:120" ? but how does it make sense?

How about

df.format({hours: 20, minutes: 3, seconds: 1234567})

"20:03:1,234,567" ?

or "20:03:1234567" ?

Currently, the spec will ask the implementation to output "20:03:1,234,567 "

FrankYFTang commented 4 months ago

@ben-allen @ryzokuken @sffc

sffc commented 4 months ago

The spec doesn't do balancing except in the narrow case of fractional second units in numeric mode. Users should eventually use Temporal.Duration to perform their balancing. So I believe this is mostly WAI. Except perhaps for this part:

"20:03:1,234,567 "

I think perhaps we should suppress grouping separators in digital format because otherwise it looks too confusing.

FrankYFTang commented 4 months ago

suppress grouping separators in digital format

Yea, that is the main reason I file this bug. But which part should we suppress grouping separators in digital format? All? or while unit is hours, minutes and seconds?

consider the following (I included the outcome of the current pec. Notice we need to consider what should do with days or larger units)

const df = new Intl.DurationFormat("en", {style: "digital"})
df.format({hours: 1234567, minutes: 20, seconds: 45}) // => "1,234,567:20:45"
df.format({hours: 12, minutes: 1234567, seconds: 20}) // => "12:1,234,567:20"
df.format({hours: 12, minutes: 34, seconds: 1234567}) // => "12:34:1,234,567"
df.format({hours: 12, minutes: 34, seconds: 56, milliseconds: 1234567}) //  => "12:34:1,290.567"
df.format({days: 1234567, hours: 3, minutes: 20, seconds: 45}). // => "1,234,567 days, 3:20:45"

what should be the use grouping suppressed output we should aim to change to?

FrankYFTang commented 4 months ago

@anba

anba commented 4 months ago

I think perhaps we should suppress grouping separators in digital format because otherwise it looks too confusing.

That suggestion sounds reasonable to me.

Using the example inputs from https://github.com/tc39/proposal-intl-duration-format/issues/192#issuecomment-2052598197:

Duration Current Proposed
{hours: 1234567, minutes: 20, seconds: 45} 1,234,567:20:45 1234567:20:45
{hours: 12, minutes: 1234567, seconds: 20} 12:1,234,567:20 12:1234567:20
{hours: 12, minutes: 34, seconds: 1234567} 12:34:1,234,567 12:34:1234567
{hours: 12, minutes: 34, seconds: 56, milliseconds: 1234567} 12:34:1,290.567 12:34:1290.567
{days: 1234567, hours: 3, minutes: 20, seconds: 45} 1,234,567 days, 3:20:45 1,234,567 days, 3:20:45
FrankYFTang commented 4 months ago

I support @anba 's proposal

sffc commented 4 months ago

TG2 notes: https://github.com/tc39/ecma402/blob/main/meetings/notes-2024-04-25.md#grouping-separators-in-durationformat

FrankYFTang commented 2 weeks ago

The PR implemented the TG2 agreed change is in https://github.com/tc39/proposal-intl-duration-format/pull/198 and got merged. I think we need someone to bring that PR to TG1 to got consensus for the change inside Stage 3.

sffc commented 2 weeks ago

TG2 discussion: https://github.com/tc39/ecma402/blob/main/meetings/notes-2024-08-22.md#durationformat-merge-issue