search

Kdyby / Translation

Integration of Symfony/Translation into Nette Framework
https://packagist.org/packages/kdyby/translation
Other
89 stars 84 forks source link

Fixed setting default locale to symfony #175

Closed mikoczy closed 3 years ago

mikoczy commented 3 years ago

The newest symfony/translation version (4.4.14) changes the default locale setting in Translator::setLocale - if the $locale value is null, the default system locale is used. This causes the if statement in Kdyby Translator::getLocale to evaluate to false, so the default locale is never used.

enumag commented 3 years ago

Does this work as expected with older sf/translation as well?

mikoczy commented 3 years ago

I did not test it, but it should. Setting NULL to locale is deprecated since v4.4.0

mikoczy commented 3 years ago

Can this be merged? I'm not sure how I could fix those tests, there are some composer errors, and it looks like phpstan is missing an error..

enumag commented 3 years ago

I could merge this, sure, but I won't make a release without passing tests anyway.

mikoczy commented 3 years ago

Great, thanks. I fixed the phpstan error, but I'm not really sure, what to do with those php 7.1 composer failures. Could you take a look please? kdyby/monolog requires tracy ^3.0, which only supports php >=7.2..

enumag commented 3 years ago

I wonder how it's possible that the tests were passing on 7.1 in previous releases. The monolog 2 and tracy 3 dependencies were in for a while now... 🤔

mikoczy commented 3 years ago

7.2 requirement was added august this year.. according to the travis build history, the last time the build passed was september last year if I'm not mistaken..

enumag commented 3 years ago

Ah okay I see the problem now. Tracy 3 is not released yet but kdyby/monolog has a dependency on it (even the stable version of kdyby/monolog). We need to fix kdyby/monolog first to work with stable version of tracy, make a release there and then translation will pass.

enumag commented 3 years ago

Note that I have no time to work on this myself.

mikoczy commented 3 years ago

I've tried adding monolog ^1.4, but it did not help. Another fix would be to drop the 7.1 support for the ^3.0 version, but that is a pretty huge breaking change...

enumag commented 3 years ago

I don't like either of those solutions anyway. The correct way is to fix kdyby/monolog to support tracy 2.x.

mikoczy commented 3 years ago

Looking at Kdyby/Monolog#35 it looks like tracy 2.6 support was dropped for no reason, or at least I dont see it.

enumag commented 3 years ago

Agreed

mikoczy commented 3 years ago

here it is, could you take a look? https://github.com/Kdyby/Monolog/pull/41

enumag commented 3 years ago

Merged and released. https://github.com/Kdyby/Monolog/releases/tag/v2.0.1

mikoczy commented 3 years ago

Great, thank you. There is only one test failing (https://travis-ci.org/github/Kdyby/Translation/jobs/733755084), and I cannot fix that. Problem is, that nette/application v3.1.0-RC and latte/latte dev-master do not work together on php 7.1, where type widening is not supported. With the --prefer-stable composer parameter it passes..

enumag commented 3 years ago

Right. Add it to matrix exclude then.

enumag commented 3 years ago

Also add 7.4 to travis

mikoczy commented 3 years ago

Finally, the tests are passing, but I had to exclude php 7.4 with prefer-lowest env.. not all of the libraries used are ready for php 7.4 in their lowest stable versions..

mikoczy commented 3 years ago

@enumag could you please create a branch on the 2.x version, so that I can commit this fix there too?

enumag commented 3 years ago

done