Twig icon indicating copy to clipboard operation
Twig copied to clipboard

Fix IntlExtension::formatDateTime uses prototype pattern if set

Open drjayvee opened this issue 2 years ago • 4 comments

See https://github.com/twigphp/intl-extra/pull/7 for details

drjayvee avatar Jun 05 '23 11:06 drjayvee

Is there any chance this could get reviewed/merged? @fabpot

We've been having this issue over at FOSSBilling/FOSSBilling for a while, and it would be great to get a fix in place.

admdly avatar Nov 24 '23 17:11 admdly

Hey Adam,

Could you please write a test for this? My other PR (#3844) was blocked until test coverage was added (and rightly so).

It should be really easy to copy one of the tests and adjust it slightly.

drjayvee avatar Nov 27 '23 11:11 drjayvee

Sadly this patch goes and breaks the usage of the IntlDateFormatter as in my testing the options set there are no longer respected. Appling the patch and using format_datetime with the date format set to none still displays a date. Likewise setting the time format to none still shows a time.

So it "fixes" the error by instead breaking it which is a shame as the current problem where format_date and format_datetime both behave identically by only following the set pattern is quite frustrating.

So with the existing twig releases format_date will still display a time if a time format is set. We also can't workaround that by manually specifying a format for a given filter as it's ignored. With this patch that behavior goes away, but seemingly because what's set via the IntlDateFormatter is completely ignored.

BelleNottelling avatar Apr 01 '24 23:04 BelleNottelling

@BelleNottelling I'm afraid I don't quite follow. It's been a while since I've looked at this bug/PR. In fact, my original explanation is from September 2021. It's possible the behavior has changed since then.

Can you show some actual code that breaks only with this PR's code? Looking at the diff, it only affects behavior when

  1. a dateFormatterPrototype is configured, and
  2. neither dateFormat or timeFormat arguments are provided (to either filter)

Likewise setting the time format to none still shows a time.

I don't see how this PR's diff affects formatting in that case.

drjayvee avatar Apr 02 '24 15:04 drjayvee