Preserve regional language selection and settings

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.

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)

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, {})