Hi internals,
I would like to bring out my latest RFC: https://wiki.php.net/rfc/reldateformatter targeting PHP 8.7
I hope you find this interesting :)
Cheers,
Weilin Du <weilindu@php.net>
Hi Weilin Du,
Really interesting RFC. Now that the Duration RFC
https://wiki.php.net/rfc/duration_class has passed and will be released
with PHP8.6
I wonder if you should consider adding its support in some of the proposed
public API ?
Thanks for your proposal,
Ignace
Hi internals,
I would like to bring out my latest RFC:
https://wiki.php.net/rfc/reldateformatter targeting PHP 8.7I hope you find this interesting :)
Cheers,
Weilin Du <weilindu@php.net
Here my two cents,
- I would be in favor of an enum instead for the constants.
- I would be in favor of an appropriate namespace to avoid conflicts.
- It seems there are missing parts, e.g. how do you plan (or do you
plan to) express the 2nd argument of ureldateformat_open().
There is probably more to it, just surface level first impressions.
Cheers.
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.
What do you think?
Cheers,
Weilin
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 SundayIf 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 David,
I tried this as an experimental feature. The implementation is way more smoother than I
expected. As the numberFormatter class in PHP side is somehow very suitable for this.
The proposed constructor is now:
public function __construct(
?string $locale = null,
int $style = self::STYLE_LONG,
int $capitalizationContext = self::CAPITALIZATION_NONE,
?NumberFormatter $numberFormatter = null,
) {}
Also, considering enums and namespaces. Tim raised this suggestion to me before. So I'd
like to put this in the future scope of this RFC. A broader ext/intl modernization RFC could
revisit namespaces and PHP enums consistently across intl classes. This is not we are
supposed to do in this specific RFC imho. However, the idea is reasonable.
Thanks for the feedback!
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,
I've applied your suggestions to the latest version of the RFC.
https://wiki.php.net/rfc/reldateformatter
> No, I hadn't taken another look at the RFC yet. I also made it clear
> that I had not given it much thought (due to time constraints). But my
> schedule should not be a blocker for RFCs to proceed.
No I think your idea is correct. So it's not your schedule blocking it. I
am happy to make the RFC looks better at the cost of restarting the process
and wait for a few more weeks for this stuff to land.
Please check the RFC if you have time. If no further suggestions I will announce
the two-week cool-down period after 8.6 branch out. That is 22 Sept. And then
I will start the intent to vote again.
Note that I haven't had time to re-implement the whole enum stuff based on
the new version of the RFC. So please ignore the implementation recently.
Thank you.
Weilin Du
Hi
Please check the RFC if you have time. If no further suggestions I will
announce
the two-week cool-down period after 8.6 branch out. That is 22 Sept.
And then
I will start the intent to vote again.
Thank you. I have read the RFC once more and it's much better now. The
remaining clunkiness of the API can reasonably justified by
“consistency” (e.g. the |false return types).
I have just one request for another example:
If a custom NumberFormatter was passed to the constructor, ICU uses it
to format the numeric offset.
Can you include an example that showcases how different NumberFormatter
configurations affect the output?
And don't forget to reset the vote by retitling it.
Best regards
Tim Düsterhus
Hi,
Thank you. I've added the example in the latest version of the RFC.
https://wiki.php.net/rfc/reldateformatter
Now as planned, I am announcing the two-week cool down period.
I will send the intent-to-vote email in 6 Oct. The voting will start at
8 Oct if no further suggestion is raised.
Thanks,
Weilin Du
Hi
Thank you. I've added the example in the latest version of the RFC.
https://wiki.php.net/rfc/reldateformatterNow as planned, I am announcing the two-week cool down period.
I will send the intent-to-vote email in 6 Oct. The voting will start at
8 Oct if no further suggestion is raised.
No further comments from my side. Thank you!
Best regards
Tim Düsterhus
Hi everyone,
I am posting this as an "intent to vote" email to announce that
the voting process of this RFC will be opened at 15 Sept if no
further opinions/objections emerge.
RFC: https://wiki.php.net/rfc/reldateformatter
I hope you love this RFC :)
Cheers,
Weilin Du
Hi
I am posting this as an "intent to vote" email to announce that
the voting process of this RFC will be opened at 15 Sept if no
further opinions/objections emerge.
Similarly to the PREG_THROW_ON_ERROR one, I didn't yet get around to
this one after my vacation. I've given it a very quick read, but not
much thought. One point for now:
The RFC specifies that the given ICU UNumberFormat is cloned internally.
Does that mean if I pass in a NumberFormatter instance and later
reconfigure using ->setAttribute(), the changes will not be applied to
IntlRelativeDateTimeFormatter? I believe this would be very unexpected
behavior.
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