Skip to content

Fix ambiguous _safer_popen_windows comment #2044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 7, 2025

Conversation

EliahKagan
Copy link
Member

@EliahKagan EliahKagan commented Jun 7, 2025

This fixes some ambiguous wording in a comment in _safer_popen_wording, where it was unclear if the secondary problem – where it would be possible to run a wrong cmd.exe-type shell – would happen under two separate circumstances, or only when both circumstances occurred together. This adjusts its wording to make clear that it is the latter.

This also fixes a minor typo in another _safer_popen_windows comment.

This might be viewed as building on the improvements in b9d9e56 (#1859), but the changes here are to comments only.

(I'll merge this once CI passes on it.)

This fixes some ambiguous wording in a comment in
`_safer_popen_wording`, where it was unclear if the secondary
problem -- where it would be possible to run a wrong `cmd.exe`-type
shell -- would happen under two separate circumstances, or only
when both circumstances occurred together. This adjusts its wording
to make clear that it is the latter.

This also fixes a minor typo in another `_safer_popen_windows`
comment.

This might be viewed as building on the improvements in b9d9e56
(gitpython-developers#1859), but the changes here are to comments only.
@EliahKagan EliahKagan marked this pull request as ready for review June 7, 2025 20:46
@EliahKagan EliahKagan merged commit b6a188b into gitpython-developers:main Jun 7, 2025
27 checks passed
@EliahKagan EliahKagan deleted the popen-comments branch June 7, 2025 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant