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?