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

Added support for parsing VEVENT from iCal format #465

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iainbeeston
Copy link

@iainbeeston iainbeeston commented Jan 12, 2019

BACKGROUND

I have an ical file I'd like to parse with ice cube. It has no recurring events, just a series of one off events. Those events are stored in the file as VEVENTs, where each event is a block of lines all describing a single event, starting with BEGIN:VEVENT and ending with END:VEVENT (more detail here).

CURRENT BEHAVIOUR

Icecube can't parse these events. It sees the DTSTART and DTEND inside the VEVENT, but applies them to the schedule.

EXPECTED BEHAVIOUR

I would have expected it to add each VEVENT as it's own recurrence time in the new schedule.

SOLUTION

I've refactored the ical parser such that it acts as a state machine. By default it works as it did before. However, if it sees a BEGIN:VEVENT line it goes into an event parsing mode, where it parses subsequent lines as events. Once it sees an END:VEVENT line, it goes back to the regular mode. When parsing events it adds a new recurrence time every time it sees a DTSTART.

NOTES

  • To keep the code simple, if there are multiple DTSTART lines within an event, it will add a new recurrence time for each of them. A VEVENT with more than one DTSTART is invalid, so this shouldn't happen in practice.
  • VEVENTs can have an end date (ie. DTEND) as well as a start date (DTSTART). I couldn't see a way of adding a duration to a recurrence time, so I've ignored the end dates for now.

@iainbeeston
Copy link
Author

The build is failing because travis is trying to use bundler 2.0 with ruby < 2.3. #459 should fix that

@iainbeeston iainbeeston force-pushed the ical-vevents branch 7 times, most recently from a5a11d7 to 318153e Compare January 16, 2019 08:58
pacso
pacso previously approved these changes Oct 21, 2021
@pacso pacso self-requested a review October 21, 2021 23:54
@pacso pacso dismissed their stale review October 21, 2021 23:54

Will defer final approval to @seejohnrun


parser, attr, occurrences = *send(parser, property, value)

case attr
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes sense to me but what do you think of passing data into the parse_line and parse_vevent_line methods so that we can remove this generic case statement. I think personally it reads a bit confusing to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense to me. Should remove an unnecessary additional case statement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@iainbeeston - are you happy to push an update to this PR?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the long wait, I've refactored this now to get rid of the extra case statement. Please let me know what you think.

@iainbeeston iainbeeston force-pushed the ical-vevents branch 3 times, most recently from d4423a5 to c991f99 Compare June 8, 2022 15:27
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.

3 participants