Fix and simplify "displayRequired" computation in PartitionDurationFormatPattern

anba commented 8 months ago

Fixes:

durationFormat.[[SecondsValue]] should be duration.[[Seconds]]
Sub-second fields were incorrectly ignored

Simplications:

Change displayRequired to a boolean.
durationFormat.[[HoursStyle]], durationFormat.[[HoursDisplay]], and duration.[[Hours]] don't have to be inspected, instead simply testing if needSeparator is true is sufficient.
needSeparator doesn't need to be reset. This step was only present for compatibility with previous revisions to output strings like 1, 03 for {hours: 1, seconds: 3} (see #170).

anba commented 7 months ago

e717eb2068d5ee9942d49fff23c738e0a8141e7f and 91f2eb0192bcff0d155ed18a50a0d2173f5a533d are purely editorial. 6b4a0b394f4c06fdf7e5a048b7a429957468c0d8 is a bug fix for #180.

FrankYFTang commented 4 months ago

yes, I think this PR is better.

ben-allen commented 4 months ago

lgtm! note that these errors are (unless I'm mistaken) also fixed in https://github.com/tc39/proposal-intl-duration-format/pull/188, which is waiting for review/re-review

ben-allen commented 3 weeks ago

closed as fixed by #188

tc39 / proposal-intl-duration-format

Fix and simplify "displayRequired" computation in PartitionDurationFormatPattern #183