-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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
Conversation
nicolas-grekas
commented
Jul 15, 2021
Q | A |
---|---|
Branch? | 5.2 |
Bug fix? | no |
New feature? | no |
Deprecations? | no |
Tickets | - |
License | MIT |
Doc PR | - |
c781a0e
to
5c5d50d
Compare
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.
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; |
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.
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.
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.
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
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.
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)) { |
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.
the empty string should not be a valid locale
Replaced by #42130 |
…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()