Skip to content

Commit 3bbbcd9

Browse files
lyydsheepclaude
andcommitted
fix(zapio): correct carriage return handling per review feedback
Based on reviewer feedback, the carriage return (\r) handling has been corrected: - Standalone \r now discards any buffered content without logging, matching the intent of issue #1055. - This prevents logging duplicate lines for progress bars (e.g., git clone progress). - Only \n and \r\n sequences trigger actual log messages. - \r\n is correctly handled as a single separator (Windows line ending). Test cases have been updated to reflect the correct behavior: - "carriage return_clears_buffer" tests buffer discard without logging - "progress-style_output" now tests only the final message is logged - "carriage_return_with_buffered_content" tests buffer discard correctly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 17a07b5 commit 3bbbcd9

File tree

2 files changed

+49
-6
lines changed

2 files changed

+49
-6
lines changed

zapio/writer.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ func (w *Writer) Write(bs []byte) (n int, err error) {
9090
// unconsumed bytes.
9191
//
9292
// It handles both newlines (\n) and carriage returns (\r).
93+
// \n and \r\n cause the buffer to be flushed to the logger.
94+
// Standalone \r discards any buffered content without logging (for overwriting progress).
9395
func (w *Writer) writeLine(line []byte) (remaining []byte) {
9496
// Find the first occurrence of either \n or \r
9597
nlIdx := bytes.IndexByte(line, '\n')
@@ -98,6 +100,7 @@ func (w *Writer) writeLine(line []byte) (remaining []byte) {
98100
// Determine which separator comes first (or if neither exists)
99101
sepIdx := -1
100102
sepLen := 0
103+
crOnly := false
101104

102105
if nlIdx >= 0 && (crIdx < 0 || nlIdx <= crIdx) {
103106
sepIdx = nlIdx
@@ -109,6 +112,7 @@ func (w *Writer) writeLine(line []byte) (remaining []byte) {
109112
sepLen = 2
110113
} else {
111114
sepLen = 1
115+
crOnly = true
112116
}
113117
}
114118

@@ -121,6 +125,13 @@ func (w *Writer) writeLine(line []byte) (remaining []byte) {
121125
// Split on the separator, buffer and flush the left.
122126
line, remaining = line[:sepIdx], line[sepIdx+sepLen:]
123127

128+
if crOnly {
129+
// A standalone \r discards any buffered content without logging.
130+
// This handles progress bars that overwrite themselves.
131+
w.buff.Reset()
132+
return remaining
133+
}
134+
124135
// Fast path: if we don't have a partial message from a previous write
125136
// in the buffer, skip the buffer and log directly.
126137
if w.buff.Len() == 0 {

zapio/writer_test.go

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,11 @@ func TestWriter(t *testing.T) {
127127
},
128128
},
129129
{
130-
desc: "carriage return creates line break",
130+
desc: "carriage return clears buffer",
131131
writes: []string{
132-
"foo\rbar\r",
132+
"foo\rbar",
133133
},
134134
want: []zapcore.Entry{
135-
{Level: zap.InfoLevel, Message: "foo"},
136135
{Level: zap.InfoLevel, Message: "bar"},
137136
},
138137
},
@@ -149,14 +148,47 @@ func TestWriter(t *testing.T) {
149148
{
150149
desc: "progress-style output with multiple updates",
151150
writes: []string{
152-
"progress: 10%\rprogress: 25%\rprogress: 50%\r",
151+
"progress: 10%\rprogress: 25%\rprogress: 50%\r\n",
153152
},
154153
want: []zapcore.Entry{
155-
{Level: zap.InfoLevel, Message: "progress: 10%"},
156-
{Level: zap.InfoLevel, Message: "progress: 25%"},
157154
{Level: zap.InfoLevel, Message: "progress: 50%"},
158155
},
159156
},
157+
{
158+
desc: "mixed newlines and carriage returns",
159+
writes: []string{
160+
"foo\nbar\r\rbaz\r\nqux\n",
161+
},
162+
want: []zapcore.Entry{
163+
{Level: zap.InfoLevel, Message: "foo"},
164+
{Level: zap.InfoLevel, Message: "baz"},
165+
{Level: zap.InfoLevel, Message: "qux"},
166+
},
167+
},
168+
{
169+
desc: "carriage return with buffered content",
170+
writes: []string{
171+
"foo",
172+
"ba",
173+
"r\rqux",
174+
"\n",
175+
},
176+
want: []zapcore.Entry{
177+
{Level: zap.InfoLevel, Message: "qux"},
178+
},
179+
},
180+
{
181+
desc: "carriage return newline with buffered content",
182+
writes: []string{
183+
"foo",
184+
"ba",
185+
"r\r\nqux\r\n",
186+
},
187+
want: []zapcore.Entry{
188+
{Level: zap.InfoLevel, Message: "foobar"},
189+
{Level: zap.InfoLevel, Message: "qux"},
190+
},
191+
},
160192
}
161193

162194
for _, tt := range tests {

0 commit comments

Comments
 (0)