zend-filter
zend-filter copied to clipboard
Improve handling of digits in CamelCaseToSeparator filter
This PR aims to add the behaviour asked in #33
Since the reason behind using the CamelCaseSeparator is tokenizing strings when case is changed, usual expectation is beginning a new token when hitting a number or digit.
I will try to summarize both current and changed behaviour on numbers with some examples:
Original Text | Current | After this PR |
---|---|---|
Foo2016Bar | Foo2016-Bar | Foo-2016-Bar |
March16Events | March16-Events | March-16-Events |
DigitSuffix42 | Digit-Suffix42 | Digit-Suffix-42 |
42metersLong | 42metersLong | 42-meters-Long |
Notice the last example: Beginning of the word with lowercase letter after
42
is also mimics a case-change. I'm still not sure is this a acceptable/desired behaviour or not.
The issue #33 is about the CamelCaseToDashFilter
but since it derived from CamelCaseToSeparator
like CamelCaseToUnderscore
, this change will affect both childs of CamelCaseToSeparator
. For this reason, I also added more tests to CamelCaseToDashTest
.
After digging dozen of approaches including other platforms such as Java and C# I came up with this final patterns with positive lookbehind and lookaheads. (Achieving same effect with a single regex seems like impossible, at least I give up)
I'm also aware of changing an existing test is not a good practice. Since the expected behaviour is changed for digits, I forced into touching the CamelCaseToUnderscoreTest
:
- Pa2_Title => Pa_2_Title
- Pa2a_Title => Pa_2a_Title
Altering the CamelCaseToUnderscoreTest
seems like mandatory if the desired behaviour will be changed.
All tests are still green.
I'm pushing this to 3.0 (which will happen this year, as we will be releasing new major versions of all components that set PHP 7.1 as the minimum supported version). While I agree it is the correct behavior, I worry that if users are expecting the current behavior currently, this would break their applications. Pushing it to a new major version allows them to upgrade at their own pace.
This repository has been closed and moved to laminas/laminas-filter; a new issue has been opened at https://github.com/laminas/laminas-filter/issues/9.
This repository has been moved to laminas/laminas-filter. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:
- Squash all commits in your branch (
git rebase -i origin/{branch}
) - Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
- Run the laminas/laminas-migration tool on the code.
- Clone laminas/laminas-filter to another directory.
- Copy the files from the second bullet point to the clone of laminas/laminas-filter.
- In your clone of laminas/laminas-filter, commit the files, push to your fork, and open the new PR. We will be providing tooling via laminas/laminas-migration soon to help automate the process.