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

Fixing EXDATE encoding #11

Closed
wants to merge 2 commits into from
Closed

Fixing EXDATE encoding #11

wants to merge 2 commits into from

Conversation

evert
Copy link

@evert evert commented Sep 23, 2011

required for supporting iCalendar (at least on OS/X 10.6). We're also
now correctly encoding the dates and times as per the iCalendar
specification.

required for supporting iCalendar (at least on OS/X 10.6). We're also
now correctly encoding the dates and times as per the iCalendar
specification.
true // $append
);
} else {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? We only support daily recurrences as the finest grained recurrence interval anyway, so DATE fields should be sufficient. And if some whacky client requires this (is this iCal again?) did you make sure that the generated DATE-TIME exception value has the correct timezone?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm misreading the code, timezone id's ignored within Kronolith_Event anyway, and everything gets converted to GMT. Although I consider this a 'bad thing', the underlying data model also does not support timezones per event, so there's little harm there.

The reason I wanted to support the 'DATETIME', is because fromiCalendar function also supports it. When iCal sends me the exception dates (correctly) for non-all-day events, the time-part simply gets cut off and stored anyway. So by adding support for the DATETIME property also in this way, at least both fromiCalendar and toiCalendar work synchronously.

The last part.. The reason I'm creating 1 EXDATE property for every exception, rather than 1 property with all the exceptions comma-separated is indeed because iCalendar has a parsing bug.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and I just had a second look. Kronolith does in fact correctly support non-all-day recurring events just fine. Maybe you're confused by the fact that Kronolith does not support a higher granularity than per-day recursion, which is correct; but my patch is still needed to support that feature correctly.

@yunosh
Copy link
Member

yunosh commented Oct 18, 2012

I merged the separate EXDATE attributes but left out the DATE-TIME changes because we don't need them at the moment, and the vCard changes because they would break the current SyncML implementation.

@yunosh yunosh closed this Oct 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants