Merge pull request #344 from pbs/OCTO-11088-fix-text-positioning

Octo 11088 fix text positioning

docs/changelog.rst

@@ -1,5 +1,13 @@
 Changelog
 ---------
+2.2.15
+^^^^^^
+- Prevent creating a repositioning command to the same coordinates.
+
+2.2.14
+^^^^^^
+- Fix an issue with WebVTT writer text positioning on break inside a cue.
+
 2.2.13
 ^^^^^^
 - Mid-row codes only add spaces only if there isn't one before.

pycaption/scc/specialized_collections.py

@@ -424,7 +424,7 @@ def interpret_command(self, command, next_command=None):
                 # command sets a different style (underline, plain)
                 # so we need to close italics if we have an open italics tag
                 # otherwise we ignore it
-                # if break is required,add style tag then break
+                # if break is required, add style tag then break
                 if self.last_style == "italics on":
                     self._collection.append(
                         _InstructionNode.create_italics_style(
@@ -446,7 +446,7 @@ def interpret_command(self, command, next_command=None):
         prev_text_node = self.get_previous_text_node()
         prev_node_is_break = prev_text_node is not None and any(
             x.is_explicit_break()
-            for x in self._collection[self._collection.index(prev_text_node) :]
+            for x in self._collection[self._collection.index(prev_text_node):]
         )
         if (
             command in MID_ROW_CODES
@@ -471,10 +471,12 @@ def _update_positioning(self, command):
 
         :type command: str
         """
+        is_offset = False
         if command in PAC_TAB_OFFSET_COMMANDS:
             prev_positioning = self._position_tracer.default
             tab_offset = PAC_TAB_OFFSET_COMMANDS[command]
             positioning = (prev_positioning[0], prev_positioning[1] + tab_offset)
+            is_offset = True
         else:
             first, second = command[:2], command[2:]
             try:
@@ -483,11 +485,25 @@ def _update_positioning(self, command):
             except KeyError:
                 # if not PAC or OFFSET we're not changing position
                 return
-        self._position_tracer.update_positioning(positioning)
+        offset_after_break = is_offset and self.has_break_before(self._collection)
+        if not offset_after_break:
+            # Tab offsets after line breaks will be ignored to avoid repositioning
+            self._position_tracer.update_positioning(positioning)
 
     def __iter__(self):
         return iter(_format_italics(self._collection))
 
+    @staticmethod
+    def has_break_before(collection):
+        if len(collection) == 0:
+            return False
+        for element in collection[::-1]:
+            if element._type == 0:
+                return False
+            elif element._type == 1:
+                return True
+        return False
+
     @classmethod
     def from_list(cls, stash_list, position_tracker):
         """Having received a list of instances of this class, creates a new

pycaption/scc/state_machines.py

@@ -42,12 +42,15 @@ def update_positioning(self, positioning):
         is_tab_offset = new_row == row and col + 1 <= new_col <= col + 3
         # One line below will be treated as line break, not repositioning
         if new_row == row + 1:
-            self._positions.append((new_row, new_col))
+            self._positions.append((new_row, col))
             self._break_required = True
             self._last_column = new_col
         # Tab offsets after line breaks will be ignored to avoid repositioning
         elif self._break_required and is_tab_offset:
             return
+        # force not to reposition on the same coordinates
+        elif positioning == current:
+            return
         else:
             # Reset the "current" position altogether.
             self._positions = [positioning]

pycaption/webvtt.py

@@ -394,7 +394,6 @@ def _group_cues_by_layout(self, nodes, caption_set):
             return []
 
         current_layout = None
-        current_node = None
 
         # A list with layout groups. Since WebVTT only support positioning
         # for different cues, each layout group has to be represented in a
@@ -408,18 +407,12 @@ def _group_cues_by_layout(self, nodes, caption_set):
                 if s and current_layout and node.layout_info != current_layout:
                     # If the positioning changes from one text node to
                     # another, a new WebVTT cue has to be created.
-                    row, column = node.position if node.position else (0, 0)
-                    prev_row, prev_column = (
-                        current_node.position if current_node.position else (0, 0)
-                    )
-                    if row != prev_row + 1:
-                        layout_groups.append((s, current_layout))
-                        s = ""
+                    layout_groups.append((s, current_layout))
+                    s = ""
                 # ATTENTION: This is where the plain unicode node content is
                 # finally encoded as WebVTT.
                 s += self._encode_illegal_characters(node.content) or " "
                 current_layout = node.layout_info
-                current_node = node
             elif node.type_ == CaptionNode.STYLE:
                 resulting_style = self._calculate_resulting_style(
                     node.content, caption_set

setup.py

@@ -20,7 +20,7 @@
 
 setup(
     name="pycaption",
-    version="2.2.13",
+    version="2.2.15",
     description="Closed caption converter",
     long_description=open(README_PATH).read(),
     author="Joe Norton",