-
Notifications
You must be signed in to change notification settings - Fork 1
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
IBX-9444: Fixed locale ach-UG #91
base: 4.6
Are you sure you want to change the base?
Conversation
|
@@ -19,7 +19,7 @@ class AvailableLocaleChoiceLoader implements ChoiceLoaderInterface | |||
{ | |||
// Acholi dialect is used by In-Context translation | |||
// and should not be present on the list of available translations. | |||
private const EXCLUDED_TRANSLATIONS = ['ach']; | |||
private const EXCLUDED_TRANSLATIONS = ['ach-UG']; |
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.
Not sure how Crowdin does things here, but just to be sure, I would leave both values if that solves the issue.
@mikadamczyk, could you sheed some light on this as well.
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.
Hi @ViniTou ,
from what I see:
TranslationCollectorPass
maps some locales to languages, see
https://github.com/ibexa/core/blob/main/src/bundle/Core/DependencyInjection/Compiler/TranslationCollectorPass.php#L22- TranslationCollectorPass
calls https://github.com/ibexa/core/blob/main/src/bundle/Core/Translation/GlobCollector.php GlobCollector
checks forxlf
files and extracts locales from filenames like
https://github.com/ibexa/i18n/blob/main/translations/ach_UG/activity-log/forms.**ach-UG**.xlf- So we will have ach-UG locale
There are several alternative options:
- Add mapping
ach-UG
->ach
to TranslationCollectorPass. Then there is no need at all to change anything in ibexa/user bundle - Or ('better safe then sorry' approach) : add both strings to
EXCLUDED_TRANSLATIONS
:
private const EXCLUDED_TRANSLATIONS = ['ach', 'ach-UG'];
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.
@reithor you are right, currently we have ach-UG
as a locale and it should be
private const EXCLUDED_TRANSLATIONS = ['ach-UG']
The 'ach' version is only valid when someone adds it as a user_preferences.additional_translations
which I bet will never happen
From what I see we are not using the language
ach
anywhere - only the localeach-UG
.PR excludes
ach-UG
from choices list - and also changes AvailableLocaleChoiceLoaderTest.php accordingly.