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

Pluralizer Case Sensitivity #1246

Merged
merged 3 commits into from
May 12, 2013
Merged

Pluralizer Case Sensitivity #1246

merged 3 commits into from
May 12, 2013

Conversation

helmut
Copy link
Contributor

@helmut helmut commented May 8, 2013

I understand that this has been discussed before but this issue keeps bugging me so I thought I would have a crack at solving it in the cleanest way I could find in the hopes of a change of heart. I have modified the pluralizer to maintain case sensitivity on irregular words and also fixes the case matching on uppercase regular words. For example...

// Without my modification on a regular word
Str::plural('dog'); // echos 'dogs' - correct!
Str::plural('Dog'); // echos 'Dogs' - correct!
Str::plural('DOG'); // echos 'DOGs' - NOT correct
// And for an irregular word
Str::plural('child'); // echos 'children' - correct!
Str::plural('Child'); // echos 'children' - NOT correct
Str::plural('CHILD'); // echos 'children' - NOT correct

And if you accept this pull request.

// With my modification on a regular word
Str::plural('dog'); // echos 'dogs' - correct!
Str::plural('Dog'); // echos 'Dogs' - correct!
Str::plural('DOG'); // echos 'DOGS' - correct!
// And for an irregular word
Str::plural('child'); // echos 'children' - correct!
Str::plural('Child'); // echos 'Children' - correct!
Str::plural('CHILD'); // echos 'CHILDREN' - correct!

Check out the implementation and if you sitll think this is too clunky then I will let it go but I think this is not a bad fix for the current inconsistency with the function.

Also discussed here #855

helmut added 2 commits May 9, 2013 09:06
Added a matchCase method to attempt to determine case format and apply it but fallback to default value if not found. Added matchCase to the irregular and inflected return values.
Add tests to support new case 'aware' plural and singular methods.
@helmut
Copy link
Contributor Author

helmut commented May 8, 2013

Not sure why the Travis 5.3 build failed. 5.4 passed ok. Says something wrong with CookieTest but don't even think the pluralizer is used in that class? My testing passed locally.

@andrewryno
Copy link
Contributor

5.3 build failed presumably from the same issue causing #1238? Error is at the same line. @taylorotwell

Removed space to trigger rebuild from travis.
@helmut
Copy link
Contributor Author

helmut commented May 8, 2013

Just removing extra space to trigger a rebuild.

@helmut
Copy link
Contributor Author

helmut commented May 9, 2013

Passed with no 'real' change in coding.

@bencorlett
Copy link
Contributor

Top job there @helmut. Good work.

@helmut
Copy link
Contributor Author

helmut commented May 9, 2013

Thanks ben... as a fellow aussie I was tempted to rename it 'pluraliser' while I was at it!

taylorotwell added a commit that referenced this pull request May 12, 2013
@taylorotwell taylorotwell merged commit 7e6723d into laravel:master May 12, 2013
@helmut helmut deleted the pluralizer-case-fix branch May 13, 2013 08:56
theninja pushed a commit to theninja/framework that referenced this pull request Feb 21, 2018
This PR changes the 'mix' helper function to take URLs coming from the hot file. This feature is fully backwards compatible.

Supporting Laravel-Mix Pull Request laravel#1246, allowing Hot Module Reloading with custom hosts and ports

Do not merge straight away, awaiting reply of laravel-mix/laravel-mix#1246

Let me know if you require tests
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.

4 participants