Skip to content

fix Windows short-circuit in man_pages.is_available#1874

Open
HrachShah wants to merge 1 commit into
httpie:masterfrom
HrachShah:fix/man-pages-windows-typo
Open

fix Windows short-circuit in man_pages.is_available#1874
HrachShah wants to merge 1 commit into
httpie:masterfrom
HrachShah:fix/man-pages-windows-typo

Conversation

@HrachShah

Copy link
Copy Markdown

Summary

is_available() guarded against Windows with os.system == 'nt'. os.system is the built-in function object from the os module, never a string, so the comparison was always False and the function never short-circuited on Windows even though that was clearly the author's intent (the surrounding code is checking whether the running OS has the man binary).

On Windows the function fell through to subprocess.run([MAN_COMMAND, ...]), which is missing from PATH and raised FileNotFoundError that was then swallowed by the bare except Exception, so users saw a confusing 'no man pages found' instead of an immediate 'not on Windows' early return.

Fix

Replace os.system with os.name — the standard Python idiom for OS detection (returns 'nt' on Windows, 'posix' on Linux/macOS).

Testing

The change is one character; manual verification: os.system == 'nt' evaluates to False on every platform including Windows, while os.name == 'nt' correctly returns True on Windows and False on Linux/macOS.

is_available guarded against Windows with `os.system == 'nt'`. os.system
is the built-in function object from the os module, never a string, so
the comparison was always False and the function never short-circuited
on Windows even though that was clearly the author's intent (the
surrounding code is checking whether the running OS has the `man`
binary). On Windows the function fell through to `subprocess.run([MAN_COMMAND, ...])`,
which is missing from PATH and raised FileNotFoundError that was then
swallowed by the bare `except Exception`, so users saw a confusing
'no man pages found' instead of an immediate 'not on Windows' early
return.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant