Implement BYSETPOS support for monthly recurrence rules with BYMONTHDAY#985
Implement BYSETPOS support for monthly recurrence rules with BYMONTHDAY#985mschroeder wants to merge 1 commit into
Conversation
kewisch
left a comment
kewisch
left a comment
There was a problem hiding this comment.
Spectacular, thanks for tackling this Martin. Good to see you're still following :)
Some minor comments if you could take a look
| let day = this._findByMonthDaySetPos(this.last.day + 1); | ||
| let attempts = 0; | ||
|
|
||
| while (!day && attempts++ < 48) { |
There was a problem hiding this comment.
Could you add a comment on why 48 attempts, here and in the last function?
| if (this.has_by_data("BYSETPOS")) { | ||
| data_valid = this._nextMonthlyByMonthDaySetPos(); | ||
| } else { |
There was a problem hiding this comment.
Have you considered cases where we have BYMONTHDAY, BYSETPOS, and other rule parts? I think theoretically a BYDAY could appears as well.
|
The BYMONTHDAY/BYSETPOS coverage is strong, especially the negative and invalid-position cases. One edge worth considering is interaction with |
3 participants
Fixes #960