-
Notifications
You must be signed in to change notification settings - Fork 11.7k
[12.x] Add ucwords to Str and Stringable #57581
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
Conversation
|
Why aren't you using PHP's native |
PHP's ucwords function doesn't work with UTF-8 strings. Same reason we don't use PHP's ucfirst function |
shaedrich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation, that makes sense 👍🏻
So, what about mb_convert_case($str, MB_CASE_TITLE, "UTF-8") then? This functionality already exists via Str::title()
src/Illuminate/Support/Str.php
Outdated
| * @param string $delimiter | ||
| * @return string | ||
| */ | ||
| public static function ucwords($string, $delimiter = ' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why ucwords() defaults to ' \t\r\n\f\v' for $delimiter, while Str::ucwords() only defaults to ' ' in that case?
Also, to make the word-splitting safe, this should be the default:
/**
* Capitalize the first character of each word in a string.
*
* @param string $string
* @param string|null $delimiter
* @return string
*/
public static function ucwords($string, $delimiter = null)
{
$words = $delimiter === null
? mb_split('\s+', $value)
: explode($delimiter, $string);
foreach ($words as &$word) {
$word = static::ucfirst($word);
}
return implode($delimiter ?? ' ', $words);
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we should more closely match PHP's native ucwords functionality, I'll push up a change.
My reason for not using |
Sorry, my bad, I forgot 🤦🏻♂️ Well, finding one thing to fit for all cases is easier said than done, actually: hotmeteor/titles#1 (comment), hotmeteor/titles#3, hotmeteor/titles#4, #49572 (comment) There have been attempts like #55852 to make this fit their specific use case. But if we did this, we'd have thousands of similar methods doing every possible permutation. |
Haha no worries. True, lots of cases for |
shaedrich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that leads us back to the actual problem: Neither PHP's MB_CASE_TITLE nor Str::title() do an actual title case or what people would expect of it (i.e. not capitalizing "minor words"), so there's Str::headline(). And since this also doesn't take localization into account among other things, we ended up with Str::apa(), which still is opinionated as it prioritizes one writing style over others and still isn't configurable (see my reply above).
So, while having an mb_ucwords() equivalent would be true to it's name, it would be kind of a band-aid to what the framework gets wrong until now in terms of either naming or functionality behind said naming ^^
Since I'm not a maintainer, I can neither approve nor reject your PR—but Taylor will. I really like your new implementation btw, though. It doesn't align with the other consistent uses (#56338), but it might be an even more precise way of handling this.
Fair points, I can see how these multiple string-casing functions can muddle the API. Although it does seem nice to provide a UTF-8 safe alternative to PHP's native |
| $this->assertSame('Laravel', Str::ucwords('laravel')); | ||
| $this->assertSame('Laravel Framework', Str::ucwords('laravel framework')); | ||
| $this->assertSame('Мама', Str::ucwords('мама')); | ||
| $this->assertSame('Мама Мыла Раму', Str::ucwords('мама мыла раму')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add your "JJ watt" example here 😉
When transforming a name in a request, I'm currently doing the following:
$name = ucwords($this->string('name')->trim()->squish()->value());After this change, we'll be able to:
$name = $this->string('name')->trim()->squish()->ucwords()->value();Maybe it's worth noting that I was using
->title()instead ofucwords, but it undesirably changes names like "JJ watt" to "Jj Watt" when I'd rather preserve "JJ".