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

Fix bug with COUNT in conjunction with EXDATE. COUNT should include e… #263

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

lucianholt97
Copy link

…xcluded dates.

Now incrementing counter outside of isExcluded check. Added new test and fixed existing test data

closes #262

…xcluded dates.

Now incrementing counter outside of isExcluded check. Added new test and fixed existing test data
@u01jmg3
Copy link
Owner

u01jmg3 commented Mar 23, 2020

@s0600204: would welcome your input on this

@s0600204
Copy link
Collaborator

Sure:

Personally I'd have thought that if the RRULE states a COUNT of 5, then 5 event instances should be spat out at the end.

However it appears I'm wrong - page 199 of the rfc5545 spec states:

[...] The final recurrence set is generated by gathering all of the start DATE-TIME values generated by any of the specified "RRULE" and "RDATE" properties, and then excluding any start DATE-TIME values specified by "EXDATE" properties. [...]

The PR conforms with the spec, is logically sound, and passes the tests. Go for it.


(The grammar-pedant in me says: within the new comment, replace "also" with "including", and capitalise the first word.)

@u01jmg3 u01jmg3 merged commit 002a3b0 into u01jmg3:master Mar 23, 2020
@u01jmg3 u01jmg3 added this to the v2.x.x milestone Mar 23, 2020
@u01jmg3
Copy link
Owner

u01jmg3 commented Mar 23, 2020

The grammar-pedant in me says: within the new comment, replace "also" with "including", and capitalise the first word.

See c8dbf16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] EXDATE not counted in processRecurrences(), leads to unwanted additional events
3 participants