From b83fdc743d852d5f69c03ceee702d017580623ea Mon Sep 17 00:00:00 2001 From: OlteanuRares Date: Mon, 5 Oct 2026 15:22:49 +0300 Subject: [PATCH] OCTO-11626 flag line-length violations caused by a missing PAC/row change --- .claude/skills/gotchas.md | 10 + docs/changelog.rst | 3 + pycaption/scc/reader.py | 41 +++- pycaption/scc/specialized_collections.py | 92 +++++++- pycaption/scc/state_machines.py | 37 +++ tests/conftest.py | 15 ++ tests/fixtures/scc.py | 278 +++++++++++++++++++++++ tests/test_scc.py | 186 ++++++++++++++- 8 files changed, 640 insertions(+), 22 deletions(-) diff --git a/.claude/skills/gotchas.md b/.claude/skills/gotchas.md index 195a20ca..935ea300 100644 --- a/.claude/skills/gotchas.md +++ b/.claude/skills/gotchas.md @@ -215,3 +215,13 @@ And: the samples decide the sensitivity. A plain-text sample reports "same" on a **Rule:** Spawn children with `sys.executable`, never a bare `python` — the interpreter running the script is the one guaranteed to exist. Any section that shells out gets an `except Exception` that degrades to a reported skip; a gate must never be able to cost the PR its report. Verify by running under `env -i PATH=/usr/bin:/bin`, which has `python3` but no `python`. **Applies to:** `check-last-pr`, any skill spawning a subprocess interpreter + +--- + +## 18. `Caption.get_text()` strips, so it hides leading/trailing whitespace changes + +**What happened:** Reviewing PR #436 (OCTO-11623, first-line NBSP indentation), a probe compared `caption.get_text()` before and after a DFXP round trip and showed the first line's NBSPs as lost — an apparent defect in the very thing the PR fixed. `Caption.get_text()` (`pycaption/base.py`) ends in `.strip()`, and Python's `str.strip()` counts U+00A0 as whitespace. The nodes were correct. + +**Rule:** Any check about indentation, NBSP, or edge whitespace must compare `caption.nodes` (or `get_text_nodes()`), never `get_text()`. The behavior gate's `text_of()` uses `get_text()` too, so it is blind to this whole class of change — its "no flow changed" says nothing about whitespace PRs. Its built-in samples also carry no NBSP indentation; add one when a PR touches whitespace handling. + +**Applies to:** `check-last-pr` diff --git a/docs/changelog.rst b/docs/changelog.rst index 08a04751..73726048 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -9,6 +9,9 @@ Changelog - 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. + - ``SCCReader`` line-length errors now end with ``(continues on the row + without a row change)`` when a line ran long because the next line's + text stayed on the same row with no PAC or row change of its own. 2.3.12 ^^^^^^ diff --git a/pycaption/scc/reader.py b/pycaption/scc/reader.py index c58e5313..24b1c4f1 100644 --- a/pycaption/scc/reader.py +++ b/pycaption/scc/reader.py @@ -112,6 +112,10 @@ _TIMECODE_RE = re.compile(r"\d{2}:\d{2}:\d{2}[:;](\d{1,2})") +# Appended to a too-long line whose text kept going on the row with no PAC or +# row change of its own. Matched downstream, so the wording is part of the API. +ROW_CONTINUATION_SUFFIX = " (continues on the row without a row change)" + class SCCReader(BaseReader): """Converts a given unicode string to a CaptionSet. @@ -243,12 +247,18 @@ def _validate_frame_numbers(self): def _validate_line_lengths(self): """Raise CaptionLineLengthError if any line exceeds 32 characters.""" - violations = [ - f"around {cap.format_start()} - {self._describe_line_length(line)}" - for cap in (c.to_real_caption() for c in self.caption_stash._collection) - for line in cap.get_text().split("\n") - if len(line) > 32 - ] + violations = [] + for precap in self.caption_stash._collection: + cap = precap.to_real_caption() + raw = "".join(cap.get_text_nodes()) + # get_text() strips the caption, dropping any lines it opens with + skipped = raw[: len(raw) - len(raw.lstrip())].count("\n") + for index, line in enumerate(cap.get_text().split("\n")): + if len(line) > 32: + description = self._describe_line_length( + line, (index + skipped) in precap.continued_rows + ) + violations.append(f"around {cap.format_start()} - {description}") if violations: raise CaptionLineLengthError( "32 character limit for caption cue in scc file.\n" @@ -256,17 +266,23 @@ def _validate_line_lengths(self): ) @staticmethod - def _describe_line_length(line): + def _describe_line_length(line, continues_row=False): """Describe a line too long for the row, separating the text from the - indentation that places it, which can be what pushes it off the row. + indentation that places it, which can be what pushes it off the row, + and naming a missing PAC or row change when that is what joined it. """ text = line.lstrip("\xa0") indent = len(line) - len(text) if not indent: - return f"{line} - Length {len(line)}" - return ( - f"{text} - Length {len(text)} plus a {indent}-column indent = {len(line)}" - ) + description = f"{line} - Length {len(line)}" + else: + description = ( + f"{text} - Length {len(text)} plus a {indent}-column indent " + f"= {len(line)}" + ) + if continues_row: + description += ROW_CONTINUATION_SUFFIX + return description @staticmethod def _fix_last_captions_without_ending(caption_list): @@ -312,6 +328,7 @@ def _translate_line(self, line): parts = r.findall(line.lower()) self.time_translator.start_at(parts[0][0]) + self.position_tracker.start_burst() word_list = parts[0][2].split(" ") for idx, word in enumerate(word_list): diff --git a/pycaption/scc/specialized_collections.py b/pycaption/scc/specialized_collections.py index f450a151..53e37e8e 100644 --- a/pycaption/scc/specialized_collections.py +++ b/pycaption/scc/specialized_collections.py @@ -53,6 +53,9 @@ def __init__(self, start=0, end=0): self.nodes = [] self.style = {} self.layout_info = None + # Indices of the lines whose text continued on the row without a PAC + # or row change, for the line length check to name the cause + self.continued_rows = set() def to_real_caption(self): """Convert this mutable holder into an immutable Caption instance. @@ -274,6 +277,7 @@ def create_and_store( _leftmost_line_starts(instructions) if caption_mode == "pop_on" else [] ) cue_index = 0 + line_index = 0 indenter = _LineIndenter(enabled=bool(leftmost_columns)) for instruction in instructions: @@ -289,8 +293,14 @@ def create_and_store( self._still_editing.append(caption) cue_index += 1 indenter.start_cue() + line_index = 0 continue + if instruction.is_explicit_break(): + line_index += 1 + elif instruction.continues_row: + caption.continued_rows.add(line_index) + text = indenter.text_for(instruction, position) _append_caption_node(caption, instruction, text, position) @@ -381,17 +391,39 @@ def add_chars(self, *chars): self._collection.append(node) self._position_tracer.acknowledge_position_changed() + # Where a continuation is recorded: on the overwrite marker if the text + # lands on columns a PAC pointed back over, so a row sent again in full + # can drop it without dropping one of text arriving later + continued = node overlap = self._position_tracer.consume_pending_overlap() if overlap and self._mark_overwrite(overlap, current_position): + continued = self._collection[-1] node = _InstructionNode.create_text(current_position) self._collection.append(node) + # The tracker is shared by the buffers, so the row it saw text on may + # be another buffer's: only this buffer's own text can be continued + if self._position_tracer.consume_row_continuation() and self._line_has_text(): + continued.continues_row = True + padding = self._position_tracer.consume_pending_padding() if padding: node.add_chars(" " * padding) node.add_chars(*chars) self._position_tracer.advance_cursor(len("".join(chars))) + def _line_has_text(self): + """Whether the line being written in this buffer already holds text + + :rtype: bool + """ + for node in reversed(self._collection): + if _ends_line(node): + return False + if node.is_text_node() and node.text: + return True + return False + def _settle_line_start(self): """Record on the break opening this line the column it now starts at @@ -645,7 +677,11 @@ def from_list(cls, stash_list, position_tracker): for idx, stash in enumerate(stash_list): # Resolved per stash: each is a row of its own, so an overwrite # is measured against its row rather than the rows joined together - new_collection.extend(_resolve_overwrites(stash._collection)) + new_collection.extend( + _clear_continuations_of_fitting_rows( + _resolve_overwrites(stash._collection) + ) + ) # use space to separate the stashes, but don't add final space if idx < len(stash_list) - 1: @@ -878,6 +914,10 @@ def __init__( # only place a later line's own column survives self.line_start = line_start self.columns = columns + # For text, whether it continued a row already holding text, with no + # PAC or row change of its own; for an overwrite, whether the text + # after it did + self.continues_row = False def add_chars(self, *args): """This being a text node, add characters to it. @@ -1099,7 +1139,10 @@ def _resolve_line_overwrites(line, line_start): """ if not any(node.is_overwrite() for node in line): return line - line = [copy.copy(node) if node.is_text_node() else node for node in line] + line = [ + copy.copy(node) if node.is_text_node() or node.is_overwrite() else node + for node in line + ] if line_start is None: line_start = next( ( @@ -1120,9 +1163,24 @@ def _resolve_line_overwrites(line, line_start): arriving += len(node.text) if line_start + _line_length(line) > 32 and arriving >= marker.columns: _drop_last_columns(line[:index], marker.columns) + if not _line_length(line[:index]): + # The row was sent again in full: nothing is left of it for + # the text arriving to have continued + _clear_continuations(line[: index + 1]) return _join_across_markers(line) +def _clear_continuations(nodes): + """Clear the row continuation of the text and overwrite nodes, the line's + own copies + + :type nodes: list[_InstructionNode] + """ + for node in nodes: + if node.is_text_node() or node.is_overwrite(): + node.continues_row = False + + def _join_across_markers(line): """Drop the overwrite markers, joining the text nodes each one split @@ -1138,7 +1196,11 @@ def _join_across_markers(line): for node in line: if node.is_overwrite(): after_marker = True + continued = node.continues_row continue + if after_marker and node.is_text_node(): + # The text the marker opened, a copy, takes on its continuation + node.continues_row |= continued if ( after_marker and node.is_text_node() @@ -1147,12 +1209,38 @@ def _join_across_markers(line): ): if node.text: joined[-1].add_chars(node.text) + joined[-1].continues_row |= node.continues_row else: joined.append(node) after_marker = False return joined +def _clear_continuations_of_fitting_rows(collection): + """Clear the row continuation of every line that fits the row + + Simulated roll-up joins its rows onto one line, which can run past column + 32 although no row did. The continuation has to be judged on its own row, + before the join, or it would take the blame for a length it did not cause. + + The nodes are cleared in place, not copied: the row's nodes are shared with + the stash it is re-read from, and a row that fits keeps fitting. + + :type collection: list[_InstructionNode] + :rtype: list[_InstructionNode] + """ + line = [] + for node in collection + [None]: + if node is not None and not _ends_line(node): + line.append(node) + continue + if _line_length(line) <= 32: + for line_node in line: + line_node.continues_row = False + line = [] + return collection + + def _drop_last_columns(line, columns): """Drop the last ``columns`` columns of text from the nodes of a line diff --git a/pycaption/scc/state_machines.py b/pycaption/scc/state_machines.py index 9ab8062f..d507dc5d 100644 --- a/pycaption/scc/state_machines.py +++ b/pycaption/scc/state_machines.py @@ -43,6 +43,10 @@ def __init__(self, positioning=None): self._pending_padding = 0 # Columns a PAC pointed back over, not yet known to be an overwrite. self._pending_overlap = 0 + # Text written next lands on a row that already holds text, reached + # with no PAC or row change of its own: a same-row PAC, or a later + # line of the file picking up where the row left off. + self._row_continued = False def update_positioning( self, positioning, column_jump_forces_reposition=False, is_offset=False @@ -171,6 +175,7 @@ def _jump_rows_forward(self, positioning, row, col, column_jump_forces_repositio self._pending_overlap = 0 self._pending_padding = 0 self._line_has_text = False + self._row_continued = False # A skipped row, or a row+1 jump with a large paint-on column jump: # use repositioning (new cue) else: @@ -218,6 +223,10 @@ def _continue_line(self, positioning, cursor, is_offset): self._last_column = new_col self._cursor_column = new_col else: + if not is_offset and self._line_has_text: + # A PAC on the row being written moves on along it instead of + # opening a row of its own + self._row_continued = True overlap = cursor - new_col if overlap < 0: # The columns between the text and the PAC are blank @@ -258,6 +267,7 @@ def _reposition(self, positioning): # and its text is no longer in front of the cursor to be overwritten self._pending_padding = 0 self._pending_overlap = 0 + self._row_continued = False def advance_cursor(self, count): """Notify the tracker that ``count`` columns of text were written. @@ -295,6 +305,22 @@ def consume_pending_overlap(self): overlap, self._pending_overlap = self._pending_overlap, 0 return overlap + def start_burst(self): + """Note that a new line of the file starts: text it writes before any + positioning command continues the row the previous line left off on. + """ + if self._line_has_text: + self._row_continued = True + + def consume_row_continuation(self): + """Return and clear whether the text arriving next continues a row + without a PAC or row change of its own. + + :rtype: bool + """ + continued, self._row_continued = self._row_continued, False + return continued + def cancel_pending_overlap(self): """Drop the overlap, the column the PAC pointed at being taken by a command rather than by text. @@ -361,6 +387,7 @@ def reset_for_new_caption(self): self._line_has_text = False self._pending_padding = 0 self._pending_overlap = 0 + self._row_continued = False # Reset positions to None so the next PAC sets position fresh self._positions = [None] @@ -405,9 +432,19 @@ def update_positioning( :param positioning: a tuple of ints (row, col) :type positioning: tuple[int] """ + # Text written before any PAC was placed on the default row + unplaced_row = ( + self.default[0] + if self._line_has_text and not any(self._positions) + else None + ) if positioning: self.default = positioning super().update_positioning( positioning, column_jump_forces_reposition, is_offset ) + if not is_offset and positioning and positioning[0] == unplaced_row: + # The PAC names the row that text already took, so the text after + # it carries on along the row instead of opening one of its own + self._row_continued = True diff --git a/tests/conftest.py b/tests/conftest.py index 2c73f3b6..33d2a51c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -97,6 +97,7 @@ sample_no_positioning_at_all_scc, sample_scc_backspace_before_a_pac_ahead_of_the_cursor, sample_scc_blank_columns_abandoned_before_a_full_line, + sample_scc_caption_opening_with_a_blank_line_then_continuing_its_row, sample_scc_created_dfxp_with_wrongly_closing_spans, sample_scc_doubled_mid_row_before_punctuation, sample_scc_doubled_pac_with_tab_offset, @@ -109,8 +110,12 @@ sample_scc_flashing_cue, sample_scc_frame_29, sample_scc_frame_30, + sample_scc_indented_line_on_the_same_row_without_a_pac, sample_scc_italics_still_open_at_repositioning, sample_scc_line_break_then_repositioning, + sample_scc_long_line_after_a_repositioning, + sample_scc_long_line_after_a_row_change, + sample_scc_long_line_then_a_line_continuing_its_row, sample_scc_mid_row_before_text_paint, sample_scc_mid_row_before_text_pop, sample_scc_mid_row_before_text_roll, @@ -129,6 +134,8 @@ sample_scc_multiple_positioning, sample_scc_no_explicit_end_to_last_caption, sample_scc_pac_pointing_back_over_a_full_row_with_less_text, + sample_scc_pac_resending_a_continued_row_in_full_past_the_last_column, + sample_scc_pac_resending_a_row_in_full_then_a_line_continuing_it, sample_scc_pac_restating_the_position_after_text, sample_scc_pac_within_a_tab_offset_of_a_mid_row_code, sample_scc_pac_within_a_tab_offset_of_an_overwrite, @@ -145,14 +152,19 @@ sample_scc_pop_on_row_and_column_jump_in_one_pac, sample_scc_pop_on_row_plus_one_large_column_jump, sample_scc_pop_on_tab_offset_on_the_line_below_an_empty_row, + sample_scc_pop_on_text_then_paint_on_line_without_a_pac, sample_scc_produces_captions_with_start_and_end_time_the_same, sample_scc_restated_origin_pac_refilling_a_long_line, + sample_scc_roll_up_long_line_after_a_carriage_return, + sample_scc_roll_up_next_line_without_a_carriage_return, sample_scc_roll_up_pac_restating_the_row_mid_row, sample_scc_roll_up_pac_restating_the_row_mid_word, sample_scc_roll_up_restated_row_rolled_into_a_long_line, sample_scc_roll_up_row_and_column_jump_in_one_pac, + sample_scc_roll_up_row_continued_without_a_pac_then_rolled, sample_scc_roll_up_row_plus_one_large_column_jump, sample_scc_roll_up_row_refilled_then_rolled, + sample_scc_roll_up_row_sent_in_one_go_then_rolled, sample_scc_roll_up_ru2, sample_scc_roll_up_ru3, sample_scc_row_jump_with_pending_reposition, @@ -168,6 +180,9 @@ sample_scc_tab_offset_after_line_break, sample_scc_tab_offset_part_way_through_a_line, sample_scc_tab_offset_resolving_behind_the_cursor, + sample_scc_text_before_its_pac_on_another_row, + sample_scc_text_before_its_pac_on_the_same_row, + sample_scc_two_cues_the_second_continuing_its_row, sample_scc_with_ampersand_character, sample_scc_with_extended_characters, sample_scc_with_italics, diff --git a/tests/fixtures/scc.py b/tests/fixtures/scc.py index 37e84d6f..4bcf10b0 100644 --- a/tests/fixtures/scc.py +++ b/tests/fixtures/scc.py @@ -1366,3 +1366,281 @@ def sample_scc_pac_within_a_tab_offset_of_a_mid_row_code(): 00:00:03:00\t942c 942f """ + + +# A roll-up row carrying on in the next line of the file with no Carriage +# Return to roll it up first: 40 columns on the base row. +@pytest.fixture(scope="session") +def sample_scc_roll_up_next_line_without_a_carriage_return(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9425 9425 94ad 94ad 9470 9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:02:00\tc1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t94ad 94ad + +00:00:04:00\t942c 942c + +""" + + +# "HI" at row 14, column 0, then a second line at row 15, column 8, whose text +# carries on in the next line of the file with no PAC: 30 characters, 8 +# columns right of the cue's origin. +@pytest.fixture(scope="session") +def sample_scc_indented_line_on_the_same_row_without_a_pac(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9440 c849 9475 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:02:00\tc1c2 43c4 4546 c7c8 494a + +00:00:03:00\t942c 942f + +""" + + +# "HI" places a first caption on row 15. The next one writes its first part +# with no PAC, so it lands on that row by default, then a PAC naming the same +# row carries the text on along it: 40 columns. +@pytest.fixture(scope="session") +def sample_scc_text_before_its_pac_on_the_same_row(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 9470 c849 942f + +00:00:02:00\t94ae 9420 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 942f + +00:00:03:00\t942c + +""" + + +# A PAC on row 14 with no text before the one on row 15, so the caption opens +# with an empty line, then the row 15 text carries on in the next line of the +# file with no PAC: 40 columns on the second line. +@pytest.fixture(scope="session") +def sample_scc_caption_opening_with_a_blank_line_then_continuing_its_row(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9440 9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:02:00\tc1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t942c 942f + +""" + + +# A two-line cue on rows 1 and 2, then a cue on row 14 whose text carries on +# in the next line of the file with no PAC: 40 columns on the second cue. +@pytest.fixture(scope="session") +def sample_scc_two_cues_the_second_continuing_its_row(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9140 c849 91e0 c849 9440 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:02:00\tc1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t942c 942f + +""" + + +# "HI" on row 14, then the next line of the file moves to row 15 before +# sending 40 columns in one go: the row changed, so nothing continued it. +@pytest.fixture(scope="session") +def sample_scc_long_line_after_a_row_change(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9440 c849 + +00:00:02:00\t9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t942c 942f + +""" + + +# "HI" on row 15, then the next line of the file repositions to row 1 before +# sending 40 columns in one go: a cue of its own, not a continued row. +@pytest.fixture(scope="session") +def sample_scc_long_line_after_a_repositioning(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9470 c849 + +00:00:02:00\t9140 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t942c 942f + +""" + + +# A roll-up row "HI" rolled up by a Carriage Return on a line of its own, then +# 40 columns sent in one go: the Carriage Return started a row of its own. +@pytest.fixture(scope="session") +def sample_scc_roll_up_long_line_after_a_carriage_return(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9425 9425 94ad 94ad 9470 9470 c849 + +00:00:02:00\t94ad 94ad + +00:00:03:00\tc1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:04:00\t942c 942c + +""" + + +# "HI" left in pop-on memory, then the next line of the file switches to +# paint-on and sends 40 columns in one go with no PAC: a memory of its own, +# so nothing on its row was continued. +@pytest.fixture(scope="session") +def sample_scc_pop_on_text_then_paint_on_line_without_a_pac(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 94ae 9470 c849 + +00:00:02:00\t9429 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t942c + +""" + + +# A 40-column line sent in one go on row 14, then a row 15 line whose text +# carries on in the next line of the file with no PAC: two lines too long, +# only the second because of a continued row. +@pytest.fixture(scope="session") +def sample_scc_long_line_then_a_line_continuing_its_row(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9440 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:02:00\t9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\tc1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:04:00\t942c 942f + +""" + + +# A roll-up row "ABCDE" carried on to "ABCDEFGHIJ" by the next line of the +# file with no PAC, then rolled up under three more rows. +@pytest.fixture(scope="session") +def sample_scc_roll_up_row_continued_without_a_pac_then_rolled(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9426 9426 94ad 94ad 9470 9470 c1c2 43c4 4580 + +00:00:02:00\t46c7 c849 4a80 + +00:00:03:00\t94ad 94ad + +00:00:04:00\t9470 cb4c cdce 4f80 + +00:00:05:00\t94ad 94ad + +00:00:06:00\t9470 d5d6 5758 d9da + +00:00:07:00\t94ad 94ad + +00:00:08:00\t9470 45ce c480 + +00:00:09:00\t94ad 94ad 942c + +""" + + +# As above, the first row "ABCDEFGHIJ" sent in one go. +@pytest.fixture(scope="session") +def sample_scc_roll_up_row_sent_in_one_go_then_rolled(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9426 9426 94ad 94ad 9470 9470 c1c2 43c4 4546 c7c8 494a + +00:00:03:00\t94ad 94ad + +00:00:04:00\t9470 cb4c cdce 4f80 + +00:00:05:00\t94ad 94ad + +00:00:06:00\t9470 d5d6 5758 d9da + +00:00:07:00\t94ad 94ad + +00:00:08:00\t9470 45ce c480 + +00:00:09:00\t94ad 94ad 942c + +""" + + +# "ABC" carried on to "ABCDE" by the next line of the file, then a PAC +# restating the row's origin sends the row again in full, 40 columns: the +# overwrite drops "ABCDE", so nothing of the row is left to have continued. +@pytest.fixture(scope="session") +def sample_scc_pac_resending_a_continued_row_in_full_past_the_last_column(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9470 c1c2 4380 + +00:00:02:00\tc445 + +00:00:03:00\t9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:04:00\t942c 942f + +""" + + +# As sample_scc_text_before_its_pac_on_the_same_row, the PAC naming row 14 +# rather than the row 15 the text before it took: no row was carried on. +@pytest.fixture(scope="session") +def sample_scc_text_before_its_pac_on_another_row(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 9470 c849 942f + +00:00:02:00\t94ae 9420 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 9440 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 942f + +00:00:03:00\t942c + +""" + + +# "ABCDE" sent again in full by a PAC restating the row's origin, 40 columns, +# then "XY" carried on by the next line of the file with no PAC: the resend +# continued nothing, but "XY" continued the row it left. +@pytest.fixture(scope="session") +def sample_scc_pac_resending_a_row_in_full_then_a_line_continuing_it(): + return """\ +Scenarist_SCC V1.0 + +00:00:01:00\t9420 942f 94ae 9420 9470 c1c2 43c4 4580 + +00:00:02:00\t9470 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 c1c2 43c4 4546 c7c8 494a cb4c cdce 4fd0 5152 d354 + +00:00:03:00\t58d9 + +00:00:04:00\t942c 942f + +""" diff --git a/tests/test_scc.py b/tests/test_scc.py index 2f9c537c..32378ef1 100644 --- a/tests/test_scc.py +++ b/tests/test_scc.py @@ -693,21 +693,28 @@ def test_roll_up_rows_are_settled_one_by_one_before_they_are_joined( simulate_roll_up=True, ) - assert "HE'S YOUNG AND WELL, OK ABCDEFGHIJ - Length 34" in str(exc_info.value) + # Joined by the simulation, not by a row continuing without a PAC + assert exc_info.value.args[0].split("\n")[2] == ( + "around 00:00:02.002 - HE'S YOUNG AND WELL, OK ABCDEFGHIJ - Length 34" + ) def test_pac_pointing_back_over_a_full_row_with_less_text_is_rejected( self, sample_scc_pac_pointing_back_over_a_full_row_with_less_text ): """Four characters cannot cover the 31 columns the PAC points back over, so dropping them all would publish "AWXYZ" as if it were the - row. The line is left long, for the length check to report. + row. The line is left long, for the length check to report, as text + a PAC naming the same row kept on it. """ with pytest.raises(CaptionLineLengthError) as exc_info: SCCReader().read( sample_scc_pac_pointing_back_over_a_full_row_with_less_text ) - assert "Length 36" in str(exc_info.value) + assert exc_info.value.args[0].split("\n")[2] == ( + "around 00:00:03.036 - ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefWXYZ - " + "Length 36 (continues on the row without a row change)" + ) def test_roll_up_row_refilled_then_rolled_keeps_the_refilled_row( self, sample_scc_roll_up_row_refilled_then_rolled @@ -1191,12 +1198,12 @@ def test_line_too_long(self, sample_scc_with_line_too_long): assert exc_info.value.args[0].startswith( "32 character limit for caption cue in scc file." ) - str_to_check = ( - "was Cal l l l l l l l l l l l l l l l l l l l l l l l l l l l l " - "Denison, a friend - Length 81" + # Sent in one go after its PAC, nothing tells too much text apart + # from a missing PAC, so no cause is named + assert exc_info.value.args[0].split("\n")[2] == ( + "around 00:00:04.733 - was Cal l l l l l l l l l l l l l l l l l l l " + "l l l l l l l l l Denison, a friend - Length 81" ) - assert "around 00:00:04.733" in exc_info.value.args[0].split("\n")[2] - assert str_to_check in exc_info.value.args[0].split("\n")[2] def test_line_too_long_names_the_indent_pushing_it_off_the_row( self, sample_scc_with_line_indented_past_the_last_column @@ -1212,6 +1219,169 @@ def test_line_too_long_names_the_indent_pushing_it_off_the_row( "Length 32 plus a 27-column indent = 59" ) + @pytest.mark.parametrize( + "fixture_name, simulate_roll_up, expected", + [ + ( + "sample_scc_indented_line_on_the_same_row_without_a_pac", + False, + "around 00:00:03.036 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJ - Length 30 " + "plus a 8-column indent = 38 " + "(continues on the row without a row change)", + ), + ( + "sample_scc_roll_up_next_line_without_a_carriage_return", + True, + "around 00:00:01.001 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40 (continues on the row without a row change)", + ), + ( + "sample_scc_text_before_its_pac_on_the_same_row", + False, + "around 00:00:02.769 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40 (continues on the row without a row change)", + ), + ( + "sample_scc_caption_opening_with_a_blank_line_then_continuing_its_row", + False, + "around 00:00:03.036 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40 (continues on the row without a row change)", + ), + ( + "sample_scc_two_cues_the_second_continuing_its_row", + False, + "around 00:00:03.036 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40 (continues on the row without a row change)", + ), + ( + "sample_scc_pac_resending_a_row_in_full_then_a_line_continuing_it", + False, + "around 00:00:04.037 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRSTXY - " + "Length 42 (continues on the row without a row change)", + ), + ], + ) + def test_line_too_long_names_the_missing_pac_or_row_change( + self, request, fixture_name, simulate_roll_up, expected + ): + """The next line's text kept going on the row with no PAC or Carriage + Return of its own, or text sent before any PAC was carried on by one + naming its row, so the error names that rather than reading as too + much text, even after a row sent again in full. A PAC naming the same + row is pinned by + test_pac_pointing_back_over_a_full_row_with_less_text_is_rejected. + The wording is matched downstream and must not change. + """ + with pytest.raises(CaptionLineLengthError) as exc_info: + SCCReader().read( + request.getfixturevalue(fixture_name), + simulate_roll_up=simulate_roll_up, + ) + + assert exc_info.value.args[0].split("\n")[2:] == [expected] + + @pytest.mark.parametrize( + "fixture_name, expected", + [ + ( + "sample_scc_long_line_after_a_row_change", + "around 00:00:03.036 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40", + ), + ( + "sample_scc_long_line_after_a_repositioning", + "around 00:00:03.036 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40", + ), + ( + "sample_scc_roll_up_long_line_after_a_carriage_return", + "around 00:00:02.002 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40", + ), + ( + "sample_scc_pac_resending_a_continued_row_in_full_past_the_last_column", + "around 00:00:04.037 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40", + ), + ( + "sample_scc_text_before_its_pac_on_another_row", + "around 00:00:02.769 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40", + ), + ], + ) + def test_line_too_long_after_a_row_of_its_own_names_no_cause( + self, request, fixture_name, expected + ): + """A row change, repositioning or Carriage Return between the lines of + the file starts a row of its own, so a long line sent after it is too + much text, not a continued row. So is a row a PAC sends again in full, + which leaves nothing of the row to continue, and text sent before any + PAC when the first one names another row. + """ + with pytest.raises(CaptionLineLengthError) as exc_info: + SCCReader().read(request.getfixturevalue(fixture_name)) + + assert exc_info.value.args[0].split("\n")[2:] == [expected] + + def test_line_too_long_after_a_switch_of_memory_names_no_cause( + self, sample_scc_pop_on_text_then_paint_on_line_without_a_pac + ): + """The row the text before the switch is on belongs to the pop-on + memory, so text written to another memory continues nothing on it. + """ + with pytest.raises(CaptionLineLengthError) as exc_info: + SCCReader().read(sample_scc_pop_on_text_then_paint_on_line_without_a_pac) + + assert exc_info.value.args[0].split("\n")[2:] == [ + "around 00:00:02.002 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - Length 40" + ] + + def test_line_too_long_names_the_cause_only_on_the_continued_line( + self, sample_scc_long_line_then_a_line_continuing_its_row + ): + """Of two lines too long in one caption, only the one that continued + its row without a PAC is said to. + """ + with pytest.raises(CaptionLineLengthError) as exc_info: + SCCReader().read(sample_scc_long_line_then_a_line_continuing_its_row) + + assert exc_info.value.args[0].split("\n")[2:] == [ + "around 00:00:04.037 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40", + "around 00:00:04.037 - ABCDEFGHIJKLMNOPQRSTABCDEFGHIJKLMNOPQRST - " + "Length 40 (continues on the row without a row change)", + ] + + def test_roll_up_row_continued_without_a_pac_rolls_as_one_sent_in_one_go( + self, + sample_scc_roll_up_row_continued_without_a_pac_then_rolled, + sample_scc_roll_up_row_sent_in_one_go_then_rolled, + ): + """Noting that a row was continued must not change how it is joined + to the rows rolled up under it, doubled separators included. + """ + continued, in_one_go = ( + [ + caption.get_text() + for caption in SCCReader() + .read(content, simulate_roll_up=True) + .get_captions("en-US") + ] + for content in ( + sample_scc_roll_up_row_continued_without_a_pac_then_rolled, + sample_scc_roll_up_row_sent_in_one_go_then_rolled, + ) + ) + + assert continued == in_one_go + assert in_one_go == [ + "ABCDEFGHIJ", + "ABCDEFGHIJ KLMNO", + "ABCDEFGHIJ KLMNO UVWXYZ", + "KLMNO UVWXYZ END", + ] + def test_mid_row_codes_not_adding_space_before_text( self, sample_scc_mid_row_before_text_pop,