OCTO-11623

docs/changelog.rst

@@ -1,5 +1,15 @@
 Changelog
 ---------
+2.3.13
+^^^^^^
+  - Fix ``DFXPWriter``, ``LegacyDFXPWriter`` and ``SAMIWriter`` dropping the
+    non-breaking spaces that indent a cue's first line, which drew centred
+    and right-justified captions flush left. They are now written as
+    `` ``; indentation on later lines is unchanged.
+  - Fix ``DFXPReader`` and ``SAMIReader`` stripping non-breaking spaces at
+    the start of a line along with the markup's pretty-print whitespace, so
+    line indentation now survives a round trip.
+
 2.3.12
 ^^^^^^
   - Fix ``SCCReader`` misclassifying mid-line PACs by measuring them from

docs/conf.py

@@ -53,9 +53,9 @@
 # built documents.
 #
 # The short X.Y version.
-version = "2.3.12"
+version = "2.3.13.dev1"
 # The full version, including alpha/beta/rc tags.
-release = "2.3.12"
+release = "2.3.13.dev1"
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

pycaption/dfxp/extras.py

@@ -10,6 +10,7 @@
 from bs4 import BeautifulSoup
 
 from ..base import BaseWriter, CaptionNode, merge_concurrent_captions
+from ..utils import encode_leading_nbsp
 from .constants import DFXP_DEFAULT_REGION
 from .writer import DFXPWriter
 
@@ -204,7 +205,7 @@ def _recreate_p_tag(self, caption, caption_style, dfxp):
         start = caption.format_start()
         end = caption.format_end()
         p = dfxp.new_tag("p", begin=start, end=end)
-        p.string = self._recreate_text(caption, dfxp)
+        p.string = encode_leading_nbsp(self._recreate_text(caption, dfxp))
 
         if dfxp.find("style", {"xml:id": "p"}):
             p["style"] = "p"

pycaption/dfxp/reader.py

@@ -48,7 +48,8 @@
 )
 
 
-_LEADING_WHITESPACE_RE = re.compile("^(?:[\n\r]+\\s*)?(.+)")
+# Pretty-print indentation only: \s would also eat U+00A0 line indentation.
+_LEADING_WHITESPACE_RE = re.compile("^(?:[\n\r]+[ \t\n\r]*)?(.+)")
 
 _DFXP_WRITING_MODE_MAP = {
     "tbrl": WritingDirectionEnum.VERTICAL_RL,

pycaption/dfxp/writer.py

@@ -13,6 +13,7 @@
 
 from ..base import BaseWriter, CaptionNode
 from ..geometry import HorizontalAlignmentEnum, LineAlignmentEnum, WritingDirectionEnum
+from ..utils import encode_leading_nbsp
 from .constants import (
     DFXP_ATTR_XML_ID,
     DFXP_ATTR_XML_LANG,
@@ -206,7 +207,9 @@ def _recreate_p_tag(
         start = caption.format_start()
         end = caption.format_end()
         p = dfxp.new_tag("p", begin=start, end=end)
-        p.string = self._recreate_text(caption, dfxp, caption_set, lang)
+        p.string = encode_leading_nbsp(
+            self._recreate_text(caption, dfxp, caption_set, lang)
+        )
 
         if dfxp.find("style", {DFXP_ATTR_XML_ID: "p"}):
             p["style"] = "p"

pycaption/sami/reader.py

@@ -4,6 +4,8 @@
 styles, margin-based positioning, and multi-language content.
 """
 
+import re
+
 from bs4 import BeautifulSoup, NavigableString
 
 from ..base import BaseReader, Caption, CaptionList, CaptionNode, CaptionSet
@@ -14,6 +16,9 @@
 
 _TAG_TO_STYLE = {"i": "italics", "b": "bold", "u": "underline"}
 
+# A line break plus the pretty-print indentation of the closing tag.
+_TRAILING_WHITESPACE_RE = re.compile("[\n\r][ \t\n\r]*$")
+
 
 class SAMIReader(BaseReader):
     """Reads SAMI caption files into a CaptionSet.
@@ -203,8 +208,9 @@ def _translate_tag(self, tag, inherit_from=None):
         if isinstance(tag, NavigableString):
             tag_text = str(tag)
             if tag_text and tag_text[0] in "\n\r":
-                tag_text = tag_text.lstrip()
-            tag_text = tag_text.rstrip("\n\r")
+                # Not a bare lstrip(): U+00A0 is line indentation, not markup.
+                tag_text = tag_text.lstrip(" \t\n\r")
+            tag_text = _TRAILING_WHITESPACE_RE.sub("", tag_text)
             if not tag_text:
                 return
             self.line.append(CaptionNode.create_text(tag_text, inherit_from))

pycaption/sami/writer.py

@@ -11,6 +11,7 @@
 
 from ..base import BaseWriter, CaptionNode
 from ..geometry import HorizontalAlignmentEnum
+from ..utils import encode_leading_nbsp
 from .constants import HORIZONTAL_ALIGNMENT_MAP, SAMI_BASE_MARKUP
 
 
@@ -104,7 +105,7 @@ def _recreate_p_tag(self, caption, sami, lang, primary, captions):
             p["style"] = p_style
 
         p["class"] = self._recreate_p_lang(caption, lang, captions)
-        p.string = self._recreate_text(caption.nodes)
+        p.string = encode_leading_nbsp(self._recreate_text(caption.nodes))
 
         sync.append(p)
 

pycaption/utils.py

@@ -11,3 +11,16 @@ def is_leaf(element):
     if not name or name == "br":
         return True
     return False
+
+
+def encode_leading_nbsp(text):
+    """Write the leading non-breaking spaces of text as   entities.
+
+    prettify() strips the ends of a paragraph's text, and Python counts U+00A0
+    as whitespace, so the indentation of a cue's first line would be lost.
+
+    :type text: str
+    :rtype: str
+    """
+    stripped = text.lstrip("\xa0")
+    return " " * (len(text) - len(stripped)) + stripped

setup.py

@@ -21,7 +21,7 @@
 
 setup(
     name="pycaption",
-    version="2.3.12",
+    version="2.3.13.dev1",
     description="Closed caption converter",
     long_description=open(README_PATH).read(),
     author="Joe Norton",

tests/conftest.py

@@ -43,6 +43,7 @@
     sample_dfxp_with_framerate_no_multiplier,
     sample_dfxp_with_inherited_style,
     sample_dfxp_with_inline_style,
+    sample_dfxp_with_nbsp_indentation,
     sample_dfxp_with_nested_spans,
     sample_dfxp_with_positioning,
     sample_dfxp_with_properly_closing_spans_output,
@@ -83,6 +84,8 @@
     sample_sami_with_multi_lang,
     sample_sami_with_multiple_p,
     sample_sami_with_multiple_span_aligns,
+    sample_sami_with_nbsp_indentation,
+    sample_sami_with_nbsp_only_line,
     sample_sami_with_p_align,
     sample_sami_with_p_and_span_align,
     sample_sami_with_separate_multi_lang,

tests/fixtures/dfxp.py

@@ -1620,3 +1620,21 @@ def sample_dfxp_with_custom_tickrate():
   </div>
  </body>
 </tt>"""
+
+
+@pytest.fixture(scope="session")
+def sample_dfxp_with_nbsp_indentation():
+    return """\
+<?xml version="1.0" encoding="utf-8"?>
+<tt xml:lang="en" xmlns="http://www.w3.org/ns/ttml"
+    xmlns:tts="http://www.w3.org/ns/ttml#styling">
+ <body>
+  <div xml:lang="en-US">
+   <p begin="00:00:01.000" end="00:00:02.000">
+    &#160;&#160;&#160;MOLLY:<br/>
+    "The Story Knife."<br/>
+    &#160;&#160;Part one
+   </p>
+  </div>
+ </body>
+</tt>"""

tests/fixtures/sami.py

@@ -722,3 +722,38 @@ def sample_sami_missing_start():
     </P></SYNC>
     </BODY></SAMI>
     """
+
+
+@pytest.fixture(scope="session")
+def sample_sami_with_nbsp_indentation():
+    return """
+<SAMI><HEAD><STYLE TYPE="text/css">
+<!--
+.ENCC {Name: English; lang: en-US; SAMI_Type: CC;}
+--></STYLE></HEAD><BODY>
+<SYNC Start="1000"><P class="ENCC">
+    &#160;&#160;&#160;MOLLY:<br/>
+    "The Story Knife."<br/>
+    &nbsp;&nbsp;Part one
+</P></SYNC>
+<SYNC Start="2000"><P class="ENCC">&nbsp;</P></SYNC>
+</BODY></SAMI>
+"""
+
+
+@pytest.fixture(scope="session")
+def sample_sami_with_nbsp_only_line():
+    return """
+<SAMI><HEAD><STYLE TYPE="text/css">
+<!--
+.ENCC {Name: English; lang: en-US; SAMI_Type: CC;}
+--></STYLE></HEAD><BODY>
+<SYNC Start="1000"><P class="ENCC">
+    one<br/>
+    &nbsp;&nbsp;
+    <br/>
+    three
+   </P></SYNC>
+<SYNC Start="2000"><P class="ENCC">&nbsp;</P></SYNC>
+</BODY></SAMI>
+"""

tests/test_dfxp.py

@@ -453,6 +453,17 @@ def test_writing_mode_is_read(self):
             WritingDirectionEnum.VERTICAL_RL
         )
 
+    def test_nbsp_line_indentation_is_kept(self, sample_dfxp_with_nbsp_indentation):
+        caption = self.reader.read(sample_dfxp_with_nbsp_indentation).get_captions(
+            "en-US"
+        )[0]
+
+        assert _texts(caption) == [
+            "\xa0\xa0\xa0MOLLY:",
+            '"The Story Knife."',
+            "\xa0\xa0Part one",
+        ]
+
 
 def _layout_at(x, y):
     """Build a top-left anchored Layout at the given percentage origin."""
@@ -462,6 +473,11 @@ def _layout_at(x, y):
     )
 
 
+def _texts(caption):
+    """Return the content of a caption's text nodes."""
+    return [node.content for node in caption.nodes if node.type_ == CaptionNode.TEXT]
+
+
 def _caption_set(nodes, layout_info=None):
     """Wrap nodes in a single-caption, single-language CaptionSet."""
     caption = Caption(1000000, 2000000, nodes, layout_info=layout_info)
@@ -866,3 +882,32 @@ def test_an_empty_layout_still_gets_no_region_of_its_own(self):
 
         by_id, paragraph_region = self._regions(dfxp)
         assert list(by_id) == [paragraph_region]
+
+
+class TestDFXPWriterNbspIndentation:
+    def setup_class(self):
+        self.nodes = [
+            CaptionNode.create_text("\xa0\xa0\xa0MOLLY:"),
+            CaptionNode.create_break(),
+            CaptionNode.create_text('"The Story Knife."'),
+            CaptionNode.create_break(),
+            CaptionNode.create_text("\xa0\xa0Part one"),
+        ]
+
+    def test_only_first_line_indentation_is_written_as_entities(self):
+        dfxp = DFXPWriter().write(_caption_set(self.nodes))
+
+        assert (
+            '&#160;&#160;&#160;MOLLY:<br/>\n    "The Story Knife."<br/>\n'
+            "    \xa0\xa0Part one"
+        ) in dfxp
+
+    def test_indentation_survives_a_round_trip(self):
+        dfxp = DFXPWriter().write(_caption_set(self.nodes))
+
+        caption = DFXPReader().read(dfxp).get_captions("en-US")[0]
+        assert _texts(caption) == [
+            "\xa0\xa0\xa0MOLLY:",
+            '"The Story Knife."',
+            "\xa0\xa0Part one",
+        ]

tests/test_dfxp_extras.py

@@ -2,6 +2,7 @@
 
 from bs4 import BeautifulSoup
 
+from pycaption.base import Caption, CaptionList, CaptionNode, CaptionSet
 from pycaption.dfxp import (
     DFXP_DEFAULT_REGION,
     DFXP_DEFAULT_REGION_ID,
@@ -146,3 +147,22 @@ def test_default_style_is_written_to_output_file(
         result = LegacyDFXPWriter().write(caption_set)
 
         assert result.count("foxy_the_squirrel") == 2
+
+    def test_nbsp_line_indentation_is_kept(self):
+        nodes = [
+            CaptionNode.create_text("\xa0\xa0\xa0MOLLY:"),
+            CaptionNode.create_break(),
+            CaptionNode.create_text("\xa0\xa0Part one"),
+        ]
+        caption_set = CaptionSet(
+            {"en-US": CaptionList([Caption(1000000, 2000000, nodes)])}
+        )
+
+        result = LegacyDFXPWriter().write(caption_set)
+
+        assert "&#160;&#160;&#160;MOLLY:<br/>\n    \xa0\xa0Part one" in result
+        caption = DFXPReader().read(result).get_captions("en-US")[0]
+        assert [n.content for n in caption.nodes if n.type_ == CaptionNode.TEXT] == [
+            "\xa0\xa0\xa0MOLLY:",
+            "\xa0\xa0Part one",
+        ]

tests/test_sami.py

@@ -2,7 +2,13 @@
 
 import pytest
 
-from pycaption import CaptionReadNoCaptions, CaptionReadSyntaxError, SAMIReader
+from pycaption import (
+    CaptionReadNoCaptions,
+    CaptionReadSyntaxError,
+    SAMIReader,
+    SAMIWriter,
+)
+from pycaption.base import Caption, CaptionList, CaptionNode, CaptionSet
 from pycaption.exceptions import CaptionReadTimingError
 from pycaption.geometry import HorizontalAlignmentEnum, Size, UnitEnum  # noqa
 from tests.mixins import ReaderTestingMixIn
@@ -207,3 +213,59 @@ def test_proper_with_timestamps_with_multiple_paragraph(
 
         assert paragraph_1.start == paragraph_2.start
         assert paragraph_1.end == paragraph_2.end
+
+    def test_nbsp_line_indentation_is_kept(self, sample_sami_with_nbsp_indentation):
+        caption = self.reader.read(sample_sami_with_nbsp_indentation).get_captions(
+            "en-US"
+        )[0]
+
+        assert _texts(caption) == [
+            "\xa0\xa0\xa0MOLLY:",
+            '"The Story Knife."',
+            "\xa0\xa0Part one",
+        ]
+
+    def test_pretty_print_whitespace_after_a_line_is_dropped(
+        self, sample_sami_with_nbsp_only_line
+    ):
+        caption = self.reader.read(sample_sami_with_nbsp_only_line).get_captions(
+            "en-US"
+        )[0]
+
+        assert _texts(caption) == ["one", "\xa0\xa0", "three"]
+
+
+def _texts(caption):
+    """Return the content of a caption's text nodes."""
+    return [node.content for node in caption.nodes if node.type_ == CaptionNode.TEXT]
+
+
+class TestSAMIWriterNbspIndentation:
+    def setup_class(self):
+        nodes = [
+            CaptionNode.create_text("\xa0\xa0\xa0MOLLY:"),
+            CaptionNode.create_break(),
+            CaptionNode.create_text('"The Story Knife."'),
+            CaptionNode.create_break(),
+            CaptionNode.create_text("\xa0\xa0Part one"),
+        ]
+        caption = Caption(1000000, 2000000, nodes)
+        self.caption_set = CaptionSet({"en-US": CaptionList([caption])})
+
+    def test_only_first_line_indentation_is_written_as_entities(self):
+        sami = SAMIWriter().write(self.caption_set)
+
+        assert (
+            '&#160;&#160;&#160;MOLLY:<br/>\n    "The Story Knife."<br/>\n'
+            "    \xa0\xa0Part one"
+        ) in sami
+
+    def test_indentation_survives_a_round_trip(self):
+        sami = SAMIWriter().write(self.caption_set)
+
+        caption = SAMIReader().read(sami).get_captions("en-US")[0]
+        assert _texts(caption) == [
+            "\xa0\xa0\xa0MOLLY:",
+            '"The Story Knife."',
+            "\xa0\xa0Part one",
+        ]