Skip to content

Commit

Permalink
Simplify _safer_popen_windows "if shell" logic (helps mypy)
Browse files Browse the repository at this point in the history
Using a separate variable, as I had done originally, does not seem
to be clearer than rebinding the parameter, and in this case the
code is simpler and can be checked by mypy without needing another
explicit type annotation to be added. This fixes one mypy error.

This also adds a comment to make clearer why the mapping passed in
is copied when modifications are needed, rather than mutated.
  • Loading branch information
EliahKagan committed Mar 7, 2024
1 parent 2212ac9 commit 38221e5
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,9 @@ def _safer_popen_windows(
# When using a shell, the shell is the direct subprocess, so the variable must be
# set in its environment, to affect its search behavior. (The "1" can be any value.)
if shell:
safer_env = {} if env is None else dict(env)
safer_env["NoDefaultCurrentDirectoryInExePath"] = "1"
else:
safer_env = env
# The original may be immutable or reused by the caller. Make changes in a copy.
env = {} if env is None else dict(env)
env["NoDefaultCurrentDirectoryInExePath"] = "1"

# When not using a shell, the current process does the search in a CreateProcessW
# API call, so the variable must be set in our environment. With a shell, this is
Expand All @@ -273,7 +272,7 @@ def _safer_popen_windows(
return Popen(
command,
shell=shell,
env=safer_env,
env=env,
creationflags=creationflags,
**kwargs,
)
Expand Down

0 comments on commit 38221e5

Please sign in to comment.