Skip to content

feat(cli): --color for --format text output - #343

Merged
digizeph merged 5 commits into
mainfrom
feat/text-color-output
Sep 10, 2026
Merged

feat(cli): --color for --format text output#343
digizeph merged 5 commits into
mainfrom
feat/text-color-output

Conversation

@digizeph

Copy link
Copy Markdown
Member

--format text printed everything in one color, so session keys, section headers, prefixes and next-hop values were hard to tell apart at a glance. This adds an opt-in ANSI accent layer. Closes #342.

Behavior

  • --color <auto|always|never>, default auto: auto colors only when stdout is a terminal, so pipes, redirects and log files stay byte-identical to today. NO_COLOR disables coloring and CLICOLOR_FORCE forces it.
  • Only --format text is colored; json/json-pretty/psv/default are untouched.
  • Accents use the basic ANSI attributes rather than fixed RGB values, so they follow the reader's terminal theme and stay legible on light and dark backgrounds: bold blue labels, bold magenta section headers, green prefixes, yellow next-hop values.
  • Library: render::text::Style::ansi() with format_record_with_style and format_record_with_hex_and_style. format_record keeps returning plain text, and Style::plain() output is byte-identical to it.

Notes

Styling is a post-pass over the block format_record renders rather than a change to the renderer, so the unstyled text cannot drift: a line the accent table does not recognize is copied through untouched, and only a missing accent (cosmetic) is possible. Tests pin strip_sgr(styled) == plain for a record whose MP_REACH_NLRI carries a comma-joined global + link-local next hop, that validation warnings are not mistaken for identifiers, and the auto/NO_COLOR/CLICOLOR_FORCE policy.

`--format text` printed one color, so session keys, section headers,
prefixes and next-hop values were indistinguishable at a glance.

Add `--color auto|always|never` (default `auto`: color only when stdout is
a terminal, so pipes and redirects stay byte-identical), honoring NO_COLOR
and CLICOLOR_FORCE. Only the text format is colored.

Library side: `render::text::Style` with `Style::ansi()` plus
`format_record_with_style` and `format_record_with_hex_and_style`;
`format_record` is unchanged.

Styling is a post-pass over the block `format_record` renders, so the
unstyled text cannot drift: a line the accent table does not recognize is
copied through untouched, and a test asserts the styled block strips back
to the plain one for a record whose MP_REACH_NLRI carries a comma-joined
global + link-local next hop. Accents use the basic ANSI attributes (bold
blue labels, bold magenta sections, green prefixes, yellow next hops) so
they follow the terminal theme on light and dark backgrounds.

Closes #342.
Copilot AI balanced review requested due to automatic review settings September 10, 2026 20:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Table-dump prefix values do not receive the advertised prefix accent.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds opt-in ANSI styling to human-readable MRT record output.

Changes:

  • Adds --color auto|always|never with environment overrides.
  • Introduces styled text-rendering APIs and tests.
  • Documents the feature in the changelog.
File summaries
File Description
src/render/text.rs Implements ANSI post-processing and tests.
src/bin/main.rs Adds CLI color policy and styled output routing.
CHANGELOG.md Documents color support.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/render/text.rs Outdated
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.73460% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.56%. Comparing base (faeab92) to head (238cf1f).

Files with missing lines Patch % Lines
src/render/text.rs 95.73% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
+ Coverage   91.50%   91.56%   +0.06%     
==========================================
  Files         103      103              
  Lines       26487    26698     +211     
==========================================
+ Hits        24236    24446     +210     
- Misses       2251     2252       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot review on #343: the prefix accent only covered the indented UPDATE
prefix lists, so the legacy table-dump `PREFIX:` line and the prefix
embedded in the RIB_AFI/RIB_GENERIC headings stayed uncolored. Accent the
labeled prefix value and the embedded one, and pin both forms with a test.
Copilot AI review requested due to automatic review settings September 10, 2026 20:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Several renderer-defined path-attribute labels incorrectly remain unstyled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/render/text.rs:200

  • ATOMIC_AGGREGATE is rendered as a bare property name without a colon, so it always falls through here and remains unaccented while other path-attribute names are colored. Handle this renderer-owned bare label explicitly.
        } else {
            None
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/render/text.rs Outdated
Copilot review on #343: ATOMIC_AGGREGATE renders without a colon, and the
raw-retained attributes render as `RAW ATTRIBUTE (type N)`, `DEPRECATED
(type N)` and `UNKNOWN (type N)`, which the accent table skipped. Match
them explicitly and keep the space heuristic for validation prose.
Copilot AI review requested due to automatic review settings September 10, 2026 20:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Some ANSI spans color metadata beyond the documented role-based palette.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/render/text.rs:163

  • For ANNOUNCED (labeled), trimmed also contains labels=[…] and an optional path-id, so this paints routing metadata green along with the prefix. The documented palette assigns green specifically to prefixes; isolate the first whitespace-delimited prefix and leave the suffix unstyled.

This issue also appears on line 194 of the same file.

src/render/text.rs:197

  • The MP_REACH_NLRI branch colors the entire value—including the AFI/SAFI and next-hop text—and even colors the family when no next hop exists. This contradicts the documented role palette where only next-hop values are yellow; split the rendered value at next-hop and accent only the address portion.
            } else if NEXT_HOP_LABELS.contains(&label) {
                Some(format!(
                    "{ANSI_LABEL}{label}:{ANSI_RESET} {ANSI_NEXT_HOP}{value}{ANSI_RESET}"
                ))
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot review on #343: labeled announcements painted their `labels=[...]`
/ `path-id` metadata green, and `MP_REACH_NLRI` painted the AFI/SAFI plus
the `next-hop` separator yellow (including a family with no next hop).
Accent only the prefix token and the next-hop address, leaving the rest of
the line plain.
Copilot AI review requested due to automatic review settings September 10, 2026 20:34
The previous commit narrowed the `MP_REACH_NLRI` accent to the address but
left this assertion expecting the colored family, so that commit's test run
failed. Assert the narrowed span.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

A new next-hop styling assertion contradicts the implementation and will fail.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/render/text.rs Outdated
Copilot AI review requested due to automatic review settings September 10, 2026 20:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The implementation preserves existing output by default and adequately tests styling and color-selection behavior.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@digizeph
digizeph merged commit d9d2094 into main Sep 10, 2026
10 checks passed
@digizeph
digizeph deleted the feat/text-color-output branch September 10, 2026 21:40
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.

feat: optional ANSI colors for --format text output

2 participants