From: Jeff King <peff@peff.net>
Date: Fri, 24 Jun 2016 15:31:21 GMT
Subject: Re: [PATCH v2] Refactor recv_sideband()
Message-ID: <20160624153121.GA2494@sigill.intra.peff.net>
In-Reply-To: <20160614210038.31465-1-lfleischer@lfos.de>

On Tue, Jun 14, 2016 at 11:00:38PM +0200, Lukas Fleischer wrote:

> Improve the readability of recv_sideband() significantly by replacing
> fragile buffer manipulations with string buffers and more sophisticated
> format strings. Note that each line is printed using a single write()
> syscall to avoid garbled output when multiple processes write to stderr
> in parallel, see 9ac13ec (atomic write for sideband remote messages,
> 2006-10-11) for details.
> 
> Also, reorganize the overall control flow, remove some superfluous
> variables and replace a custom implementation of strpbrk() with a call
> to the standard C library function.

I happened to be looking at the color-printing code yesterday, and was
reminded that on Windows, fprintf is responsible for converting ANSI
codes into colors the terminal can show:

  $ git grep -A2 IMPORTANT color.h
  color.h: * IMPORTANT: Due to the way these color codes are emulated on Windows,
  color.h- * write them only using printf(), fprintf(), and fputs(). In particular,
  color.h- * do not use puts() or write().

Your patch converts some fprintf calls to write. What does this mean
on Windows for:

  1. Remote servers which send ANSI codes and expect them to look
     reasonable (this might be a losing proposition already, as the
     server won't know anything about the user's terminal, or whether
     output is going to a file).

  2. The use of ANSI_SUFFIX in this function, which uses a similar
     escape code.

-Peff