Skip to content

Commit bc63b02

Browse files
Byroncodex
Byron
and
codex
committed
fix: restore Actor.name_email_regex (#2220)
<!-- agent --> GitPython 3.1.60 removed the Actor.name_email_regex class attribute while replacing internal actor parsing, which broke downstream consumers such as python-semantic-release. Bring back the historical compiled pattern for downstream compatibility, but emit DeprecationWarning whenever the class member is accessed. The warning explains its quadratic behavior on long malformed strings and points callers to the public Actor(name, email) constructor when fields are separate, or direct string parsing when they are not. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com> # 252a84ac Add public Actor.from_string constructor (#2220) Add public Actor.from_string constructor (#2220) Promote the existing actor identity parser to Actor.from_string while retaining _from_string as a compatibility alias. Point the name_email_regex deprecation warning to the new public constructor or Actor(name, email). Validation: - actor-related test/test_actor.py and test/test_util.py cases: 12 passed - ruff check and format checks passed - basedpyright git/util.py: no errors Co-authored-by: Codex GPT-5 <codex@openai.com> # dd2181d8 Address review feedback about Actor subclasses Address review feedback about Actor subclasses Review feedback: Actor.from_string() should preserve the class on which the public classmethod is called instead of always returning a base Actor. Construct through cls on both parsing paths and cover subclass construction directly. Validation: - actor-related test/test_actor.py and test/test_util.py cases: 13 passed - ruff check and format checks passed - basedpyright git/util.py: no errors # fcf1ecb1 Address review feedback about constructor typing Address review feedback about constructor typing Review feedback: Actor.from_string() constructs subclasses at runtime but its Actor return annotation prevents type checkers from preserving subclass-specific members. Use the project existing Python 3.7-compatible bound TypeVar pattern so the declared return type follows cls. Validation: - actor-related test/test_actor.py and test/test_util.py cases: 13 passed - ruff check and format checks passed - basedpyright git/util.py: no errors Co-authored-by: Codex GPT-5 <codex@openai.com>
1 parent a9fb008 commit bc63b02

2 files changed

Lines changed: 49 additions & 12 deletions

File tree

‎git/util.py‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
Sequence,
8181
Tuple,
8282
TYPE_CHECKING,
83+
Type,
8384
TypeVar,
8485
Union,
8586
cast,
@@ -108,6 +109,7 @@
108109

109110
T_IterableObj = TypeVar("T_IterableObj", bound=Union["IterableObj", "Has_id_attribute"], covariant=True)
110111
# So IterableList[Head] is subtype of IterableList[IterableObj].
112+
T_Actor = TypeVar("T_Actor", bound="Actor")
111113

112114
_logger = logging.getLogger(__name__)
113115

@@ -853,11 +855,27 @@ def update(self, *args: Any, **kwargs: Any) -> None:
853855
self._callable(*args, **kwargs)
854856

855857

858+
class _DeprecatedActorNameEmailRegex:
859+
_pattern = re.compile(r"(.*) <(.*?)>")
860+
861+
def __get__(self, _instance: Any, _owner: Any) -> Pattern[str]:
862+
warnings.warn(
863+
"Actor.name_email_regex is deprecated and will be removed in GitPython 4.0.0 because searching long "
864+
"malformed strings with it can take quadratic time. Use Actor.from_string() to parse actor identities, "
865+
"or Actor(name, email) when the fields are already separate.",
866+
DeprecationWarning,
867+
stacklevel=2,
868+
)
869+
return self._pattern
870+
871+
856872
class Actor:
857873
"""Actors hold information about a person acting on the repository. They can be
858874
committers and authors or anything with a name and an email as mentioned in the git
859875
log entries."""
860876

877+
name_email_regex = _DeprecatedActorNameEmailRegex()
878+
861879
# ENVIRONMENT VARIABLES
862880
# These are read when creating new commits.
863881
env_author_name = "GIT_AUTHOR_NAME"
@@ -891,7 +909,7 @@ def __repr__(self) -> str:
891909
return '<git.Actor "%s <%s>">' % (self.name, self.email)
892910

893911
@classmethod
894-
def _from_string(cls, string: str) -> "Actor":
912+
def from_string(cls: Type[T_Actor], string: str) -> T_Actor:
895913
"""Create an :class:`Actor` from a string.
896914
897915
:param string:
@@ -906,10 +924,12 @@ def _from_string(cls, string: str) -> "Actor":
906924
left_bracket = line.find("<")
907925
right_bracket = line.find(">", left_bracket + 1)
908926
if left_bracket >= 0 and right_bracket >= 0:
909-
return Actor(line[:left_bracket].rstrip(), line[left_bracket + 1 : right_bracket])
927+
return cls(line[:left_bracket].rstrip(), line[left_bracket + 1 : right_bracket])
910928

911929
# Assume the best and use the whole string as name.
912-
return Actor(string, None)
930+
return cls(string, None)
931+
932+
_from_string = from_string
913933

914934
@classmethod
915935
def _main_actor(

‎test/test_actor.py‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6-
from git import Actor
6+
from unittest import mock
77

8+
from git import Actor
89
from test.lib import TestBase
910

1011

1112
class TestActor(TestBase):
1213
def test_from_string_should_separate_name_and_email(self):
13-
a = Actor._from_string("Michael Trier <mtrier@example.com>")
14+
a = Actor.from_string("Michael Trier <mtrier@example.com>")
1415
self.assertEqual("Michael Trier", a.name)
1516
self.assertEqual("mtrier@example.com", a.email)
1617

@@ -23,18 +24,34 @@ def test_from_string_should_separate_name_and_email(self):
2324
assert len(m) == 1
2425

2526
def test_from_string_should_handle_just_name(self):
26-
a = Actor._from_string("Michael Trier")
27+
a = Actor.from_string("Michael Trier")
2728
self.assertEqual("Michael Trier", a.name)
2829
self.assertEqual(None, a.email)
2930

31+
def test_from_string_constructs_subclass(self):
32+
class DerivedActor(Actor):
33+
pass
34+
35+
self.assertIsInstance(DerivedActor.from_string("name <email>"), DerivedActor)
36+
3037
def test_from_string_handles_unterminated_email_without_regex_backtracking(self):
3138
value = "A" * 20_000 + " <unterminated"
32-
actor = Actor._from_string(value)
33-
self.assertNotIn("name_email_regex", vars(Actor))
39+
with mock.patch.object(Actor, "name_email_regex", None):
40+
actor = Actor.from_string(value)
3441
self.assertEqual(actor, Actor(value, None))
3542

43+
def test_name_email_regex_is_available_but_deprecated(self):
44+
with self.assertWarns(DeprecationWarning) as context:
45+
match = Actor.name_email_regex.match("Michael Trier <mtrier@example.com>")
46+
47+
message = str(context.warning)
48+
self.assertIn("Actor.from_string()", message)
49+
self.assertIn("Actor(name, email)", message)
50+
assert match is not None
51+
self.assertEqual(match.groups(), ("Michael Trier", "mtrier@example.com"))
52+
3653
def test_from_string_does_not_parse_across_lines(self):
37-
self.assertEqual(Actor._from_string("x <a>\n y <b>"), Actor("x", "a"))
54+
self.assertEqual(Actor.from_string("x <a>\n y <b>"), Actor("x", "a"))
3855

3956
def test_from_string_uses_git_delimiters(self):
4057
for value, expected in (
@@ -45,12 +62,12 @@ def test_from_string_uses_git_delimiters(self):
4562
("Name <email", Actor("Name <email", None)),
4663
("Name email>", Actor("Name email>", None)),
4764
):
48-
self.assertEqual(Actor._from_string(value), expected)
65+
self.assertEqual(Actor.from_string(value), expected)
4966

5067
def test_should_display_representation(self):
51-
a = Actor._from_string("Michael Trier <mtrier@example.com>")
68+
a = Actor.from_string("Michael Trier <mtrier@example.com>")
5269
self.assertEqual('<git.Actor "Michael Trier <mtrier@example.com>">', repr(a))
5370

5471
def test_str_should_alias_name(self):
55-
a = Actor._from_string("Michael Trier <mtrier@example.com>")
72+
a = Actor.from_string("Michael Trier <mtrier@example.com>")
5673
self.assertEqual(a.name, str(a))

0 commit comments

Comments
 (0)