From 6c6ae8954f1401f77ca51f32d3335c251e19e546 Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Thu, 29 Oct 2026 12:12:40 -0600 Subject: [PATCH 1/3] Preserve full locale identity in language settings --- agent-notes/improve-language-selection.md | 26 +++++++++ app_elements/i18n.py | 49 ++++++++++------- tests/test_language.py | 64 +++++++++++++++++++++++ 3 files changed, 119 insertions(+), 20 deletions(-) create mode 100644 agent-notes/improve-language-selection.md create mode 100644 tests/test_language.py diff --git a/agent-notes/improve-language-selection.md b/agent-notes/improve-language-selection.md new file mode 100644 index 0000000..6869f57 --- /dev/null +++ b/agent-notes/improve-language-selection.md @@ -0,0 +1,26 @@ +# 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 + +Babel resolves zh_CN to zh_Hans_CN, so matching must canonicalize the requested +locale as well as using the locale object's full identity. + +# 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..86b19cd 100644 --- a/app_elements/i18n.py +++ b/app_elements/i18n.py @@ -1,5 +1,3 @@ -from __future__ import absolute_import - import locale import sys @@ -24,35 +22,46 @@ 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() + try: + requested = str(babel.Locale.parse(requested)).casefold() + except (babel.UnknownLocaleError, ValueError): + pass + 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..234eabc --- /dev/null +++ b/tests/test_language.py @@ -0,0 +1,64 @@ +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 = [ + Locale.parse(identifier) for identifier in identifiers + ] + panel = component.LanguageSelectionPanel() + panel.render() + return panel + + def test_qread_options_preserves_exact_regional_locale_on_save(self): + with load_component("app_elements.i18n", locale="zh-CN.UTF-8") as (component, app, modules): + panel = self.panel(component, modules, ["zh_TW", "zh_CN", "en_US"]) + index = next(n for n, item in enumerate(panel.locales) if str(item) == "zh_Hans_CN") + 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"], "zh_Hans_CN") + + 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, ["fr_FR", "fr_CA", "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", ["fr", "en_US"], "fr"), + ("fr_BE", ["fr_CA", "en_US"], "fr_CA"), + ("de_DE", ["fr", "en_US"], "en_US"), + ("de_DE", ["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, ["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, {}) From 49cdf22298331e4fa8fa7286bc99c051fd1cbc37 Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Thu, 29 Oct 2026 12:15:11 -0600 Subject: [PATCH 2/3] Match the locale identities supplied by translation discovery --- agent-notes/improve-language-selection.md | 8 +++++-- app_elements/i18n.py | 5 ----- tests/test_language.py | 26 +++++++++++++++-------- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/agent-notes/improve-language-selection.md b/agent-notes/improve-language-selection.md index 6869f57..968eb09 100644 --- a/agent-notes/improve-language-selection.md +++ b/agent-notes/improve-language-selection.md @@ -17,8 +17,12 @@ set_items. The earlier investigation's claim to the contrary was corrected. # Surprises -Babel resolves zh_CN to zh_Hans_CN, so matching must canonicalize the requested -locale as well as using the locale object's full identity. +PR #2's initial CI passed, but the Chinese test constructed locales with +Locale.parse, unlike the real i18n_core.locale_from_locale_id producer, which uses +Locale(language, territory). Parsing added a script absent from catalog identities. +Removed that unnecessary requested-locale parsing and corrected the existing +Chinese regression to use real producer-shaped objects for both CN and TW. +Persisting str(locale) now preserves the identity supplied by i18n_core exactly. # Next step diff --git a/app_elements/i18n.py b/app_elements/i18n.py index 86b19cd..96d14ba 100644 --- a/app_elements/i18n.py +++ b/app_elements/i18n.py @@ -1,7 +1,6 @@ import locale import sys -import babel import i18n_core from gui_builder import fields from wx_utils import forms as wx_forms @@ -40,10 +39,6 @@ def render(self, *args, **kwargs): index = 0 for requested in (application.locale, i18n_core.DEFAULT_LOCALE): requested = requested.split(".")[0].split("@")[0].replace("-", "_").casefold() - try: - requested = str(babel.Locale.parse(requested)).casefold() - except (babel.UnknownLocaleError, ValueError): - pass if requested in identifiers: index = identifiers.index(requested) break diff --git a/tests/test_language.py b/tests/test_language.py index 234eabc..7b7cfa4 100644 --- a/tests/test_language.py +++ b/tests/test_language.py @@ -8,20 +8,28 @@ class LanguageSelectionTests(unittest.TestCase): def panel(self, component, modules, identifiers): modules["i18n_core"].get_available_locales.return_value = [ - Locale.parse(identifier) for identifier in identifiers + Locale.parse(identifier) if isinstance(identifier, str) else identifier + for identifier in identifiers ] panel = component.LanguageSelectionPanel() panel.render() return panel - def test_qread_options_preserves_exact_regional_locale_on_save(self): - with load_component("app_elements.i18n", locale="zh-CN.UTF-8") as (component, app, modules): - panel = self.panel(component, modules, ["zh_TW", "zh_CN", "en_US"]) - index = next(n for n, item in enumerate(panel.locales) if str(item) == "zh_Hans_CN") - 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"], "zh_Hans_CN") + def test_existing_regional_catalog_identity_round_trips(self): + # i18n_core.locale_from_locale_id constructs Locale(language, territory), + # not Locale.parse (which adds a likely Chinese script). + for territory in ("CN", "TW"): + with self.subTest(territory=territory): + with load_component("app_elements.i18n", locale="zh-%s.UTF-8" % territory) as (component, app, modules): + panel = self.panel(component, modules, [ + Locale("zh", "TW"), Locale("zh", "CN"), Locale("en", "US") + ]) + identifier = "zh_%s" % 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): From 631f283a436c776f7617810c26c6ae899e9a8056 Mon Sep 17 00:00:00 2001 From: Christopher Toth Date: Thu, 29 Oct 2026 12:17:02 -0600 Subject: [PATCH 3/3] Exercise regional selection with supported catalog identities --- agent-notes/improve-language-selection.md | 15 +++++++----- tests/test_language.py | 29 +++++++++++------------ 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/agent-notes/improve-language-selection.md b/agent-notes/improve-language-selection.md index 968eb09..cb1c6b7 100644 --- a/agent-notes/improve-language-selection.md +++ b/agent-notes/improve-language-selection.md @@ -17,12 +17,15 @@ set_items. The earlier investigation's claim to the contrary was corrected. # Surprises -PR #2's initial CI passed, but the Chinese test constructed locales with -Locale.parse, unlike the real i18n_core.locale_from_locale_id producer, which uses -Locale(language, territory). Parsing added a script absent from catalog identities. -Removed that unnecessary requested-locale parsing and corrected the existing -Chinese regression to use real producer-shaped objects for both CN and TW. -Persisting str(locale) now preserves the identity supplied by i18n_core exactly. +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 diff --git a/tests/test_language.py b/tests/test_language.py index 7b7cfa4..0b79f94 100644 --- a/tests/test_language.py +++ b/tests/test_language.py @@ -8,23 +8,22 @@ class LanguageSelectionTests(unittest.TestCase): def panel(self, component, modules, identifiers): modules["i18n_core"].get_available_locales.return_value = [ - Locale.parse(identifier) if isinstance(identifier, str) else identifier - for identifier in identifiers + identifier for identifier in identifiers ] panel = component.LanguageSelectionPanel() panel.render() return panel def test_existing_regional_catalog_identity_round_trips(self): - # i18n_core.locale_from_locale_id constructs Locale(language, territory), - # not Locale.parse (which adds a likely Chinese script). - for territory in ("CN", "TW"): - with self.subTest(territory=territory): - with load_component("app_elements.i18n", locale="zh-%s.UTF-8" % territory) as (component, app, modules): + # 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("zh", "TW"), Locale("zh", "CN"), Locale("en", "US") + Locale("fr"), Locale("fr", "CA"), Locale("hi", "IN"), Locale("en", "US") ]) - identifier = "zh_%s" % territory + 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 @@ -33,7 +32,7 @@ def test_existing_regional_catalog_identity_round_trips(self): 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, ["fr_FR", "fr_CA", "en_US"]) + 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)) @@ -42,10 +41,10 @@ def test_regions_have_distinct_native_and_english_labels(self): def test_base_language_then_default_then_first_available(self): for requested, available, expected in [ - ("fr_BE", ["fr", "en_US"], "fr"), - ("fr_BE", ["fr_CA", "en_US"], "fr_CA"), - ("de_DE", ["fr", "en_US"], "en_US"), - ("de_DE", ["fr"], "fr"), + ("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): @@ -57,7 +56,7 @@ 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, ["en_US"]) + panel = self.panel(component, modules, [Locale("en", "US")]) panel.language.get_index.return_value = index panel.set_config_values() self.assertEqual(app.config, {})