dulwich.cli: Reconcile parse_commit_range and parse_committish_range (#1705)

dulwich/cli.py

@@ -44,7 +44,7 @@ from .client import GitProtocolError, get_transport_and_path
 from .errors import ApplyDeltaError
 from .index import Index
 from .objects import valid_hexsha
-from .objectspec import parse_committish_range
+from .objectspec import parse_commit_range
 from .pack import Pack, sha_to_hex
 from .repo import Repo
 
@@ -2477,7 +2477,7 @@ class cmd_format_patch(Command):
         committish = None
         if args.committish:
             with Repo(".") as r:
-                range_result = parse_committish_range(r, args.committish)
+                range_result = parse_commit_range(r, args.committish)
                 if range_result:
                     committish = range_result
                 else:
@@ -2555,10 +2555,10 @@ class cmd_bundle(Command):
         elif parsed_args.refs:
             for ref_arg in parsed_args.refs:
                 if ".." in ref_arg:
-                    range_result = parse_committish_range(repo, ref_arg)
+                    range_result = parse_commit_range(repo, ref_arg)
                     if range_result:
                         start_commit, end_commit = range_result
-                        prerequisites.append(start_commit)
+                        prerequisites.append(start_commit.id)
                         # For ranges like A..B, we need to include B if it's a ref
                         # Split the range to get the end part
                         end_part = ref_arg.split("..")[1]

dulwich/objectspec.py

@@ -21,7 +21,6 @@
 
 """Object specification."""
 
-from collections.abc import Iterator
 from typing import TYPE_CHECKING, Optional, Union
 
 from .objects import Commit, ShaFile, Tag, Tree
@@ -210,9 +209,9 @@ def parse_refs(container, refspecs):
     return ret
 
 
-def parse_committish_range(
+def parse_commit_range(
     repo: "Repo", committish: Union[str, bytes]
-) -> Optional[tuple[bytes, bytes]]:
+) -> Optional[tuple["Commit", "Commit"]]:
     """Parse a string referring to a commit range.
 
     Args:
@@ -221,7 +220,7 @@ def parse_committish_range(
 
     Returns:
       None if committish is a single commit reference
-      A tuple of (start_commit_id, end_commit_id) if it's a range
+      A tuple of (start_commit, end_commit) if it's a range
     Raises:
       KeyError: When the commits can not be found
       ValueError: If the range can not be parsed
@@ -240,25 +239,7 @@ def parse_committish_range(
     start_commit = parse_commit(repo, start_ref)
     end_commit = parse_commit(repo, end_ref)
 
-    return (start_commit.id, end_commit.id)
-
-
-def parse_commit_range(
-    repo: "Repo", committishs: Union[str, bytes]
-) -> Iterator["Commit"]:
-    """Parse a string referring to a range of commits.
-
-    Args:
-      repo: A `Repo` object
-      committishs: A string referring to a range of commits.
-    Returns: An iterator over `Commit` objects
-    Raises:
-      KeyError: When the reference commits can not be found
-      ValueError: If the range can not be parsed
-    """
-    committishs = to_bytes(committishs)
-    # TODO(jelmer): Support more than a single commit..
-    return iter([parse_commit(repo, committishs)])
+    return (start_commit, end_commit)
 
 
 class AmbiguousShortId(Exception):

dulwich/porcelain.py

@@ -4653,7 +4653,15 @@ def format_patch(
                 pass
         elif isinstance(committish, tuple):
             # Handle commit range (start, end)
-            start_id, end_id = committish
+            start_commit, end_commit = committish
+
+            # Extract commit IDs from commit objects if needed
+            from .objects import Commit
+
+            start_id = (
+                start_commit.id if isinstance(start_commit, Commit) else start_commit
+            )
+            end_id = end_commit.id if isinstance(end_commit, Commit) else end_commit
 
             # Walk from end back to start
             walker = r.get_walker(include=[end_id], exclude=[start_id])

tests/test_objectspec.py

@@ -59,12 +59,21 @@ class ParseCommitRangeTests(TestCase):
 
     def test_nonexistent(self) -> None:
         r = MemoryRepo()
-        self.assertRaises(KeyError, parse_commit_range, r, "thisdoesnotexist")
+        self.assertRaises(KeyError, parse_commit_range, r, "thisdoesnotexist..HEAD")
 
     def test_commit_by_sha(self) -> None:
         r = MemoryRepo()
         c1, c2, c3 = build_commit_graph(r.object_store, [[1], [2, 1], [3, 1, 2]])
-        self.assertEqual([c1], list(parse_commit_range(r, c1.id)))
+        self.assertIsNone(parse_commit_range(r, c1.id))
+
+    def test_commit_range(self) -> None:
+        r = MemoryRepo()
+        c1, c2, c3 = build_commit_graph(r.object_store, [[1], [2, 1], [3, 1, 2]])
+        result = parse_commit_range(r, f"{c1.id.decode()}..{c2.id.decode()}")
+        self.assertIsNotNone(result)
+        start_commit, end_commit = result
+        self.assertEqual(c1, start_commit)
+        self.assertEqual(c2, end_commit)
 
 
 class ParseCommitTests(TestCase):