Skip to content

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 18, 2019

Remove empty needle as failure case, normalise implementations and error messages.

Removed test file for bug 63943 as it was covering if the correct warning message is displayed.
Split non 7-bit ASCII characters test into a separate variation test.

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

There are many unnecessary changes in here -- in most cases it should be sufficient to just drop the check for empty needle and not change anything else.

What's the reason for all the error message changes?

@Girgias
Copy link
Member Author

Girgias commented Aug 18, 2019

The error message change is because strpos() and stripos() both use "Offset not contained in string" as their error message. That's why I've changed the error messages strings for strrpos() and strripos() to mimic their non reverse counter-part.

@Girgias Girgias changed the title Improve strpos function family implementation Improve strpos and strstr function family implementation Aug 18, 2019
@nikic
Copy link
Member

nikic commented Aug 19, 2019

Could you please split the changes to the strrpos/strripos error handling / implementation into a separate PR?

@Girgias
Copy link
Member Author

Girgias commented Aug 26, 2019

Test failures maybe due to being rebased before 0038db2

@Girgias Girgias force-pushed the improve-strpos-family-function branch from 7019f9c to 1953a7e Compare August 26, 2019 11:39
Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Please also add a note to UPGRADING with a list of functions that now accept an empty needle.


Warning: stristr(): Empty needle in %s on line %d
bool(false)
string(11) "Hello World"
Copy link
Member

Choose a reason for hiding this comment

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

You can drop this file. These are no longer error conditions, and the result is already checked in stristr.phpt.

@Girgias
Copy link
Member Author

Girgias commented Aug 26, 2019

Merged in as 6d57848

@Girgias Girgias closed this Aug 26, 2019
@Girgias Girgias deleted the improve-strpos-family-function branch August 26, 2019 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants