-
Notifications
You must be signed in to change notification settings - Fork 41
replaced usage of zend filters w/ hardcoded versions #14
Conversation
Could use some guidance on how to address coverage for test cases involving pcre w/ unicode that is based on environment extension mbstring being loaded. |
{ | ||
if (static::$underscoreToStudlyCaseFilter instanceof FilterChain) { | ||
return static::$underscoreToStudlyCaseFilter; | ||
if (static::$underscoreToCamelCaseFilter === null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (! static::$underscoreToCamelCaseFilter) {
@jeremygiberson patch looks really good so far! |
*/ | ||
protected static $camelCaseToUnderscoreFilter; | ||
private static $camelCaseToUnderscoreFilter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to document this visibility change in the upgrade notes
Linking #12 |
Working on addressing the coverage issue. |
…tests to provide coverage
When mbstrings extension is not present the filter has different output behaviour. The behaviour difference is a degradation rather than a break. Ie when an underscore proceeds a unicode lower case character it will leave that character pair alone. And when A camel cased character is unicode it will separate it with an underscore but leave the unicode character upper case. These changed behaviours leave extract/hydrate functional in both directions.
A test asset contained input filter usage but in unused code path. So I removed it which frees us from the require-dev zend-inputfilter dependency
@jeremygiberson need rebase against master |
@samsonasik done |
I've done the following:
I had hoped to push back to your branch, @jeremygiberson , but it's evidently locked to maintainers. As such, I went ahead and merged to develop. We should be able to release either tomorrow, or early next week, following some additional refactors to the component to make use of PHP 7.1. |
Patch zendframework#14 inlined the `CamelCaseToUnderscore` and `UnderscoreToCamelCase` filters in the package. However, in doing so, it added behavior that was not expected, nor present in the original: splitting words on numeric boundaries. Furthermore, the functionality was unpredictable; some boundaries were matched, while others were not, which led to an inability to predict what would happen. The problem stemmed from the addition of a regex in each of the unicode and non-unicode matching pattern sets, as well as corresponding replacement sets. These have been reverted to match those in zend-filter, which restores behavior to how it worked in version 2. Additional test cases were provided to the `UnderscoreToCamelCaseTest` to demonstrate the reverse operations as well. One note: when converting from underscore to camel case and word splits happen before numeric values, there is no way to guarantee the opposite operation (camel case to underscore) will resolve to the original value. If such input is expected, it should likely use a custom filter.
Address issue 12 with Ocramius suggestion to hard code the filters to decouple from zend-filter dependency.
The code for the filters is not "simple" so I decided to keep them separate classes for testing purposes. For the what the filters are necessary for, decided not to translate the subclassing that zend-filter uses and went with single concrete implementations for underscore
_
separators. Imported the test cases from zend-filter to make sure we had same coverage.I changed visibility from protected to private on
UnderscoreNamingStrategy
.Finally, I removed require-dev zend-filter from composer.json