Skip to content

feat(color): add semantic terminal coloring via color_mode config (#24)#72

Merged
thomaschristory merged 15 commits into
mainfrom
feat/issue-24-rich-coloring
May 21, 2026
Merged

feat(color): add semantic terminal coloring via color_mode config (#24)#72
thomaschristory merged 15 commits into
mainfrom
feat/issue-24-rich-coloring

Conversation

@thomaschristory
Copy link
Copy Markdown
Owner

Summary

  • Adds ColorMode StrEnum (auto|on|off) to config with defaults.color_mode: auto default
  • Adds resolve_color() and color: bool to RuntimeContext; resolved once at bootstrap via sys.stdout.isatty()
  • Semantic table cell coloring: status strings (active→green, failed→red, planned→yellow), booleans, and empty cells
  • Colorizes delete success messages (deleted in yellow, already absent dimmed)
  • color_mode: off suppresses all Rich markup including existing error/explain panels

Test Plan

  • just test — 656 passed, 1 skipped
  • just lint — ruff + mypy --strict clean
  • Manual: nsc <resource> list in a TTY shows colored status cells
  • Manual: nsc <resource> list | cat produces plain output (auto mode)
  • Manual: set defaults.color_mode: off in config — no ANSI anywhere
  • Manual: set defaults.color_mode: on — colors even when piped

Closes #24

🤖 Generated with Claude Code

Thomas Christory and others added 10 commits May 14, 2026 12:11
Add resolve_color(mode, *, is_tty) function to resolve color mode to boolean.
Add color: bool = False field to RuntimeContext.
Add color: bool = False parameter to emit_envelope signature.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t_envelope

- Update render_to_rich_stderr signature to accept color: bool = False
- When color=True, use force_terminal=True for ANSI codes
- When color=False, use no_color=True and highlight=False to suppress markup
- Complete emit_envelope body to pass color to render_to_rich_stderr
- Add comprehensive tests for color parameter behavior

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add explicit OutputFormat.TABLE check before applying color in delete responses
- Set no_color=False on Console to enable ANSI output to test streams
- Fix line-length issue by splitting console instantiation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Thomas Christory and others added 2 commits May 14, 2026 12:55
Moves the duplicated Console-construction pattern into nsc/output/_console.py
and updates table.py, errors.py, explain.py, and handlers.py to use it;
also removes the spurious no_color=False default from the delete helpers.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@thomaschristory
Copy link
Copy Markdown
Owner Author

Code review

Found 1 issue:

  1. make_console(color=True) does not disable Rich's auto-highlighter, causing unintended bold/style artifacts on plain strings. When color=True, the factory uses force_terminal=True but leaves highlight=True (the default). Rich's highlighter bolds parentheses, colorizes IP addresses, and applies other patterns to plain text — so (no records) renders as \x1b[1m(\x1b[0mno records\x1b[1m)\x1b[0m (bolded parens) and [dim]already absent (no change)[/] gets extra bold on the parens inside the dim span.

def make_console(stream: TextIO, *, color: bool, soft_wrap: bool = False) -> Console:
if color:
return Console(file=stream, force_terminal=True, soft_wrap=soft_wrap)
return Console(file=stream, no_color=True, highlight=False, soft_wrap=soft_wrap)

Fix: add highlight=False to the color=True branch:

def make_console(stream: TextIO, *, color: bool, soft_wrap: bool = False) -> Console:
    if color:
        return Console(file=stream, force_terminal=True, highlight=False, soft_wrap=soft_wrap)
    return Console(file=stream, no_color=True, highlight=False, soft_wrap=soft_wrap)

Verified against the live Rich library: plain console.print('(no records)') with force_terminal=True emits \x1b[1m(\x1b[0mno records\x1b[1m)\x1b[0m; adding highlight=False gives the expected clean (no records).

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@thomaschristory
Copy link
Copy Markdown
Owner Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@thomaschristory
Copy link
Copy Markdown
Owner Author

Advisory findings (below confidence threshold)

These issues scored 75/100 — verified real but not certain enough to block. Flagged for awareness:

  1. sys.stdout.isatty() used to gate color for both stdout and stderr — In color_mode: auto, piping stdout (e.g. nsc list | jq) sets color=False, silencing Rich error panels on stderr even though stderr is still a TTY. Reverse: redirecting stderr while stdout is a TTY leaks ANSI codes into the file.

debug=state.debug,
page_size=state.config.defaults.page_size,
color=resolve_color(state.config.defaults.color_mode, is_tty=sys.stdout.isatty()),
)

  1. API-sourced values passed unescaped into Rich markup stringsenv.error, env.endpoint, and env.details come from HTTP responses and are interpolated directly into f"[bold red]{env.type.value}[/]: {env.error}". A server returning a value containing [ / ] can garble output or raise MarkupError. Same applies to _format_cell returning raw text when color=True.

console = make_console(stream, color=color, soft_wrap=True)
body_lines = [
f"[bold red]{env.type.value}[/]: {env.error}",
]
if env.endpoint:
body_lines.append(f"endpoint: {env.endpoint}")
if env.method is not None:
body_lines.append(f"method: {env.method.value}")
if env.status_code is not None:
body_lines.append(f"status: {env.status_code}")
if env.operation_id:
body_lines.append(f"op: {env.operation_id}")
if env.attempt_n is not None:
body_lines.append(f"attempt: {env.attempt_n}")
if env.audit_log_path:
body_lines.append(f"audit: {env.audit_log_path}")
if env.details:
body_lines.append(f"details: {env.details}")
console.print(Panel("\n".join(body_lines), title="nsc error", border_style="red"))

https://github.com/thomaschristory/netbox-super-cli/blob/bc56b2a9b437326dcb5e380d1f6c36029f1d0e4e/nsc/output/table.py#L62-L80

  1. make_console(color=True) omits highlight=False — The no-color path sets highlight=False but the color path does not, so Rich's auto-highlighter (numbers → magenta, URLs → blue) fires on top of intentional status markup.

https://github.com/thomaschristory/netbox-super-cli/blob/bc56b2a9b437326dcb5e380d1f6c36029f1d0e4e/nsc/output/_console.py#L10-L14

  1. explain.py color path has no testsrender_to_rich_stdout gained color: bool = False but unlike errors.py (→ test_errors_color.py) and table.py (→ test_table_color.py), no test_explain_color.py was added. CLAUDE.md requires TDD.

def render_to_rich_stdout(trace: ExplainTrace, *, stream: TextIO, color: bool = False) -> None:
console = make_console(stream, color=color, soft_wrap=True)
body = [

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Thomas Christory and others added 2 commits May 21, 2026 23:13
# Conflicts:
#	nsc/cli/globals.py
#	nsc/output/errors.py
#	uv.lock
…, disable highlighter

- make_console(color=True) now sets highlight=False so Rich's auto-highlighter
  no longer bolds parens / colorizes numbers in plain output.
- Escape API/user-sourced text (env.error/endpoint/details, table cells,
  explain reasoning) before it reaches Rich markup parsing, on every output
  path — Rich parses markup even in no-color mode.
- RuntimeContext gains color_stderr, resolved from sys.stderr.isatty()
  independently of stdout; emit_envelope calls now use it so `nsc ... 2>file`
  no longer leaks ANSI into the redirected file.
- Add test_console.py, test_explain_color.py, test_globals_color.py, and
  markup-escape regression tests for table/errors/explain.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thomaschristory
Copy link
Copy Markdown
Owner Author

Picked up review fixes. Merged latest main (resolved conflicts in globals.py/errors.py/uv.lock), addressed all review feedback: highlight=False on color console, escape API/user text before Rich markup (table/errors/explain, both color modes), and split RuntimeContext.color_stderr from stdout color so 2>file no longer leaks ANSI. 668 tests pass, lint+mypy clean locally. Waiting on CI.

…test

PR #44 added a "Fetch and cache the live schema now?" confirm after
`nsc login --new`, but the e2e test fed only the token line — the confirm
then hit EOF and aborted (exit 1). e2e has been red on main since #44.
Feed `n` to decline the fetch; the test only asserts profile creation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@thomaschristory thomaschristory merged commit 1604fb6 into main May 21, 2026
11 checks passed
thomaschristory added a commit that referenced this pull request May 21, 2026
Release prep for v1.0.5 — semantic terminal coloring (issue #24, PR #72).
@thomaschristory thomaschristory deleted the feat/issue-24-rich-coloring branch May 21, 2026 21:46
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.

add some coloring via rich

1 participant