Skip to content

Fixed cmd2 bypassing NO_COLOR and allow_style when setting prompt-toolkit's color depth. #1706

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.

Sign up for GitHub

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
kmvanbrunt merged 2 commits into from
Jul 7, 2026
Merged

Fixed cmd2 bypassing NO_COLOR and allow_style when setting prompt-toolkit's color depth. #1706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf99bc3
Fixed cmd2 bypassing NO_COLOR and allow_style when setting prompt-too…
kmvanbrunt Jul 7, 2026
2cb95e2
Fixed handling of NO_COLOR environment variable.
kmvanbrunt Jul 7, 2026
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Bug Fixes
- Fixed type hinting so that methods decorated with `with_annotated` no longer trigger spurious
mypy errors and preserve their original signature.
- Fixed cmd2 bypassing NO_COLOR and allow_style when setting prompt-toolkit's color depth.

- Experimental features
- `@with_annotated` now supports `frozenset[T]` collection parameters, alongside the existing
Expand Down
5 changes: 3 additions & 2 deletions cmd2/cmd2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
from prompt_toolkit.input import DummyInput, create_input
from prompt_toolkit.key_binding import KeyBindings
from prompt_toolkit.output import DummyOutput, create_output
from prompt_toolkit.output.color_depth import ColorDepth
from prompt_toolkit.patch_stdout import patch_stdout
from prompt_toolkit.shortcuts import CompleteStyle, PromptSession, choice, set_title
from prompt_toolkit.styles import DynamicStyle
Expand Down Expand Up @@ -202,6 +201,7 @@ def __init__(self, msg: str = "") -> None:
Cmd2History,
Cmd2Lexer,
pt_filter_style,
pt_resolve_color_depth,
)
from .utils import (
Settable,
Expand Down Expand Up @@ -778,7 +778,7 @@ def _(event: Any) -> None: # pragma: no cover
kwargs: dict[str, Any] = {
"auto_suggest": AutoSuggestFromHistory() if auto_suggest else None,
"bottom_toolbar": self.get_bottom_toolbar if enable_bottom_toolbar else None,
"color_depth": ColorDepth.TRUE_COLOR,
"color_depth": pt_resolve_color_depth(),
"complete_style": CompleteStyle.MULTI_COLUMN,
"complete_in_thread": complete_in_thread,
"complete_while_typing": False,
Expand Down Expand Up @@ -1443,6 +1443,7 @@ def allow_style(self) -> ru.AllowStyle:
def allow_style(self, value: ru.AllowStyle) -> None:
"""Setter property needed to support do_set when it updates allow_style."""
ru.ALLOW_STYLE = value
self.main_session.color_depth = pt_resolve_color_depth()

@tleonhardt tleonhardt Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is a common pattern for subclasses of cmd2.Cmd to configure properties like self.allow_style in their __init__ before calling super().__init__(). Doing so here will crash the application because main_session has not been instantiated yet. Using getattr would makes this setter safe at any point in the lifecycle, i.e.:

if getattr(self, "main_session", None) is not None:
    self.main_session.color_depth = pt_resolve_color_depth()

@kmvanbrunt kmvanbrunt Jul 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is true for a few of our settables. They expect certain instance members to exist before than can safely be set.

  1. allow_style
  2. always_prefix_settables
  3. traceback_show_locals
  4. traceback_width

@tleonhardt tleonhardt Jul 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should either attempt to ruggedize their use like I was suggesting above or do a very good job of documenting that these are special and certain instance members need to exist before they can be safely set (likely by saying don't set them before calling parent class dunder init).

@kmvanbrunt kmvanbrunt Jul 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think it's best to leave the code alone. __init__() functions are meant to establish the baseline state for an instance. Setting a parent's instance variables before calling super().__init__() doesn't seem safe since those values could be overwritten by __init__().


@property
def traceback_show_locals(self) -> bool:
Expand Down
9 changes: 9 additions & 0 deletions cmd2/pt_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Utilities for integrating prompt_toolkit with cmd2."""

import os
import re
from collections.abc import (
Callable,
Expand All @@ -21,6 +22,7 @@
from prompt_toolkit.formatted_text import ANSI
from prompt_toolkit.history import History
from prompt_toolkit.lexers import Lexer
from prompt_toolkit.output.color_depth import ColorDepth
from rich.style import Style, StyleType

from . import (
Expand Down Expand Up @@ -75,6 +77,13 @@ def pt_filter_style(text: str | ANSI) -> str | ANSI:
return text if isinstance(text, ANSI) else ANSI(text)


def pt_resolve_color_depth() -> ColorDepth:
"""Determine the prompt-toolkit ColorDepth based on NO_COLOR and ru.ALLOW_STYLE."""
if os.environ.get("NO_COLOR") or ru.ALLOW_STYLE == ru.AllowStyle.NEVER:
return ColorDepth.DEPTH_1_BIT
return ColorDepth.TRUE_COLOR


@lru_cache(maxsize=256)
def rich_to_pt_color(color: "Color | None") -> str:
"""Convert a rich Color object to a prompt_toolkit color string."""
Expand Down
5 changes: 4 additions & 1 deletion cmd2/styles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ class Cmd2Style(StrEnum):
Cmd2Style.COMMAND_LINE: Style(color=Color.CYAN, bold=True),
Cmd2Style.COMPLETION_MENU: Style(color="#000000", bgcolor="#bbbbbb"), # prompt-toolkit default
Cmd2Style.COMPLETION_MENU_COMPLETION: Style(), # prompt-toolkit default
Cmd2Style.COMPLETION_MENU_CURRENT: Style(color=Color.GREEN, bgcolor=Color.BLACK), # This style swaps FG and BG colors
# Defined with swapped FG/BG colors because prompt-toolkit applies 'reverse' by default.
# This yields a black-on-green highlight in color mode, while maintaining a fallback
# reverse-video highlight when colors are disabled (e.g. NO_COLOR=1 or allow_style=never).
Cmd2Style.COMPLETION_MENU_CURRENT: Style(color=Color.GREEN, bgcolor=Color.BLACK),
Cmd2Style.COMPLETION_MENU_META: Style(color="#000000", bgcolor="#bbbbbb"), # prompt-toolkit default
Cmd2Style.COMPLETION_MENU_META_CURRENT: Style(color=Color.BLACK, bgcolor=Color.BRIGHT_GREEN),
Cmd2Style.ERROR: Style(color=Color.BRIGHT_RED),
Expand Down
5 changes: 4 additions & 1 deletion tests/test_cmd2.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ def test_set_no_settables(base_app) -> None:
],
)
@with_ansi_style(ru.AllowStyle.TERMINAL)
def test_set_allow_style(base_app, new_val, is_valid, expected) -> None:
def test_set_allow_style(base_app: cmd2.Cmd, new_val: str, is_valid: bool, expected: ru.AllowStyle) -> None:
from cmd2.pt_utils import pt_resolve_color_depth

# Use the set command to alter allow_style
out, err = run_cmd(base_app, f"set allow_style {new_val}")
assert base_app.last_result is is_valid
Expand All @@ -255,6 +257,7 @@ def test_set_allow_style(base_app, new_val, is_valid, expected) -> None:
if is_valid:
assert out
assert not err
assert base_app.main_session.color_depth == pt_resolve_color_depth()


def test_set_traceback_show_locals(base_app: cmd2.Cmd) -> None:
Expand Down
30 changes: 30 additions & 0 deletions tests/test_pt_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
ANSI,
to_formatted_text,
)
from prompt_toolkit.output.color_depth import ColorDepth
from rich.table import Table

import cmd2
Expand All @@ -25,6 +26,7 @@
from cmd2.pt_utils import (
Cmd2Lexer,
pt_filter_style,
pt_resolve_color_depth,
)

from .conftest import with_ansi_style
Expand Down Expand Up @@ -96,6 +98,34 @@ def test_pt_filter_style_never() -> None:
assert result == su.strip_style(styled)


@with_ansi_style(ru.AllowStyle.ALWAYS)
def test_pt_resolve_color_depth_always(monkeypatch) -> None:
# Clear NO_COLOR to ensure colors are not suppressed
monkeypatch.delenv("NO_COLOR", raising=False)
assert pt_resolve_color_depth() == ColorDepth.TRUE_COLOR


@with_ansi_style(ru.AllowStyle.TERMINAL)
def test_pt_resolve_color_depth_terminal(monkeypatch) -> None:
# Mock NO_COLOR to an empty string to ensure colors are not suppressed
monkeypatch.setenv("NO_COLOR", "")
assert pt_resolve_color_depth() == ColorDepth.TRUE_COLOR


@with_ansi_style(ru.AllowStyle.NEVER)
def test_pt_resolve_color_depth_never(monkeypatch) -> None:
# Under NEVER, colors are suppressed regardless of NO_COLOR
monkeypatch.delenv("NO_COLOR", raising=False)
assert pt_resolve_color_depth() == ColorDepth.DEPTH_1_BIT


@with_ansi_style(ru.AllowStyle.ALWAYS)
def test_pt_resolve_color_depth_no_color(monkeypatch) -> None:
# Mock NO_COLOR to ensure colors are suppressed
monkeypatch.setenv("NO_COLOR", "1")
assert pt_resolve_color_depth() == ColorDepth.DEPTH_1_BIT


class TestCmd2Lexer:
@with_ansi_style(ru.AllowStyle.NEVER)
def test_lex_document_no_style(self, mock_cmd_app):
Expand Down
Loading