[RFC] When importing events preserve VTIMEZONES #513
Conversation
|
If there's anything that I can test out I'll be happy to :) |
| # print all sub-events | ||
| for sub_event in vevent: | ||
| cal = icalendar.Calendar.from_ical(vevent) | ||
| for sub_event in [item for item in cal.walk() if item.name == 'VEVENT']: |
There was a problem hiding this comment.
You can just have a if item.name == 'VEVENT': continue in the for-loop. This creates a new list for no good reason.
There was a problem hiding this comment.
Using () instead of [] would at least make it a generator comprehension instead of listcomp, and have the same effect of skipping creation of an intermediate list.
There was a problem hiding this comment.
There's still overhead. I don't see a reason to use either list comprs or generator exprs.
| same UID, i.e., one "master" event and (optionally) 1+ RECURRENCE-ID events | ||
| :type vevent: list(str) | ||
| """ | ||
| # TODO re-enable random_uid |
There was a problem hiding this comment.
calling khal import --random-uid somefile.ics would import all events but assign a random uid to each VEVENT, I want to re-enable that functionality.
| if datetime_.tzinfo is None and 'TZID' in item.params and \ | ||
| item.params['TZID'] not in missing_tz: | ||
| logger.warn( | ||
| 'Cannot find timezone `{}` in .ics file, using default timezone. ' |
There was a problem hiding this comment.
I think khal should print the event and ask for the tz
There was a problem hiding this comment.
probably a good idea, not sure if we need to address it with this PR though.
There was a problem hiding this comment.
It might be useful as a separate command, like repair, which might eventually have other features as well.
I agree that it's best left outside the scope of this issue (so we can close this bug faster).
| 'information in {} in {}'.format(prop, href, calendar)) | ||
| logger.warn( | ||
| "{} localized in invalid or incomprehensible timezone `{}` in {}/{}. " | ||
| "This could lead to this event being wronlgy displayed." |
| @@ -37,12 +38,14 @@ | |||
|
|
|||
|
|
|||
| def timefstr(dtime_list, timeformat): | |||
| """converts a time (as a string) to a datetimeobject | |||
There was a problem hiding this comment.
Unrelated change, also why is that API designed like that? Why not take just a time as a string? Why does it delete the item from the list?
There was a problem hiding this comment.
for consistency reason with the other xfstr()s
| :rtype list: | ||
| """ | ||
| cal = icalendar.Calendar.from_ical(ics) | ||
| vevents = [item for item in cal.walk() if item.name == 'VEVENT'] |
There was a problem hiding this comment.
Unnecessary list. Can be again done in a single loop.
geier
force-pushed
the
fix_import_timezone
branch
from
412f444 to
e2842df
Compare
geier
force-pushed
the
fix_import_timezone
branch
2 times, most recently
from
d103d67 to
21adf38
Compare
geier
force-pushed
the
fix_import_timezone
branch
from
21adf38 to
9334546
Compare
geier
force-pushed
the
fix_import_timezone
branch
from
9334546 to
7b360dc
Compare
|
@pimutils/packaging There is a new release of khal (0.8.4), as it fixes a bug in |
|
Updating the port right now. I guess it might get committed during the following week or so. |
|
Couldn't upgrade the port now as it contains py3-specific code. Could you kindly release something like 0.7.5? |
|
Khal 0.8 doesn't support Python 2. |
|
Ok, it appears @geier offered to backport this to 0.7 anyway, so nvm |
|
@untitaker thank you for the info! |
|
The same thing applies for vdirsyncer 0.13 btw, but it has been outputting warnings about Python 2 for a long time now. |
|
BTW feel free to open issues if you want to have a different way to be notified about releases. |
|
Building for Rawhide now; will copy down to stable releases afterwards. As for notifications, a separate issue with a better subject line would be better at least. |
|
Fedora 23 is still using the Python 2 version, so a new 0.7 would be good there. |
|
Regarding notifications, I wish a system that notifies me via email. The mentions thing implemented here works just fine in this case. If the separate issue approach mentioned above works the same way, I'm for it. |
|
Let's continue in pimutils/pimutils.org#5 |
This is an important fix for an issue reported in #498 by @waynew.
The underlying issue can lead to data-loss or rather wrongly displayed events, see draft for release announcement.
@untitaker @hobarrera while this is not entirely finished yet, it would be great if you had the time to look over this already, as I want to get it out as soon as possible.
Todo
sort_keyintokhal/utils.pythat needs to be resolved first, but should be okay, as we keep the original order and when actually inserting it into the db we order it correctlyfixes #498