Merge pull request #2187 from pick7/codex/remote-progress-return-type

Improve RemoteProgress parse return typing

Author: Byron

git/util.py

@@ -620,13 +620,17 @@ def __init__(self) -> None:
         self.error_lines: List[str] = []
         self.other_lines: List[str] = []
 
-    def _parse_progress_line(self, line: AnyStr) -> None:
+    def _parse_progress_line(self, line: AnyStr) -> Optional[object]:
         """Parse progress information from the given line as retrieved by
         :manpage:`git-push(1)` or :manpage:`git-fetch(1)`.
 
         - Lines that do not contain progress info are stored in :attr:`other_lines`.
         - Lines that seem to contain an error (i.e. start with ``error:`` or ``fatal:``)
           are stored in :attr:`error_lines`.
+
+        The base implementation returns ``None``. Subclasses may return another
+        value for compatibility with existing overrides, but callers should treat
+        the return value as unspecified.
         """
         # handle
         # Counting objects: 4, done.
@@ -641,7 +645,7 @@ def _parse_progress_line(self, line: AnyStr) -> None:
 
         if self._cur_line.startswith(("error:", "fatal:")):
             self.error_lines.append(self._cur_line)
-            return
+            return None
 
         cur_count, max_count = None, None
         match = self.re_op_relative.match(line_str)
@@ -651,7 +655,7 @@ def _parse_progress_line(self, line: AnyStr) -> None:
         if not match:
             self.line_dropped(line_str)
             self.other_lines.append(line_str)
-            return
+            return None
         # END could not get match
 
         op_code = 0
@@ -682,7 +686,7 @@ def _parse_progress_line(self, line: AnyStr) -> None:
             self.line_dropped(line_str)
             # Note: Don't add this line to the other lines, as we have to silently
             # drop it.
-            return
+            return None
         # END handle op code
 
         # Figure out stage.
@@ -708,6 +712,7 @@ def _parse_progress_line(self, line: AnyStr) -> None:
             max_count and float(max_count),
             message,
         )
+        return None
 
     def new_message_handler(self) -> Callable[[str], None]:
         """
@@ -717,7 +722,7 @@ def new_message_handler(self) -> Callable[[str], None]:
         """
 
         def handler(line: AnyStr) -> None:
-            return self._parse_progress_line(line.rstrip())
+            self._parse_progress_line(line.rstrip())
 
         # END handler
 

test/deprecation/test_remote_progress.py

@@ -0,0 +1,19 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Tests for static characteristics of remote progress helpers."""
+
+from typing import AnyStr
+
+from git import RemoteProgress
+
+
+def test_parse_progress_line_override_can_return_non_none() -> None:
+    """Subclass overrides may return a value without violating base typing."""
+
+    class ReturningProgress(RemoteProgress):
+        def _parse_progress_line(self, line: AnyStr) -> str:
+            super()._parse_progress_line(line)
+            return "handled"
+
+    assert ReturningProgress()._parse_progress_line("unmatched progress") == "handled"