Skip to content

Commit f556490

Browse files
committed
Clarify USE_SHELL warning helper signature
This is a minor refactor of how `_warn_use_shell` can be, and is, invoked. The `_warn_use_shell` helper function in `git.cmd` takes a single `bool`-valued argumnet `extra_danger`, which is conceptually associated with having a `True` value of `USE_SHELL`, but the association is not necessarily obvious. Specifically: - For the warning given when reading `USE_SHELL` on the `Git` class or through an instance, `extra_danger` is always `False`. This is so even if the `USE_SHELL` value is currently `True`, because the danger that arises from `True` occurs internally. - For the warning given when writing `USE_SHELL`, which can only be done on the `Git` class and not on or through an instance, `extra_danger` is the value set for the attribute. This is because setting `USE_SHELL` to `True` incurs the danger described in #1896. When reading the code, which passed `extra_danger` positionally, the meaning of the parameter may not always have been obvious. This makes the `extra_danger` parameter keyword-only, and passes it by keyword in all invocations, so that its meaning is clearer.
1 parent b6a188b commit f556490

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

git/cmd.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@ def __del__(self) -> None:
550550
)
551551

552552

553-
def _warn_use_shell(extra_danger: bool) -> None:
553+
def _warn_use_shell(*, extra_danger: bool) -> None:
554554
warnings.warn(
555555
_USE_SHELL_DANGER_MESSAGE if extra_danger else _USE_SHELL_DEFAULT_MESSAGE,
556556
DeprecationWarning,
@@ -566,12 +566,12 @@ class _GitMeta(type):
566566

567567
def __getattribute(cls, name: str) -> Any:
568568
if name == "USE_SHELL":
569-
_warn_use_shell(False)
569+
_warn_use_shell(extra_danger=False)
570570
return super().__getattribute__(name)
571571

572572
def __setattr(cls, name: str, value: Any) -> Any:
573573
if name == "USE_SHELL":
574-
_warn_use_shell(value)
574+
_warn_use_shell(extra_danger=value)
575575
super().__setattr__(name, value)
576576

577577
if not TYPE_CHECKING:
@@ -988,7 +988,7 @@ def __init__(self, working_dir: Union[None, PathLike] = None) -> None:
988988

989989
def __getattribute__(self, name: str) -> Any:
990990
if name == "USE_SHELL":
991-
_warn_use_shell(False)
991+
_warn_use_shell(extra_danger=False)
992992
return super().__getattribute__(name)
993993

994994
def __getattr__(self, name: str) -> Any:

0 commit comments

Comments
 (0)