Skip to content

[String] Introduce underscore() method #34781

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

[String] Introduce underscore() method #34781

wants to merge 12 commits into from

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Dec 3, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #34777
License MIT
Doc PR TBA symfony/symfony-docs#...

Provides underscore() method to String component. Similar to snake() but also adds _ before numbers.

assertSame('something_1', u('Something1')->underscore()->toString());

This pattern is used eg. by Doctrine ORM UnderscoreNamingStrategy with number aware enabled.

@simPod simPod force-pushed the string-underscore branch from f38cae4 to c1876cd Compare December 3, 2019 13:03
stof
stof requested changes Dec 3, 2019
View reviewed changes
src/Symfony/Component/String/AbstractUnicodeString.php Outdated
public function underscore(): parent
{
$str = $this->camel()->snake();
$str->string = mb_strtolower(preg_replace('~(?<=[a-z])([0-9])~u', '_$1', $str->string), 'UTF-8');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doing camel -> snake -> regexp replacement -> lowercase, where snake itself is already doing camel -> title -> regex replacement ->lowercase with a regex keeping numbers in the previous word ? This duplicates the camelcase conversion, performs 2 regexp replacement where one could be enough, and converts to lowercase twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently doing camel -> title -> regex -> lowercase. Good?

stof
stof reviewed Dec 3, 2019
View reviewed changes
@nicolas-grekas nicolas-grekas added this to the next milestone Dec 3, 2019
nicolas-grekas
nicolas-grekas reviewed Dec 3, 2019
View reviewed changes
@simPod
Copy link
Contributor Author

simPod commented Dec 3, 2019

I think I addressed all comments, thank you. Did I miss to test something?

@simPod simPod requested a review from nicolas-grekas December 5, 2019 13:36
@simPod simPod requested a review from stof December 5, 2019 13:36
nicolas-grekas
nicolas-grekas reviewed Dec 5, 2019
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm genuinely wondering about the difference with snake()
Yes, I can personnally understand it by looking at the code and maybe the tests, but for anyone else after (future me included), when should I use snake or underscore? Is that obvious to anyone?

@stof
Copy link
Member

stof commented Dec 5, 2019

I'm genuinely wondering about the difference with snake()
Yes, I can personnally understand it by looking at the code and maybe the tests, but for anyone else after (future me included), when should I use snake or underscore? Is that obvious to anyone?

We clearly need some phpdoc explanation the difference (or maybe we should not have both ?)

@simPod
Copy link
Contributor Author

simPod commented Dec 5, 2019

I have not found any "string manipulation standards". But looking through internet, I found there's generally consensus on that snake does not consider numbers as another "word".

But difference should be documented for sure.

@javiereguiluz
Copy link
Member

I agree that this may be confusing. I can think of two possible solutions:

  • Not adding this and "forcing" people to use "pure snake case" (same as we do in the Slugger ... see [String] Slugify a string with an apostrophe #34822, where we made a decision about how apostrophes should behave in Symfony's Slugger and we don't make it configurable)
  • Renaming to snake_numbers(), snake_numeric(), etc. to clearly show that this is a snake variant.

@simPod simPod force-pushed the string-underscore branch from cb51e19 to c773e1f Compare December 7, 2019 22:53
@simPod
Copy link
Contributor Author

simPod commented Dec 7, 2019

snakeNumeric() sounds feasible. Also added docs to make the difference clear.

@simPod simPod requested a review from nicolas-grekas December 7, 2019 23:24
@gharlan
Copy link
Contributor

gharlan commented Dec 11, 2019

This pattern is used eg. by Doctrine ORM UnderscoreNamingStrategy with number aware enabled.

Are you sure?
https://github.com/doctrine/orm/blob/2.7/UPGRADE.md#deprecated-number-unaware-doctrineormmappingunderscorenamingstrategy

base64Encoded is converted to base64_encoded, not to base_64_encoded.
So I guess your Something1 would be converted to something1 by the number aware UnderscoreNamingStrategy from doctrine, not to something_1.

@simPod
Copy link
Contributor Author

simPod commented Dec 11, 2019

@gharlan you're correct, the strategy changes a1B to a1_b. My bad on that one.

@ro0NL
Copy link
Contributor

ro0NL commented Dec 12, 2019

is "underscore case" some widely adopted pattern? I cant find any reference about it ... as opposed to "snake case": https://en.wikipedia.org/wiki/Snake_case

Nor does "snake numeric case" seem to exist. Im 👎 for inventing our own cases :)

@nicolas-grekas
Copy link
Member

I'm sorry but I'm 👎 too: looking for prior art on the topic, it's not clear at all that there is anything named "underscore case" that differs from "snake case".
Thus, your requirement looks specific and cannot make it into a dedicated method to me.

@simPod
Copy link
Contributor Author

simPod commented Dec 16, 2019

Ok, thanks for feedback!

@simPod simPod closed this Dec 16, 2019
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@simPod simPod deleted the string-underscore branch April 15, 2022 11:42
This was referenced Feb 25, 2024
Closed
Closed
@VincentLanglet
Copy link
Contributor

VincentLanglet commented Mar 5, 2024 •
edited
Loading

is "underscore case" some widely adopted pattern? I cant find any reference about it ... as opposed to "snake case": en.wikipedia.org/wiki/Snake_case

It's used by Symfony for the template name https://github.com/symfony/symfony/tree/7.1/src/Symfony/Bridge/Twig/Resources/views/Form instead of "pure" snake case. (Which is weird when symfony recommend snake case https://symfony.com/doc/current/best_practices.html#use-snake-case-for-template-names-and-variables)

Should Symfony rename these templates then ?

@Seb33300 Seb33300 mentioned this pull request Apr 6, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof Awaiting requested review from stof

@nicolas-grekas nicolas-grekas Awaiting requested review from nicolas-grekas

Assignees
No one assigned
Labels
Feature Status: Needs Review
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

[String] FR: snake case but with underscore before numbers
8 participants
@simPod @stof @javiereguiluz @gharlan @ro0NL @nicolas-grekas @VincentLanglet @carsonbot