[prev in list] [next in list] [prev in thread] [next in thread]
List: perl-datetime
Subject: Re: issues with Data::ICal::DateTime and possible DateTime::Set
From: "Flavio S. Glock" <fglock () gmail ! com>
Date: 2017-08-21 18:00:25
Message-ID: CAHMRfDxxN_e9=2HEF626SQYdPxnQYJpMrTGPfbc1fqc2ivaseg () mail ! gmail ! com
[Download RAW message or body]
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 <fglock@gmail.com>:
> 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 <fglock@gmail.com>:
>
>> (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 => $dt19970902T090000_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 => $dt19970902T090000_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:
>>
>> <quote>
>>
>> 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.</quote>
>>
>> I *think* DateTime::Event::ICal is doing the right thing, but the module documentation is not clear:
>>
>> <quote>
>>
>> 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.
>>
>> </quote>
>>
>> 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 <fglock@gmail.com>:
>>
>>> 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 <slrn406268377@gmail.com>:
>>> > 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 <th.j.v.hoesel@me.com>
>>> 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
>>> >
>>>
>>
>>
>>
>
[Attachment #3 (text/html)]
<div dir="ltr">Data::ICal::DateTime 0.82 implements the fix discussed \
below.<div><br></div><div><div><font face="monospace, monospace">0.82 - Mon 21 Aug \
2017</font></div><div><font face="monospace, monospace"> * Set recurrence \
timezone to the same as the 'start' attribute (fix github issue \
#2)</font></div><div><br></div><div class="gmail_extra"><br><div \
class="gmail_quote">2017-08-18 21:10 GMT+02:00 Flavio S. Glock <span dir="ltr"><<a \
href="mailto:fglock@gmail.com" \
target="_blank">fglock@gmail.com</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>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).</div><div><br></div><font face="monospace, monospace">diff --git \
a/lib/Data/ICal/DateTime.pm b/lib/Data/ICal/DateTime.pm<br>index ecf9e2f..ab49e78 \
100644<br>--- a/lib/Data/ICal/DateTime.pm<br>+++ b/lib/Data/ICal/DateTime.pm<br>@@ \
-499,6 +499,7 @@ sub _rule_set {<br> for (@{ $self->property($name) }) \
{<br> my $recur = \
DateTime::Format::ICal->parse_<wbr>recurrence(recurrence => $_->value, \
dtstart => $start);<br> # \
$recur->set_time_zone($_-><wbr>parameters->{TZID}) if \
$_->parameters->{TZID};<br>+ \
$recur->set_time_zone($start-<wbr>>time_zone) if \
!$start->time_zone->is_<wbr>floating;<br> $set = \
$set->union($recur);<br> }<br> # \
$set->set_time_zone($tz);</font><div><div class="gmail-h5"><div><font \
face="monospace, monospace"><br></font><div><div class="gmail_extra"><br><div \
class="gmail_quote">2017-08-18 19:27 GMT+02:00 Flavio S. Glock <span dir="ltr"><<a \
href="mailto:fglock@gmail.com" \
target="_blank">fglock@gmail.com</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div \
class="gmail-m_-3591553219584233613HOEnZb"><div \
class="gmail-m_-3591553219584233613h5"><div dir="ltr"><div class="gmail_quote"><span \
style="font-family:arial,helvetica,sans-serif;color:rgb(0,0,0);white-space:pre-wrap">(forwarding \
to DateTime list because this looks interesting)</span></div><div \
class="gmail_quote"><font color="#000000" face="arial, helvetica, sans-serif"><span \
style="white-space:pre-wrap"><br></span></font><div dir="ltr">I've created this \
test - <a href="https://gist.github.com/fglock/cf1117ad000b41d9e5dbee6fa8b78993" \
target="_blank">https://gist.github.com/fglock<wbr>/cf1117ad000b41d9e5dbee6fa8b78<wbr>993</a><br><br>this \
fails (set time zone implicitly through dtstart):<br><br><font face="monospace, \
monospace"> $a = DateTime::Event::ICal->recur(<br> dtstart \
=> $dt19970902T090000_tz ,<br> freq => 'daily',<br> \
count => 10 )<br> ->intersection( $period_1995_1999 \
);</font><div><br></div><div>this other test works (time zone is set explicitly for \
the whole recurrence):</div><div><br></div><div><div><font face="monospace, \
monospace"> $a = DateTime::Event::ICal->recur(</font></div><div><font \
face="monospace, monospace"> dtstart => $dt19970902T090000_tz \
,</font></div><div><font face="monospace, monospace"> freq => \
'daily',</font></div><div><font face="monospace, monospace"> \
count => 10 )</font></div><div><font face="monospace, monospace"> \
->set_time_zone( "America/New_York" )</font></div><div><font \
face="monospace, monospace"> ->intersection( $period_1995_1999 \
);</font></div></div><div><br></div><div>I believe this is the correct behaviour \
(though it should warn!), because </div><div><a \
href="http://www.ietf.org/rfc/rfc2445.txt" \
target="_blank">http://www.ietf.org/rfc/rfc244<wbr>5.txt</a><br></div><div>says:</div><div><br></div><div><quote></div><div><pre \
style="word-wrap:break-word">Dawson & Stenerson Standards Track \
[Page 117]<br> <br>RFC 2445 iCalendar \
November 1998<br><br><br> The "DTSTART" and "DTEND" property \
pair or "DTSTART" and "DURATION"<br> property pair, specified \
within the iCalendar object defines the<br> first instance of the recurrence. \
When used with a recurrence rule,<br> the "DTSTART" and \
"DTEND" properties MUST be specified in local time<br> and the \
appropriate set of "VTIMEZONE" calendar components MUST be<br> \
included. For detail on the usage of the "VTIMEZONE" calendar<br> \
component, see the "VTIMEZONE" calendar component definition.<font \
color="#000000"><span \
style="white-space:pre-wrap"></quote></span></font></pre><pre \
style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><font face="arial, \
helvetica, sans-serif">I *think* DateTime::Event::ICal is doing the right thing, but \
the module documentation is not clear:</font></pre><pre \
style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><font face="arial, \
helvetica, sans-serif"><quote></font></pre><pre \
style="word-wrap:break-word"><font color="#000000"><span \
style="white-space:pre-wrap"><font face="monospace, monospace">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.</font><font face="arial, \
helvetica, sans-serif"> </font></span></font></pre><div><font face="arial, helvetica, \
sans-serif"></quote></font></div><pre \
style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><font face="arial, \
helvetica, sans-serif">It *could* as well also use the time_zone from \
"dtstart".</font></pre><pre \
style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><font face="arial, \
helvetica, sans-serif">Simon: do you think this explains the problem, and can this \
can be fixed in your module maybe?</font></pre><pre \
style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><font face="arial, \
helvetica, sans-serif">Flávio S. Glock</font><br></pre><pre \
style="color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap"><font face="arial, \
helvetica, sans-serif"><br></font></pre></div></div><div \
class="gmail-m_-3591553219584233613m_1848727811611487409HOEnZb"><div \
class="gmail-m_-3591553219584233613m_1848727811611487409h5"><div \
class="gmail_extra"><br><div class="gmail_quote">2017-08-18 18:28 GMT+02:00 Flavio S. \
Glock <span dir="ltr"><<a href="mailto:fglock@gmail.com" \
target="_blank">fglock@gmail.com</a>></span>:<br><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex">just to confirm, I did a fresh install and I \
get:<br> <br>
t/01.parse_recurring.t ...... 1/18<br>
# Failed test at t/01.parse_recurring.t line 25.<br>
# got: 'floating'<br>
# expected: 'Europe/London'<br>
<br>
I'm investigating a bit<br>
<br>
Flávio<br>
<div class="gmail-m_-3591553219584233613m_1848727811611487409m_-2462350057489538370HOEnZb"><div \
class="gmail-m_-3591553219584233613m_1848727811611487409m_-2462350057489538370h5"><br>
<br>
2017-08-18 17:33 GMT+02:00 Th.J. van Hoesel <<a \
href="mailto:slrn406268377@gmail.com" \
target="_blank">slrn406268377@gmail.com</a>>:<br> > just fiddling along, it \
works if I first install FGLOCK/DateTime-Event-Recurren<wbr>ce-0.16<br> ><br>
>> On 18 Aug 2017, at 15:34, Th.J. van Hoesel <<a \
href="mailto:th.j.v.hoesel@me.com" target="_blank">th.j.v.hoesel@me.com</a>> \
wrote:<br> >><br>
>> Hi Flávio, Hello Simon,<br>
>><br>
>> 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.<br> >><br>
>> 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.<br> >><br>
>> 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.<br> >><br>
>> 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<br> >><br>
>> Theo<br>
>><br>
>> 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<br> ><br>
</div></div></blockquote></div><br></div>
</div></div></div><br></div>
</div></div></blockquote></div><br></div></div></div></div></div></div>
</blockquote></div><br></div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic