Skip to content

[String] Revert "Fixed u()->snake(), b()->snake() and s()->snake() methods" #57616

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

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #57612
License MIT

This PR reverts #57497 for BC reasons, but keeps the test cases we added in the process. Those test cases allowed to spot a real issue where the ascii and unicode implementations didn't agree on the resulting camel case for SYMFONY IS GREAT. Both implementations now result in SYMFONYISGREAT (likely introduced in #47423).

@carsonbot carsonbot added this to the 5.4 milestone Jul 1, 2024
@nicolas-grekas nicolas-grekas changed the title String inflection [String] Revert "Fixed u()->snake(), b()->snake() and s()->snake() methods (arczinosek)" Jul 1, 2024
@nicolas-grekas nicolas-grekas changed the title [String] Revert "Fixed u()->snake(), b()->snake() and s()->snake() methods (arczinosek)" [String] Revert "Fixed u()->snake(), b()->snake() and s()->snake() methods" Jul 1, 2024
@fabpot fabpot force-pushed the string-inflection branch from 4364734 to 6397c38 Compare July 1, 2024 16:36
@fabpot
Copy link
Member

fabpot commented Jul 1, 2024

Thank you @nicolas-grekas.

@fabpot fabpot merged commit e3625f2 into symfony:5.4 Jul 1, 2024
4 of 5 checks passed
fabpot added a commit that referenced this pull request Jul 6, 2024
This PR was merged into the 5.4 branch.

Discussion
----------

[String] test: kebab-case to snake_case

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes?
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

In Symfony 7.1.1, `kebab-case` strings casted to `snake_case` properly. The changes made in 7.1.2 broke this functionality and kept it `kebab-case`.

It would be nice to have a `kebab-case` test added to the list to clearly define the expected behavior.

Relates to: #57497, #57612, #57616

Commits
-------

a6d0f3b test: kebab-case to snake_case
@nicolas-grekas nicolas-grekas deleted the string-inflection branch July 18, 2024 08:41
@kevinpapst
Copy link

This is still not correct or at least contains a BC break @nicolas-grekas

I did not check whether this was introduced by the original change or your fix.

The string 123-customer,with/special#name was previously (for years) converted to 123-customer_with_special_name.
Now it returns 123-customer,with/special#name, so many characters are not converted as expected.

New bug report?

@nicolas-grekas
Copy link
Member Author

Better: a PR! 🙏

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.

4 participants