Add style/type to ListFormatter public API

Description

C API for ListFormatter doesn't have support for style/type of the formatter.

The unstable API has `style` parameter which can be "standard", "duration", or "duration-short".

I assume that the selection has been based on CLDR categories that we recently updated ( http://unicode.org/cldr/trac/ticket/9361 ).

We now have two types of lists - standard and unit (and we're talking about adding a third for formatting list of numbers)

And we have three styles of lists - long, short and narrow.

It would be great to get the stable API to support those two parameters.

Activity

Show:
Shane Carr
October 9, 2019, 9:42 PM

Comment from Jeff Walden via email:

----------

We do sometimes use unstable ICU C++ API, if we must and the need is sufficiently important and the C++ API looks unofficially pretty stable. It appears

is the only extra unstable API needed. (We depend on unstable icu::Locale::Locale(const char*) for other reasons already.)

U_HIDE_INTERNAL_API suggests ICU developers would change this without even thinking about it. I think it doesn't affect whether a system ICU would provide it (e.g. Fedora has it). We technically could take the dependency, but I think U_HIDE_INTERNAL_API concerns push this past the point where I want to take it.

That said, a straightforward

patterned off the existing

where type/style are conjunction/disjunction/unit and narrow/short/long enums looks easy. I'll look into writing such a patch and posting it and mailing icu-design tomorrow. We take local patches that upstream has landed but not shipped yet more easily than we'll take dodgy C++ unstable API use. Not quite a workaround, but I guess this has to happen sooner or later, might as well be sooner.

Shane Carr
October 9, 2019, 9:47 PM

The style method was added to C++ in

https://github.com/unicode-org/icu/commit/ecd7ea193b6c8740c06ebaa59a771afee993029a

If the behavior is stable, I would support promoting this method from internal to draft. An argument could even be made to promote this from internal to stable "de facto".

A C API can also be added.

Jeff Walden
October 9, 2019, 10:50 PM

I did the local hacking to churn out a patch for the API as proposed above. (Roughly – I changed “style” mentions to “width” because it seemed maybe a better name, and moreover ListFormatter::createInstance called its different/distinct parameter style so it seemed unwise to use the same name.) I’ve tried to test it, but for some reason my locally-built ICU tip throws an exception when trying to call umtx_lock nested beneath the new API entrypoint, and that could easily be me screwing up how ICU was built, so I don’t know if it actually works. But it’s gotta be at least a good start.

Jeff Walden
October 9, 2019, 10:52 PM

(Not certain whether I have time to send the icu-design mail, in part for lack of time, but also because I think my existing subscription probably is no longer accessible to me because MIT’s mail system abruptly stopped supporting +foo notation and so I’ve probably lost account access. I really ought resubscribe, but…time.)

Shane Carr
October 25, 2019, 3:47 AM

Thanks for the work! The API surface looks good. I like changing out the strings in the old @internal API for enums.

I cloned your branch and added the following changes:

  1. Added tests

  2. Added C++ version of the API

  3. Changed C to invoke the C++ version

I sent an API proposal and a PR:

https://github.com/unicode-org/icu/pull/894

Other notes:

  • We typically use SHORT as the default width in APIs, not LONG.

  • You can use C++ in implementation files. We have a helper class CharString to do string concatenation so you don't have to do all the strcpy stuff. However, this is moot since I rewired the C implementation to invoke the new C++ factory method.

Assignee

Shane Carr

Reporter

TracBot

Components

Labels

Reviewer

None

Priority

major

Time Needed

Days

Fix versions

Configure