Re: issues with Data::ICal::DateTime and possible DateTime::Set

[Flavio S. Glock]

Data::ICal::DateTime 0.82 implements the fix discussed below.

0.82 - Mon 21 Aug 2017
* Set recurrence timezone to the same as the 'start' attribute (fix
github issue #2)


2017-08-18 21:10 GMT+02:00 Flavio S. Glock :

> This patch fixes the problem - but as I explained in the previous mail,
> I'm not sure if it is the right thing to do (the alternative is that
> DateTime::Event::ICal didn't ignore the time_zone in dtstart).
>
> diff --git a/lib/Data/ICal/DateTime.pm b/lib/Data/ICal/DateTime.pm
> index ecf9e2f..ab49e78 100644
> --- a/lib/Data/ICal/DateTime.pm
> +++ b/lib/Data/ICal/DateTime.pm
> @@ -499,6 +499,7 @@ sub _rule_set {
>  for (@{ $self->property($name) }) {
>  my $recur   = DateTime::Format::ICal->parse_recurrence(recurrence
> => $_->value, dtstart => $start);
>  # $recur->set_time_zone($_->parameters->{TZID}) if
> $_->parameters->{TZID};
> +$recur->set_time_zone($start->time_zone) if
> !$start->time_zone->is_floating;
>  $set = $set->union($recur);
>  }
>  # $set->set_time_zone($tz);
>
>
> 2017-08-18 19:27 GMT+02:00 Flavio S. Glock :
>
>> (forwarding to DateTime list because this looks interesting)
>>
>> I've created this test - https://gist.github.com/fglock
>> /cf1117ad000b41d9e5dbee6fa8b78993
>>
>> this fails (set time zone implicitly through dtstart):
>>
>> $a = DateTime::Event::ICal->recur(
>> dtstart => $dt19970902T09_tz ,
>> freq => 'daily',
>> count => 10 )
>> ->intersection( $period_1995_1999 );
>>
>> this other test works (time zone is set explicitly for the whole
>> recurrence):
>>
>> $a = DateTime::Event::ICal->recur(
>> dtstart => $dt19970902T09_tz ,
>> freq => 'daily',
>> count => 10 )
>> ->set_time_zone( "America/New_York" )
>> ->intersection( $period_1995_1999 );
>>
>> I believe this is the correct behaviour (though it should warn!), because
>> http://www.ietf.org/rfc/rfc2445.txt
>> says:
>>
>> 
>>
>> Dawson & Stenerson  Standards Track   [Page 117]
>> 
>> RFC 2445   iCalendar   November 1998
>>
>>
>>The "DTSTART" and "DTEND" property pair or "DTSTART" and "DURATION"
>>property pair, specified within the iCalendar object defines the
>>first instance of the recurrence. When used with a recurrence rule,
>>the "DTSTART" and "DTEND" properties MUST be specified in local time
>>and the appropriate set of "VTIMEZONE" calendar components MUST be
>>included. For detail on the usage of the "VTIMEZONE" calendar
>>component, see the "VTIMEZONE" calendar component definition.
>>
>> I *think* DateTime::Event::ICal is doing the right thing, but the module 
>> documentation is not clear:
>>
>> 
>>
>> This method takes parameters which correspond to the rule parts
>> specified in section 4.3.10 of RFC 2445.  Rather than rewrite that RFC
>> here, you are encouraged to read that first if you want to understand
>> what all these parameters represent.
>>
>> 
>>
>> It *could* as well also use the time_zone from "dtstart".
>>
>> Simon: do you think this explains the problem, and can this can be fixed in 
>> your module maybe?
>>
>> Flávio S. Glock
>>
>>
>>
>> 2017-08-18 18:28 GMT+02:00 Flavio S. Glock :
>>
>>> just to confirm, I did a fresh install and I get:
>>>
>>> t/01.parse_recurring.t .. 1/18
>>> #   Failed test at t/01.parse_recurring.t line 25.
>>> #  got: 'floating'
>>> # expected: 'Europe/London'
>>>
>>> I'm investigating a bit
>>>
>>> Flávio
>>>
>>>
>>> 2017-08-18 17:33 GMT+02:00 Th.J. van Hoesel :
>>> > just fiddling along, it works if I first install
>>> FGLOCK/DateTime-Event-Recurrence-0.16
>>> >
>>> >> On 18 Aug 2017, at 15:34, Th.J. van Hoesel 
>>> wrote:
>>> >>
>>> >> Hi Flávio, Hello Simon,
>>> >>
>>> >> i've some issues with installing Data::Ical::DateTime and I can not
>>> exactly say who is in the right or who is in the wrong, that would require
>>> more investigation.
>>> >>
>>> >> Data::Ical::DateTime is depending on DateTime::Event::Recurrence,
>>> with is dependent on DateTime::Set. When that was changed back in november
>>> 2015, that is when Data::ICal::DateTime started failing on CPAN testers.
>>> >>
>>> >> Maybe one does a wrong call and should actually had expected to get
>>> "floating" timezones, maybe one does expected that his timezone from the
>>> .ics test file would be honoured.
>>> >>
>>> >> Could you please be so kind and have a look, you probably do
>>> understand the problem-space much better than I do. Would I have
>>> understood, I probably would had sent a bug fix
>>> >>
>>> >> Theo
>>> >>
>>> >> PS. It is part of the "Act-out-of-the-Box" install script, and it
>>> just looks ugly to do a force install, but it will do the job for now
>>> >
>>>
>>
>>
>>
>

Re: issues with Data::ICal::DateTime and possible DateTime::Set

[Flavio S. Glock]

This patch fixes the problem - but as I explained in the previous mail, I'm
not sure if it is the right thing to do (the alternative is that
DateTime::Event::ICal didn't ignore the time_zone in dtstart).

diff --git a/lib/Data/ICal/DateTime.pm b/lib/Data/ICal/DateTime.pm
index ecf9e2f..ab49e78 100644
--- a/lib/Data/ICal/DateTime.pm
+++ b/lib/Data/ICal/DateTime.pm
@@ -499,6 +499,7 @@ sub _rule_set {
 for (@{ $self->property($name) }) {
 my $recur   = DateTime::Format::ICal->parse_recurrence(recurrence
=> $_->value, dtstart => $start);
 # $recur->set_time_zone($_->parameters->{TZID}) if
$_->parameters->{TZID};
+$recur->set_time_zone($start->time_zone) if
!$start->time_zone->is_floating;
 $set = $set->union($recur);
 }
 # $set->set_time_zone($tz);


2017-08-18 19:27 GMT+02:00 Flavio S. Glock :

> (forwarding to DateTime list because this looks interesting)
>
> I've created this test - https://gist.github.com/fglock
> /cf1117ad000b41d9e5dbee6fa8b78993
>
> this fails (set time zone implicitly through dtstart):
>
> $a = DateTime::Event::ICal->recur(
> dtstart => $dt19970902T09_tz ,
> freq => 'daily',
> count => 10 )
> ->intersection( $period_1995_1999 );
>
> this other test works (time zone is set explicitly for the whole
> recurrence):
>
> $a = DateTime::Event::ICal->recur(
> dtstart => $dt19970902T09_tz ,
> freq => 'daily',
> count => 10 )
> ->set_time_zone( "America/New_York" )
> ->intersection( $period_1995_1999 );
>
> I believe this is the correct behaviour (though it should warn!), because
> http://www.ietf.org/rfc/rfc2445.txt
> says:
>
> 
>
> Dawson & Stenerson  Standards Track   [Page 117]
> 
> RFC 2445   iCalendar   November 1998
>
>
>The "DTSTART" and "DTEND" property pair or "DTSTART" and "DURATION"
>property pair, specified within the iCalendar object defines the
>first instance of the recurrence. When used with a recurrence rule,
>the "DTSTART" and "DTEND" properties MUST be specified in local time
>and the appropriate set of "VTIMEZONE" calendar components MUST be
>included. For detail on the usage of the "VTIMEZONE" calendar
>component, see the "VTIMEZONE" calendar component definition.
>
> I *think* DateTime::Event::ICal is doing the right thing, but the module 
> documentation is not clear:
>
> 
>
> This method takes parameters which correspond to the rule parts
> specified in section 4.3.10 of RFC 2445.  Rather than rewrite that RFC
> here, you are encouraged to read that first if you want to understand
> what all these parameters represent.
>
> 
>
> It *could* as well also use the time_zone from "dtstart".
>
> Simon: do you think this explains the problem, and can this can be fixed in 
> your module maybe?
>
> Flávio S. Glock
>
>
>
> 2017-08-18 18:28 GMT+02:00 Flavio S. Glock :
>
>> just to confirm, I did a fresh install and I get:
>>
>> t/01.parse_recurring.t .. 1/18
>> #   Failed test at t/01.parse_recurring.t line 25.
>> #  got: 'floating'
>> # expected: 'Europe/London'
>>
>> I'm investigating a bit
>>
>> Flávio
>>
>>
>> 2017-08-18 17:33 GMT+02:00 Th.J. van Hoesel :
>> > just fiddling along, it works if I first install
>> FGLOCK/DateTime-Event-Recurrence-0.16
>> >
>> >> On 18 Aug 2017, at 15:34, Th.J. van Hoesel 
>> wrote:
>> >>
>> >> Hi Flávio, Hello Simon,
>> >>
>> >> i've some issues with installing Data::Ical::DateTime and I can not
>> exactly say who is in the right or who is in the wrong, that would require
>> more investigation.
>> >>
>> >> Data::Ical::DateTime is depending on DateTime::Event::Recurrence, with
>> is dependent on DateTime::Set. When that was changed back in november 2015,
>> that is when Data::ICal::DateTime started failing on CPAN testers.
>> >>
>> >> Maybe one does a wrong call and should actually had expected to get
>> "floating" timezones, maybe one does expected that his timezone from the
>> .ics test file would be honoured.
>> >>
>> >> Could you please be so kind and have a look, you probably do
>> understand the problem-space much better than I do. Would I have
>> understood, I probably would had sent a bug fix
>> >>
>> >> Theo
>> >>
>> >> PS. It is part of the "Act-out-of-the-Box" install script, and it just
>> looks ugly to do a force install, but it will do the job for now
>> >
>>
>
>
>