Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified behavior to treat "floating" events found in a calendar with… #7

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

franc6
Copy link
Contributor

@franc6 franc6 commented Apr 4, 2022

… X-WR-TIMEZONE as if they are in the time zone specified

The code was crashing because of the check in is_UTC not checking if a
timezone existed or not. If there was no timezone, then the code
crashed. This matches the case of a "floating" event, which is normally
interpreted as being in the current time zone.

This change treats these "floating" events as if they are in the
timezone specified by X-WR-TIMEZONE, and not as "floating" events.
Based on u01jmg3/ics-parser#245 and
https://blog.jonudell.net/2011/10/17/x-wr-timezone-considered-harmful/,
and a calendar a user sent me, I think this is what was intended by the
calendars' authors. It's hard to say for sure, though, since I've been
unable to find any definitive documentation, only educated guesses.

Note: New tests were added, and the existing tests still pass with
these changes.

… X-WR-TIMEZONE as if they are in the time zone specified

The code was crashing because of the check in is_UTC not checking if a
timezone existed or not.  If there was no timezone, then the code
crashed.  This matches the case of a "floating" event, which is normally
interpreted as being in the current time zone.

This change treats these "floating" events as if they are in the
timezone specified by X-WR-TIMEZONE, and not as "floating" events.
Based on u01jmg3/ics-parser#245 and
https://blog.jonudell.net/2011/10/17/x-wr-timezone-considered-harmful/,
and a calendar a user sent me, I think this is what was intended by the
calendars' authors.  It's hard to say for sure, though, since I've been
unable to find any definitive documentation, only educated guesses.
@niccokunzmann
Copy link
Owner

Hi, thanks for taking the time to write all the tests, too! This looks like a well-tested PR to me.

@niccokunzmann niccokunzmann merged commit 564c1ae into niccokunzmann:master Apr 4, 2022
niccokunzmann added a commit that referenced this pull request Apr 4, 2022
@niccokunzmann
Copy link
Owner

I released version v0.0.5 which includes this fix.

@franc6 franc6 deleted the no_tz_floating branch April 5, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants