Skip to content

[Translation] remove unused code #42128

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

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.2
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

@carsonbot carsonbot added this to the 5.2 milestone Jul 15, 2021
@carsonbot carsonbot changed the title [Translation] remove dead code [Translation] remove unused code Jul 15, 2021
Copy link
Member Author

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

small mess here...

@@ -150,7 +150,7 @@ public function addResource(string $format, $resource, string $locale, string $d
public function setLocale(string $locale)
{
$this->assertValidLocale($locale);
$this->locale = $locale ?? (class_exists(\Locale::class) ? \Locale::getDefault() : 'en');
$this->locale = $locale;
Copy link
Member Author

Choose a reason for hiding this comment

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

in 4.4, we deprecated calling setLocale with null

we could make the empty string fallback to \Locale::getDefault(), but there are tests that fail if we do.

Copy link
Contributor

Choose a reason for hiding this comment

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

ref #38230

we could still deprecate empty string on 4.4 isnt it? also the trait currenly stll allows "" and "0", perhaps we should consider a shared LocaleAwareTrait

Copy link
Member Author

@nicolas-grekas nicolas-grekas Jul 15, 2021

Choose a reason for hiding this comment

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

we can't deprecate anymore on 4.4
looks like we (I) missed why #38230 was proposed...

@@ -444,7 +444,7 @@ protected function computeFallbackLocales(string $locale)
*/
protected function assertValidLocale(string $locale)
{
if (null !== $locale && 1 !== preg_match('/^[a-z0-9@_\\.\\-]*$/i', $locale)) {
if (!preg_match('/^[a-z0-9@_\\.\\-]+$/i', $locale)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

the empty string should not be a valid locale

@nicolas-grekas
Copy link
Member Author

Replaced by #42130

fabpot added a commit that referenced this pull request Jul 20, 2021
…s-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Translation] fix fallback to Locale::getDefault()

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

Replaces #42128 and #38230

Provides consistent behavior with `TranslatorTrait::getLocale()`.

Commits
-------

20120a3 [Translator] fix fallback to Locale::getDefault()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants