fix: preserve ANSI color across newlines in DiffPrettyText#154
fix: preserve ANSI color across newlines in DiffPrettyText#154Yanhu007 wants to merge 1 commit intosergi:masterfrom
Conversation
DiffPrettyText previously split insert/delete text on newlines and reset the color code before each newline. This caused the color to not visually extend across line breaks, making multi-line insertions and deletions appear only partially colored. Simplify to wrap the entire diff text in a single color escape sequence pair, which correctly maintains the color through newlines. Fixes sergi#138
utafrali
left a comment
There was a problem hiding this comment.
Clean fix that correctly solves the multi-line coloring bug and cuts the implementation down meaningfully. The one behavior change worth knowing: the ANSI reset now follows any trailing newline in the diff text rather than preceding it, which is fine for terminal foreground colors but reduces compatibility with per-line ANSI processing.
|
|
||
| _, _ = buff.WriteString("\x1b[32m") | ||
| _, _ = buff.WriteString(text) | ||
| _, _ = buff.WriteString("\x1b[0m") |
There was a problem hiding this comment.
When text ends with \n (common in line-oriented diffs), the reset \x1b[0m now lands after the final newline — e.g., \x1b[31mb\nc\n\x1b[0m. The old code placed the reset before the last \n.
For direct terminal output with foreground-only colors this is harmless, but it breaks per-line ANSI processing: if a caller splits the result on \n to render lines independently, the last line of a colored segment will carry an unclosed start code and the line immediately following will begin with a dangling \x1b[0m. If that use case matters, a small guard would fix it:
if strings.HasSuffix(text, "\n") {
buff.WriteString("\x1b[32m")
buff.WriteString(strings.TrimSuffix(text, "\n"))
buff.WriteString("\x1b[0m\n")
} else {
buff.WriteString("\x1b[32m")
buff.WriteString(text)
buff.WriteString("\x1b[0m")
}Not blocking — just worth a conscious decision either way.
Fixes #138
Problem
DiffPrettyTextsplits insert/delete text on newlines and resets the ANSI color escape before each\n. This causes multi-line diffs to appear only partially colored — the color is lost at each line break.Fix
Wrap the entire diff text in a single color escape pair instead of resetting per-line:
This also simplifies the code from 22 lines to 7 lines.
All existing tests pass (updated expected values for the new coloring behavior).