diff --git a/docs/changelog.rst b/docs/changelog.rst index 8e419373..08a04751 100644 --- a/docs/changelog.rst +++ b/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 diff --git a/docs/conf.py b/docs/conf.py index 98b8e0a8..52359eb2 100644 --- a/docs/conf.py +++ b/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. diff --git a/pycaption/dfxp/extras.py b/pycaption/dfxp/extras.py index 383d3f1b..e0cc8fc6 100644 --- a/pycaption/dfxp/extras.py +++ b/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" diff --git a/pycaption/dfxp/reader.py b/pycaption/dfxp/reader.py index 25821c18..521e3f3b 100644 --- a/pycaption/dfxp/reader.py +++ b/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, diff --git a/pycaption/dfxp/writer.py b/pycaption/dfxp/writer.py index 8981c1ba..747d3964 100644 --- a/pycaption/dfxp/writer.py +++ b/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" diff --git a/pycaption/sami/reader.py b/pycaption/sami/reader.py index bc4a7601..08fb1107 100644 --- a/pycaption/sami/reader.py +++ b/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)) diff --git a/pycaption/sami/writer.py b/pycaption/sami/writer.py index d2dda8a8..257fb549 100644 --- a/pycaption/sami/writer.py +++ b/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) diff --git a/pycaption/utils.py b/pycaption/utils.py index d3efd654..e2bb1889 100644 --- a/pycaption/utils.py +++ b/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 diff --git a/setup.py b/setup.py index 2f41389a..1414d94a 100644 --- a/setup.py +++ b/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", diff --git a/tests/conftest.py b/tests/conftest.py index 50f93cce..2c73f3b6 100644 --- a/tests/conftest.py +++ b/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, diff --git a/tests/fixtures/dfxp.py b/tests/fixtures/dfxp.py index 0ab1a5e5..ab1388d4 100644 --- a/tests/fixtures/dfxp.py +++ b/tests/fixtures/dfxp.py @@ -1620,3 +1620,21 @@ def sample_dfxp_with_custom_tickrate(): """ + + +@pytest.fixture(scope="session") +def sample_dfxp_with_nbsp_indentation(): + return """\ + + + +
+

+    MOLLY:
+ "The Story Knife."
+   Part one +

+
+ +""" diff --git a/tests/fixtures/sami.py b/tests/fixtures/sami.py index 08d6382e..107b747a 100644 --- a/tests/fixtures/sami.py +++ b/tests/fixtures/sami.py @@ -722,3 +722,38 @@ def sample_sami_missing_start():

""" + + +@pytest.fixture(scope="session") +def sample_sami_with_nbsp_indentation(): + return """ + +

+    MOLLY:
+ "The Story Knife."
+   Part one +

 +

 

 + +""" + + +@pytest.fixture(scope="session") +def sample_sami_with_nbsp_only_line(): + return """ + +

+ one
+    +
+ three +

+

 

 + +""" diff --git a/tests/test_dfxp.py b/tests/test_dfxp.py index b96c5c3a..1c7cf576 100644 --- a/tests/test_dfxp.py +++ b/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 ( + '   MOLLY:
\n "The Story Knife."
\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", + ] diff --git a/tests/test_dfxp_extras.py b/tests/test_dfxp_extras.py index 673707b4..65e6415a 100644 --- a/tests/test_dfxp_extras.py +++ b/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 "   MOLLY:
\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", + ] diff --git a/tests/test_sami.py b/tests/test_sami.py index 0811ed1b..b0cab043 100644 --- a/tests/test_sami.py +++ b/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 ( + '   MOLLY:
\n "The Story Knife."
\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", + ]