Buffer partial lines in sessionStderr until a terminator arrives

1eb0d68→main·

Soph·2mo ago·3 files·+56 added/-12 removed

prefixedLineWriter inside internal/gitproto writes a sideband progress line in two Write calls — first the "source: " or "target: " prefix, then the content with its '\r' or '\n' terminator. sessionStderr was treating each Write as its own logical line, so verbose runs printed the prefix on a row by itself, then the content on the next row, instead of "source: Counting objects: 10%" on a single row.

Buffer partial-line input in a strings.Builder per writer instance and only flush on a terminator. The prefix lands in the buffer, the chunk's terminator triggers one combined notify("source: Counting objects: 10%"). Switch the type to use pointer receivers since each writer now carries state, and update construction sites in newSession to pass &sessionStderr{...} for the slog handler and both Conn.ProgressOut hooks.

Sessions

aabc1d4bcd1fView transcript

Changes

3

• internal/syncer

• Mprogress.go+16/-8

• Mprogress_test.go+37/-1

• Msyncer.go+3/-3

202 unmodified lines

203
204
205
206
207
208
209
206
207
208
209
210
211
212
213
214
215
211
216
217
218
219
5 unmodified lines

225
226
227
223
228
229
230
226
227
231
232
233
234
235
236
237
238

202 unmodified lines

// above the in-place ticker frame instead of clobbering it. Falls back
// to os.Stderr when no reporter is active.
//
// Each Write may carry multiple lines or carriage-returned in-place
// updates; both '\n' and '\r' are treated as line ends so the reporter
// receives one notify per logical line.
type sessionStderr struct{ s *syncSession }
// Partial-line writes are buffered until a '\n' or '\r' terminator
// arrives. This matters for prefixedLineWriter, which writes a logical
// line in two calls — first the prefix ("source: "), then the content
// with terminator — and would otherwise produce two separate notify
// frames split mid-line. Use as a pointer (the buffer is stateful).
type sessionStderr struct {
    s   *syncSession
    buf strings.Builder
}

func (w sessionStderr) Write(b []byte) (int, error) {
func (w *sessionStderr) Write(b []byte) (int, error) {
    if w.s == nil || w.s.progress == nil {
        n, err := os.Stderr.Write(b)
        if err != nil {
5 unmodified lines

for s != "" {
            i := strings.IndexAny(s, "\r\n")
            if i < 0 {
                w.s.progress.notify(s)
                w.buf.WriteString(s)
                break
            }
            if i > 0 {
                w.s.progress.notify(s[:i])
            w.buf.WriteString(s[:i])
            line := w.buf.String()
            w.buf.Reset()
            if line != "" {
                w.s.progress.notify(line)
            }
            s = s[i+1:]
        }
    }

Minternal/syncer/progress.go+16/-8

252 unmodified lines

253
254
255
256
256
257
258
259
10 unmodified lines

270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311

252 unmodified lines

p.render(false) // give notify something to clear

sess := &syncSession{progress: p}
sink := sessionStderr{s: sess}
sink := &sessionStderr{s: sess}

// Multi-line write (e.g. a slog line followed by a sideband line)
    // must produce one notify per logical line — both '\n' and '\r' are
10 unmodified lines

}
}

// TestSessionStderrBuffersPartialLines covers the prefixedLineWriter
// pattern in internal/gitproto: a logical line arrives in two writes —
// first the "source: " prefix, then the content with terminator. The
// buffered writer must combine them into one notify call instead of
// emitting the prefix on its own row.
func TestSessionStderrBuffersPartialLines(t *testing.T) {
    t.Parallel()
    stats := newStats(true)
    stats.setSideDisplay("source", "github.com")
    stats.side("source").bytes.Store(1024)

var buf bytes.Buffer
    p := newProgressReporter(&buf, stats, 0)
    p.render(false)

sess := &syncSession{progress: p}
sink := &sessionStderr{s: sess}

if _, err := sink.Write([]byte("source: ")); err != nil {
        t.Fatalf("prefix write: %v", err)
    }
    if _, err := sink.Write([]byte("Counting objects: 10%\r")); err != nil {
        t.Fatalf("content write: %v", err)
    }

out := buf.String()
    if !strings.Contains(out, "source: Counting objects: 10%") {
        t.Errorf("expected joined line 'source: Counting objects: 10%%', got %q", out)
    }
    // Reject the bug shape: prefix on its own line followed by content
    // on a separate line.
    if strings.Contains(out, "source: \n") || strings.Contains(out, "source:\n") {
        t.Errorf("prefix should not be emitted as a standalone line: %q", out)
    }
}

func TestProgressReporterNotifyClearsAndRedraws(t *testing.T) {
    t.Parallel()
    stats := newStats(true)