Skip to content

Fix the Windows check in the man page lookup - #1937

Open
Kjubikstronk wants to merge 1 commit into
httpie:masterfrom
Kjubikstronk:fix-man-pages-windows-guard
Open

Fix the Windows check in the man page lookup#1937
Kjubikstronk wants to merge 1 commit into
httpie:masterfrom
Kjubikstronk:fix-man-pages-windows-guard

Conversation

@Kjubikstronk

Copy link
Copy Markdown

Closes #1898.

Problem

httpie/output/ui/man_pages.py guards on os.system == 'nt', which compares the built-in function os.system to a string. That is never true, so the Windows branch is dead code.

I happened to be on Windows, so I could check the actual behaviour rather than just read it. Simulating a man on PATH, which is what you get with Git Bash or WSL installed:

running on Windows      : True
os.system == 'nt'       : False   <- the guard
os.name                 : 'nt'
is_available('http')    : True    <- expected False on Windows
subprocess.run called   : True    <- the guard should have prevented this

So it isn't only that the comparison is wrong — HTTPie really does go on to shell out to man on Windows and, if one is present, renders man pages. Where no man exists the function still returns False, but only because the FileNotFoundError is caught by the bare except Exception, which is accidental rather than intended.

After the change, on the same machine:

is_available('http') on Windows: False
subprocess.run called          : False

Fix

httpie/compat.py already defines is_windows, and config.py, context.py, uploads.py, internal/daemons.py and output/writer.py all use it. Reusing it keeps the platform check in one place:

from httpie.compat import is_windows
...
if NO_MAN_PAGES or is_windows:
    return False

The issue suggested sys.platform == 'win32', which is what is_windows is defined as — this just routes through the existing constant instead of adding a second spelling. httpie.context already imports from httpie.compat, so this introduces no new dependency or import cycle.

Tests

There were no tests for this module, which is how the guard stayed broken. Added tests/test_man_pages.py covering the Windows short-circuit, the HTTPIE_NO_MAN_PAGES short-circuit, and the three outcomes of the man call — found, not found, and man not installed. The two short-circuit cases assert that subprocess.run is never reached, since that is the actual point of the guard.

The tests patch the module-level flags rather than the real platform, so they exercise the Windows path on any OS.

tests/test_man_pages.py .....                     [100%]
5 passed

flake8 is clean on both files.

I also ran tests/test_cli_ui.py, tests/test_cli_utils.py and the doctests under httpie/output and httpie/cli. One unrelated failure, test_cli_utils.py::test_lazy_choices_help, is pre-existing — it fails identically with this change stashed, and is a Python 3.14 argparse behaviour change (the lazy getter is now called during help formatting). I did not run the full suite, as the dev extra pins werkzeug<2.1.0, which will not install on 3.14.

Changelog

Added an entry under a new 3.2.5-dev (unreleased) heading, following the format of the 3.3.0-dev section used before the 3.2.2 release. I guessed the version number since master had no unreleased section — happy to renumber or drop it.

`is_available()` guarded on `os.system == 'nt'`, comparing the built-in
function to a string, so the branch was never taken. On Windows the
lookup fell through to running `man` and only returned False because the
resulting FileNotFoundError was swallowed -- where a `man` is on PATH,
from Git Bash or WSL, HTTPie went on to render man pages.

Use `is_windows` from `httpie.compat`, which the rest of the codebase
already uses for this.
@Kjubikstronk

Kjubikstronk commented Aug 15, 2026

Copy link
Copy Markdown
Author

Heads-up for whoever reviews this: the failing Tests jobs are not caused by this PR.

The same failures are present on master with no PR involved — comparing this run against the master run from 2026-07-25 (1fc01be7), the assertions are identical:

FAILED tests/test_encoding.py::test_terminal_output_request_charset_detection[big5-…]
FAILED tests/test_encoding.py::test_terminal_output_response_charset_detection[big5-…]
  AssertionError: assert '卷首卷首…' in '…뻥솤뻥솤…'
FAILED tests/test_cli_ui.py::test_naked_invocation[args3-…]
FAILED tests/test_uploads.py::test_chunked_stdin

Every Tests run on master since at least 2026-04-30 has failed (Apr 30, May 7, May 12, Jun 5, Jun 25, Jun 30, Jul 25), as have the recent pull request runs from other branches. The big5 cases look like drift in the charset detection dependency — the sample text now decodes as Korean instead of Chinese.

This PR touches one line in httpie/output/ui/man_pages.py, which none of those tests exercise. The Code Style Check and Check Markdown Style jobs, which do cover the change, both pass, and the added tests/test_man_pages.py passes locally.

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.

Bug: Windows guard in is_available() compares os.system (a function) to 'nt' — always False

1 participant