search

tc39 / proposal-intl-duration-format

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

A couple of fixes noticed after implementing the latest spec in LibJS #203

Open trflynn89 opened 2 months ago

trflynn89 commented 2 months ago

I also noticed an inconsistency with how the [[DigitalFormat]] internal slot is referred to.

In the DurationFormat constructor, the fields of the digital format are applied directly on the DurationFormat object:

18. Let hoursMinutesSeparator be digitalFormat.[[HoursMinutesSeparator]].
19. Set durationFormat.[[HoursMinutesSeparator]] to hoursMinutesSeparator.

But in the FormatNumeric* AOs, it reads like they were applied to a [[DigitalFormat]] middle-man internal slot:

durationFormat.[[DigitalFormat]].[[HoursMinutesSeparator]].

I'm happy to clean that up, but didn't know which reference was intended.

ben-allen commented 3 weeks ago

Thank you greatly!

re: the [[DigitalFormat]] slot, the intent was indeed to have [[HoursMinutesSeparator]] and [[MinutesSecondsSeparator]] directly on the DurationFormat object. I've pushed a new commit to do this.

This seems to me to be a bugfix rather than a normative change to take to plenary -- do you think I can go ahead and merge it? @gibson042