Skip to content

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...

{
$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...

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