-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
bug: X-WR-TIMEZONE not supported #71
Comments
For choosing way 2, I created this: https://github.com/niccokunzmann/x-wr-timezone My idea is to integrate the @NicoHood @sirdan69 @asimonson1125 @martinm76 Notifying you as you helped in this regard. Also inviting you to collaborate! For reference, a discussion in |
I think the idea is to optionally depend on your new library and if it is available, it will normalize the input ical file? Or maybe you could just add the normalize to the example and its all good, no need to change a single line of this library? |
Hm. My thought would be that Users of this library want it easy and not Test it out and run into a x-wr-timezone calendar later, untested. I would integrate it and use it by default, so that the meaning of x-wr-timezone is supported by default. This means also changing the major Version number. You would be able to opt out, though. It only affects results of no timezone is used in the arguments. What do you think of that? It's probably good to list pro and con of each Option. |
Very interesting. I'll try it out on my work calendar soon and see if there is any difference from the modification I made inline when parsing it afterwards. |
Pro (integrating):
Con (not integrating)
Suggestion: |
@martinm76 thanks for checking! @NicoHood Thanks for the big list. I created #77 so you can have a look. For some reason, I still have problems with the tests - I think, having a few more example calendars would be useful. All tests run when I integrate So: a good example would be a calendar using
If you can provide me with an ics file that does that, would be awesome! At the moment, I cannot create that myself - would take me hours and frozen fingers. |
v1.0.0b supports X-WR-TIMEZONE. Please test it:) Thanks for your support! |
I decided to include the small library. It does not really change much, only if you use at and between without timezone as argument. The tests indicated that no behavior that was tested needed to change. |
Sounds good! I did a quick test on my work calendar and running my script on the original and the one piped though x-wr-tinezone, with ny astimezone calls intact, and the final result was identical. The calendars differed a fair bit, but most of it seemed to be down to the order of the fields. I only checked for a week, but it parsed the entire calendar without issue, so pretty sure it is doing exactly what it should 😀👍🏻🙏🏻 |
Hi Nico, I tried my test case against your latest changes around x-wr-timezone and it works where it used to crash. This was for the case where Confluence-created calendar contained only all-day events. Thanks for all your work on this issue! |
thanks for your feedback! I will close this. It looks like it does about what it should. |
Describe the bug
It seems that several people encounter a behaviour which is unexpected: When X-WR-TIMEZONE is present, Google and other calendars show a events at a different time in different time zones.
To Reproduce
See Issues #53, #59, #61
Expected behavior
The expected behaviour of this module is to handle X-WR-TIMEZONE as these issues clearly show.
Version:
Additional context
Suggested implementation
This varies:
An untested implementation is discussed here.
We're using Polar.sh so you can upvote and help fund this issue. We receive the funding once the issue is completed & confirmed by you. Thank you in advance for helping prioritize & fund our work.
The text was updated successfully, but these errors were encountered: