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

Update str_plural() to add intval($count) #14501

Closed
wants to merge 1 commit into from
Closed

Update str_plural() to add intval($count) #14501

wants to merge 1 commit into from

Conversation

SaeedPrez
Copy link
Contributor

A lot of the time when you use str_plural() you get the $count value from a form request which returns the number as a string and therefore this helper won't work as expected. This small change should make sure the $count is in integer format.

A lot of the time when you use `str_plural()` you get the $count value from a form request which returns the number as a string and therefore this helper won't work as expected. This small change should make sure the $count is in integer format.
@barryvdh
Copy link
Contributor

Shouldn't that happen in the Pluralizer class?

public static function plural($value, $count = 2)
{
if ($count === 1 || static::uncountable($value)) {
return $value;
}
$plural = Inflector::pluralize($value);
return static::matchCase($plural, $value);
}

@SaeedPrez
Copy link
Contributor Author

Hi Barry,

It doesn't return in singular when I try str_plural('car', $request->input('quantity')); where quantity is a HTML number field. However it does work as expected when I use it with intvar().

Also when I try var_dump($request->input('quantity)) it gives me string "1".

Another option would be to change $count === 1 to $count == 1 in Pluralizer.php

@barryvdh
Copy link
Contributor

I meant that you can do the intval() casting further down the code. Now your just fixing the helper, not the Str::plural() method or the Pluralizer class itself.

@SaeedPrez
Copy link
Contributor Author

Ah, sorry.. I thought you meant it should already happen there.

This is my first pull request ever ☺ Should I close this and make a new pull request for the Pluralizer.php?

@SaeedPrez
Copy link
Contributor Author

Closing this one as I've created a new pull request as suggested..

#14502

@SaeedPrez SaeedPrez closed this Jul 27, 2016
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.

2 participants