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

2017-08-18 Thread 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
>> >
>>
>
>
>


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

2017-08-18 Thread 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
> >
>