Re: [PATCH v2] Refactor recv_sideband()

2016-06-28 Thread Johannes Schindelin
Hi Junio, On Tue, 28 Jun 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > Erm, sorry... > > > > On Tue, 28 Jun 2016, Johannes Schindelin wrote: > > > >> [...] we actually do not override fprintf() at all anymore [*1*] [...] > > > > ... forgot the > >

Re: [PATCH v2] Refactor recv_sideband()

2016-06-28 Thread Junio C Hamano
Johannes Schindelin writes: > Erm, sorry... > > On Tue, 28 Jun 2016, Johannes Schindelin wrote: > >> [...] we actually do not override fprintf() at all anymore [*1*] [...] > > ... forgot the > > Footnote *1*: In Git for Windows, we actually *do* override fprintf(), >

Re: [PATCH v2] Refactor recv_sideband()

2016-06-28 Thread Johannes Schindelin
Erm, sorry... On Tue, 28 Jun 2016, Johannes Schindelin wrote: > [...] we actually do not override fprintf() at all anymore [*1*] [...] ... forgot the Footnote *1*: In Git for Windows, we actually *do* override fprintf(), thanks to using gettext, but it is *gettext* that is doing the overriding

Re: [PATCH v2] Refactor recv_sideband()

2016-06-28 Thread Johannes Schindelin
Hi Peff, On Mon, 27 Jun 2016, Jeff King wrote: > On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote: > > > It's just you used xwrite() there that introduced a different issue. > > Wouldn't replacing it with fwrite(stderr) without changing anything > > else solve that? > > I am

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Junio C Hamano
Lukas Fleischer writes: > I do not see how using fwrite() buys us anything. Neither fwrite() nor > fputs() nor fprintf() guarantee to call write() only once. That is not the point. Your first attempt split what used to be a single fprintf(), which (as Nico explained)

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Lukas Fleischer
On Mon, 27 Jun 2016 at 22:47:59, Nicolas Pitre wrote: > On Mon, 27 Jun 2016, Lukas Fleischer wrote: > > > On Mon, 27 Jun 2016 at 19:50:13, Junio C Hamano wrote: > > > Jeff King writes: > > > > > > > On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote: > > > > > > > >>

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Nicolas Pitre
On Mon, 27 Jun 2016, Lukas Fleischer wrote: > On Mon, 27 Jun 2016 at 19:50:13, Junio C Hamano wrote: > > Jeff King writes: > > > > > On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote: > > > > > >> It's just you used xwrite() there that introduced a different issue. >

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Lukas Fleischer
On Mon, 27 Jun 2016 at 19:50:13, Junio C Hamano wrote: > Jeff King writes: > > > On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote: > > > >> It's just you used xwrite() there that introduced a different issue. > >> Wouldn't replacing it with fwrite(stderr) without

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Junio C Hamano
Jeff King writes: > On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote: > >> It's just you used xwrite() there that introduced a different issue. >> Wouldn't replacing it with fwrite(stderr) without changing anything >> else solve that? > > I am having trouble actually

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Jeff King
On Mon, Jun 27, 2016 at 08:54:22AM -0700, Junio C Hamano wrote: > It's just you used xwrite() there that introduced a different issue. > Wouldn't replacing it with fwrite(stderr) without changing anything > else solve that? I am having trouble actually seeing how the ANSI-emulation code gets

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Junio C Hamano
Lukas Fleischer writes: > On Fri, 24 Jun 2016 at 20:32:28, Junio C Hamano wrote: >> On Fri, Jun 24, 2016 at 11:14 AM, Jeff King wrote: >> > >> > I do wonder about the ANSI_SUFFIX thing, though. >> >> compat/winansi.c seems to have a provision for 'K' (and

Re: [PATCH v2] Refactor recv_sideband()

2016-06-27 Thread Lukas Fleischer
On Fri, 24 Jun 2016 at 20:32:28, Junio C Hamano wrote: > On Fri, Jun 24, 2016 at 11:14 AM, Jeff King wrote: > > > > I do wonder about the ANSI_SUFFIX thing, though. > > compat/winansi.c seems to have a provision for 'K' (and obviously 'm' > for colors). So we probably want to

Re: [PATCH v2] Refactor recv_sideband()

2016-06-24 Thread Dennis Kaarsemaker
On vr, 2016-06-24 at 14:14 -0400, Jeff King wrote: > On Fri, Jun 24, 2016 at 07:45:04PM +0200, Johannes Schindelin wrote: >> Do we *actually* send color via the sideband, like, ever? > We don't, but remember that we forward arbitrary output from hooks. > If the consensus is "nah, it is probably

Re: [PATCH v2] Refactor recv_sideband()

2016-06-24 Thread Junio C Hamano
On Fri, Jun 24, 2016 at 11:14 AM, Jeff King wrote: > > I do wonder about the ANSI_SUFFIX thing, though. compat/winansi.c seems to have a provision for 'K' (and obviously 'm' for colors). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to

Re: [PATCH v2] Refactor recv_sideband()

2016-06-24 Thread Jeff King
On Fri, Jun 24, 2016 at 07:45:04PM +0200, Johannes Schindelin wrote: > > $ 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, > >

Re: [PATCH v2] Refactor recv_sideband()

2016-06-24 Thread Johannes Schindelin
Hi Peff, On Fri, 24 Jun 2016, Jeff King wrote: > 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

Re: [PATCH v2] Refactor recv_sideband()

2016-06-24 Thread Jeff King
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

Re: [PATCH v2] Refactor recv_sideband()

2016-06-19 Thread Lukas Fleischer
On Wed, 15 Jun 2016 at 09:14:54, Lukas Fleischer wrote: > What we could do is reintroduce the local prefix variable I had in v1 > and use that to store whether a prefix needs to be prepended or not. If > we do that and if we are fine with strbuf memory being (potentially) > allocated and

Re: [PATCH v2] Refactor recv_sideband()

2016-06-14 Thread Jeff King
On Tue, Jun 14, 2016 at 02:25:42PM -0700, Junio C Hamano wrote: > > 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 find that calling the loop "a custom

Re: [PATCH v2] Refactor recv_sideband()

2016-06-14 Thread Junio C Hamano
Lukas Fleischer writes: Lukas Fleischer writes: > Improve the readability of recv_sideband() significantly by replacing s/significantly //; "making it readable" is already a subjective goodness criterion, and you do not have to make it sound even more

Re: [PATCH v2] Refactor recv_sideband()

2016-06-14 Thread Lukas Fleischer
On Tue, 14 Jun 2016 at 23:00:38, 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

[PATCH v2] Refactor recv_sideband()

2016-06-14 Thread Lukas Fleischer
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,