Instead of using the config translation label, in #3019298 we hardcode the source language of the config to English and then run these labels again through t(). All these objects are config entities so instead of using locale to do the translation, we should depend on the config translation.
This is the original issue:
FILE: ...les/admin_toolbar/admin_toolbar_tools/admin_toolbar_tools.module
----------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------
100 | WARNING | Only string literals should be passed to t() where
| | possible
110 | WARNING | Only string literals should be passed to t() where
| | possible
233 | WARNING | Only string literals should be passed to t() where
| | possible
277 | WARNING | Only string literals should be passed to t() where
| | possible
321 | WARNING | Only string literals should be passed to t() where
| | possible
539 | WARNING | Only string literals should be passed to t() where
| | possible
571 | WARNING | Only string literals should be passed to t() where
| | possible
----------------------------------------------------------------------
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3026337_11.patch | 4.34 KB | mpp |
| #3 | admin_toolbar-string_literals_on_t-3026337-3-D8.patch | 3.89 KB | thalles |
Comments
Comment #2
thallesComment #3
thallesFollow the patch
Comment #5
adriancidHi @thalles as the problem says:
In this case, I don't think we are improving nothing, you're only changing the variable with a placeholder, but in the end, we are only translating a variable.
Comment #6
thallesOk! Thanks!
Comment #7
thallesHello @adriancid, sorry for reopening, but we have a good reason to change, in some places before the deploy runs a sniffer, which can block in case of occurrences, but feel free to accept the patch or not.
Thank you for your attention!
Comment #8
adriancidThanks, @thalles I will check the patch, but why you run a sniffer against contributed modules, you should do this only for your custom modules.
Comment #9
thallesThis helps us contribute to the community.
Comment #10
mpp commentedInstead of putting the source label through the translation helper we should get the translated labels (e.g. with $entity->getTranslation()).
Comment #11
mpp commentedAfter some more debugging I found out that a bug has been introduced in #3019298.
Instead of using the config translation label, in #3019298 we hardcode the source language of the config to English and then run these labels again through t(). All these objects are config entities so instead of using locale to do the translation, we should depend on the config translation.
I added a patch that will fix the original issue ("String literals on t()") but that will also remove the redundant t() and the unneeded hardcoding of English config.
We could put this back in place in a follow up ticket as a feature to allow people to define the language for the menu. But as it now stands this is a bug.
Comment #12
gábor hojtsyConfig translation should not be hacked around like this IMHO indeed. Theme name looks like an exception that it is not config, but that should not be translated either, we don't translate component name.
Comment #13
fernly commentedLooks good. Thx mpp.
Comment #15
adriancidThanks to @all