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

Improve handling of digits in CamelCaseToSeparator filter #9

Open
weierophinney opened this issue Dec 31, 2019 · 1 comment
Open

Improve handling of digits in CamelCaseToSeparator filter #9

weierophinney opened this issue Dec 31, 2019 · 1 comment

Comments

@weierophinney
Copy link
Member

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.


Originally posted by @edigu at zendframework/zend-filter#45

@weierophinney
Copy link
Member Author

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.


Originally posted by @weierophinney at zendframework/zend-filter#45 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant