Strip ANSI color codes from output file while keeping console colors#2293
Strip ANSI color codes from output file while keeping console colors#2293Mzack9999 merged 4 commits intoprojectdiscovery:devfrom
Conversation
WalkthroughAdds an internal ANSI escape-code remover to the runner package and uses a conditional wrapper to strip ANSI sequences from plain text output when the NoColor option is set; JSON/CSV outputs and public APIs are unchanged. (33 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner
participant Formatter
participant Handler as handleStripAnsiCharacters()
participant stripANSI as stripANSI()
participant Output
Runner->>Formatter: Prepare response (resp.str)
alt Plain text output (NoColor=true)
Formatter->>Handler: handleStripAnsiCharacters(resp.str, true)
Handler->>stripANSI: stripANSI(resp.str)
stripANSI-->>Handler: cleaned string
Handler-->>Formatter: cleaned string
Formatter->>Output: write cleaned string
else Plain text output (NoColor=false)
Formatter->>Handler: handleStripAnsiCharacters(resp.str, false)
Handler-->>Formatter: original resp.str
Formatter->>Output: write original resp.str
else JSON/CSV output path
Formatter->>Output: write raw resp.str (no stripping)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
runner/runner.go (2)
104-105: LGTM! Efficient use of precompiled regex.The precompiled ANSI regex pattern correctly matches standard SGR (Select Graphic Rendition) escape sequences for color codes, which is appropriate for the stated use case of removing colors from file output.
Optional enhancement: Consider adding a comment explaining the regex pattern for maintainability:
+// ansiRegex matches ANSI SGR (Select Graphic Rendition) escape sequences +// Pattern: ESC [ <params> m where params are digits/semicolons var ansiRegex = regexp.MustCompile(`\x1b\[[0-9;]*m`)Note: The current pattern covers standard color codes but doesn't match cursor movement or other non-SGR sequences, which is acceptable for this use case.
2710-2713: LGTM! Clean implementation with efficient regex usage.The
stripANSIfunction correctly uses the precompiled regex for optimal performance. The implementation is straightforward and thread-safe.Recommended enhancement: Add unit tests to verify ANSI stripping behavior:
func TestStripANSI(t *testing.T) { tests := []struct { input string expected string }{ {"plain text", "plain text"}, {"\x1b[31mred text\x1b[0m", "red text"}, {"\x1b[1;32mbold green\x1b[0m", "bold green"}, {"mixed \x1b[31mred\x1b[0m text", "mixed red text"}, } for _, tt := range tests { if got := stripANSI(tt.input); got != tt.expected { t.Errorf("stripANSI(%q) = %q, want %q", tt.input, got, tt.expected) } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
runner/runner.go(3 hunks)
⏰ 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). (8)
- GitHub Check: Test Builds (macOS-latest)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Functional Test (windows-latest)
- GitHub Check: Analyze (go)
- GitHub Check: Functional Test (ubuntu-latest)
- GitHub Check: Functional Test (macOS-latest)
- GitHub Check: release-test
🔇 Additional comments (1)
runner/runner.go (1)
1186-1190: LGTM! Logic correctly handles ANSI stripping for file output.The conditional logic properly achieves the PR objective:
- When colors are enabled (
NoColor=false): Console output remains colorized viaresp.str, while file output is cleaned by stripping ANSI codes- When colors are disabled (
NoColor=true): Both console and file output contain no ANSI codes (no stripping needed)This implementation successfully addresses the issue where users previously had to disable console colors to get clean file output.
|
@Mzack9999 it saves an unnecessary call to stripANSI when the user is using the -nc flag. |
|
@ayanrajpoot10 Thanks for pointing out the optimization. I've summarized it into an helper function |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
runner/runner.go (1)
1351-1356: Consider improving clarity with better naming or inlining.The logic is correct and implements the optimization from the PR comments. However, the function and parameter names could be clearer:
handleStripAnsiCharacters→stripANSIConditionallyormaybeStripANSIskip→noColororskipStrippingAlternatively, you could inline this at the call site as originally suggested in the PR comments, which would be equally clear:
- plainFile.WriteString(handleStripAnsiCharacters(resp.str, r.options.NoColor) + "\n") + if r.options.NoColor { + plainFile.WriteString(resp.str + "\n") + } else { + plainFile.WriteString(stripANSI(resp.str) + "\n") + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
runner/runner.go(4 hunks)
⏰ 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). (7)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Test Builds (macOS-latest)
- GitHub Check: Analyze (go)
- GitHub Check: Functional Test (windows-latest)
- GitHub Check: Functional Test (macOS-latest)
- GitHub Check: Functional Test (ubuntu-latest)
🔇 Additional comments (3)
runner/runner.go (3)
104-105: LGTM! Good use of package-level precompiled regex.The ANSI regex pattern correctly matches SGR color codes and is precompiled for efficient reuse across all file writes.
1186-1186: Correct implementation with optimization in place.The ANSI stripping is properly applied only to file output while preserving console colors. The optimization suggested in the PR comments (skipping the strip operation when
-ncis used) is correctly implemented here.
2713-2716: LGTM! Clean implementation.The function correctly uses the precompiled regex to strip ANSI codes. The implementation is simple, efficient, and well-documented.
2 participants
This PR adds support for removing ANSI color codes from output files while still allowing colored output in the console.
-ncsee zero impact, and stripping ANSI codes adds only a small cost for colored lines.Summary by CodeRabbit