[Intl] Add `DateIntervalFormatter` #39912

YaFou · 2021-01-20T20:52:49Z

Q	A
Branch?	5.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #29929
License	MIT
Doc PR	TODO

$formatter = new DateIntervalFormatter();
$formatter->formatInterval(new \DateInterval('P1DT5H2M')); // one day, 5 hours and 2 minutes

You can set a precision. The precision cuts the formatted content to a given number of elements:

$formatter = new DateIntervalFormatter();
$formatter->formatInterval(new \DateInterval('P1DT20H2M'), 2); // one day and 20 hours

src/Symfony/Component/Translation/CHANGELOG.md

src/Symfony/Component/Translation/Formatter/DateIntervalFormatter.php

ro0NL · 2021-01-21T08:05:54Z

there's an interesting ICU concept: listPattern
https://github.com/unicode-org/icu/blob/6092942e23785d85de5b40ea2f47768f7fa9345b/icu4c/source/data/locales/en.txt#L2007

relative/duration time formats
https://github.com/unicode-org/icu/blob/6092942e23785d85de5b40ea2f47768f7fa9345b/icu4c/source/data/locales/en.txt#L1441-L1456
https://github.com/unicode-org/icu/blob/6092942e23785d85de5b40ea2f47768f7fa9345b/icu4c/source/data/unit/en.txt#L286

ideally we'd implement this at the ICU message format level 😁 but that's far fetched.

overall, having a generic localized interval formatter in the Intl component seems more useful.

fancyweb · 2021-01-21T08:57:11Z

overall, having a generic localized interval formatter in the Intl component seems more useful.

I agree and we could use it in the Validator component too for #33401

src/Symfony/Component/Intl/Tests/DateIntervalFormatter/DateIntervalFormatterTest.php

YaFou · 2021-06-26T09:03:22Z

Ready for review

HeahDude · 2021-06-26T11:29:26Z

src/Symfony/Bridge/Twig/Extension/IntlExtension.php

+use Twig\Extension\AbstractExtension;
+use Twig\TwigFilter;
+
+class IntlExtension extends AbstractExtension


What about doing this in https://github.com/twigphp/intl-extra/blob/2.x/IntlExtension.php instead?
Currently it may conflict with https://github.com/twigphp/twig-extra-bundle/blob/2.x/Resources/config/intl.php.

I think @HeahDude's idea is a good one. Can you please remove that code from this PR and move it to twig instead?

sstok · 2021-07-04T13:04:12Z

FYI https://carbon.nesbot.com/docs/#api-interval provides a fully localized version of DateInterval with forHumans().

OskarStark · 2021-08-04T20:30:18Z

Open to finish this PR @YaFou ?

YaFou · 2021-08-04T21:12:04Z

Open to finish this PR @YaFou ?

For me, it is finished. I saw comments which suggest to add ICU but I don't think we have to add something to big to Symfony core.

nicolas-grekas

I'm mixed here: I get the value, but what about i18n? Is the language designed to be an implementation detail or should the interface allow declaring it?

In the end, my question ends up as: shouldn't we delegate this to a 3rd party lib, eg Carbon?

nicolas-grekas · 2021-10-18T16:59:06Z

src/Symfony/Bridge/Twig/Extension/IntlExtension.php

+use Twig\Extension\AbstractExtension;
+use Twig\TwigFilter;
+
+class IntlExtension extends AbstractExtension


I think @HeahDude's idea is a good one. Can you please remove that code from this PR and move it to twig instead?

nicolas-grekas · 2021-10-18T17:00:27Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.php

@@ -158,5 +160,8 @@
            ->args([service(ContainerInterface::class)])
            ->tag('container.service_subscriber', ['id' => 'translator'])
            ->tag('kernel.cache_warmer')
+
+        ->set('translation.formatter.date_interval', DateIntervalFormatter::class)


I don't get the need for this service

nicolas-grekas · 2021-10-18T17:01:05Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/intl.php

+return static function (ContainerConfigurator $container) {
+    $container->services()
+        ->set('intl.date_interval_formatter', DateIntervalFormatter::class)
+            ->alias(DateIntervalFormatterInterface::class, 'intl.date_interval_formatter')


alias should be unindented by one

nicolas-grekas · 2021-10-18T17:01:30Z

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

@@ -574,6 +575,10 @@ public function load(array $configs, ContainerBuilder $container)
            'kernel.locale_aware',
            'kernel.reset',
        ]);
+
+        if (class_exists(DateIntervalFormatterInterface::class)) {


interface_exists, since it's not a class but an interface

nicolas-grekas · 2021-10-18T17:01:49Z

src/Symfony/Component/Intl/CHANGELOG.md

+5.4
+---
+
+* Add `DateIntervalFormatter` to humanize a `DateInterval` or a difference between two `DateTimeInterface` objects


missing space at start of line

nicolas-grekas · 2021-10-18T17:03:23Z

src/Symfony/Component/Intl/DateIntervalFormatter/DateIntervalFormatterInterface.php

+    /**
+     * @param \DateInterval|string $interval
+     */
+    public function formatInterval($interval, int $precision = 0): string;


what is $precision supposed to mean? reading the interface should provide a clue

fabpot · 2021-10-30T09:21:37Z

Having something like this in core without i18n is a no-go for me. As soon as we will merge it, people will start asking for it and they will be right.
Let's close for now.

carsonbot added Status: Needs Review Translation Feature labels Jan 20, 2021

carsonbot changed the title ~~[Translation] [WIP] Add DateIntervalFormatter~~ [Translator] [WIP] Add DateIntervalFormatter Jan 20, 2021

YaFou force-pushed the date-interval-formatter branch 2 times, most recently from 0169445 to 5189d2d Compare January 20, 2021 20:54

YaFou mentioned this pull request Jan 20, 2021

[Translation] Add human readable DateInterval #29929

Closed

OskarStark reviewed Jan 21, 2021

View reviewed changes

src/Symfony/Component/Translation/CHANGELOG.md Outdated Show resolved Hide resolved

OskarStark reviewed Jan 21, 2021

View reviewed changes

src/Symfony/Component/Translation/Formatter/DateIntervalFormatter.php Outdated Show resolved Hide resolved

YaFou force-pushed the date-interval-formatter branch from 2888345 to 7f6cbba Compare January 21, 2021 10:04

YaFou requested a review from yceruto as a code owner January 21, 2021 10:04

YaFou force-pushed the date-interval-formatter branch 2 times, most recently from b43f917 to 0ed15bd Compare January 21, 2021 10:08

nicolas-grekas added this to the 5.x milestone Jan 21, 2021

OskarStark reviewed Jan 27, 2021

View reviewed changes

src/Symfony/Component/Intl/Tests/DateIntervalFormatter/DateIntervalFormatterTest.php Show resolved Hide resolved

YaFou changed the title ~~[Translator] [WIP] Add DateIntervalFormatter~~ [Intl] [WIP] Add DateIntervalFormatter Jun 26, 2021

YaFou force-pushed the date-interval-formatter branch 5 times, most recently from 1c09134 to 894cc36 Compare June 26, 2021 09:01

YaFou changed the title ~~[Intl] [WIP] Add DateIntervalFormatter~~ [Intl] Add DateIntervalFormatter Jun 26, 2021

YaFou requested a review from OskarStark June 26, 2021 09:03

[Translation] Add DateIntervalFormatter

86a078a

YaFou force-pushed the date-interval-formatter branch from 894cc36 to 86a078a Compare June 26, 2021 09:05

HeahDude reviewed Jun 26, 2021

View reviewed changes

OskarStark changed the title ~~[Intl] Add DateIntervalFormatter~~ [Intl] Add DateIntervalFormatter Aug 4, 2021

nicolas-grekas reviewed Oct 18, 2021

View reviewed changes

fabpot closed this Oct 30, 2021

Uh oh!

[Intl] Add DateIntervalFormatter #39912

[Intl] Add DateIntervalFormatter #39912

Uh oh!

Conversation

YaFou commented Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ro0NL commented Jan 21, 2021

Uh oh!

fancyweb commented Jan 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

YaFou commented Jun 26, 2021

Uh oh!

HeahDude Jun 26, 2021

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sstok commented Jul 4, 2021

Uh oh!

OskarStark commented Aug 4, 2021

Uh oh!

YaFou commented Aug 4, 2021

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Oct 18, 2021

Choose a reason for hiding this comment

Uh oh!

fabpot commented Oct 30, 2021

Uh oh!

Uh oh!

[Intl] Add `DateIntervalFormatter` #39912

[Intl] Add `DateIntervalFormatter` #39912

YaFou commented Jan 20, 2021 •

edited

Loading

fancyweb commented Jan 21, 2021 •

edited

Loading

nicolas-grekas Oct 18, 2021 •

edited

Loading

nicolas-grekas Oct 18, 2021 •

edited

Loading