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

[5.2] Issue #11285 - Ability to capitalize translation placeholders #11337

Merged
merged 2 commits into from
Dec 14, 2015

Conversation

AdenFraser
Copy link
Contributor

Ability to capitalize translation placeholders for both Validation Rules and Strings.

As suggested in Issue #11285.

The makeReplacements method (on the Translator) and the doReplacements() method (on the Validator) now first check and string replace all capital attributes/placeholders with the same case (all capital) replacement, followed by uppercased attributes/placeholders and finally replacing the lowercase attributes/placeholders as before.

@AdenFraser AdenFraser force-pushed the issue-11285 branch 2 times, most recently from c73d181 to 31b9d09 Compare December 14, 2015 01:27
} elseif (strpos($line, ':'.ucfirst($key)) !== false) {
$line = str_replace(':'.ucfirst($key), ucfirst($value), $line);
}

$line = str_replace(':'.$key, $value, $line);
Copy link
Member

Choose a reason for hiding this comment

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

It's not possible to do following?

$search = [':'.strtoupper($key), ':'.ucfirst($key), ':'.$key];
$replace = [strtoupper($value), ucfirst($value), $value];

$line = str_replace($search, $replace, $line);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had realized the strpos() conditionals are pointless overhead (too much Whisky this evening perhaps!) when I saw your comment. That's a much better improvement!

Aden Fraser added 2 commits December 14, 2015 01:39
@taylorotwell taylorotwell merged commit f8be484 into laravel:5.2 Dec 14, 2015
@anteriovieira
Copy link
Contributor

@AdenFraser , I suggest add tests for this change.

@AdenFraser
Copy link
Contributor Author

Probably a wise suggestion actually, will do!

@anteriovieira
Copy link
Contributor

Feel free.

@jaketoolson
Copy link

Nice! I do like what this adds, albeit small, its handy.

@Propaganistas
Copy link
Contributor

Great that my proposal made it into core.

Could this be backported to a 5.1 maintenance release please?

@GrahamCampbell
Copy link
Member

No, because it's breaking.

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.

7 participants