From 40ddc98f5c040b9a44e8d061be1b011b3e91f3db Mon Sep 17 00:00:00 2001 From: Max Karolinskiy <41635752+mkarolin@users.noreply.github.com> Date: Wed, 17 Dec 2025 11:55:16 -0500 Subject: [PATCH] Fixes l10n scripts. (#32846) - Fixes validation of ICU plural strings. The previous pattern didn't allow for 'precise' (e.g =1) and 'ends on' (one) clauses to be present at the same time. - Fixes bug in adding installer strings when combining override xtb with the original xtb. --- script/lib/l10n/crowdin/pull.py | 70 ++++++++++++++++++++++----------- script/lib/l10n/grd_utils.py | 17 ++++---- 2 files changed, 56 insertions(+), 31 deletions(-) diff --git a/script/lib/l10n/crowdin/pull.py b/script/lib/l10n/crowdin/pull.py index 81c62b9fdaa..9ff1d2ca559 100755 --- a/script/lib/l10n/crowdin/pull.py +++ b/script/lib/l10n/crowdin/pull.py @@ -220,35 +220,61 @@ def create_xtb_format_translationbundle_tag(lang): return translationbundle_tag +def check_plural_string_clauses(all_text, clauses_text): + """Validates clauses of a plural ICU string""" + clause_pattern = re.compile( + r"(zero|one|two|few|many|other|=\d+)\s*\{([^{}]+|\{[^{}]+\})+\}") + clause_counts = {} + unmatched_parts = [] + last_end = 0 + for match in clause_pattern.finditer(clauses_text): + keyword = match.group(1) + clause_counts[keyword] = clause_counts.get(keyword, 0) + 1 + start, end = match.span() + unmatched_parts.append(clauses_text[last_end:start]) + last_end = end + unmatched_parts.append(clauses_text[last_end:]) + unmatched_text = (' '.join(unmatched_parts).strip()).strip() + if len(unmatched_text): + error = (f"Plural string has invalid format:\n{'-' * 10}\n" + f"{all_text}\n{'-' * 10}\nUnmatched parts:\n" + f"{unmatched_text}\n{'-' * 10}") + raise ValueError(error) + dups = [(key, value) for key, value in clause_counts.items() if value > 1] + if dups: + dups_details = ', '.join(f"'{key}' ({value} of them)" + for key, value in dups) + error = (f"Plural string has duplicate clauses: {dups_details}\n" + f"{'-' * 10}\n{all_text}\n{'-' * 10}") + raise ValueError(error) + # This is also a fall through when clause_counts is empty. + if 'other' not in clause_counts: + error = ("Plural string is missing the required 'other' clause:\n" + f"{'-' * 10}\n{all_text}\n{'-' * 10}") + raise ValueError(error) + + def check_plural_string_formatting(grd_string_content, translation_content): """Checks 'plural' string formatting in translations""" - pattern = re.compile(r"\s*{(.*,\s*plural,)(\s*offset:[0-2])?" - r"(\s*(=0|zero)\s*{(.*)})?" - r"(\s*(=1|one)\s*{(.*)})?" - r"(\s*(=2|two)\s*{(.*)})?" - r"(\s*(few)\s*{(.*)})?" - r"(\s*(many)\s*{(.*)})?" - r"(\s*other\s*{(.*)})?" - r"\s*}\s*$") - if pattern.match(grd_string_content) is not None: - if pattern.match(translation_content) is None: - error = ('Translation of plural string:\n' - '-----------\n' - f"{grd_string_content}\n" - '-----------\n' - 'does not match:\n' - '-----------\n' - f"{translation_content}\n" - '-----------\n') + outer_pattern = re.compile( + r"^\s*\{\s*([^,]*,\s*plural,)(\s*offset:[0-2])?(.*)\}\s*$", re.DOTALL) + match = outer_pattern.match(grd_string_content) + if match: + check_plural_string_clauses(grd_string_content, match.group(3)) + translation_match = outer_pattern.match(translation_content) + if not translation_match: + error = (f"Translation of plural string:\n{'-' * 10}\n" + f"{grd_string_content}\n{'-' * 10}\ndoes not match:\n" + f"{'-' * 10}\n{translation_content}\n{'-' * 10}") raise ValueError(error) + check_plural_string_clauses(translation_content, + translation_match.group(3)) else: # This finds plural strings that the pattern above doesn't catch leading_pattern = re.compile(r"\s*{.*,\s*plural,.*") if leading_pattern.match(grd_string_content) is not None: - error = ('Uncaught plural pattern:\n' - '-----------\n' - f"{grd_string_content}\n" - '-----------\n') + error = (f"Uncaught plural pattern:\n{'-' * 10}\n" + f"{grd_string_content}\n{'-' * 10}\n") raise ValueError(error) diff --git a/script/lib/l10n/grd_utils.py b/script/lib/l10n/grd_utils.py index 1fe19e1c725..f33e104b771 100755 --- a/script/lib/l10n/grd_utils.py +++ b/script/lib/l10n/grd_utils.py @@ -169,15 +169,14 @@ def update_xtbs_locally(grd_file_path, brave_source_root, only_for_lang): chromium_grd_strings = get_grd_strings( chromium_grd_file_path, validate_tags=False) # Special treatment for brave_strings.grd - brave_strings_string_ids = [] + extra_brave_strings_string_ids = [] if os.path.basename(grd_file_path) == 'brave_strings.grd': assert len(grd_strings) == len(chromium_grd_strings) + \ len(GOOGLE_CHROME_STRINGS_MIGRATION_MAP) + \ len(INSTALLER_STRINGS) - brave_strings_string_ids = remove_installer_strings( - remove_google_chrome_strings(grd_strings, - GOOGLE_CHROME_STRINGS_MIGRATION_MAP), - INSTALLER_STRINGS) + extra_brave_strings_string_ids = remove_google_chrome_strings( + grd_strings, GOOGLE_CHROME_STRINGS_MIGRATION_MAP) + \ + remove_installer_strings(grd_strings, INSTALLER_STRINGS) assert len(grd_strings) == len(chromium_grd_strings), ( f'String count in {grd_file_path} and in {chromium_grd_file_path} do' + f'not match: {len(grd_strings)} vs {len(chromium_grd_strings)}.') @@ -222,8 +221,8 @@ def update_xtbs_locally(grd_file_path, brave_source_root, only_for_lang): # Special treatment for brave_strings.grd if os.path.basename(grd_file_path) == 'brave_strings.grd': - add_google_chrome_translations(xtb_file, xml_tree, - brave_strings_string_ids) + add_extra_translations_from_brave_xtb( + xtb_file, xml_tree, extra_brave_strings_string_ids) transformed_content = (b'\n' + lxml.etree.tostring(xml_tree, pretty_print=True, @@ -428,8 +427,8 @@ def remove_installer_strings(brave_grd_strings, installer_string): return string_ids -def add_google_chrome_translations(brave_strings_xtb_file, xml_tree, - string_ids): +def add_extra_translations_from_brave_xtb(brave_strings_xtb_file, xml_tree, + string_ids): brave_xtb_tree = lxml.etree.parse(brave_strings_xtb_file) translationbundle = xml_tree.xpath('//translationbundle')[0] for string_id in string_ids: