lpil / icalendar

🗓️ A small library for reading and writing ICalendar files.
MIT License
103 stars 56 forks source link

[RFC] RRULE Support #10

Closed johnhamelink closed 5 years ago

johnhamelink commented 8 years ago

This PR will provide full support for the RRULE specification within iCalendar.

Todo List:

johnhamelink commented 8 years ago

@lpil ready for reivew 😄

lpil commented 7 years ago

Hi @johnhamelink ! Sorry I let this go stale. Where are with this?

johnhamelink commented 7 years ago

@lpil it's me who's let it go stale, sorry :) I need to handle errors better by using {:error, err} and {:ok, output} for ICalendar.to_ics/1 - I've been super busy recently and haven't had the chance to finish it off.

lpil commented 7 years ago

Ah! OK. No rush. Just wanted to make sure I wasn't getting in your way :)

walter commented 7 years ago

👍 on this, I'm also interested in collaborating on the occurrences work in #5

walter commented 7 years ago

Oh one other note. Thinking that an Ecto type for RRULE might be useful for embeds. Going to experiment, will keep you posted.

johnhamelink commented 7 years ago

@walter if you want to chat in more detail about that I'm on the Elixir Slack channel, would be interesting to see how we could collab.

walter commented 6 years ago

Long overdue update. I've put in some changes as a PR for @johnhamelink on his fork.

I've also extracted my RRULE type here:

https://github.com/walter/icalendar_rrule_type

lpil commented 6 years ago

Seems there's lots of good work here that we could use. Is there much to do before you'd consider this good to merge in?

walter commented 6 years ago

Details on remaining work on this PR for @johnhamelink fork:

https://github.com/johnhamelink/icalendar/pull/1

Relevant bit:

My memory is rusty, but IIRC what remains is changing the api contract for to_ics to return a tuple.

That along with change to from_ics to return tuple means this feature introduces breaking changes to icalendar.

So definitely need to consider how to introduce the breaking changes before merging.

johnhamelink commented 6 years ago

Hey guys! 😄 It's been a very long time now since I made this PR. I'd still love to see proper RRULE support in iCalendar.

We aren't yet using Elixir for the portion of our codebase which uses the iCal spec (it's currently managed in ruby by a fork of the ice_cube library.

If my memory serves me correctly, I stopped short of completing this because I needed to make multiple breaking changes which @walter referred to above. I'll need to review how much of this codebase already has breaking changes as opposed to the breaking changes which are required...

@walter - I'm happy to work with you to help get this merged into a PR (with breaking changes) which could be released as part of a major revision of this library.

lpil commented 6 years ago

I'm very happy to have everything broken and a new major version released :)

archseer commented 6 years ago

Hello folks! I'm interested in finishing up this PR. I took johnhamelink:feature/rrule, merged https://github.com/johnhamelink/icalendar/pull/1 from walter:feature/rrule, then merged lpil:master to get everything up to date. I'll also get the error handling changes in place.

You can see my branch here: https://github.com/polyfox/icalendar/commits/feature/rrule

I've fixed the tests, but added a new failing case for the recurring rule: BYDAY rule can also specify a count prefix, which tells us which occurrence inside the frequency we want. Examples make this much easier to understand:

Monthly on the 1st Friday for ten occurrences:
  RRULE:FREQ=MONTHLY;COUNT=10;BYDAY=1FR

Every other month on the 1st and last Sunday of the month for 10
occurrences:
  RRULE:FREQ=MONTHLY;INTERVAL=2;COUNT=10;BYDAY=1SU,-1SU

Monthly on the second to last Monday of the month for 6 months:
  RRULE:FREQ=MONTHLY;COUNT=6;BYDAY=-2MO

Every 20th Monday of the year, forever:
  RRULE:FREQ=YEARLY;BYDAY=20MO

The parser is currently parsing the days into an array of atoms:

{"BYDAY=MO,TU", %RRULE{by_day: [:monday, :tuesday]}}

Would handling this with a tuple be acceptable? e.g.

{"BYDAY=20MO,TU", %RRULE{by_day: [{20, :monday}, :tuesday]}}
{"BYDAY=-1MO", %RRULE{by_day: [{-1, :monday}]}}
walter commented 6 years ago

Good to see someone take this up again @archSeer. Hopefully we can get it wrapped up. Tuple seems worth trying for BYDAY.

lpil commented 5 years ago

Is this still wanted?

walter commented 5 years ago

Sadly my project that needed it is long dead and I'm don't have any spare cycles to follow up on this.

lpil commented 5 years ago

No problem, I'll close this for now and someone else can open it if desired.