Concatenate/merge followups

.coveragerc

@@ -1,6 +1,7 @@
 [report]
 exclude_lines =
     NotImplemented
+    if TYPE_CHECKING:
     pragma: no cover
+    raise OptionError
     warnings.warn
-    if TYPE_CHECKING:

babel/messages/frontend.py

@@ -146,15 +146,15 @@ 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):
         if not self.finalized:
             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",
         )
@@ -964,16 +964,17 @@ 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(
+                key = template._key_for(message.id, message.context)
+                message_counts[key] += 1
+                message_strings[key].add(
                     message.string if isinstance(message.string, str) else tuple(message.string),
                 )
             templates.append((filename, template))
@@ -992,11 +993,12 @@ def run(self):
                 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:
+                if count > 1 and not self.use_first and len(message_strings[key]) > 1:
                     filename = os.path.basename(path)
                     catalog.add_conflict(message, filename, template.project, template.version)
                     message.flags |= {'fuzzy'}
@@ -1099,34 +1101,45 @@ def finalize_options(self):
 
     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
 
     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,
             no_fuzzy_matching=self.no_fuzzy_matching,
         )
 
         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()
+            current = catalog.get(message.id, message.context)
+            if current is None:  # The compendium does not add messages missing from the template.
+                continue
 
-                current.string = message.string
-                if current.fuzzy:
-                    current.flags.remove('fuzzy')
+            if current.string 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

babel/messages/pofile.py

@@ -28,6 +28,8 @@
 
 _unescape_re = re.compile(r'\\([\\trn"])')
 
+CONFLICT_MARKER = "#-#-#-#-#"
+
 
 def unescape(string: str) -> str:
     r"""Reverse `escape` the given string.
@@ -352,7 +354,7 @@ def parse(self, fileobj: IO[AnyStr] | Iterable[AnyStr]) -> None:
             if needs_decode:
                 line = line.decode(self.catalog.charset)
             if line[:1] == '#':
-                if line[1:2] == '-':
+                if line.startswith(CONFLICT_MARKER) and line.endswith(CONFLICT_MARKER):
                     self._invalid_pofile(line, lineno, 'cannot parse po file with conflicts')
 
                 if line[1:2] == '~':
@@ -649,36 +651,37 @@ 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:
+                try:
+                    string = string[plural_index]
+                except IndexError:
+                    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)):
@@ -751,8 +754,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'

tests/messages/frontend/test_concat.py

@@ -331,3 +331,124 @@ def test_conflicted_po_raises_on_read(tmp_path):
     with pytest.raises(PoFileError):
         with open(conflicted) as f:
             read_po(f, abort_invalid=True)
+
+
+def test_non_utf8_input(concat_cmd, tmp_path):
+    input_file = tmp_path / 'latin1.po'
+    output_file = tmp_path / 'output.po'
+    with open(input_file, 'wb') as file:
+        catalog = Catalog(locale='fr', charset='iso-8859-1')
+        catalog.add('coffee', string='café')
+        pofile.write_po(file, catalog)
+
+    concat_cmd.input_files = [str(input_file)]
+    concat_cmd.output_file = str(output_file)
+    concat_cmd.finalize_options()
+    concat_cmd.run()
+
+    with open(output_file, 'rb') as file:
+        assert pofile.read_po(file)['coffee'].string == 'café'
+
+
+def test_unique_treats_contextual_messages_as_distinct(concat_cmd, tmp_path):
+    input_files = []
+    for filename, context, string in (
+        ('button.po', 'button', 'Open button'),
+        ('menu.po', 'menu', 'Open menu'),
+    ):
+        path = tmp_path / filename
+        with open(path, 'wb') as file:
+            catalog = Catalog(locale='en')
+            catalog.add('Open', string=string, context=context)
+            pofile.write_po(file, catalog)
+        input_files.append(str(path))
+
+    output_file = tmp_path / 'output.po'
+    concat_cmd.input_files = input_files
+    concat_cmd.output_file = str(output_file)
+    concat_cmd.unique = True
+    concat_cmd.finalize_options()
+    concat_cmd.run()
+
+    with open(output_file, 'rb') as file:
+        catalog = pofile.read_po(file)
+    assert catalog.get('Open', 'button').string == 'Open button'
+    assert catalog.get('Open', 'menu').string == 'Open menu'
+
+
+def test_contextual_conflict_is_written(concat_cmd, tmp_path):
+    input_files = []
+    for filename, string in (('first.po', 'Open'), ('second.po', 'Öffnen')):
+        path = tmp_path / filename
+        with open(path, 'wb') as file:
+            catalog = Catalog(locale='de')
+            catalog.add('open', string=string, context='menu')
+            pofile.write_po(file, catalog)
+        input_files.append(str(path))
+
+    output_file = tmp_path / 'output.po'
+    concat_cmd.input_files = input_files
+    concat_cmd.output_file = str(output_file)
+    concat_cmd.finalize_options()
+    concat_cmd.run()
+
+    with open(output_file, 'rb') as file:
+        message = pofile.read_po(file).get('open', 'menu')
+    assert message.fuzzy
+    assert 'first.po' in message.string
+    assert 'Open' in message.string
+    assert 'second.po' in message.string
+    assert 'Öffnen' in message.string
+
+
+def test_contextual_plural_is_written(concat_cmd, tmp_path):
+    input_file = tmp_path / 'input.po'
+    output_file = tmp_path / 'output.po'
+    with open(input_file, 'wb') as file:
+        catalog = Catalog(locale='en')
+        catalog.add(
+            ('item', 'items'),
+            string=('One item', 'Many items'),
+            context='inventory',
+        )
+        pofile.write_po(file, catalog)
+
+    concat_cmd.input_files = [str(input_file)]
+    concat_cmd.output_file = str(output_file)
+    concat_cmd.finalize_options()
+    concat_cmd.run()
+
+    with open(output_file, 'rb') as file:
+        message = pofile.read_po(file).get(('item', 'items'), 'inventory')
+    assert message.string == ('One item', 'Many items')
+
+
+def test_plural_conflict_is_valid_po(concat_cmd, tmp_path):
+    input_files = []
+    for filename, strings in (
+        ('first.po', ('One item', 'Many items')),
+        ('second.po', ('Ein Element', 'Viele Elemente')),
+    ):
+        path = tmp_path / filename
+        with open(path, 'wb') as file:
+            catalog = Catalog(locale='en')
+            catalog.add(('item', 'items'), string=strings)
+            pofile.write_po(file, catalog)
+        input_files.append(str(path))
+
+    output_file = tmp_path / 'output.po'
+    concat_cmd.input_files = input_files
+    concat_cmd.output_file = str(output_file)
+    concat_cmd.finalize_options()
+    concat_cmd.run()
+
+    content = output_file.read_text()
+    assert content.count('msgstr[0]') == 1
+    assert content.count('msgstr[1]') == 1
+
+    with open(output_file, 'rb') as file:
+        message = pofile.read_po(file, abort_invalid=True)['item']
+    assert 'One item' in message.string[0]
+    assert 'Ein Element' in message.string[0]
+    assert 'Many items' in message.string[1]
+    assert 'Viele Elemente' in message.string[1]