From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sun, 09 Aug 2009 20:17:26 GMT
Subject: Re: [PATCH 1/8] Use a clearer style to issue commands to remote helpers
Message-ID: <alpine.DEB.1.00.0908092211030.8306@pacific.mpi-cbg.de>
In-Reply-To: <alpine.LNX.2.00.0908091526350.27553@iabervon.org>

Hi,

On Sun, 9 Aug 2009, Daniel Barkalow wrote:

> This style is overkill for some commands, but it's worthwhile to use
> the same style to issue all commands, and it's useful to avoid
> open-coding string lengths.
> 
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---

Thanks.  I am positively surprised that you did this patch in spite of the 
basis already being in 'next'.

> +	strbuf_addstr(&buf, "capabilities\n");
> +	write_in_full(helper->in, buf.buf, buf.len);
> +	strbuf_reset(&buf);

Hmm.  I think you use the same paradigm three times.  Maybe it is time to 
heed the DRY principle and introduce a helper function?

Maybe something like

void fprintf_in_full(struct strbuf *buf, int fd, const char *format, ...)
{
	va_list ap;

	va_start(ap, format);
	strbuf_reset(buf);
	strbuf_vaddf(buf, format, ap);
	write_in_full(fd, buf->buf, buf->len);
	va_end(ap);
}

But I keep forgetting that there is no strbuf_vaddf() in 'next'... maybe 
it is high time that I forget my promise and re-submit that series?

Ciao,
Dscho