[Translation][cache fallback] keep only missing messages in fallbackCatalogue. #14318
Conversation
aitboudad
commented
Apr 11, 2015
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Fixed tickets | #14315 |
| Tests pass? | yes |
| License | MIT |
eda1193 to
00f84f0
Compare
| @@ -444,6 +430,14 @@ private function getFallbackContent(MessageCatalogue $catalogue) | |||
| $fallbackSuffix = ucfirst(preg_replace($replacementPattern, '_', $fallback)); | |||
| $currentSuffix = ucfirst(preg_replace($replacementPattern, '_', $current)); | |||
|
|
|||
| $fallbackMessages = $fallbackCatalogue->all(); | |||
There was a problem hiding this comment.
This should be in else to avoid call if not needed as you replace it if not in debug anyway...
|
I don't think this optimizes anything? |
547c8a9 to
41fd50a
Compare
41fd50a to
41f1ecc
Compare
|
@mpdude this PR is about fixing a bug in the previous optimization. and keeping only the missing messages rather than the whole fallback catalogue is about optimizing memory usage. |
|
@nicolas-grekas we have a Fatal Error related to the Debug component in the deps=high build when running the TwigBundle tests. It would be great to investigate it |
|
@stof sure, I understand what it is about. The thing is that this optimization and dropping the messages causes unintuitive and possibly BC breaking behaviour when getting the catalogue from the translator (#14315). As this PR passes the tests from #14315 I wonder if it really fixes the problem or if the actual optimization does not happen anymore at all, although (at a glimpse) the code suggests it. |
|
No, it's not useless. It shows that your fix here does not suffice. Run it against a Symfony version before the optimization was introduced and it will pass. |
|
@mpdude ok no problem for me :) |
|
I have an idea how we might be able to rescue this. It's not gonna be beautiful, but might work. Need to think about it a little more and will get back to you. |