tc39 / proposal-intl-duration-format

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

Normative: Delegate to NumberFormat to handle unit formattting #122

Closed FrankYFTang closed 2 years ago

FrankYFTang commented 2 years ago

For the case of style "short", "long" and "narrow" and resolve the plural issue and reduce unnecessary locale data handling.

This should also resolve the issue mentioned in https://github.com/tc39/proposal-intl-duration-format/issues/118

@ryzokuken @sffc @anba @gibson042 @michaelficarra @rbuckton @rkirsling -any opinion of moving toward this direction?

ryzokuken commented 2 years ago

@FrankYFTang thanks for this, I'd missed this simpler way of formatting the results which also avoids duplicating code as well as spec text.

anba commented 2 years ago

Probably worth mentioning this depends on https://github.com/tc39/ecma402/issues/702 resp. https://github.com/tc39/ecma402/issues/704

ryzokuken commented 2 years ago

@anba are we blocked on that right away or for Stage 4? Practically speaking, I suppose you'd need the change in order to implement this to begin with.

anba commented 2 years ago

are we blocked on that right away or for Stage 4?

I don't know. Are there any strict guidelines which describe cases like this? It's probably okay to merge this PR, as long as there's an <emu-note> to point out that this depends on the linked ECMA-402 issues. That way we don't risk that there's any confusion for other implementers of Intl.DurationFormat who didn't read this PR.

Practically speaking, I suppose you'd need the change in order to implement this to begin with.

Implementers can work around the unit option validation checks in the Intl.NumberFormat constructor if needed. But it's easier if the ECMA-402 issue is resolved first.

ryzokuken commented 2 years ago

@anba alright, thanks, that sounds good to me. I'll add a note and merge this.

FrankYFTang commented 2 years ago

Probably worth mentioning this depends on tc39/ecma402#702 resp. tc39/ecma402#704

Ah... I totally forgot about that dependency. Thanks for pointing that out.