Re: [PATCH 2/8] Use a clearer style to issue commands to remote helpers

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 4, 2009, 10:40 UTC
Message-ID: <alpine.DEB.1.00.0909041236290.4605@intel-tinevez-2-302>
In-Reply-To: <alpine.LNX.2.00.0909032213200.28290@iabervon.org>

Hi,

On Thu, 3 Sep 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.

Why do I have to study the patch to find out what "this style" is? And why do you not even _try_ to convince people that "it's worthwhile", say, by giving some example?

Show 14 quoted lines

> 
> diff --git a/transport-helper.c b/transport-helper.c
> index 4684877..b1ea7e6 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)
>  		die("Unable to run helper: git %s", helper->argv[0]);
>  	data->helper = helper;
>  
> -	write_in_full(data->helper->in, "capabilities\n", 13);
> +	strbuf_addstr(&buf, "capabilities\n");
> +	write_in_full(helper->in, buf.buf, buf.len);
> +	strbuf_reset(&buf);
> +

If you use that paradigm more often, why not rather introduce something like

	void strbuf_flush(struct strbuf *buf, int fd) {
		write_in_full(fd, buf->buf, buf->len);
		strbuf_reset(buf);
	}

instead of violating the DRY principle?

Ciao, Dscho