merged branch fabpot/timezones (PR twigphp#876)

This PR was merged into the master branch. Commits ------- f99504a changed the date filter behavior to always apply the default timezone, except if false is passed as the timezone d3b0cbd merged branch jmikola/patch-1 (PR twigphp#845) a6a1ef0 Avoid setting timezones on DateIntervals Discussion ---------- changed the date filter behavior to always apply the default timezone, except if false is passed as the timezone This PR is based on twigphp#845, and addresses issue twigphp#778. Right now, the timezone management when using the date filter is counter-intuitive. When you pass a DateTime object, the default timezone is not applied, but in all other cases, it is. This PR makes the behavior more consistent by always applying the default timezone (which is probably what users want most of the time), and this can be disabled by explicitly passing ``false`` as the timezone value. This is a BC break but I think that not many consciously rely on the current behavior. --------------------------------------------------------------------------- by fabpot at 2012-10-30T15:49:13Z @jmikola Can you have a look at this PR and tell me your opinion about this change? --------------------------------------------------------------------------- by jmikola at 2012-11-02T20:17:30Z @fabpot: Will take a look at this tonight. I missed the notifications as I was offline for a few days. --------------------------------------------------------------------------- by jmikola at 2012-11-03T04:09:54Z I read through all of the issues and PR's and this looks reasonable. Are we missing a test case for passing in a DateTime object with `false` for the timezone? I do see one example where we render the current date using the `'e'` format string and provide `false` for the timezone, but that's it. --------------------------------------------------------------------------- by fabpot at 2012-11-03T07:09:08Z ok, I've added an additional test just to be sure.
fabpot · Nov 3, 2012 · 96b3ce6 · 96b3ce6
2 parents 0879472 + f99504a
commit 96b3ce6
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 35 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,6 @@
 * 1.11.0 (2012-XX-XX)
 
+ * changed the date filter behavior to always apply the default timezone, except if false is passed as the timezone
  * fixed bitwise operator precedences
  * added the template_from_string function
  * fixed default timezone usage for the date function

diff --git a/doc/filters/date.rst b/doc/filters/date.rst
@@ -10,6 +10,9 @@
 .. versionadded:: 1.6.1
     The default timezone support has been added in Twig 1.6.1.
 
+.. versionadded:: 1.11.0
+    The introduction of the false value for the timezone was introduced in Twig 1.11.0
+
 The ``date`` filter formats a date to a given format:
 
 .. code-block:: jinja
@@ -24,17 +27,20 @@ instance, to display the current date, filter the word "now":
 
     {{ "now"|date("m/d/Y") }}
 
-To escape words and characters in the date format use ``\\`` in front of each character:
+To escape words and characters in the date format use ``\\`` in front of each
+character:
 
 .. code-block:: jinja
 
     {{ post.published_at|date("F jS \\a\\t g:ia") }}
 
-You can also specify a timezone:
+If the value passed to the ``date`` filter is ``null``, it will return the
+current date by default. If an empty string is desired instead of the current
+date, use a ternary operator:
 
 .. code-block:: jinja
 
-    {{ post.published_at|date("m/d/Y", "Europe/Paris") }}
+    {{ post.published_at is empty ? "" : post.published_at|date("m/d/Y") }}
 
 If no format is provided, Twig will use the default one: ``F j, Y H:i``. This
 default can be easily changed by calling the ``setDateFormat()`` method on the
@@ -46,20 +52,30 @@ dates and the second one is the default format for date intervals:
     $twig = new Twig_Environment($loader);
     $twig->getExtension('core')->setDateFormat('d/m/Y', '%d days');
 
-The default timezone can also be set globally by calling ``setTimezone()``:
+Timezone
+--------
 
-.. code-block:: php
+By default, the date is displayed by applying the default timezone (the one
+specified in php.ini or declared in Twig -- see below), but you can override
+it by explicitly specifying a timezone:
 
-    $twig = new Twig_Environment($loader);
-    $twig->getExtension('core')->setTimezone('Europe/Paris');
+.. code-block:: jinja
 
-If the value passed to the ``date`` filter is ``null``, it will return the
-current date by default. If an empty string is desired instead of the current
-date, use a ternary operator:
+    {{ post.published_at|date("m/d/Y", "Europe/Paris") }}
+
+If the date is already a DateTime object, and if you want to keep its current
+timezone, pass ``false`` as the timezone value:
 
 .. code-block:: jinja
 
-    {{ post.published_at is empty ? "" : post.published_at|date("m/d/Y") }}
+    {{ post.published_at|date("m/d/Y", false) }}
+
+The default timezone can also be set globally by calling ``setTimezone()``:
+
+.. code-block:: php
+
+    $twig = new Twig_Environment($loader);
+    $twig->getExtension('core')->setTimezone('Europe/Paris');
 
 .. _`strtotime`:    http://www.php.net/strtotime
 .. _`DateTime`:     http://www.php.net/DateTime

diff --git a/lib/Twig/Extension/Core.php b/lib/Twig/Extension/Core.php
@@ -393,12 +393,7 @@ 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 (null !== $timezone) {
-            $date = clone $date;
-            $date->setTimezone($timezone instanceof DateTimeZone ? $timezone : new DateTimeZone($timezone));
-        }
-
+    if ($date instanceof DateInterval) {
         return $date->format($format);
     }
 
@@ -420,12 +415,7 @@ function twig_date_format_filter(Twig_Environment $env, $date, $format = null, $
  */
 function twig_date_modify_filter(Twig_Environment $env, $date, $modifier)
 {
-    if ($date instanceof DateTime) {
-        $date = clone $date;
-    } else {
-        $date = twig_date_converter($env, $date);
-    }
-
+    $date = twig_date_converter($env, $date, false);
     $date->modify($modifier);
 
     return $date;
@@ -449,28 +439,32 @@ function twig_date_modify_filter(Twig_Environment $env, $date, $modifier)
 function twig_date_converter(Twig_Environment $env, $date = null, $timezone = null)
 {
     // determine the timezone
-    if (null === $timezone) {
-        $timezone = $env->getExtension('core')->getTimezone();
+    if (!$timezone) {
+        $defaultTimezone = $env->getExtension('core')->getTimezone();
     } elseif (!$timezone instanceof DateTimeZone) {
-        $timezone = new DateTimeZone($timezone);
+        $defaultTimezone = new DateTimeZone($timezone);
+    } else {
+        $defaultTimezone = $timezone;
     }
 
     if ($date instanceof DateTime) {
         $date = clone $date;
-        $date->setTimezone($timezone);
+        if (false !== $timezone) {
+            $date->setTimezone($defaultTimezone);
+        }
 
         return $date;
     }
 
     $asString = (string) $date;
     if (ctype_digit($asString) || (!empty($asString) && '-' === $asString[0] && ctype_digit(substr($asString, 1)))) {
         $date = new DateTime('@'.$date);
-        $date->setTimezone($timezone);
+        $date->setTimezone($defaultTimezone);
 
         return $date;
     }
 
-    return new DateTime($date, $timezone);
+    return new DateTime($date, $defaultTimezone);
 }
 
 /**

diff --git a/test/Twig/Tests/Fixtures/filters/date.test b/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') }}
@@ -20,8 +21,9 @@
 {{ date5|date('d/m/Y') }}
 {{ date6|date('d/m/Y H:i:s P', 'Europe/Paris') }}
 {{ date6|date('d/m/Y H:i:s P', 'Asia/Hong_Kong') }}
+{{ date6|date('d/m/Y H:i:s P', false) }}
 {{ date6|date('e', 'Europe/Paris') }}
-{{ date6|date('e') }}
+{{ date6|date('e', false) }}
 --DATA--
 date_default_timezone_set('UTC');
 return array(
@@ -31,6 +33,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 +46,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
@@ -52,5 +56,6 @@ December 30, 1969 23:59
 30/12/1969
 04/10/2010 19:45:00 +02:00
 05/10/2010 01:45:00 +08:00
+04/10/2010 13:45:00 -04:00
 Europe/Paris
-America/New_York
+America/New_York
diff --git a/test/Twig/Tests/Fixtures/filters/date_interval.test b/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