Best-match pattern for "sS" uses <appendItem> data

Description

From <https://unicode.org/reports/tr35/tr35-dates.html#Matching_Skeletons>:

3. A requested skeleton that includes both seconds and fractional seconds (e.g. “mmssSSS”) is allowed to match a dateFormatItem skeleton that includes seconds but not fractional seconds (e.g. “ms”). In this case the requested sequence of ‘S’ characters (or its length) should be retained separately and used when adjusting the pattern, as described below.

and

If the requested skeleton included both seconds and fractional seconds and the dateFormatItem skeleton included seconds but not fractional seconds, then the seconds field of the corresponding pattern should be adjusted by appending the locale’s decimal separator, followed by the sequence of ‘S’ characters from the requested skeleton.

ICU appears to implement this specification when more than one 'S' character is present, for example the best-match pattern for the skeleton "sSS" is "s.SS" or for "sSSS" is "s.SSS". But when only a single 'S' character is present, ICU instead uses the <appendItem> data, that means the skeleton "sS" returns the best-match pattern "S ('second': s)".

Is the use of "S ('second': s)" correct or should instead "s.S" be used?


(Setting the "ecmascript" and "ecma402" labels, because it affects <https://github.com/tc39/ecma402/pull/347>.)

Activity

Show:
Frank Yung-Fong Tang
March 18, 2020, 10:48 PM

fixed

Shane Carr
January 9, 2020, 5:37 PM

Mihai, can you take a look?

Frank Yung-Fong Tang
December 20, 2019, 7:00 PM

Currently the above tests show

 

FAIL: Format pattern; got "S"; expected "s.S"

FAIL: Format pattern; got "SS"; expected "s.SS"

FAIL: Format pattern; got "SSS"; expected "s.SSS"

FAIL: Format pattern; got "SSSS"; expected "s.SSSS"

FAIL: Format pattern; got "S"; expected "ss.S"

FAIL: Format pattern; got "SS"; expected "ss.SS"

FAIL: Format pattern; got "SSS"; expected "ss.SSS"

FAIL: Format pattern; got "SSSS"; expected "ss.SSSS"

 

 

Frank Yung-Fong Tang
December 19, 2019, 8:45 PM

When I patch it into v8 it looks like the PR does not really address this part.

Please add unit test in icu4c/source/test/intltest/dtfmttst.cpp
to verify the following cases:

(not sure the expected values are all correct above)

Mihai Nita
November 20, 2019, 5:26 PM

It is fixed, but I get “You don’t have permission to transition this issue”

 

Fixed

Assignee

Mihai Nita

Reporter

André Bargull

Components

Labels

Priority

major

Time Needed

Days

Fix versions