Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion progressbar/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def from_env(cls) -> ColorSupport:
break
elif '256' in value:
support = max(cls.XTERM_256, support)
elif value == 'xterm':
elif 'xterm' in value:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The environment variable check for 'xterm' is case-sensitive. Since terminal environment variables (like TERM or COLORTERM) can sometimes be set in uppercase or mixed case (e.g., XTERM, Xterm), it is safer to perform a case-insensitive check by lowercasing the value before checking.

Suggested change
elif 'xterm' in value:
elif 'xterm' in value.lower():

support = max(cls.XTERM, support)
elif env_flag(variable, default=False):
# Generic truthy flags such as `FORCE_COLOR=1` enable
Expand Down
26 changes: 26 additions & 0 deletions tests/test_color.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,32 @@ def test_color_support_from_env(monkeypatch, variable, value) -> None:
env.ColorSupport.from_env()


@pytest.mark.parametrize(
('term', 'expected'),
[
# Bare ``xterm`` and any ``xterm-*`` variant advertise (at least) 16
# color support, matching the documented "if they contain ``xterm``"
# behaviour and ``is_ansi_terminal``'s ``^xterm`` prefix match.
('xterm', env.ColorSupport.XTERM),
('xterm-color', env.ColorSupport.XTERM),
('xterm-16color', env.ColorSupport.XTERM),
('xterm-kitty', env.ColorSupport.XTERM),
('xterm-ghostty', env.ColorSupport.XTERM),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure that case-insensitive terminal names (like XTERM) are correctly supported, we should add a test case with uppercase characters.

Suggested change
('xterm', env.ColorSupport.XTERM),
('xterm-color', env.ColorSupport.XTERM),
('xterm-16color', env.ColorSupport.XTERM),
('xterm-kitty', env.ColorSupport.XTERM),
('xterm-ghostty', env.ColorSupport.XTERM),
('xterm', env.ColorSupport.XTERM),
('XTERM', env.ColorSupport.XTERM),
('xterm-color', env.ColorSupport.XTERM),
('xterm-16color', env.ColorSupport.XTERM),
('xterm-kitty', env.ColorSupport.XTERM),
('xterm-ghostty', env.ColorSupport.XTERM),

# A ``256`` anywhere in the value still wins over plain xterm.
('xterm-256color', env.ColorSupport.XTERM_256),
('screen-256color', env.ColorSupport.XTERM_256),
],
)
def test_color_support_from_env_term(monkeypatch, term, expected) -> None:
if os.name == 'nt':
# Windows has special handling so we need to disable that to make the
# tests work properly
monkeypatch.setattr(os, 'name', 'posix')

monkeypatch.setenv('TERM', term)
assert env.ColorSupport.from_env() == expected


@pytest.mark.parametrize(
'variable',
[
Expand Down