Everything

php

[RFC] Add IntlDateTimePatternGenerator

php.internals

Mel Dafert

5 years ago

Hi Internals,

I would like to propose an RFC https://wiki.php.net/rfc/intldatetimepatterngenerator to add IntlDateTimePatternGenerator
which exposes ICU's ability to flexibly create localized date/time formatting patterns from a skeleton.

Previous discussion: https://externals.io/message/113831
Implementation: https://github.com/php/php-src/pull/6771

Proposed signature:
```
class IntlDateTimePatternGenerator
{
public function __construct(?string $locale = null) {}

public static function create(?string $locale = null): ?IntlDateTimePatternGenerator {}

public function getBestPattern(string $skeleton): string|false {}
}
```

There is an open question about what this should be named `IntlDatePatternGenerator` instead, both for brevity and to keep consistency
with `IntlDateFormatter`. (Note that the underlying ICU classes are called `DateTimePatternGenerator` and `DateFormatter`, respectively.)
I am open to switching to the shorter form.

Another open question is the signature, as currently both `__construct` and `create` are exposed (like `IntlDateFormatter`).
Other classes inside the intl extension (like `IntlCalendar`) only provide a static `create` method, and leave the `__construct` method
private. Is there a preferred way, or are both equivalent?

Regards,
Mel

Larry Garfield

5 years ago

On Sat, Apr 24, 2021, at 10:12 AM, Mel Dafert wrote:

> Hi Internals,
>
> I would like to propose an RFC
> https://wiki.php.net/rfc/intldatetimepatterngenerator to add
> IntlDateTimePatternGenerator
> which exposes ICU's ability to flexibly create localized date/time
> formatting patterns from a skeleton.
>
> Previous discussion: https://externals.io/message/113831
> Implementation: https://github.com/php/php-src/pull/6771
>
> Proposed signature:
> ```
> class IntlDateTimePatternGenerator
> {
>     public function __construct(?string $locale = null) {}
>
>     public static function create(?string $locale = null):
> ?IntlDateTimePatternGenerator {}
>
>     public function getBestPattern(string $skeleton): string|false {}
> }
> ```
>
> There is an open question about what this should be named
> `IntlDatePatternGenerator` instead, both for brevity and to keep
> consistency
> with `IntlDateFormatter`. (Note that the underlying ICU classes are
> called `DateTimePatternGenerator` and `DateFormatter`, respectively.)
> I am open to switching to the shorter form.
>
> Another open question is the signature, as currently both `__construct`
> and `create` are exposed (like `IntlDateFormatter`).
> Other classes inside the intl extension (like `IntlCalendar`) only
> provide a static `create` method, and leave the `__construct` method
> private. Is there a preferred way, or are both equivalent?

For both points, I'd say "consistency wins by default."  So if the other classes in that package already skip "Time" and use ::create() instead of a constructor, you should do the same as well unless there's a very good reason not to.

(Consistency with everything else in PHP that's not that package is a possible reason not to, I grant, hence why it's somewhat subjective.)

--Larry Garfield

Mel Dafert

5 years ago

> For both points, I'd say "consistency wins by default." So if the other classes in that package already skip "Time" and
> use ::create() instead of a constructor, you should do the same as well unless there's a very good reason not to.

The problem with ::create is that in the intl extension itself there are already two different ways used
(`IntlDateFormatter` exposes both, `IntlCalendar::__construct` is private)

Mel Dafert

5 years ago

>For both points, I'd say "consistency wins by default."  So if the other classes in that package already skip "Time" and use ::create() instead of a constructor, you should do the same as well unless there's a very good reason not to.

I have updated the RFC to use switch to the naming without "Time", and added some notes about the naming options.

(On a side note, I realized that HHVM/Hack also implements it without the "Time" part:
https://docs.hhvm.com/hack/reference/class/IntlDatePatternGenerator/ )

I have also removed the open question about the `::create()` method, since the current form is consistent with `IntlDateFormatter`, which is what we want.

Mel Dafert

5 years ago

I am planning to open the vote on this tomorrow, if there aren't any objections.