adding DatePeriod::getRecurrences method see bug #75113#3888
adding DatePeriod::getRecurrences method see bug #75113#3888nyamsprod wants to merge 5 commits intophp:masterfrom nyamsprod:bugfix/adding-DatePeriod-getRecurrences-method
Conversation
ext/date/php_date.c
Outdated
|
|
||
| dpobj = Z_PHPPERIOD_P(ZEND_THIS); | ||
|
|
||
| if (!dpobj->recurrences) { |
There was a problem hiding this comment.
@derickr seems dpobj->recurrences by default equals 1 instead of NULL maybe we should substract 1 to the value and return if the substracted value equals 0 🤔
ext/date/php_date.c
Outdated
|
|
||
| dpobj = Z_PHPPERIOD_P(ZEND_THIS); | ||
|
|
||
| if (0 == dpobj->recurrences - 1) { |
There was a problem hiding this comment.
That's incorrect. By default, the start date is also included as a recurrence.
If you set the DatePeriod::EXCLUDE_START_DATE option, then this does not happen.
So instead of using - 1, you should be using - dpobj->include_start_date.
It's probably wise to add a test case for this too.
| string(12) "+0-0-1-0-0-0" | ||
| NULL | ||
| int(4) | ||
| NULL No newline at end of file |
There was a problem hiding this comment.
Please make sure the last line has a new line too.
|
Committed and merged after rebasing this to PHP-7.2 |
|
@derickr Why putting a new method into |
|
|
||
| $periodWithRecurrencesWithoutStart = new DatePeriod($start, $interval, $recurrences, DatePeriod::EXCLUDE_START_DATE); | ||
|
|
||
| var_dump($periodWithRecurrences->getRecurrences()); |
There was a problem hiding this comment.
Typo? I assume this should be $periodWithRecurrencesWithoutStart instead.
@derickr here's the PR to add
DatePeriod::getRecurrencesI hope I've follow your guidance correctly If something is missing please let me know.