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

[5.5] Snake and Kebab handle lower case words as expected. #18764

Merged
merged 1 commit into from
Apr 11, 2017
Merged

[5.5] Snake and Kebab handle lower case words as expected. #18764

merged 1 commit into from
Apr 11, 2017

Conversation

PeterDKC
Copy link
Contributor

It seems like unexpected results to return "snaked" words only if those words or letters start with uppercase and to explicitly ignore a non-letter character followed by an letter.

Current behavior:

>>> \Illuminate\Support\Str::snake('Random words Here')
=> "randomwords_here"
>>> Illuminate\Support\Str::snake('Random wORDS  p     here')
=> "randomw_o_r_d_sphere"

Proposed change passes current unit tests and doesn't change existing behavior.

@taylorotwell taylorotwell merged commit 8877cf6 into laravel:master Apr 11, 2017
@ntzm
Copy link
Contributor

ntzm commented Apr 11, 2017

This could be a BC for someone who is using these functions for URL slugs or identifiers

@jhoff
Copy link
Contributor

jhoff commented Apr 11, 2017

@marktopper This wouldn't re-case words, only upper cases the first, so the delimiter is still added before each upper case letter. In that example, it would be random_w_o_r_d_s_p_here.

@PeterDKC
Copy link
Contributor Author

This could be a BC for someone who is using these functions for URL slugs or identifiers

@ntzm I considered that quite thoroughly, but I feel like anyone using it for that would have accounted for it locally when they actually tried to use it, which is exactly why we discovered it was behaving oddly. We were writing factories for User Permissions, which are kebabbed: is-admin or marketing-manager for instance. instead we got:

>>> $faker->words(3, true)
=> "delectus quos possimus"
>>> Illuminate\Support\Str::kebab($faker->words(3, true))
=> "etaperiamex"

Clearly the intended result is not to have onelongwordsausage, so I feel like anyone who was actually looking for slugs would have accounted for it if they payed attention to the output at all. If they didn't, then yes, they might need to adjust some things.

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

Successfully merging this pull request may close these issues.

None yet

4 participants