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

Str\Grapheme\reverse() function #239

Merged
merged 8 commits into from
Oct 18, 2021
Merged

Str\Grapheme\reverse() function #239

merged 8 commits into from
Oct 18, 2021

Conversation

yivi
Copy link
Contributor

@yivi yivi commented Oct 18, 2021

Add Str\Grapheme\reverse() to match Str\reverse() and Str\Byte\reverse()

@yivi yivi changed the title Str Grapheme reverse() function Str\Grapheme\reverse() function Oct 18, 2021
@coveralls
Copy link

coveralls commented Oct 18, 2021

Pull Request Test Coverage Report for Build 1356158078

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0001%) to 99.936%

Totals Coverage Status
Change from base Build 1355021051: 0.0001%
Covered Lines: 3130
Relevant Lines: 3132

💛 - Coveralls

@yivi
Copy link
Contributor Author

yivi commented Oct 18, 2021

Regarding the coverage decrease... I'm afraid I do not yet know how to produce an input that grapheme_strlen would choke on.

Docs (https://www.php.net/manual/en/function.grapheme-strlen.php) say the function returns int|false|null with false and on failure (?) and, with no indication of when null would be returned.

I'm trying to see how to create a string that would be an invalid UTF-8 string to see if we can recover the lost coverage.

@yivi
Copy link
Contributor Author

yivi commented Oct 18, 2021

Ok, got it. Pushing recovered coverage. :)

@azjezz azjezz added this to the 1.9.0 milestone Oct 18, 2021
@azjezz azjezz added hacktoberfest hacktoberfest-accepted Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed by the requestor out of politeness Type: Enhancement Most issues will probably ask for additions or changes. labels Oct 18, 2021
@azjezz
Copy link
Owner

azjezz commented Oct 18, 2021

Thank you @yivi 🎉

@azjezz azjezz merged commit 08a6730 into azjezz:1.9.x Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest hacktoberfest-accepted Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed by the requestor out of politeness Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants