From 253099fe91744801c39b41527e126c85c17ec003 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 7 Jun 2025 18:28:17 -0400 Subject: [PATCH] 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 argument `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. --- git/cmd.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 71096197c..15d7820df 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -550,7 +550,7 @@ def __del__(self) -> None: ) -def _warn_use_shell(extra_danger: bool) -> None: +def _warn_use_shell(*, extra_danger: bool) -> None: warnings.warn( _USE_SHELL_DANGER_MESSAGE if extra_danger else _USE_SHELL_DEFAULT_MESSAGE, DeprecationWarning, @@ -566,12 +566,12 @@ class _GitMeta(type): def __getattribute(cls, name: str) -> Any: if name == "USE_SHELL": - _warn_use_shell(False) + _warn_use_shell(extra_danger=False) return super().__getattribute__(name) def __setattr(cls, name: str, value: Any) -> Any: if name == "USE_SHELL": - _warn_use_shell(value) + _warn_use_shell(extra_danger=value) super().__setattr__(name, value) if not TYPE_CHECKING: @@ -988,7 +988,7 @@ def __init__(self, working_dir: Union[None, PathLike] = None) -> None: def __getattribute__(self, name: str) -> Any: if name == "USE_SHELL": - _warn_use_shell(False) + _warn_use_shell(extra_danger=False) return super().__getattribute__(name) def __getattr__(self, name: str) -> Any: