Refactor XML processing

Refactored XML normalization and macro handling, including extraction of XML declaration and em-dash normalization into helper methods.

Author: adunning

src/style_variant_builder/build.py

@@ -9,14 +9,17 @@
 import subprocess
 import sys
 import tempfile
+from contextlib import suppress
 from dataclasses import dataclass
+from itertools import chain
 from pathlib import Path
 
 from lxml import etree
 
 from style_variant_builder.prune import CSLPruner
 
 logging.basicConfig(level=logging.INFO, format="%(message)s")
+TEMPLATE_SUFFIX = "-template.csl"
 
 
 class ColourFormatter(logging.Formatter):
@@ -99,7 +102,7 @@ def _get_diff_files(self) -> list[Path]:
                     f"Error reading diff file {diff_file.name}: {e}",
                     exc_info=True,
                 )
-        all_diffs = sorted(list(filname_diffs) + reference_diffs)
+        all_diffs = sorted(chain(filname_diffs, reference_diffs))
         if not all_diffs:
             raise FileNotFoundError(
                 f"No diff files found for style family '{self.style_family}' in {self.diffs_dir}"
@@ -146,8 +149,11 @@ def _insert_notice_comment_xml(self, pruner: CSLPruner) -> None:
             logging.error("Cannot insert notice comment: XML root is None.")
             return
 
-        comment_text = "This file was generated by the Style Variant Builder <https://github.com/citation-style-language/style-variant-builder>. To contribute changes, modify the template and regenerate variants."
-        comment_node = etree.Comment(f" {comment_text} ")
+        comment_node = etree.Comment(
+            " This file was generated by the Style Variant Builder "
+            "<https://github.com/citation-style-language/style-variant-builder>. "
+            "To contribute changes, modify the template and regenerate variants. "
+        )
         # Insert as the first child of the root <style> element
         pruner.root.insert(0, comment_node)
         # Insert a tail newline and indentation after the comment to ensure separation and correct indentation for <info>
@@ -226,8 +232,9 @@ def build_variants(self) -> tuple[int, int]:
                 )
                 self.failed_variants += 1
             finally:
-                if patched_file is not None and patched_file.exists():
-                    patched_file.unlink(missing_ok=True)
+                if patched_file is not None:
+                    with suppress(FileNotFoundError):
+                        patched_file.unlink()
 
         return (self.successful_variants, self.failed_variants)
 
@@ -262,7 +269,7 @@ def generate_diff_files(self) -> None:
                     f"Error reading development file {dev_file.name}: {e}",
                     exc_info=True,
                 )
-        dev_files = sorted(list(expected_dev_files) + additional_dev_files)
+        dev_files = sorted(chain(expected_dev_files, additional_dev_files))
 
         if not dev_files:
             logging.warning(
@@ -291,8 +298,7 @@ def generate_diff_files(self) -> None:
                     continue
                 diff_path = self.diffs_dir / dev_file.with_suffix(".diff").name
                 self.diffs_dir.mkdir(parents=True, exist_ok=True)
-                with diff_path.open("w", encoding="utf-8") as dfile:
-                    dfile.write("".join(diff))
+                diff_path.write_text("".join(diff), encoding="utf-8")
                 logging.info(f"Generated diff file: {diff_path}")
             except Exception as e:
                 logging.error(
@@ -356,12 +362,13 @@ def main() -> int:
     args = parser.parse_args()
 
     # Automatically determine style families by scanning template files.
-    template_files = list(args.templates_path.glob("*-template.csl"))
+    template_files = list(args.templates_path.glob(f"*{TEMPLATE_SUFFIX}"))
     if not template_files:
         logging.error(f"No template files found in {args.templates_path}.")
         return 1
     style_families = [
-        template.stem.replace("-template", "") for template in template_files
+        template.stem.removesuffix(TEMPLATE_SUFFIX.removesuffix(".csl"))
+        for template in template_files
     ]
 
     overall_success = True

src/style_variant_builder/prune.py

@@ -13,7 +13,13 @@
 from lxml import etree
 
 logging.basicConfig(level=logging.INFO, format="%(message)s")
-NS = "{http://purl.org/net/xbiblio/csl}"
+
+NSMAP = {"csl": "http://purl.org/net/xbiblio/csl"}
+
+
+def _tag(local_name: str) -> str:
+    """Helper to construct fully-qualified tag names."""
+    return f"{{{NSMAP['csl']}}}{local_name}"
 
 
 @dataclass(slots=True)
@@ -69,9 +75,8 @@ def collect_macro_definitions(self) -> None:
             logging.error(msg)
             raise ValueError(msg)
         self.macro_defs = {}
-        for elem in self.root.iter(f"{NS}macro"):
-            name = elem.attrib.get("name")
-            if name:
+        for elem in self.root.iter(_tag("macro")):
+            if name := elem.attrib.get("name"):
                 self.macro_defs[name] = elem
 
     def flatten_layout_macros(self) -> int:
@@ -90,30 +95,27 @@ def flatten_layout_macros(self) -> int:
             raise ValueError(msg)
 
         updated = 0
-        for layout in self.root.iter(f"{NS}layout"):
+        for layout in self.root.iter(_tag("layout")):
             # Consider only real element children, ignore comments/whitespace
             children = [ch for ch in list(layout) if isinstance(ch.tag, str)]
             if len(children) != 1:
                 continue
             only_child = children[0]
-            if only_child.tag != f"{NS}text":
+            if only_child.tag != _tag("text"):
                 continue
 
             # Require a pure macro call to avoid changing semantics if other attributes exist
             # Build a concrete dict[str, str] of attributes for type safety
             attrs: dict[str, str] = {
                 str(k): str(v) for k, v in only_child.attrib.items()
             }
-            macro_attr = attrs.get("macro")
             # Ensure the attribute is a string (lxml can expose AnyStr)
-            if not isinstance(macro_attr, str):
+            if not isinstance(macro_attr := attrs.get("macro"), str):
                 continue
-            macro_name: str = macro_attr
-            if not macro_name or len(attrs) != 1:
+            if not macro_attr or len(attrs) != 1:
                 continue
 
-            macro_def = self.macro_defs.get(macro_name)
-            if macro_def is None:
+            if (macro_def := self.macro_defs.get(macro_attr)) is None:
                 # Unknown macro; skip
                 continue
 
@@ -131,46 +133,46 @@ def flatten_layout_macros(self) -> int:
             self.build_parent_map()
         return updated
 
-    def gather_macro_refs(self, element: etree._Element) -> set:
+    def gather_macro_refs(self, element: etree._Element) -> set[str]:
         """Recursively collect macro names to which the element refers."""
-        refs = set()
-        macro_attr = element.attrib.get("macro")
-        if macro_attr:
+        refs: set[str] = set()
+        if macro_attr := element.attrib.get("macro"):
             refs.add(macro_attr)
         for child in element:
             refs.update(self.gather_macro_refs(child))
         return refs
 
-    def build_used_macros(self) -> set:
+    def build_used_macros(self) -> set[str]:
         if self.root is None:
             msg = "Root is None. Ensure parse_xml() is called successfully."
             logging.error(msg)
             raise ValueError(msg)
-        used_macros = set()
+        used_macros: set[str] = set()
 
         def add_macro_and_deps(macro_name: str) -> None:
             if macro_name in used_macros:
                 return
             used_macros.add(macro_name)
-            macro = self.macro_defs.get(macro_name)
-            if macro is not None:
+            if (macro := self.macro_defs.get(macro_name)) is not None:
                 refs = self.gather_macro_refs(macro)
                 for ref in refs:
                     add_macro_and_deps(ref)
 
         # Entry points: <citation> and <bibliography>
-        entry_tags = [f"{NS}citation", f"{NS}bibliography"]
-        entry_macro_refs = set()
-        for tag in entry_tags:
-            for entry in self.root.iter(tag):
-                entry_macro_refs.update(self.gather_macro_refs(entry))
+        entry_tags = [_tag("citation"), _tag("bibliography")]
+        entry_macro_refs = {
+            ref
+            for tag in entry_tags
+            for entry in self.root.iter(tag)
+            for ref in self.gather_macro_refs(entry)
+        }
         entry_macro_refs.update(self.gather_macro_refs(self.root))
         for ref in entry_macro_refs:
             add_macro_and_deps(ref)
         return used_macros
 
     def prune_macros(self) -> None:
-        total_removed = []
+        total_removed_count = 0
         while True:
             used_macros = self.build_used_macros()
             removed = []
@@ -183,15 +185,15 @@ def prune_macros(self) -> None:
                         logging.debug(f"Removed macro: {name}")
             if removed:
                 self.modified = True
-                total_removed.extend(removed)
+                total_removed_count += len(removed)
                 self.collect_macro_definitions()
                 self.build_parent_map()
             else:
                 logging.debug("No unused macros found.")
                 break
-        if total_removed:
+        if total_removed_count > 0:
             logging.info(
-                f"Removed a total of {len(total_removed)} unused macros."
+                f"Removed a total of {total_removed_count} unused macros."
             )
         else:
             logging.info("No macros pruned.")
@@ -200,16 +202,24 @@ def remove_xml_model_declarations(self, xml_data: str) -> str:
         """Remove any xml-model declarations from the XML content."""
         return re.sub(r"<\?xml-model [^>]+>\n?", "", xml_data)
 
-    def normalize_xml_content(self, xml_data: bytes) -> bytes:
-        """Revert Python changes to XML content and reorder the default-locale attribute in <style> tags."""
-        text = xml_data.decode("utf-8")
-        text = self.remove_xml_model_declarations(text)
-        text = re.sub(
+    def _normalize_xml_declaration(self, text: str) -> str:
+        """Ensure XML declaration uses double quotes."""
+        return re.sub(
             r"<\?xml version='1\.0' encoding='utf-8'\?>",
             '<?xml version="1.0" encoding="utf-8"?>',
             text,
         )
-        text = re.sub(r"—+", lambda m: "&#8212;" * len(m.group(0)), text)
+
+    def _escape_em_dashes(self, text: str) -> str:
+        """Replace em-dashes with numeric entities."""
+        return re.sub(r"—+", lambda m: "&#8212;" * len(m.group(0)), text)
+
+    def normalize_xml_content(self, xml_data: bytes) -> bytes:
+        """Revert Python changes to XML content and reorder the default-locale attribute in <style> tags."""
+        text = xml_data.decode("utf-8")
+        text = self.remove_xml_model_declarations(text)
+        text = self._normalize_xml_declaration(text)
+        text = self._escape_em_dashes(text)
 
         # Collapse XML fragments inside multi-line comments to match CSL repository indentation
         # Pattern captures the entire comment body (non-greedy) so we can post-process line breaks
@@ -232,12 +242,8 @@ def collapse_comment(match: re.Match) -> str:
                         seq.append(lines[i])
                         i += 1
                     # Collapse sequence
-                    first_indent_match = re.match(r"^([ \t]*)", seq[0])
-                    indent = (
-                        first_indent_match.group(1)
-                        if first_indent_match
-                        else ""
-                    )
+                    indent_match = re.match(r"^([ \t]*)", seq[0])
+                    indent = indent_match.group(1) if indent_match else ""
                     collapsed = indent + "".join(s.strip() for s in seq)
                     new_lines.append(collapsed)
                 else:
@@ -251,18 +257,16 @@ def collapse_comment(match: re.Match) -> str:
         # Reorder the default-locale attribute to the end in <style ...> tags.
         def reorder_default_locale(match: re.Match) -> str:
             attribs = match.group(1)
-            # Find all attributes in the tag.
             attrs = re.findall(r'(\S+="[^"]*")', attribs)
-            new_attrs: list[str] = []
-            default_locale_attr: str | None = None
-            for attr in attrs:
-                if attr.startswith("default-locale="):
-                    default_locale_attr = attr
-                else:
-                    new_attrs.append(attr)
-            if default_locale_attr is not None:
-                new_attrs.append(default_locale_attr)
-            new_attribs = " ".join(new_attrs)
+
+            # Separate default-locale from other attributes
+            other_attrs = [
+                a for a in attrs if not a.startswith("default-locale=")
+            ]
+            locale_attrs = [a for a in attrs if a.startswith("default-locale=")]
+
+            # Reorder: other attributes first, then locale
+            new_attribs = " ".join(other_attrs + locale_attrs)
             return f"<style {new_attribs}>"
 
         text = re.sub(r"<style\s+([^>]+)>", reorder_default_locale, text)
@@ -306,15 +310,9 @@ def save(self) -> None:
                 xml_data = self.reindent_xml_bytes(xml_data)
                 # Ensure XML declaration uses double quotes
                 xml_text = xml_data.decode("utf-8")
-                xml_text = re.sub(
-                    r"<\?xml version='1\.0' encoding='utf-8'\?>",
-                    '<?xml version="1.0" encoding="utf-8"?>',
-                    xml_text,
-                )
+                xml_text = self._normalize_xml_declaration(xml_text)
                 # lxml will decode character entities during reparse; restore em-dashes as numeric entities
-                xml_text = re.sub(
-                    r"—+", lambda m: "&#8212;" * len(m.group(0)), xml_text
-                )
+                xml_text = self._escape_em_dashes(xml_text)
                 self.output_path.write_text(xml_text, encoding="utf-8")
             except Exception as e:
                 logging.error(