[RFC] IntlRelativeDateTimeFormatter View as Markdown

Hi Ignace, David,

Thanks for the feedback. IMHO I tend to not accept Duration object as a parameter.

IntlRelativeDateTimeFormatter formats a caller-selected offset and unit. Time\Duration
represents stopwatch time as seconds and nanoseconds. This cause a huge amount of
issue which immediately comes into my head when thinking of this.

The API looks like

$fmt->format(3, UNIT_DAY); // in 3 days
$fmt->format(2, UNIT_MONTH); // in 2 months
$fmt->format(-1, UNIT_SUNDAY);// last Sunday

If we are now accepting Durations, they look like

Time\Duration::fromMinutes(90)

We don't know how to deal with 90 minutes here. It can be 90 minutes or 1.5 hour.

Nevertheless, what about weekdays? things like UNIT_SUNDAY are not durations.
Not to mention months, quarters, and years need calendar context.

For enums and namespaces: I kept class constants and the global Intl* class name to stay
consistent with the existing ext/intl API, such as IntlDateFormatter, IntlListFormatter
IntlNumberRangeFormatter, and IntlDatePatternGenerator. I don't want to make
IntlRelativeDateTimeFormatter somehow special here just because this is added later.
I agree that enums and namespaces would be nicer in isolation, indeed. But,
introducing them for only this one formatter would make the API inconsistent with the rest
of ext/intl.

For the second argument of ureldatefmt_open(): the initial implementation will pass NULL,
so ICU uses the default number formatter for the selected locale. This is intended.
Exposing a custom NumberFormatter is possible future scope, but it needs extra care
because ICU adopts ownership of the supplied UNumberFormat, so PHP would need to
clone the underlying formatter before passing it to ICU.

Well I guess you have time until next release to try out the value of this.

What do you think?

Cheers,
Weilin

Hi

For enums and namespaces: I kept class constants and the global Intl*
class name to stay
consistent with the existing ext/intl API, such as IntlDateFormatter,
IntlListFormatter
IntlNumberRangeFormatter, and IntlDatePatternGenerator. I don't want to
make
IntlRelativeDateTimeFormatter somehow special here just because this is
added later.
I agree that enums and namespaces would be nicer in isolation,
indeed. But, 
introducing them for only this one formatter would make the API
inconsistent with the rest
of ext/intl.

As I had mentioned in the voting thread, I think there is a reasonable
middleground to be achieved here:

 final class IntlRelativeDateTimeFormatter
 {
     public function __construct(
         ?string $locale = null,
         IntlRelativeDateTimeFormatterStyle $style = 

IntlRelativeDateTimeFormatterStyle::Long,
IntlRelativeDateTimeFormatterCapitalization
$capitalizationContext =
IntlRelativeDateTimeFormatterCapitalization::None,
?NumberFormatter $numberFormatter = null,
) {}

     public function format(int|float $offset, 

IntlRelativeDateTimeFormatterUnit $unit): string|false {}

     public function formatNumeric(int|float $offset, int $unit): 

string|false {}

     public function combineDateAndTime(string $relativeDate, string 

$time): string|false {}

     public function getErrorCode(): int {}

     public function getErrorMessage(): string {}
 }

 enum IntlRelativeDateTimeFormatterStyle {
     case Long;
     case Short;
     case Narrow;
 }

 enum IntlRelativeDateTimeFormatterCapitalization {
     case None;
     case MiddleOfSentence;
     case BeginningOfSentence;
     case UiListAndMenu;
     case Standalone;
 }

 enum IntlRelativeDateTimeFormatterUnit {
     case Year;
     case Quarter;
     case Month;
     case Week;
     case Day;
     case Hour;
     case Minute;
     case Second;
     // ...
 }

The practical change for the users is just moving some separators
around:

 $formatter->format(-1, IntlRelativeDateTimeFormatter::UNIT_DAY)
 // becomes
 $formatter->format(-1, IntlRelativeDateTimeFormatterUnit::Day)

Basically the first component of the constant name moves in front of the
:: (and underscores are replaced by pascal case).

No namespaces are introduced, leaving a clean place for a redesigned
Intl API that doesn't just expose the ICU API as-is, but the main
benefits of using enums are preserved: Better discoverability and
autocompletion in IDEs. Cleaner documentation (it's possible to document
both the enum and the individual cases). Simplified input checks in the
internal implementation (error handling is implicitly provided by the
engine with the Z_PARAM_ENUM specifier).

I believe that users will value the clearer API signatures provided by
enums over the tiny inconsistency of placing the “Unit”, “Style” and
“Capitalization” in front of the :: instead of after - and enums
themselves are already used in PHP’s stdlib (e.g. RoundingMode), so
users have already encountered them.

Best regards
Tim Düsterhus

Hi Tim,

Yes. The clone was necessary because ICU takes ownership
of the UNumberFormat passed to it.

And yes, this would be unexpected. Technically it is possible
for us to fix this in the php side. My idea is to make the internal
ICU formatter refreshed lazily before the next format() or
formatnumeric() call, and by refreshing we observe if there is
updates to the internal retained numberformatter.

I think this is a good catch user-side. Will add this in the RFC
and implementation. Thanks!

Weilin