fix: only emit ANSI color when stdout is a TTY by larsch · Pull Request #551 · commit-check/commit-check · GitHub

Problem

The CLI emitted raw ANSI color escapes unconditionally. RED, GREEN,
YELLOW, and RESET_COLOR were hardcoded escape sequences in
commit_check/__init__.py, and print_error_message / print_suggestion
wrapped output in them without checking whether stdout is a terminal. Piped or
redirected output (CI logs, agent harnesses, files) therefore contained escape
garbage.

Fix

Add a supports_color() probe that mirrors the existing
supports_hyperlinks() convention, and gate the constants themselves:

RED = "\033[91m" if supports_color() else ""

FORCE_COLOR overrides in both directions (0 off, anything else on),
matching the FORCE_HYPERLINK convention already documented in
supports_hyperlinks().
A non-TTY stdout disables color.
TERM=dumb and an empty TERM (TERM=) disable color — an empty value
is the same deliberate "no terminal type" signal as dumb, while an unset
TERM still allows color on a real terminal.
No new dependencies.

Moving the decision to the constant definitions means commit_check/util.py
is unchanged: the print functions keep interpolating the constants, which are
already empty when color is off.

Tests

supports_color() behavior across FORCE_COLOR, TTY, and TERM cases.
Constant values (empty vs. raw escape codes) verified via module reload.

Summary by CodeRabbit

New Features
- Added automatic detection of terminal color support.
- Honors explicit color settings, terminal environment values, and whether output is connected to an interactive terminal.
- ANSI colors are now enabled only when supported, improving output in redirected or non-color environments.
Tests
- Added coverage for forced color, piped output, terminal settings, and color configuration behavior.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f8690345-28e5-4992-95b2-ae41cebf46d5

📥 Commits

Reviewing files that changed from the base of the PR and between 41df972 and 616bdaf.

📒 Files selected for processing (2)

commit_check/__init__.py
tests/util_test.py

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The package adds supports_color() and uses it to enable or disable ANSI color constants. Tests cover TTY status, TERM, FORCE_COLOR, and module reload behavior.

Changes

Terminal color support

Layer / File(s)	Summary
Detect and validate color support commit_check/__init__.py, tests/util_test.py	supports_color() evaluates TTY status, TERM, and FORCE_COLOR. ANSI constants become empty when color is unsupported. Tests cover detection and reloaded constants.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 616bd

This localized CLI change gates ANSI color for non-TTY output, and no concrete user or production risk remains at the current head; it is merge-ready after normal checks.

Suggested reviewers: shenxianpeng

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: limiting ANSI color output to TTY-supported stdout.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.14%. Comparing base (41df972) to head (616bdaf).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #551      +/-   ##
==========================================
+ Coverage   98.13%   98.14%   +0.01%     
==========================================
  Files          12       12              
  Lines        1337     1348      +11     
==========================================
+ Hits         1312     1323      +11     
  Misses         25       25

☔ 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.

Thank you for your PR!

Merging this PR will not alter performance

✅ 490 untouched benchmarks
🆕 9 new benchmarks
⏩ 121 skipped benchmarks1

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
🆕	test_constants_empty_when_color_off	N/A	12.1 ms	N/A
🆕	test_constants_set_when_color_on	N/A	11.6 ms	N/A
🆕	test_dumb_term_turns_color_off	N/A	2.9 ms	N/A
🆕	test_empty_term_turns_color_off	N/A	2.9 ms	N/A
🆕	test_force_empty_falls_through_to_detection	N/A	2.9 ms	N/A
🆕	test_force_zero_turns_color_off	N/A	2.8 ms	N/A
🆕	test_forced_even_when_piped	N/A	2.8 ms	N/A
🆕	test_not_supported_when_piped	N/A	2.8 ms	N/A
🆕	test_unset_term_still_allows_color_on_a_tty	N/A	2.9 ms	N/A

Comparing larsch:bugfix/tty-aware-color-output (616bdaf) with main (60d7586)2

121 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on main (41df972) during the generation of this report, so 60d7586 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

supports_color() from #551 answered FORCE_COLOR, the TTY and TERM, but not NO_COLOR — the variable users actually export globally to opt out of color (https://no-color.org). Any non-empty value now disables color, outranking detection and yielding only to an explicit FORCE_COLOR. Two gaps in the #551 tests are closed alongside. The reload-based tests recomputed the module constants under a patched environment and left the last reload's values in place for every test that ran afterwards; a fixture now re-derives them on teardown. And nothing exercised the copies commit_check.util binds at import — the ones the print functions actually read — so two subprocess tests now run the real import path end to end and assert on what gets printed.

fix: only emit ANSI color when stdout is a TTY

616bdaf

larsch requested a review from a team as a code owner August 16, 2026 10:31

larsch requested review from shenxianpeng and removed request for a team August 16, 2026 10:31

github-actions Bot added the bug Something isn't working label Aug 16, 2026

shenxianpeng approved these changes Aug 16, 2026

View reviewed changes

shenxianpeng merged commit b446cb8 into commit-check:main Aug 16, 2026
26 checks passed

shenxianpeng mentioned this pull request Aug 16, 2026

fix: support the NO_COLOR convention for disabling color #552

Merged

fix: only emit ANSI color when stdout is a TTY by larsch · Pull Request #551 · commit-check/commit-check · GitHub

Uh oh!

Uh oh!

fix: only emit ANSI color when stdout is a TTY - #551

fix: only emit ANSI color when stdout is a TTY#551
shenxianpeng merged 1 commit into
commit-check:mainfrom
larsch:bugfix/tty-aware-color-output

larsch commented Aug 16, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 16, 2026 •
edited
Loading

Uh oh!

Uh oh!

sonarqubecloud Bot commented Aug 16, 2026

Uh oh!

codecov Bot commented Aug 16, 2026

Uh oh!

shenxianpeng left a comment

Uh oh!

Uh oh!

codspeed-hq Bot commented Aug 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: only emit ANSI color when stdout is a TTY by larsch · Pull Request #551 · commit-check/commit-check · GitHub

Uh oh!

Uh oh!

fix: only emit ANSI color when stdout is a TTY - #551

fix: only emit ANSI color when stdout is a TTY#551shenxianpeng merged 1 commit intocommit-check:maincommit-check/commit-check:mainfrom larsch:bugfix/tty-aware-color-outputlarsch/commit-check:bugfix/tty-aware-color-outputCopy head branch name to clipboard

Conversation

larsch commented Aug 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Tests

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

sonarqubecloud Bot commented Aug 16, 2026

Quality Gate passed

Uh oh!

codecov Bot commented Aug 16, 2026

Codecov Report

Uh oh!

shenxianpeng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq Bot commented Aug 16, 2026

Merging this PR will not alter performance

Performance Changes

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: only emit ANSI color when stdout is a TTY#551
shenxianpeng merged 1 commit into
commit-check:mainfrom
larsch:bugfix/tty-aware-color-output

larsch commented Aug 16, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Aug 16, 2026 •
edited
Loading