Bug fix: IntlExtension used dateFormatterPrototype's time type instead of zone #6
Conversation
…th its time type Looks like a copy-paste bug (the previous line uses `getTimeType()` correctly.
|
Actually, more work needs to be done here! First, as I point out in twigphp/Twig#3568, What's more, |
| @@ -307,7 +314,7 @@ public function formatTime(Environment $env, $date, ?string $timeFormat = 'mediu | |||
| return $this->formatDateTime($env, $date, 'none', $timeFormat, $pattern, $timezone, $calendar, $locale); | |||
| } | |||
|
|
|||
| private function createDateFormatter(?string $locale, ?string $dateFormat, ?string $timeFormat, string $pattern, \DateTimeZone $timezone, string $calendar): \IntlDateFormatter | |||
| private function createDateFormatter(?string $locale, ?string $dateFormat, ?string $timeFormat, string $pattern, ?\DateTimeZone $timezone, string $calendar): \IntlDateFormatter | |||
There was a problem hiding this comment.
this change breaks the computation of $hash without a dateFormatterPrototype as it calls getName() on null
There was a problem hiding this comment.
You're right, I overlooked that.
if ($this->dateFormatterPrototype), then that case is already handled.
How about calculating the hash with '...' . ($timezone ?: '') . '...' to handle the last case?
There was a problem hiding this comment.
well, the issue is also that you won't pass a $timezone to the IntlDateFormatter constructor right now, which changes the behavior compared to today (and may break things for the vast majority of users not using the prototype, in an effort to make the prototype respect its timezone)
There was a problem hiding this comment.
I force-pushed a fix for this
|
I've just stumbled upon this PR as this repository is not monitored (it's a read-only subtree split of the main twigphp/twig repository) which is where development happens. Feel free to reopen there. Thank you. |
See twigphp/intl-extra#6 for more details
…totype (drjayvee) This PR was squashed before being merged into the 3.x branch. Discussion ---------- Fix IntlExtension::formatDateTime use of date formatter prototype See twigphp/intl-extra#6 for more details Commits ------- c75762c Fix IntlExtension::formatDateTime use of date formatter prototype
…totype (drjayvee) This PR was squashed before being merged into the 3.x branch. Discussion ---------- Fix IntlExtension::formatDateTime use of date formatter prototype See #6 for more details Commits ------- c75762c3 Fix IntlExtension::formatDateTime use of date formatter prototype
Looks like a copy-paste bug (the previous line uses
getTimeType()correctly.