date-time icon indicating copy to clipboard operation
date-time copied to clipboard

Add formatting API

Open jiripudil opened this issue 3 years ago • 10 comments

This is my take on the formatting API (#3). It adds the format() method to Local(Date|DateTime|Time) and ZonedDateTime. The method accepts an implementation of the DateTimeFormatter interface.

The API is designed pretty much like parsing, only backwards. The format() method in each date-time class creates an instance of DateTimeFormatContext that holds individual fields of the value. This context is then sent into the formatter's format() method and a formatted string is returned.

The context contains the individual fields, but also exposes the original value as a whole. This gives custom implementations finer control over the specifics of their formatting.

Built-in formatters

This PR provides two elementary implementations of a DateTimeFormatter:

  • NativeFormatter is the go-to implementation for developers used to the native PHP's formatting options and format string alphabet. It delegates to the native DateTime's format() method, making sure that the format string doesn't refer to any fields that are not available on the formatted value.

    $localDate->format(NativeFormatter::of('d.m.Y'));
    // returns '20.06.2022'
    
    $localTime->format(NativeFormatter::of('d.m.Y'));
    // throws exception, format string refers to fields not available on LocalTime
    
  • IntlFormatter works on top of ext-intl. It has several factory methods:

    • ofDate(), ofTime(), and ofDateTime() take a locale and one of the predefined ICU formats for date, time, or both, respectively. Again, it prevents you from formatting a LocalTime with a date-based formatter.

      $localDate->format(IntlFormatter::ofDate('en_US', IntlFormatter::LONG));
      // returns 'June 20, 2022'
      
      $localTime->format(IntlFormatter::ofDate('en_US', IntlFormatter::LONG));
      // throws exception, cannot use a date-based formatter on LocalTime
      
    • ofPattern() gives you the liberty to set your own custom pattern. This uses the ICU SimpleFormat a.k.a. the one Java uses:

      $localDate->format(IntlFormatter::of('en_US', 'MMMM d, y'));
      // returns 'June 20, 2022'
      
      $localTime->format(IntlFormatter::ofDate('en_US', 'MMMM d, y'));
      // throws exception, cannot use a date-based formatter on LocalTime
      
    • I am particularly fond of ofSkeleton() which utilizes the IntlDatePatternGenerator added in PHP 8.1. This method only requires you to specify what kind of data you want in the pattern, and it generates the most fitting pattern for you:

      $localDate->format(IntlFormatter::ofSkeleton('en_US', 'yMMMMd'));
      // returns 'June 20, 2022'
      
      $localTime->format(IntlFormatter::ofSkeleton('en_US', 'yMMMMd'));
      // throws exception, cannot use a date-based formatter on LocalTime
      

Parity with parser

I was shortly considering sharing the same classes for formatting and parsing, like Java does. I decided not to, though.

I've skimmed through the several discussions in ECMAScript's Temporal API proposal and the outcome appears to be that parsing non-standard localized date strings is fragile, undeterministic guesswork and should be left to the application code that knows best what formats it works with. And there's already PatternParser for that.

While we technically could write a strict parser that works on top of DateTime::createFromFormat or IntlDateFormatter::parse, it would add additional difficulties such as splitting the result of these method calls to a set of fields in DateTimeParseResult. We can revisit the idea later if it is desired, and introduce it as a new DateTimeParser implementation.

jiripudil avatar Jun 20 '22 07:06 jiripudil

@jiripudil friendly pring :slightly_smiling_face: , do you plan to keep working on this ?

tigitz avatar Jul 19 '22 09:07 tigitz

Hi, yes, sure, as soon as I'm able :) I was on a few vacations recently and now I need some time to catch up with work stuff first

jiripudil avatar Jul 26 '22 06:07 jiripudil

I had a need for this today. looks like a great PR!

bendavies avatar Oct 12 '22 07:10 bendavies

@BenMorel what do you think about this PR? Maybe some one should help @jiripudil to make it mergeble? I think, custom formatting is one of the things that this lib is really miss.

solodkiy avatar Jul 04 '23 23:07 solodkiy

Admittedly, this doesn't scratch my itch that much anymore, and I keep putting it off because it feels scary to get back into all of it after so much time. But I guess it's one of those things where you just need to start and then it gets done – which I'd like to do because I've already set this off, and I too believe this library and its users could benefit from a formatting API.

I'd like to hear @BenMorel's thoughts on the direction of it too. There are some great suggestions regarding the API in the review comments, such as

Also from a DX perspective it could become daunting to type the Formatter class every time. I'm wondering if a toNativeFormat and toIntlFormat could be considered.

I think that's a discussion that needs to be had before we can move this forward properly.

jiripudil avatar Jul 06 '23 20:07 jiripudil

@jiripudil First of all a big thank you for your PR and sorry for the (huge) delay!

I took the time to review your PR today, this is excellent work, so let's move forward with this API.

I learned from your PR that the format Java Time uses is actually the ICU format, and it's super cool that php-intl already has support for it and that we don't have to re-implement it!

A bit of early feedback:

  • let's target the next version (0.6) that will require PHP 8.1 (I plan to release it before then end of the year)
    • could you please make your PR target the v0.6 branch and rebase it?
    • you can then remove the code that checks for PHP_VERSION_ID < 80100
  • instead of introducing a DateTimeFormatContext with a factory method for each supported date/time class, I'd like to move forward with #67 first, and have each date/time class report directly the fields it supports. This will bring two improvements:
    • we won't need a DateTimeFormatContext at all
    • we should be able to make formatters more generic, and work with more date/time classes, for example YearMonth: instead of determining the date/time classes supported by each symbol, we should be able to map each symbol to a date/time field that's required for formatting!

I'd like to hear @BenMorel's thoughts on the direction of it too. There are some great suggestions regarding the API in the review comments, such as

Also from a DX perspective it could become daunting to type the Formatter class every time. I'm wondering if a toNativeFormat and toIntlFormat could be considered.

That's my point of view as well. format(DateTimeFormatter) may be left as is (but may just become a proxy for $formatter->format($this) if all objects implement a Temporal interface), but adding convenience methods such as formatNative() and formatIntlPattern() will be appreciable. Proposals for namings welcome.

A couple questions:

  • How does $locale affect IntlFormatter::ofPattern()? Should we keep this parameter, or just hardcode an arbitrary value if the locale doesn't affect the output?
  • How do $dateType and $timeType affect an IntlDateFormatter when $pattern is set to a non-empty string? From my early testing it looks like these parameters are simply ignored in this case, but this doesn't seem to be documented.

(It looks like IntlDateFormatter's constructor accepts too many parameters at the same time, and would have benefited from factory methods similar to those you created on IntlFormatter!)

BenMorel avatar Oct 15 '23 19:10 BenMorel

Hi, thanks for the feedback!

let's target the next version (0.6) that will require PHP 8.1

Ok, sure, I'll rebase the PR and update the code accordingly.

instead of introducing a DateTimeFormatContext with a factory method for each supported date/time class, I'd like to move forward with #67 first

That sounds great, looking forward to it!

How does $locale affect IntlFormatter::ofPattern()?

It does greatly, because some patterns need to be localized, such as day-of-week (eeee) or month names (LLLL).

How do $dateType and $timeType affect an IntlDateFormatter when $pattern is set to a non-empty string? From my early testing it looks like these parameters are simply ignored in this case, but this doesn't seem to be documented.

Yep, I couldn't find it documented either, but my understanding is the same as yours: they are used to create the internal formatter and assemble the initial pattern, which then gets overridden by a custom pattern if you provide one (see code).

jiripudil avatar Oct 22 '23 12:10 jiripudil

How does $locale affect IntlFormatter::ofPattern()?

It does greatly, because some patterns need to be localized, such as day-of-week (eeee) or month names (LLLL).

Could we detect if the given pattern contains symbols that need to be localized, and make $locale optional if none are detected?

Yep, I couldn't find it documented either, but my understanding is the same as yours: they are used to create the internal formatter and assemble the initial pattern, which then gets overridden by a custom pattern if you provide one (see code).

Thank you for digging into the php-src code! :slightly_smiling_face:

BenMorel avatar Oct 22 '23 14:10 BenMorel

Could we detect if the given pattern contains symbols that need to be localized, and make $locale optional if none are detected?

Well, then there are locales that use different numeral systems (mainly Eastern Arabic numerals), so pretty much every symbol might be affected by the locale...

jiripudil avatar Oct 22 '23 19:10 jiripudil