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

Refactor inflector class #116

Merged
merged 2 commits into from
Jan 15, 2019
Merged

Refactor inflector class #116

merged 2 commits into from
Jan 15, 2019

Conversation

jwage
Copy link
Member

@jwage jwage commented Jan 10, 2019

fixes #115

  • Move accented characters array to private constant.
  • Handle preg_replace returning null instead of casting to string.
  • Remove duplicates from accented characters array

@jwage jwage added this to the v2.0.0 milestone Jan 10, 2019
@jwage jwage self-assigned this Jan 10, 2019
@jwage jwage requested a review from Majkl578 January 10, 2019 15:39
@jwage jwage force-pushed the refactor-inflector-class branch 3 times, most recently from 8c3883c to 87f1cf7 Compare January 10, 2019 19:57
@SenseException
Copy link
Member

It's hard to review (diff) the changes into ACCENTED_CHARACTERS. I seems like that the previous version had more elements in the array and now a different order. Can you please make your changes there more transparent?

@jwage
Copy link
Member Author

jwage commented Jan 14, 2019

The array had duplicate keys defined at the end of the array. They existed but as chr() calls before. As soon as we converted that array to not use chr(), phpstan pointed that out. So the duplicates got cleaned up and the values moved up.

@jwage
Copy link
Member Author

jwage commented Jan 14, 2019

@SenseException Could you elaborate more on what you are looking to see done here?

@jwage jwage merged commit 003f2dd into master Jan 15, 2019
@jwage jwage deleted the refactor-inflector-class branch January 15, 2019 17:18
@SenseException
Copy link
Member

I just needed more input about the changes in detail as a help for my review and your explanation helped me.

@alcaeus alcaeus modified the milestones: 2.0.0, 1.4.0 May 6, 2020
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.

Fix (string) casting on preg_replace calls
3 participants