Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Format: new functions for readable date-time conversion #1758

Merged
merged 2 commits into from
May 17, 2024
Merged

Format: new functions for readable date-time conversion #1758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4cb1f78
Format: new functions for readable date-time conversion
yookoala Nov 20, 2023
b6769e6
Fix typo in FormatTest
yookoala Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/Services/Format.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,41 @@ public static function dateTime($dateString, $format = false)
/**
* Formats a YYYY-MM-DD date as a readable string with month names.
*
* @param DateTime|string $dateString The date string to format.
* @param string $pattern (Optional) The pattern string for Unicode formatting suppored by
* IntlDateFormatter::setPattern().
*
* See: https://unicode-org.github.io/icu/userguide/format_parse/datetime/
* Default: 'MMM d, Y'
*
* @return string The formatted date string.
*/
public static function dateIntlReadable($dateString, $pattern = 'MMM d, yyyy'): string
{
if (empty($dateString)) {
return '';
}
if (!static::$intlFormatterAvailable) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can handle this with an exception, because currently there will be lots of installations that don't have the PHP intl library (since it was added as a requirement in recent versions), so needs to fallback to regular date formatting. I wonder about a third parameter, called "fallbackPattern" that uses a plain date() format for what to fallback to. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strftime() is set to be deprecated in PHP 8.1 and will probably be removed in PHP 9. To work with international dates, I think intl extension is probably the only way forward. Besides, it is more sensible for user to find out there's a missing dependency by exception than the date silently not working.

One way to work-around the hard dependency-change is to add symfony/polyfill-intl-icu as a project requirement. That way if intl is not installed to the system, the polyfill will provide an IntlDateFormatter implementation to keep everything running.

Copy link
Member

@SKuipers SKuipers Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agreed that intl is the way forward. For the fallback, I was thinking perhaps a plain, non-internationalised Y-m-d using the date method, as a simple way to ensure that the date will still display even if the intl method isn't available, and avoid adding an exception. I know there are installations that don't have intl, so we can't rely on it 100%, but I think it's okay to not have translated strings in this case, as long as the date displays in a basic form. Most dates in Gibbon are already in the basic form, so the readable method will be an added bonus for systems with intl, for now.

Copy link
Member Author

@yookoala yookoala Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SKuipers: The major problem I can see is that both dateReadable() and dateTimeReadable() accept a $pattern for user to specify the localized date / time format. And some usage of these 2 functions do specify the pattern.

Unfortunately, the formatting string used by IntlDateFormatter, as specified in ICU standard by Unicode Consortium, is not compatible with that of PHP's own date() or DateTimeInterface::format().

That means if the function user specify the localized date with ICU format, it is not compatible with date() and thus cannot be used with date() as a fallback.

So without intl extension, I see 3 behaviours that might work:

  1. the new formatting function negate the provided $pattern and returns date / time string of fixed format; or
  2. provide compatible IntlDateFormatter implementation by polyfill library; or
  3. raise an exception.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Koala, 1 is what I was thinking of. I realize they're not compatible, which is why I was thinking of a fallback parameter as the third param. For 2, I don't see that it's worth it to add a polyfill for such a small piece of functionality when we're moving towards all systems having intl. For 3, we have to be backwards compatible, which leads back to 1. If dateIntlReadable had a simple date('Y-m-d') fallback when it's not possible to display an internationalised string, I think that's perfectly fine, as the important data is still displayed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So something like this?

    public static function dateIntlReadable($dateString, $pattern = 'MMM d, yyyy', $fallbackPattern = 'M d, Y'): string {
        ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this eases the library install transition, the code would be quite painful to maintain in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Yes, I realize it's a bit more cumbersome in the meantime, but important to maintain backwards compatibility, as we have thousands of installs out there, and php extension requirements are some of the harder changes for IT personnel to make, so we have to ease things in gently and keep everything backwards compatible.

Copy link
Member Author

@yookoala yookoala Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the stable way forward would be:

  1. Have the dateIntlReadable() and dateTimeIntlReadable() with the fallback string variable with comparable DateTime formatting string.
  2. If intl is not installed, fallback to use DateTime() and $fallbackPattern to format the date in English only.

Correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sounds good. Thanks for moving this one forward! 😄

throw new \Exception('IntlDateFormatter not available.');
}
$formatter = new \IntlDateFormatter(
static::$settings['code'],
\IntlDateFormatter::FULL,
\IntlDateFormatter::FULL,
null,
null,
$pattern,
);
return mb_convert_case(
$formatter->format(static::createDateTime($dateString)),
MB_CASE_TITLE,
);
}

/**
* Formats a YYYY-MM-DD date as a readable string with month names.
*
* @deprecated v27.0.00 Use dateIntlReadable() instead.
* @param DateTime|string $dateString
* @return string
*/
Expand All @@ -140,6 +175,24 @@ public static function dateReadable($dateString, $format = '%b %e, %Y')
/**
* Formats a YYYY-MM-DD date as a readable string with month names and times.
*
* @param DateTime|string $dateString The date string to format.
* @param string $pattern (Optional) The pattern string for Unicode formatting suppored by
* IntlDateFormatter::setPattern().
*
* See: https://unicode-org.github.io/icu/userguide/format_parse/datetime/
* Default: 'MMM d, Y'
*
* @return string The formatted date string.
*/
public static function dateTimeIntlReadable($dateString, $pattern = 'MMM d, yyyy HH:mm'): string
{
return static::dateIntlReadable($dateString, $pattern);
}

/**
* Formats a YYYY-MM-DD date as a readable string with month names and times.
*
* @deprecated v27.0.00 Use dateTimeIntlReadable() instead.
* @param DateTime|string $dateString
* @return string
*/
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/Services/FormatTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ public function testFormatsDateTimeWithFormat()
public function testFormatsReadableDates()
{
$this->assertEquals('May 18, 2018', Format::dateReadable('2018-05-18'));
$this->assertEquals('May 18, 2018', Format::dateIntlReadable('2018-05-18'));
$this->assertEquals('May 18, 2018 13:24', Format::dateTimeReadable('2018-05-18 13:24'));
$this->assertEquals('May 18, 2018 13:24', Format::dateTimeIntlReadable('2018-05-18 13:24'));
}

public function testFormatsDateRanges()
Expand Down