Skip to content

fix: preserve ANSI color across newlines in DiffPrettyText#154

Open
Yanhu007 wants to merge 1 commit intosergi:masterfrom
Yanhu007:fix/pretty-text-newline-coloring
Open

fix: preserve ANSI color across newlines in DiffPrettyText#154
Yanhu007 wants to merge 1 commit intosergi:masterfrom
Yanhu007:fix/pretty-text-newline-coloring

Conversation

@Yanhu007
Copy link
Copy Markdown

Fixes #138

Problem

DiffPrettyText splits 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:

// Before: splits on newlines, resets color at each break
lines := strings.Split(text, "\n")
for i, line := range lines {
    buff.WriteString("\x1b[32m")
    buff.WriteString(line)
    buff.WriteString("\x1b[0m\n") // color lost here
}

// After: single color wrap, newlines stay colored
buff.WriteString("\x1b[32m")
buff.WriteString(text)
buff.WriteString("\x1b[0m")

This also simplifies the code from 22 lines to 7 lines.

All existing tests pass (updated expected values for the new coloring behavior).

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
Copy link
Copy Markdown

@utafrali utafrali left a comment

Choose a reason for hiding this comment

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

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")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

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.

DiffPrettyText does does not colour all diffs with newlines

2 participants