diff --git a/.coveragerc b/.coveragerc index 32128ff86..5b463415b 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,6 +1,7 @@ [report] exclude_lines = NotImplemented + if TYPE_CHECKING: pragma: no cover + raise OptionError warnings.warn - if TYPE_CHECKING: diff --git a/babel/messages/catalog.py b/babel/messages/catalog.py index 5e6c28255..be6d13ba6 100644 --- a/babel/messages/catalog.py +++ b/babel/messages/catalog.py @@ -800,7 +800,7 @@ def add_conflict(self, message: Message, filename: str, project: str, version: s """ key = self._key_for(message.id, message.context) self._conflicts[key].append({ - 'message': message, + 'message': message.clone(), # The catalog copy is mutable 'filename': filename, 'project': project, 'version': version, @@ -813,8 +813,9 @@ def get_conflicts(self, id: _MessageID, context: str | None = None) -> list[Conf :param context: optional message context (msgctxt) :return: list of :class:`ConflictInfo` dicts, or an empty list if none """ - key = self._key_for(id, context) - return self._conflicts.get(key, []) + # Guard against catalogs serialized prior to conflicts existing + conflicts = getattr(self, '_conflicts', None) or {} + return conflicts.get(self._key_for(id, context), []) def add( self, diff --git a/babel/messages/frontend.py b/babel/messages/frontend.py index 89826e834..e06ed447f 100644 --- a/babel/messages/frontend.py +++ b/babel/messages/frontend.py @@ -149,7 +149,7 @@ def __init__(self, dist=None): self.help = 0 self.finalized = 0 - def initialize_options(self): + def initialize_options(self): # pragma: no cover pass def ensure_finalized(self): @@ -157,7 +157,7 @@ def ensure_finalized(self): self.finalize_options() self.finalized = 1 - def finalize_options(self): + def finalize_options(self): # pragma: no cover raise RuntimeError( f"abstract method -- subclass {self.__class__} must override", ) @@ -886,6 +886,17 @@ def run(self): return +def _is_translated(message) -> bool: + """Whether a message actually carries a translation. + + An empty msgstr (or a plural tuple of empty msgstrs) is not a definition, + it just means the message is known but untranslated. + """ + if isinstance(message.string, (list, tuple)): + return any(message.string) + return bool(message.string) + + class ConcatenateCatalog(CommandMixin): description = 'concatenates the specified PO files into single one' user_options = [ @@ -954,46 +965,73 @@ def finalize_options(self): def _collect_message_info(self): templates: list[tuple[str, Catalog]] = [] message_counts: Counter[_MessageID] = Counter() - message_strings: dict[_MessageID, set[str | tuple[str, ...]]] = defaultdict(set) + message_strings: dict[_MessageID, set[_MessageID]] = defaultdict(set) for filename in self.input_files: - with open(filename) as pofile: + with open(filename, 'rb') as pofile: template = read_po(pofile) for message in template: if not message.id: continue - message_counts[message.id] += 1 - message_strings[message.id].add( - message.string if isinstance(message.string, str) else tuple(message.string), - ) + key = template._key_for(message.id, message.context) + message_counts[key] += 1 + if _is_translated(message): + message_strings[key].add( + message.string if isinstance(message.string, str) else tuple(message.string), + ) templates.append((filename, template)) return templates, message_counts, message_strings def run(self): - catalog = Catalog(fuzzy=False) templates, message_counts, message_strings = self._collect_message_info() - for path, template in templates: - if catalog.locale is None: - catalog.locale = template.locale + # Take the header from the first input preferring one that has a locale. + header_source = next( + (template for _, template in templates if template.locale is not None), + templates[0][1], + ) + catalog = Catalog(fuzzy=False) + catalog.header_comment = header_source.header_comment + catalog.mime_headers = header_source.mime_headers + if len({template.charset for _, template in templates}) > 1: + # If charsets disagree, use UTF-8 as a common denominator. + catalog.charset = 'utf-8' + + for path, template in templates: for message in template: if not message.id: continue - count = message_counts[message.id] + key = template._key_for(message.id, message.context) + count = message_counts[key] if count <= self.more_than or (self.less_than is not None and count >= self.less_than): continue - if count > 1 and not self.use_first and len(message_strings[message.id]) > 1: - filename = os.path.basename(path) - catalog.add_conflict(message, filename, template.project, template.version) - message.flags |= {'fuzzy'} + translated = _is_translated(message) + conflicting = translated and not self.use_first and len(message_strings[key]) > 1 + current = catalog.get(message.id, message.context) + was_translated = current is not None and _is_translated(current) + was_fuzzy = current is not None and current.fuzzy catalog[message.id] = message + stored = catalog.get(message.id, message.context) + + if translated and not was_translated and stored.pluralizable == message.pluralizable: + stored.string = message.string + was_fuzzy = message.fuzzy + + if was_fuzzy: + stored.flags.add('fuzzy') + elif current is not None: + stored.flags.discard('fuzzy') + + if conflicting: + filename = os.path.basename(path) + catalog.add_conflict(message, filename, template.project, template.version) + stored.flags.add('fuzzy') - catalog.fuzzy = any(message.fuzzy for message in catalog) output_file = self.output_file if not output_file or output_file == '-': @@ -1082,9 +1120,14 @@ def finalize_options(self): self._finalize_width() + if self.compendium is None: + self.compendium = [] + elif isinstance(self.compendium, str): + self.compendium = [self.compendium] + def _get_messages_from_compendiums(self, compendium_paths): for file_path in compendium_paths: - with open(file_path) as pofile: + with open(file_path, 'rb') as pofile: catalog = read_po(pofile) for message in catalog: yield message, file_path @@ -1092,9 +1135,9 @@ def _get_messages_from_compendiums(self, compendium_paths): def run(self): def_file, ref_file = self.input_files - with open(def_file) as pofile: + with open(def_file, 'rb') as pofile: catalog = read_po(pofile) - with open(ref_file) as pofile: + with open(ref_file, 'rb') as pofile: ref_catalog = read_po(pofile) catalog.update( ref_catalog, @@ -1102,18 +1145,36 @@ def run(self): ) for message, compendium_path in self._get_messages_from_compendiums(self.compendium): - if (current := catalog.get(message.id)) and (not current.string or current.fuzzy or self.compendium_overwrite): - if self.compendium_overwrite and not current.fuzzy and current.string: - catalog.obsolete[message.id] = current.clone() + if not message.id or not _is_translated(message): + # Only translated compendium entries are of any use. + continue - current.string = message.string - if current.fuzzy: - current.flags.remove('fuzzy') + current = catalog.get(message.id, message.context) + if current is None: + # The compendium does not add messages missing from the template. + continue + + if current.pluralizable != message.pluralizable: + continue + + if _is_translated(current) and not current.fuzzy: + if not self.compendium_overwrite: + # Keep existing translations unless explicitly overwriting them. + continue + + # Preserve the translation being replaced as an obsolete message. + key = catalog._key_for(current.id, current.context) + catalog.obsolete[key] = current.clone() + + current.string = message.string + if message.fuzzy: + current.flags.add('fuzzy') + else: + current.flags.discard('fuzzy') - if not self.no_compendium_comment: - current.auto_comments.append(compendium_path) + if not self.no_compendium_comment: + current.auto_comments.append(compendium_path) - catalog.fuzzy = any(message.fuzzy for message in catalog) output_path = def_file if self.update else self.output_file if self.update and self.backup: diff --git a/babel/messages/pofile.py b/babel/messages/pofile.py index 63401f10d..c8b7cc868 100644 --- a/babel/messages/pofile.py +++ b/babel/messages/pofile.py @@ -29,6 +29,8 @@ _unescape_re = re.compile(r'\\([\\abfnrtv"])') _unescape_map = {'a': '\a', 'b': '\b', 'f': '\f', 'n': '\n', 'r': '\r', 't': '\t', 'v': '\v'} +CONFLICT_MARKER = "#-#-#-#-#" + def unescape(string: str) -> str: r"""Reverse `escape` the given string. @@ -644,36 +646,38 @@ def _format_comment(comment, prefix=''): for line in comment_wrapper.wrap(comment): yield f"#{prefix} {line.strip()}\n" - def _format_conflict_comment(file, project, version, prefix=''): - comment = f"#-#-#-#-# {file} ({project} {version}) #-#-#-#-#" - yield f"{normalize(comment, prefix=prefix, width=width)}\n" - - def _format_conflict(key: str | tuple[str, str], conflicts: list[ConflictInfo], prefix=''): + def _get_conflict_string(conflicts: list[ConflictInfo], plural_index: int | None = None) -> str: + parts = [] for conflict in conflicts: - message = conflict['message'] - if message.context: - yield from _format_conflict_comment(conflict['filename'], conflict['project'], conflict['version'], prefix=prefix) - yield f"{prefix}msgctxt {normalize(message.context, prefix=prefix, width=width)}\n" + parts.append( + f"{CONFLICT_MARKER} " + f"{conflict['filename']} ({conflict['project']} {conflict['version']})" + f" {CONFLICT_MARKER}", + ) + string = conflict['message'].string + if plural_index is not None: + if isinstance(string, (list, tuple)): + string = string[plural_index] if plural_index < len(string) else '' + elif plural_index: + # A singular translation only contributes to the singular form. + string = '' + parts.append(string) + return '\n'.join(parts) - if isinstance(key, (list, tuple)): - yield f"{prefix}msgid {normalize(key[0], prefix=prefix, width=width)}\n" - yield f"{prefix}msgid_plural {normalize(key[1], prefix=prefix, width=width)}\n" - else: - yield f"{prefix}msgid {normalize(key, prefix=prefix, width=width)}\n" - yield f"{prefix}msgstr {normalize('', prefix=prefix, width=width)}\n" + def _format_conflict(message, conflicts: list[ConflictInfo], prefix=''): + if message.context: + yield f"{prefix}msgctxt {normalize(message.context, prefix=prefix, width=width)}\n" - for conflict in conflicts: - message = conflict['message'] - yield from _format_conflict_comment(conflict['filename'], conflict['project'], conflict['version'], prefix=prefix) - if isinstance(key, (list, tuple)): - for idx in range(catalog.num_plurals): - try: - string = message.string[idx] - except IndexError: - string = '' - yield f"{prefix}msgstr[{idx:d}] {normalize(string, prefix=prefix, width=width)}\n" - else: - yield f"{normalize(message.string, prefix=prefix, width=width)}\n" + if isinstance(message.id, (list, tuple)): + yield f"{prefix}msgid {normalize(message.id[0], prefix=prefix, width=width)}\n" + yield f"{prefix}msgid_plural {normalize(message.id[1], prefix=prefix, width=width)}\n" + for idx in range(catalog.num_plurals): + string = _get_conflict_string(conflicts, plural_index=idx) + yield f"{prefix}msgstr[{idx:d}] {normalize(string, prefix=prefix, width=width)}\n" + else: + yield f"{prefix}msgid {normalize(message.id, prefix=prefix, width=width)}\n" + string = _get_conflict_string(conflicts) + yield f"{prefix}msgstr {normalize(string, prefix=prefix, width=width)}\n" def _format_message(message, prefix=''): if isinstance(message.id, (list, tuple)): @@ -745,8 +749,8 @@ def _format_message(message, prefix=''): norm_previous_id = normalize(message.previous_id[1], width=width) yield from _format_comment(f'msgid_plural {norm_previous_id}', prefix='|') - if len(conflicts := catalog.get_conflicts(message.id)) > 0: - yield from _format_conflict(message.id, conflicts) + if conflicts := catalog.get_conflicts(message.id, message.context): + yield from _format_conflict(message, conflicts) else: yield from _format_message(message) yield '\n' diff --git a/tests/messages/frontend/test_concat.py b/tests/messages/frontend/test_concat.py index 28a0091e9..0490e54f2 100644 --- a/tests/messages/frontend/test_concat.py +++ b/tests/messages/frontend/test_concat.py @@ -22,7 +22,7 @@ from babel.messages import Catalog, frontend, pofile from babel.messages.frontend import OptionError from tests.messages.consts import TEST_PROJECT_DISTRIBUTION_DATA -from tests.messages.utils import Distribution +from tests.messages.utils import Distribution, run_command, write_po_file @pytest.fixture(autouse=True) @@ -331,3 +331,82 @@ def test_conflicted_po_raises_on_read(tmp_path): with pytest.raises(PoFileError): with open(conflicted) as f: read_po(f, abort_invalid=True) + + +@pytest.mark.parametrize('plural_first', [True, False]) +def test_conflicts_with_context_and_plurals(concat_cmd, tmp_path, plural_first): + """Conflicts must be keyed by context and written as valid PO, plural or not.""" + file_plural = (('file', 'files'), ('eine Datei', '%d Dateien')) + file_singular = ('file', 'one file') + first = write_po_file(tmp_path / 'first.po', [ + ('open', 'Open', {'context': 'menu'}), + (('item', 'items'), ('One item', 'Many items')), + file_plural if plural_first else file_singular, + ], locale='de') + second = write_po_file(tmp_path / 'second.po', [ + ('open', 'Öffnen', {'context': 'menu'}), + (('item', 'items'), ('Ein Element', 'Viele Elemente')), + file_singular if plural_first else file_plural, + ], locale='de') + output_file = tmp_path / 'output.po' + catalog = run_command(concat_cmd, [first, second], output_file) + + message = catalog.get('open', 'menu') + assert message.fuzzy + for part in ('first.po', 'Open', 'second.po', 'Öffnen'): + assert part in message.string + singular, plural = catalog['item'].string + assert 'One item' in singular and 'Ein Element' in singular + assert 'Many items' in plural and 'Viele Elemente' in plural + # A singular/plural conflict keeps whole translations, not sliced characters. + content = output_file.read_text() + for translation in ('eine Datei', '%d Dateien', 'one file'): + assert translation in content + + +@pytest.mark.parametrize('translated_first', [True, False]) +def test_untranslated_entry_is_not_a_definition(concat_cmd, tmp_path, translated_first): + """An empty msgstr must not conflict with, or win over, a real translation.""" + translated = write_po_file(tmp_path / 'de.po', [ + ('hello', 'Guten Tag'), + (('one file', '%d files'), ('eine Datei', '%d Dateien')), + ], locale='de') + template = write_po_file(tmp_path / 'messages.pot', [('hello', None), (('one file', '%d files'), None)]) + inputs = [translated, template] if translated_first else [template, translated] + catalog = run_command(concat_cmd, inputs, tmp_path / 'output.po') + for id, string in (('hello', 'Guten Tag'), ('one file', ('eine Datei', '%d Dateien'))): + assert catalog[id].string == string + assert not catalog[id].fuzzy + + +def test_singular_translation_does_not_invade_a_plural_message(concat_cmd, tmp_path): + plural = write_po_file(tmp_path / 'plural.po', [(('one', 'many'), None)], locale='fi') + singular = write_po_file(tmp_path / 'singular.po', [('one', 'yksi')], locale='fi') + message = run_command(concat_cmd, [plural, singular], tmp_path / 'output.po')['one'] + assert message.pluralizable + assert not any(message.string) + + +def test_use_first_does_not_inherit_later_fuzzy_flag(concat_cmd, tmp_path): + first = write_po_file(tmp_path / 'first.po', [('hello', 'Hallo')], locale='de') + second = write_po_file(tmp_path / 'second.po', [('hello', 'Servus', {'flags': ['fuzzy']})], locale='de') + message = run_command(concat_cmd, [first, second], tmp_path / 'output.po', use_first=True)['hello'] + assert message.string == 'Hallo' + assert not message.fuzzy + + +def test_conflict_does_not_make_the_header_fuzzy(concat_cmd, po_files, tmp_path): + catalog = run_command(concat_cmd, po_files, tmp_path / 'output.po') + assert catalog['almost_same'].fuzzy + assert not catalog.fuzzy + + +def test_header_is_taken_from_an_input_with_a_locale(concat_cmd, tmp_path): + template = write_po_file(tmp_path / 'messages.pot', [(('file', 'files'), None)]) + translated = write_po_file( + tmp_path / 'pl.po', [(('file', 'files'), ('plik', 'pliki', 'plikow'))], + locale='pl', project='Foobar', version='1.0', + ) + catalog = run_command(concat_cmd, [template, translated], tmp_path / 'output.po') + assert (catalog.project, catalog.version) == ('Foobar', '1.0') + assert catalog['file'].string == ('plik', 'pliki', 'plikow') diff --git a/tests/messages/frontend/test_merge.py b/tests/messages/frontend/test_merge.py index 6e6c2f45f..ecdf7e2a9 100644 --- a/tests/messages/frontend/test_merge.py +++ b/tests/messages/frontend/test_merge.py @@ -14,6 +14,7 @@ import pathlib import shutil +import sys import pytest from freezegun import freeze_time @@ -21,7 +22,7 @@ from babel.messages import Catalog, frontend, pofile from babel.messages.frontend import OptionError from tests.messages.consts import TEST_PROJECT_DISTRIBUTION_DATA -from tests.messages.utils import Distribution +from tests.messages.utils import Distribution, run_command, write_po_file @pytest.fixture(autouse=True) @@ -327,3 +328,67 @@ def test_compendium_not_applied_for_absent_messages(merge_cmd, merge_files, tmp_ content = output_file.read_text() active_section = content.split('#~')[0] assert 'word5' not in active_section + + +def test_compendium_matches_message_context(merge_cmd, tmp_path): + def_file = write_po_file(tmp_path / 'def.po', [ + ('save', None), + ('save', 'Old translation', {'context': 'menu'}), + ], locale='es') + ref_file = write_po_file(tmp_path / 'ref.pot', [('save', None), ('save', None, {'context': 'menu'})]) + compendium = write_po_file(tmp_path / 'compendium.po', [('save', 'Guardar', {'context': 'menu'})], locale='es') + catalog = run_command( + merge_cmd, [def_file, ref_file], tmp_path / 'output.po', + compendium=[str(compendium)], compendium_overwrite=True, no_fuzzy_matching=True, + ) + assert catalog.get('save').string == '' + assert catalog.get('save', 'menu').string == 'Guardar' + assert catalog.obsolete[('save', 'menu')].string == 'Old translation' + + +def test_compendium_only_applies_usable_translations(merge_cmd, tmp_path): + ids = ['review', 'hello', ('item', 'items'), ('file', 'files'), 'dog'] + # Not utf-8, to make sure the inputs are decoded with their declared charset. + def_file = write_po_file(tmp_path / 'def.po', [ + ('review', None), + ('hello', 'Grüß dich', {'flags': ['fuzzy']}), + (('item', 'items'), None), + (('file', 'files'), None), + ('dog', None), + ], locale='de', charset='iso-8859-1') + ref_file = write_po_file(tmp_path / 'ref.pot', [(id, None) for id in ids]) + compendium = write_po_file(tmp_path / 'compendium.po', [ + ('review', 'Überprüfen', {'flags': ['fuzzy']}), + ('hello', None), + (('item', 'items'), ('Ein Element', 'Viele Elemente')), + ('file', 'eine Datei'), + (('dog', 'dogs'), ('ein Hund', 'Hunde')), + ], locale='de', charset='iso-8859-1') + catalog = run_command( + merge_cmd, [def_file, ref_file], tmp_path / 'output.po', + compendium=[str(compendium)], no_fuzzy_matching=True, + ) + assert catalog['review'].string == 'Überprüfen' + assert catalog['review'].fuzzy # the compendium's fuzzy flag is kept + assert catalog['hello'].string == 'Grüß dich' # not wiped out by an untranslated entry + assert catalog['item'].string == ('Ein Element', 'Viele Elemente') # untranslated plurals get filled + assert not any(catalog['file'].string) # a singular entry doesn't translate a plural... + assert catalog['dog'].string == '' # ...or vice versa + + +def test_cli_merge_without_compendium(merge_files, tmp_path, monkeypatch): + temp_def, temp_ref, _ = merge_files + output_file = tmp_path / 'output.po' + monkeypatch.setattr(sys, 'argv', ['pybabel']) + frontend.CommandLineInterface().run(['pybabel', 'merge', '-o', str(output_file), str(temp_def), str(temp_ref)]) + with open(output_file, 'rb') as file: + assert pofile.read_po(file)['word1'].string == 'Word 1' + + +def test_fuzzy_message_does_not_make_the_header_fuzzy(merge_cmd, tmp_path): + """A fuzzy header makes `pybabel compile` skip the whole catalog.""" + def_file = write_po_file(tmp_path / 'def.po', [('Hello world', 'Hallo Welt')], locale='de', fuzzy=False) + ref_file = write_po_file(tmp_path / 'ref.pot', [('Hello world!', None)]) + catalog = run_command(merge_cmd, [def_file, ref_file], tmp_path / 'output.po') + assert catalog['Hello world!'].fuzzy + assert not catalog.fuzzy diff --git a/tests/messages/test_pofile_write.py b/tests/messages/test_pofile_write.py index 6f02b5b42..0b6fe6ade 100644 --- a/tests/messages/test_pofile_write.py +++ b/tests/messages/test_pofile_write.py @@ -475,3 +475,14 @@ def test_a_template_declares_no_plural_forms(): buf = BytesIO() pofile.write_po(buf, catalog) assert 'Plural-Forms' not in buf.getvalue().decode() + + +def test_generate_po_without_conflict_support(): + """`generate_po` accepts catalog-like objects that predate conflict tracking.""" + catalog = Catalog(locale='de') + catalog.add('hello', string='Hallo') + del catalog._conflicts + + buf = BytesIO() + pofile.write_po(buf, catalog) + assert 'msgstr "Hallo"' in buf.getvalue().decode() diff --git a/tests/messages/utils.py b/tests/messages/utils.py index ecd8a2b26..870fdc84e 100644 --- a/tests/messages/utils.py +++ b/tests/messages/utils.py @@ -1,5 +1,10 @@ from __future__ import annotations +import pathlib + +from babel.messages.catalog import Catalog +from babel.messages.pofile import read_po, write_po + CUSTOM_EXTRACTOR_COOKIE = "custom extractor was here" @@ -22,3 +27,25 @@ def get_version(self) -> str: @property def packages(self) -> list[str]: return self.attrs['packages'] + + +def write_po_file(path: pathlib.Path, messages, **catalog_kwargs) -> pathlib.Path: + """Write `(id, string[, add_kwargs])` tuples to `path` as a PO file.""" + catalog = Catalog(**catalog_kwargs) + for id, string, *extra in messages: + catalog.add(id, string=string, **(extra[0] if extra else {})) + with open(path, 'wb') as file: + write_po(file, catalog) + return path + + +def run_command(cmd, input_files, output_file: pathlib.Path, **options) -> Catalog: + """Run a concat/merge command object and read back the catalog it wrote.""" + cmd.input_files = [str(path) for path in input_files] + cmd.output_file = str(output_file) + for name, value in options.items(): + setattr(cmd, name, value) + cmd.finalize_options() + cmd.run() + with open(output_file, 'rb') as file: + return read_po(file)