merged branch jmikola/patch-1 (PR #845)

This PR was merged into the master branch.

Commits
-------

a6a1ef0 Avoid setting timezones on DateIntervals

Discussion
----------

Avoid setting timezones on DateIntervals

Edit: originally, I opened this to ensure the default Twig timezone was always set on incoming DateTime objects, but I came to realize why that behavior would be undesirable. The idea was prompted by this line from: http://twig.sensiolabs.org/doc/filters/date.html

> The default timezone can also be set globally by calling `setTimezone()`.

Anyway, I did notice an edge case where the code might call `DateInterval::setTimezone()`, so this PR attempts to correct that. Additionally, I added some tests for passing a DateTimeZone object to the `date` filter.

---------------------------------------------------------------------------

by jmikola at 2012-09-22T17:18:56Z

@fabpot: One question came up as I was looking through this a second time: if `twig_date_converter()` receives a DateTime argument, it will override its timezone regardless of the logic path within (either from the `$timezone` argument, Twig default, or the system default).

At first, I thought that it was redundant for `twig_date_format_filter()` not to utilize `twig_date_converter()`, but it looks like that was a careful decision; otherwise, there'd be no way to display DateTimes with their internal timezone. This PR would remove that functionality, so I need to reconsider.

Having said that, there does appear to be a bug in `twig_date_format_filter()` in that it could end up calling a nonexistent `setTimezone()` method on a DateInterval object. Would it be preferable to throw an exception or simply ignore the `$timezone` argument if the `$date` argument is a DateInterval?

---------------------------------------------------------------------------

by fabpot at 2012-10-30T08:56:31Z

I don't know if not changing the timezone on DateTime object was a conscious/careful decision or not. The documentation is not so clear about that, and there is even a bug report about this behavior: #778.

For consistency, I think we can always set the timezone and allow disabling that by passing `false` for the timezone.

Does it sounds good?

lib/Twig/Extension/Core.php

@@ -393,7 +393,11 @@ function twig_date_format_filter(Twig_Environment $env, $date, $format = null, $
         $format = $date instanceof DateInterval ? $formats[1] : $formats[0];
     }
 
-    if ($date instanceof DateInterval || $date instanceof DateTime) {
+    if ($date instanceof DateInterval) {
+        return $date->format($format);
+    }
+
+    if ($date instanceof DateTime) {
         if (null !== $timezone) {
             $date = clone $date;
             $date->setTimezone($timezone instanceof DateTimeZone ? $timezone : new DateTimeZone($timezone));

test/Twig/Tests/Fixtures/filters/date.test

@@ -11,6 +11,7 @@
 {{ date2|date }}
 {{ date2|date('d/m/Y') }}
 {{ date2|date('d/m/Y H:i:s', 'Europe/Paris') }}
+{{ date2|date('d/m/Y H:i:s', timezone1) }}
 {{ date2|date('d/m/Y H:i:s') }}
 {{ date3|date }}
 {{ date3|date('d/m/Y') }}
@@ -31,6 +32,7 @@ return array(
     'date4' => 1286199900,
     'date5' => -86410,
     'date6' => new DateTime('2010-10-04 13:45', new DateTimeZone('America/New_York')),
+    'timezone1' => new DateTimeZone('America/New_York'),
 )
 --EXPECT--
 October 4, 2010 13:45
@@ -43,6 +45,7 @@ UTC
 October 4, 2010 13:45
 04/10/2010
 04/10/2010 15:45:00
+04/10/2010 09:45:00
 04/10/2010 13:45:00
 October 4, 2010 13:45
 04/10/2010

test/Twig/Tests/Fixtures/filters/date_interval.test

@@ -3,14 +3,17 @@
 --CONDITION--
 version_compare(phpversion(), '5.3.0', '>=')
 --TEMPLATE--
-{{ date6|date }}
-{{ date6|date('%d days %h hours') }}
+{{ date1|date }}
+{{ date1|date('%d days %h hours') }}
+{{ date1|date('%d days %h hours', timezone1) }}
 --DATA--
 date_default_timezone_set('UTC');
 return array(
-    'date5' => -86410,
-    'date6' => new DateInterval('P2D'),
+    'date1' => new DateInterval('P2D'),
+    // This should have no effect on DateInterval formatting
+    'timezone1' => new DateTimeZone('America/New_York'),
 )
 --EXPECT--
 2 days
 2 days 0 hours
+2 days 0 hours