1719678 - Unify date interval formatting/ DateTime(Range) formatToParts in SpiderMonkey

Status: RESOLVED FIXED

(Core :: Internationalization, task, P3)

Description

[Greg Tatum [:gregtatum]]

In https://searchfox.org/mozilla-central/source/js/src/builtin/intl/DateTimeFormat.cpp

The calls to create and use UDateIntervalFormat need to use a mozilla:intl component.

This will remove the following import:

#include "unicode/udateintervalformat.h"

Comment 1

[Dan Minor [:dminor]]

Hi Yoshi, Greg and I think this would be a good next project for you when you have the time.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Cool, thanks. I'll work on this, although I am on parental leave and the progress will be slower.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1719678 - Part 1: Add a mozilla::intl::DateIntervalFormat.

Add a mozilla::intl::DateIntervalFormat class, with two methods:

• TryFormatCalendar
• TryFormatDateTime

These methods also take a callback, the callback will be called with the
UFormattedValue* argument.

However, there's a failure when running check_vanilla_allocations.py.

The callback is a std::function, and gcc will call operator new if the
lambda is too large, which is detected by check_vanilla_allocations.py
and causes failure.

https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/std_function.h#L161

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1719678 - Part 2: Unify FormatDateTimeToParts and FormatDateTimeRangeToParts.

Add DateTimeFormat::TryFormatToParts and
DateIntervalFormat::TryFormattedToParts methods.

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Greg, could you check my comment in mozphab?
https://phabricator.services.mozilla.com/D125642#4086962

(copy from mozphab)

For getting the formatted string of FormatDateTimeRange, there are two interface calls:

Call DateIntervalFormat::TryFormatCalendar or TryFormatDateTime, and store the formatted value in AutoFormattedDateInterval
Use the AutoFormattedDateInterval to check if the date ranges are "practically equal" by calling DateIntervalFormat::DateFieldsPracticallyEqual

Do you think this DateIntervalFormat::DateFieldsPracticallyEqual interface makes sense?
Or should I add an extra parameter 'isEqual' in TryFormatCalendar/TryFormatDateTime?

Thanks

Comment 7

[Greg Tatum [:gregtatum]]

Do you think this DateIntervalFormat::DateFieldsPracticallyEqual interface makes sense?
Or should I add an extra parameter 'isEqual' in TryFormatCalendar/TryFormatDateTime?

It seems like the current approach adds implementation details to the unified API. My preference looking at things would be to add an extra parameter, especially if it reduces the API size. Generally we're trying to keep ICU-specific things hidden away as much as possible.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Greg:
In my patches, I only handle the UTF-16 case,
(For example. DateTimeFormat::TryFormatToParts only accepts buffer with CharType is char16_t)

should I file a new bug to tackling the UTF-8 case?

Comment 9

[Greg Tatum [:gregtatum]]

I would only file UTF-8 support if we actually plan on using them in Gecko somewhere. Otherwise it's probably fine to limit scope. We can always file follow-up tracking issues if we need to switch for ICU4X. At this point, I'd prefer to keep the scope more limited here.

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Bug 1719678 - Part 3: Remove js::intl::FormattedValueToString.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4678d4fbb260
Part 1: Add a mozilla::intl::DateIntervalFormat. r=platform-i18n-reviewers,anba,gregtatum,tcampbell
https://hg.mozilla.org/integration/autoland/rev/af56d234b8c1
Part 2: Unify FormatDateTimeToParts and FormatDateTimeRangeToParts. r=platform-i18n-reviewers,anba,gregtatum,tcampbell
https://hg.mozilla.org/integration/autoland/rev/35b801064f12
Part 3: Remove js::intl::FormattedValueToString. r=tcampbell

Comment 12

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4678d4fbb260
https://hg.mozilla.org/mozilla-central/rev/af56d234b8c1
https://hg.mozilla.org/mozilla-central/rev/35b801064f12