chore: simplify format flags handling

Move merging to a single location where errors can be handled
consistently.

See #17816

Author: nijel

weblate/formats/base.py

@@ -15,6 +15,7 @@
 from django.http import HttpResponse
 from django.utils.functional import cached_property
 from django.utils.translation import gettext
+from pyparsing import ParseException
 from translate.misc.multistring import multistring
 from translate.storage.base import TranslationStore as TranslateToolkitStore
 from translate.storage.base import TranslationUnit as TranslateToolkitUnit
@@ -28,9 +29,10 @@
 from weblate.utils.state import STATE_EMPTY, STATE_TRANSLATED
 
 if TYPE_CHECKING:
-    from collections.abc import Callable, Iterator, Sequence
+    from collections.abc import Callable, Generator, Iterator, Sequence
 
     from django_stubs_ext import StrOrPromise
+    from lxml import etree
 
     from weblate.trans.file_format_params import FileFormatParams
     from weblate.trans.models import Component, Translation, Unit
@@ -153,6 +155,8 @@ class TranslationUnit:
     parent: TranslationFormat
     mainunit: InnerUnit
     empty_unit_ok: ClassVar[bool] = False
+    remove_flags: ClassVar[list[str]] = []
+    add_flags: ClassVar[list[str]] = []
 
     def __init__(
         self,
@@ -199,17 +203,30 @@ def locations(self) -> str:
         """Return comma separated list of locations."""
         return ""
 
-    @cached_property
-    def flags(self) -> Flags:
-        """Return flags or typecomments from units."""
-        flags = Flags()
+    def get_extra_flags(self) -> Generator[str | etree._Element | Flags]:
         # add default flags based location extensions
         for location in self.locations.split(","):
             _, extension = os.path.splitext(location.split(":")[0].strip())
             if extension == ".rst":
-                flags.merge("rst-text")
+                yield "rst-text"
             elif extension in {".md", ".markdown"}:
-                flags.merge("md-text")
+                yield "md-text"
+        yield from self.add_flags
+
+    @cached_property
+    def flags(self) -> Flags:
+        """Return flags or typecomments from units."""
+        flags = Flags()
+        for extra in self.get_extra_flags():
+            try:
+                flags.merge(extra)
+            except (ParseException, ValueError) as error:
+                msg = f"Could not parse flags: {extra}: {error}"
+                raise ValueError(msg) from error
+
+        for remove in self.remove_flags:
+            flags.remove(remove)
+
         return flags
 
     @cached_property

weblate/formats/convert.py

@@ -84,6 +84,8 @@ def source(self):
 
 
 class ConvertXliffUnit(XliffUnit):
+    remove_flags: ClassVar[list[str]] = ["xml-text"]
+
     def is_fuzzy(self, fallback=False):
         """Check whether unit needs editing."""
         return fallback
@@ -98,12 +100,6 @@ def is_translated(self):
             return self.has_translation()
         return self.unit is not None
 
-    @cached_property
-    def flags(self):
-        flags = super().flags
-        flags.remove("xml-text")
-        return flags
-
 
 class ConvertFormat(TranslationFormat):
     """

weblate/formats/multi.py

@@ -17,9 +17,14 @@
 from .ttkit import CSVFormat
 
 if TYPE_CHECKING:
+    from collections.abc import Generator
+
+    from lxml import etree
     from translate.storage.base import TranslationStore
     from translate.storage.base import TranslationUnit as TranslateToolkitUnit
 
+    from weblate.checks.flags import Flags
+
 
 class MultiUnit(TranslationUnit):
     units: list[TranslationUnit]
@@ -105,12 +110,11 @@ def set_state(self, state) -> None:
         for unit in self.units:
             unit.set_state(state)
 
-    @cached_property
-    def flags(self):
-        flags = super().flags
+    def get_extra_flags(self) -> Generator[str | etree._Element | Flags]:
+        yield from super().get_extra_flags()
+
         for unit in self.units:
-            flags.merge(unit.flags)
-        return flags
+            yield unit.flags
 
     def has_unit(self) -> bool:
         return all(unit.has_unit() for unit in self.units)

weblate/formats/ttkit.py

@@ -22,7 +22,6 @@
 from django.utils.translation import gettext, gettext_lazy
 from lxml import etree
 from lxml.etree import XMLSyntaxError
-from pyparsing import ParseException
 from translate.misc import quote
 from translate.misc.multistring import multistring
 from translate.misc.xml_helpers import setXMLspace
@@ -74,7 +73,7 @@
 )
 
 if TYPE_CHECKING:
-    from collections.abc import Callable
+    from collections.abc import Callable, Generator
 
     from translate.storage.base import TranslationUnit as TranslateToolkitUnit
 
@@ -180,19 +179,17 @@ def set_state(self, state) -> None:
         if hasattr(self.unit, "markapproved"):
             self.unit.markapproved(state == STATE_APPROVED)
 
-    @cached_property
-    def flags(self):
+    def get_extra_flags(self) -> Generator[str | etree._Element | Flags]:
         """
         Return flags from unit.
 
-        We currently extract maxwidth attribute.
+        We currently extract from XML.
         """
-        flags = super().flags
+        yield from super().get_extra_flags()
         if hasattr(self.unit, "xmlelement"):
-            flags.merge(self.unit.xmlelement)
+            yield self.unit.xmlelement
         if self.template is not None and hasattr(self.template, "xmlelement"):
-            flags.merge(self.template.xmlelement)
-        return flags
+            yield self.template.xmlelement
 
     def clone_template(self) -> None:
         super().clone_template()
@@ -577,6 +574,9 @@ def target(self):
 class PoUnit(TTKitUnit):
     """Wrapper for gettext PO unit."""
 
+    # Fuzzy flag is not useful, it is exposed as state instead
+    remove_flags: ClassVar[list[str]] = ["fuzzy"]
+
     def set_state(self, state) -> None:
         """Set fuzzy /approved flag on translated unit."""
         super().set_state(state)
@@ -585,17 +585,14 @@ def set_state(self, state) -> None:
             self.unit.prev_msgid_plural = []
             self.unit.prev_msgctxt = []
 
-    @cached_property
-    def flags(self):
-        """Return flags or typecomments from units."""
-        flags = super().flags
-        try:
-            flags.merge(Flags(*self.mainunit.typecomments))
-        except ParseException as error:
-            msg = f"Could not parse flags: {self.mainunit.typecomments!r}: {error}"
-            raise ValueError(msg) from error
-        flags.remove({"fuzzy"})
-        return flags
+    def get_extra_flags(self) -> Generator[str | etree._Element | Flags]:
+        """
+        Return flags from unit.
+
+        We currently extract from typecomments.
+        """
+        yield from super().get_extra_flags()
+        yield from self.mainunit.typecomments
 
     @cached_property
     def previous_source(self):
@@ -847,6 +844,8 @@ def target(self):
 class RichXliffUnit(XliffUnit):
     """Wrapper unit for XLIFF with XML elements."""
 
+    add_flags: ClassVar[list[str]] = ["xml-text"]
+
     @cached_property
     def source(self):
         """Return source string from a Translate Toolkit unit."""
@@ -871,12 +870,6 @@ def target(self):
 
         return rich_to_xliff_string(self.unit.rich_target)
 
-    @cached_property
-    def flags(self):
-        flags = super().flags
-        flags.merge("xml-text")
-        return flags
-
     def set_target(self, target: str | list[str]) -> None:
         """Set translation unit target."""
         self._invalidate_target()
@@ -1021,7 +1014,7 @@ def flags(self):
             # WebExtension placeholders
             placeholder_ids = [f"${key.upper()}$" for key in placeholders]
             flags.merge("case-insensitive")
-        flags.merge(f"placeholders:{Flags.format_flag(placeholder_ids)}")
+        flags.set_value("placeholders", Flags.format_flag(placeholder_ids))
         return flags
 
 

weblate/formats/txt.py

@@ -24,8 +24,11 @@
 from weblate.utils.errors import report_error
 
 if TYPE_CHECKING:
-    from collections.abc import Callable
+    from collections.abc import Callable, Generator
 
+    from lxml import etree
+
+    from weblate.checks.flags import Flags
     from weblate.trans.file_format_params import FileFormatParams
 
 
@@ -177,13 +180,10 @@ def context(self):
         """Return context of message."""
         return self.mainunit.location
 
-    @cached_property
-    def flags(self):
-        """Return flags from unit."""
-        flags = super().flags
+    def get_extra_flags(self) -> Generator[str | etree._Element | Flags]:
+        yield from super().get_extra_flags()
         if self.mainunit.flags:
-            flags.merge(self.mainunit.flags)
-        return flags
+            yield self.mainunit.flags
 
     def set_target(self, target: str | list[str]) -> None:
         """Set translation unit target."""