Fix: Respect --no-color flag in syntax highlighting

[osterman]

what

• The HighlightCodeWithConfig function in pkg/utils/highlight_utils.go has been updated to correctly respect the --no-color flag and Settings.Terminal.Color configuration.
• Previously, this function did not check these settings, leading to colored output even when colors were explicitly disabled for JSON and YAML formatting.
• This fix ensures that all syntax highlighting, including for JSON and YAML outputs, adheres to the terminal color settings.

why

• This addresses a regression introduced on May 14, 2025, when the --no-color flag was added to the CLI.
• While the markdown renderer was correctly updated to respect the flag, the core syntax highlighting function (HighlightCodeWithConfig) was overlooked.
• This resulted in an inconsistency where JSON and YAML outputs continued to display color codes despite the user's intent to disable them, impacting commands like atmos describe stacks.
• The fix aligns the behavior of all code highlighting with the user's --no-color preference and terminal color settings.

references

• closes Describe stacks --no-color options is not working #1651
• DEV-3701
• Audit Report: NO_COLOR_BUG_AUDIT.md

Summary by CodeRabbit

• Bug Fixes

  • The --no-color flag now works correctly across all syntax highlighting utilities, ensuring ANSI color codes are excluded from output when the flag is enabled.
  • Improved handling of nil configurations to prevent errors during output highlighting.

• Tests

  • Added comprehensive tests to verify no-color behavior across highlighting features and the Describe Stacks command.

[coderabbitai]

📝 Walkthrough

Walkthrough

Fixes the --no-color flag not being respected in the describe stacks command by adding nil-config handling and respecting NoColor/Color settings in the highlight utility, paired with unit and integration tests validating the behavior.

Changes

Cohort / File(s)	Change Summary
Core highlighting logic pkg/utils/highlight_utils.go	Adds nil-check for config parameter; derives colorDisabled from config settings (NoColor takes precedence over Color); extends highlight-eligibility condition to skip when colorDisabled is true
Highlight utility tests pkg/utils/highlight_utils_no_color_test.go	New test file validating no-color behavior: tests NoColor/Color flag precedence, ANSI escape sequence absence in JSON/YAML output, and nil-config handling across multiple highlight functions
Integration tests tests/describe_stacks_no_color_test.go	New test file for describe stacks no-color flag; defines scenarios with NoColor true/false, validates ANSI color code presence/absence in output via helper function

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

The changes are narrowly scoped to one utility function with straightforward nil-checking and conditional logic. The test additions are well-structured and validate expected behavior clearly. The logic density is low, and changes follow consistent patterns across test scenarios.

Possibly related PRs

• Colorize Atmos Describe Commands when TTY attached #907: Introduced the HighlightCodeWithConfig function that this PR extends with NoColor/Color flag support and nil-config handling.

Suggested reviewers

• aknysh

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Fix: Respect --no-color flag in syntax highlighting" is concise, specific, and directly reflects the primary change in the pull request. It clearly summarizes the main objective: ensuring that the syntax highlighting utilities properly honor the --no-color flag when processing code, JSON, and YAML outputs. The title is appropriately focused and would give a teammate scanning commit history a clear understanding of the fix.
Linked Issues Check	✅ Passed	The code changes directly address the requirements outlined in linked issues #1651 and DEV-3701, which report that atmos describe stacks --no-color produces colored output despite the flag being specified. The implementation adds a nil-check for config in HighlightCodeWithConfig, derives a colorDisabled flag from the terminal color settings (with NoColor taking precedence), and extends the highlight-eligibility condition to skip coloring when disabled. The three new test functions (TestHighlightCodeWithConfig_RespectsNoColorFlag, TestPrintAsYAML_RespectsNoColorFlag, TestPrintAsJSON_RespectsNoColorFlag) and the describe stacks integration test validate that colored output is correctly suppressed when --no-color is specified, meeting the expected behavior stated in both issues.
Out of Scope Changes Check	✅ Passed	All code changes remain focused on the stated objective of fixing the --no-color flag in syntax highlighting. The modifications to highlight_utils.go target the core issue, the new test files verify the fix works across multiple output formats (JSON, YAML, and describe stacks), and the nil-config handling is a necessary safeguard for the feature. There are no changes that diverge from the scope of addressing the --no-color flag behavior described in the linked issues.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/dev-3701-describe-stacks-no-color-options-is-not-working

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

NO_COLOR_BUG_AUDIT.md (1)

45-64: Thorough audit documentation.

The root cause analysis and fix documentation are excellent. One minor markdown improvement: the fenced code block on line 45 is missing a language identifier (the static analysis tool flagged this). Adding a language identifier improves rendering:

-```
+```text
 User runs: atmos describe stacks --no-color
   ↓
 cmd/describe_stacks.go: processes --no-color flag

Not critical, but cleaner markdown.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6e04105 and 286fd9f.

📒 Files selected for processing (4)

• NO_COLOR_BUG_AUDIT.md (1 hunks)
• pkg/utils/highlight_utils.go (1 hunks)
• pkg/utils/highlight_utils_no_color_test.go (1 hunks)
• tests/describe_stacks_no_color_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

pkg/**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

Place business logic in pkg rather than in cmd

Files:

• pkg/utils/highlight_utils.go
• pkg/utils/highlight_utils_no_color_test.go

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All Go comments must end with periods; applies to single-line, multi-line, inline, and documentation comments (golangci-lint godot).
Group imports into three sections (stdlib, 3rd-party, Atmos), separated by blank lines; sort alphabetically within each group; preserve existing aliases.
Configuration loading must use Viper with precedence CLI → ENV → files → defaults; bind config name atmos and add path, AutomaticEnv, and ATMOS prefix.
All errors must be wrapped using static errors (defined in errors/errors.go); use errors.Join for multiple errors; fmt.Errorf with %w for context; use errors.Is for checks; never compare error strings.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; never use logging for UI.
Most text UI must go to stderr; only data/results to stdout; prefer utils.PrintfMessageToTUI for UI messages.
All new configurations must support Go templating using existing utilities and available template functions.
Prefer SDKs over external binaries for cross-platform support; use filepath/os/runtime for portability.
For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
80% minimum coverage on new/changed lines and include unit tests for new features; add integration tests for CLI using tests/ fixtures.
Always bind environment variables with viper.BindEnv and provide ATMOS_ alternatives for every env var.
Use structured logging with levels (Fatal>Error>Warn>Debug>Trace); avoid string interpolation and ensure logging does not affect execution.
Prefer re...

Files:

• pkg/utils/highlight_utils.go
• pkg/utils/highlight_utils_no_color_test.go
• tests/describe_stacks_no_color_test.go

**/!(*_test).go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

Document all exported functions, types, and methods with Go doc comments

Add defer perf.Track() to all public functions and critical private functions; include a blank line after the call; use package-prefixed names; pass atmosConfig when present, else nil.

Files:

• pkg/utils/highlight_utils.go

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios

**/*_test.go: Use table-driven unit tests for pure functions and focus on behavior; co-locate tests; target >80% coverage for pkg/ and internal/exec/.
Always use t.Skipf() with a clear reason; never use t.Skip() or t.Skipf without a reason.

Files:

• pkg/utils/highlight_utils_no_color_test.go
• tests/describe_stacks_no_color_test.go

pkg/**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests for packages live under pkg/ alongside implementation files.

Files:

• pkg/utils/highlight_utils_no_color_test.go

tests/**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

Integration tests live under tests/ using fixtures in tests/test-cases/.

Files:

• tests/describe_stacks_no_color_test.go

🧠 Learnings (1)

📚 Learning: 2025-06-07T19:27:40.807Z

Learnt from: samtholiya
PR: cloudposse/atmos#1266
File: pkg/utils/highlight_utils.go:0-0
Timestamp: 2025-06-07T19:27:40.807Z
Learning: In pkg/utils/highlight_utils.go, the global variable `isTermPresent` that caches terminal detection at package initialization is an intentional design choice and should not be changed to per-call detection.

Applied to files:

• pkg/utils/highlight_utils.go

🧬 Code graph analysis (3)

pkg/utils/highlight_utils.go (1)

pkg/schema/schema.go (2)

• Settings (685-689)
• Terminal (208-216)

pkg/utils/highlight_utils_no_color_test.go (4)

pkg/schema/schema.go (4)

• AtmosConfiguration (27-65)
• AtmosSettings (251-271)
• Terminal (208-216)
• SyntaxHighlighting (242-249)

pkg/utils/highlight_utils.go (1)

• HighlightCodeWithConfig (80-128)

pkg/utils/yaml_utils.go (1)

• GetHighlightedYAML (91-103)

pkg/utils/json_utils.go (1)

• GetHighlightedJSON (28-46)

tests/describe_stacks_no_color_test.go (3)

pkg/schema/schema.go (4)

• AtmosConfiguration (27-65)
• AtmosSettings (251-271)
• Terminal (208-216)
• SyntaxHighlighting (242-249)

pkg/utils/yaml_utils.go (1)

• GetHighlightedYAML (91-103)

pkg/utils/json_utils.go (1)

• GetHighlightedJSON (28-46)

🪛 LanguageTool

NO_COLOR_BUG_AUDIT.md

[grammar] ~140-~140: Please add a punctuation mark at the end of paragraph.
Context: ... → No colors 5. Color=true → Colors enabled ### Testing the Fix After implementin...

(PUNCTUATION_PARAGRAPH_END)

---

[style] ~225-~225: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...separate --no-color handling if they're affected. ## Workarounds (Current) Users can w...

(EN_REPEATEDWORDS_AFFECT)

---

[style] ~264-~264: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ... existed for ~5 months from May 14, 2025 to October 17, 2025. ## References - ...

(MISSING_COMMA_AFTER_YEAR)

---

[style] ~271-~271: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...ed), affects all versions since May 14, 2025 - Introduced in: PR #1227 (commit `...

(MISSING_COMMA_AFTER_YEAR)

---

[style] ~272-~272: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...PR #1227 (commit 8d4bfcf7b) - May 14, 2025 - Fixed in: Current fix (October 17...

(MISSING_COMMA_AFTER_YEAR)

---

[grammar] ~327-~327: Please add a punctuation mark at the end of paragraph.
Context: ...6. ✅ Work for both JSON and YAML output formats ## Next Steps 1. ✅ Create tests repro...

(PUNCTUATION_PARAGRAPH_END)

🪛 markdownlint-cli2 (0.18.1)

NO_COLOR_BUG_AUDIT.md

45-45: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Analyze (go)
• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Summary

🔇 Additional comments (6)

tests/describe_stacks_no_color_test.go (2)

14-88: Well-structured integration test.

The table-driven approach and TTY-aware assertions are solid. You're correctly handling the CI environment limitation by only asserting color absence (not presence), since TTY detection will prevent highlighting in non-terminal environments anyway.

---

90-97: Good ANSI detection helper.

Covers the standard escape sequence patterns. The three checks handle different representations of the ESC character (octal and hex variants).

pkg/utils/highlight_utils_no_color_test.go (3)

14-118: Comprehensive unit test with excellent coverage.

The four scenarios properly validate all flag combinations, including the critical precedence rule (NoColor overrides Color). The TTY-aware logic is pragmatic - logging expected color behavior but only asserting absence to avoid false failures in CI.

---

120-149: LGTM!

Straightforward test for YAML highlighting with no-color flag. Follows the established pattern.

---

151-180: LGTM!

Mirrors the YAML test for JSON output. Good parallel coverage.

pkg/utils/highlight_utils.go (1)

86-91: Verification confirms the implementation is correct—no issues found.

The config loading logic (pkg/config/config.go:84-89) explicitly manages Color based on the NO_COLOR env var: when set to "false", Color becomes true; when set to "true", Color becomes false. The default behavior (both fields false) correctly disables colors, and test cases across the codebase validate this precedence. The logic at line 88 is sound.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

pkg/utils/highlight_utils_no_color_test.go (1)

104-115: Strengthen positive test case assertion or skip explicitly.

The test only asserts the negative case (no color) but logs without asserting when colors are expected. This leaves a gap in coverage for the "Color flag set to true should enable colors" scenario.

Consider one of these approaches:

• If TTY detection is unreliable in test environments, use t.Skipf("Skipping color-enabled assertion: TTY detection unreliable in CI") for that specific test case
• Alternatively, assert when hasColor is true but add a clear comment explaining the limitation: if hasColor { assert.True(t, hasColor, "Colors should be present when Color=true in TTY") }

Apply this diff to improve the assertion logic:

 		if tt.expectColor {
-			// In a terminal, we expect colors
-			// But this test might run in CI where there's no TTY
-			// So we'll just document the expected behavior
-			t.Logf("Expected colors (hasColor=%v): %s", hasColor, tt.description)
+			// Note: This assertion may not trigger in CI without a TTY.
+			if hasColor {
+				assert.True(t, hasColor, "Output should contain ANSI escape codes when Color=true in a TTY environment")
+			} else {
+				t.Logf("Skipped color assertion (no TTY detected): %s", tt.description)
+			}
 		} else {

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 286fd9f and 0362ceb.

📒 Files selected for processing (2)

• pkg/utils/highlight_utils.go (1 hunks)
• pkg/utils/highlight_utils_no_color_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/utils/highlight_utils.go

🧰 Additional context used

📓 Path-based instructions (4)

pkg/**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

Place business logic in pkg rather than in cmd

Files:

• pkg/utils/highlight_utils_no_color_test.go

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios

**/*_test.go: Use table-driven unit tests for pure functions and focus on behavior; co-locate tests; target >80% coverage for pkg/ and internal/exec/.
Always use t.Skipf() with a clear reason; never use t.Skip() or t.Skipf without a reason.

Files:

• pkg/utils/highlight_utils_no_color_test.go

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All Go comments must end with periods; applies to single-line, multi-line, inline, and documentation comments (golangci-lint godot).
Group imports into three sections (stdlib, 3rd-party, Atmos), separated by blank lines; sort alphabetically within each group; preserve existing aliases.
Configuration loading must use Viper with precedence CLI → ENV → files → defaults; bind config name atmos and add path, AutomaticEnv, and ATMOS prefix.
All errors must be wrapped using static errors (defined in errors/errors.go); use errors.Join for multiple errors; fmt.Errorf with %w for context; use errors.Is for checks; never compare error strings.
Distinguish structured logging from UI output: UI prompts/status/errors to stderr; data/results to stdout; never use logging for UI.
Most text UI must go to stderr; only data/results to stdout; prefer utils.PrintfMessageToTUI for UI messages.
All new configurations must support Go templating using existing utilities and available template functions.
Prefer SDKs over external binaries for cross-platform support; use filepath/os/runtime for portability.
For non-standard execution paths, capture telemetry via telemetry.CaptureCmd or telemetry.CaptureCmdString without user data.
80% minimum coverage on new/changed lines and include unit tests for new features; add integration tests for CLI using tests/ fixtures.
Always bind environment variables with viper.BindEnv and provide ATMOS_ alternatives for every env var.
Use structured logging with levels (Fatal>Error>Warn>Debug>Trace); avoid string interpolation and ensure logging does not affect execution.
Prefer re...

Files:

• pkg/utils/highlight_utils_no_color_test.go

pkg/**/*_test.go

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests for packages live under pkg/ alongside implementation files.

Files:

• pkg/utils/highlight_utils_no_color_test.go

🧬 Code graph analysis (1)

pkg/utils/highlight_utils_no_color_test.go (4)

pkg/schema/schema.go (4)

• AtmosConfiguration (27-65)
• AtmosSettings (251-271)
• Terminal (208-216)
• SyntaxHighlighting (242-249)

pkg/utils/highlight_utils.go (1)

• HighlightCodeWithConfig (80-133)

pkg/utils/yaml_utils.go (1)

• GetHighlightedYAML (91-103)

pkg/utils/json_utils.go (1)

• GetHighlightedJSON (28-46)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: Build (macos-latest, macos)
• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Analyze (go)
• GitHub Check: Lint (golangci)
• GitHub Check: Summary

🔇 Additional comments (3)

pkg/utils/highlight_utils_no_color_test.go (3)

121-149: Well-structured test.

This test correctly verifies that GetHighlightedYAML respects the NoColor flag by checking for absence of ANSI escape codes.

---

152-180: Well-structured test.

This test correctly verifies that GetHighlightedJSON respects the NoColor flag by checking for absence of ANSI escape codes.

---

183-196: Good edge case coverage.

This test properly validates the nil config handling, ensuring no panic and returning plain code. This aligns well with the nil-check added to HighlightCodeWithConfig in the main implementation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.00%. Comparing base (80680b6) to head (bf29bc9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1652      +/-   ##
==========================================
+ Coverage   65.98%   66.00%   +0.02%     
==========================================
  Files         343      343              
  Lines       38686    38693       +7     
==========================================
+ Hits        25526    25540      +14     
+ Misses      11163    11157       -6     
+ Partials     1997     1996       -1     

Flag	Coverage Δ
unittests	66.00% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/utils/highlight_utils.go	12.59% <100.00%> (+4.78%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

These changes were released in v1.195.0-rc.1.