diff --git a/agent-notes/improve-language-selection.md b/agent-notes/improve-language-selection.md new file mode 100644 index 0000000..cb1c6b7 --- /dev/null +++ b/agent-notes/improve-language-selection.md @@ -0,0 +1,33 @@ +# Goal + +Fix language selection and persistence for existing product options dialogs. + +# State + +Isolated branch based on improve/integration-contract (PR #1). Existing public +API is preserved; no consuming app edits or live language switching. + +# Decisions and why + +Use Babel's complete locale identity for matching/saving and full display names +for regional labels. Prefer exact locale, then base language, another matching +regional locale, then the default. No selection does not save a setting. +The original set_value population call is valid: ChoiceField delegates it to +set_items. The earlier investigation's claim to the contrary was corrected. + +# Surprises + +PR #2's initial CI passed, but the Chinese fixture used Locale.parse, unlike the +real i18n_core.locale_from_locale_id producer (Locale(language, territory)). The +direct Chinese constructor fails with UnknownLocaleError under Babel 2.17.0, so +those locales are skipped by that producer; claiming they reached the panel was +wrong. Removed unnecessary requested-locale parsing and corrected the existing +regression to use supported real catalog identities: this package's fr/fr_CA and +QRead's hi_IN. Every fixture now uses the producer's direct-constructor shape. +Persisting str(locale) preserves the supplied identity. Chinese discovery is an +upstream i18n_core limitation, not fixed or claimed fixed in this PR. + +# Next step + +Publish the focused PR and read hosted CI. Tests use real Babel and controlled +fields/services; no live wx or screen-reader result is claimed. No merge/release. diff --git a/app_elements/i18n.py b/app_elements/i18n.py index 58763b2..96d14ba 100644 --- a/app_elements/i18n.py +++ b/app_elements/i18n.py @@ -1,9 +1,6 @@ -from __future__ import absolute_import - import locale import sys -import babel import i18n_core from gui_builder import fields from wx_utils import forms as wx_forms @@ -24,35 +21,42 @@ class LanguageSelectionPanel(wx_forms.AutoSizedPanel): def __init__(self, *args, **kwargs): super(LanguageSelectionPanel, self).__init__(*args, **kwargs) locales = list(i18n_core.get_available_locales(application.name)) - if sys.version_info[0] < 3: - self.locales = sorted( - locales, key=lambda i: i.language, cmp=locale.strcoll) - else: - self.locales = sorted( - locales, key=lambda i: locale.strxfrm(i.language)) + self.locales = sorted(locales, key=lambda i: locale.strxfrm(str(i))) def render(self, *args, **kwargs): super(LanguageSelectionPanel, self).render(*args, **kwargs) locales = [ "{name} ({english_name})".format( - name=i.language_name, english_name=i.english_name + name=i.get_display_name(i), english_name=i.get_display_name("en") ) for i in self.locales ] self.language.set_value(locales) - current_locale = application.locale.split(".")[0] - languages = [i.language for i in self.locales] - try: - index = languages.index(current_locale.split("_")[0]) - except ValueError: - index = languages.index(i18n_core.DEFAULT_LOCALE.split("_")[0]) + self.language.set_enabled(bool(self.locales)) + if not self.locales: + return + identifiers = [str(i).casefold() for i in self.locales] + index = 0 + for requested in (application.locale, i18n_core.DEFAULT_LOCALE): + requested = requested.split(".")[0].split("@")[0].replace("-", "_").casefold() + if requested in identifiers: + index = identifiers.index(requested) + break + language = requested.split("_")[0] + if language in identifiers: + index = identifiers.index(language) + break + matches = [n for n, item in enumerate(self.locales) if item.language == language] + if matches: + index = matches[0] + break self.language.set_index(index) def set_config_values(self): - locale = self.locales[self.language.get_index()] + index = self.language.get_index() + if index is None or not 0 <= index < len(self.locales): + return + locale = self.locales[index] if "UI" not in application.config: application.config["UI"] = {} - locale_id = locale.language - if locale.territory: - locale_id += "_%s" % locale.territory - application.config["UI"]["language"] = locale_id + application.config["UI"]["language"] = str(locale) diff --git a/tests/test_language.py b/tests/test_language.py new file mode 100644 index 0000000..0b79f94 --- /dev/null +++ b/tests/test_language.py @@ -0,0 +1,71 @@ +import unittest + +from babel import Locale + +from support import load_component + + +class LanguageSelectionTests(unittest.TestCase): + def panel(self, component, modules, identifiers): + modules["i18n_core"].get_available_locales.return_value = [ + identifier for identifier in identifiers + ] + panel = component.LanguageSelectionPanel() + panel.render() + return panel + + def test_existing_regional_catalog_identity_round_trips(self): + # Existing catalogs include fr_CA/fr in this package and hi_IN in QRead. + # Construct locales the same way i18n_core.locale_from_locale_id does. + for language, territory in (("fr", "CA"), ("hi", "IN")): + with self.subTest(language=language, territory=territory): + with load_component("app_elements.i18n", locale="%s-%s.UTF-8" % (language, territory)) as (component, app, modules): + panel = self.panel(component, modules, [ + Locale("fr"), Locale("fr", "CA"), Locale("hi", "IN"), Locale("en", "US") + ]) + identifier = "%s_%s" % (language, territory) + index = next(n for n, item in enumerate(panel.locales) if str(item) == identifier) + panel.language.set_index.assert_called_once_with(index) + panel.language.get_index.return_value = index + panel.set_config_values() + self.assertEqual(app.config["UI"]["language"], identifier) + + def test_regions_have_distinct_native_and_english_labels(self): + with load_component("app_elements.i18n", locale="fr_CA") as (component, _, modules): + panel = self.panel(component, modules, [Locale("fr"), Locale("fr", "CA"), Locale("en", "US")]) + labels = panel.language.set_value.call_args.args[0] + self.assertEqual(len(labels), len(set(labels))) + self.assertTrue(any("Canada" in label for label in labels)) + selected = panel.language.set_index.call_args.args[0] + self.assertEqual(str(panel.locales[selected]), "fr_CA") + + def test_base_language_then_default_then_first_available(self): + for requested, available, expected in [ + ("fr_BE", [Locale("fr"), Locale("en", "US")], "fr"), + ("fr_BE", [Locale("fr", "CA"), Locale("en", "US")], "fr_CA"), + ("de_DE", [Locale("fr"), Locale("en", "US")], "en_US"), + ("de_DE", [Locale("fr")], "fr"), + ]: + with self.subTest(requested=requested, available=available): + with load_component("app_elements.i18n", locale=requested) as (component, _, modules): + panel = self.panel(component, modules, available) + index = panel.language.set_index.call_args.args[0] + self.assertEqual(str(panel.locales[index]), expected) + + def test_no_selection_does_not_write_configuration(self): + for index in (None, -1, 99): + with self.subTest(index=index): + with load_component("app_elements.i18n") as (component, app, modules): + panel = self.panel(component, modules, [Locale("en", "US")]) + panel.language.get_index.return_value = index + panel.set_config_values() + self.assertEqual(app.config, {}) + + def test_no_available_locales_disables_control_and_preserves_config(self): + with load_component("app_elements.i18n") as (component, app, modules): + panel = self.panel(component, modules, []) + panel.language.set_enabled.assert_called_once_with(False) + panel.language.set_index.assert_not_called() + panel.language.get_index.return_value = None + panel.set_config_values() + self.assertEqual(app.config, {})