{"thread":{"id":"20840","subject":"[PATCH 2/8] Use a clearer style to issue commands to remote helpers","startedAt":"2009-09-04T02:13:51Z","lastAt":"2009-09-04T10:40:05Z","messageCount":2,"participants":["Daniel Barkalow","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"122378","messageId":"alpine.LNX.2.00.0909032213200.28290@iabervon.org","threadId":"20840","inReplyTo":null,"subject":"[PATCH 2/8] Use a clearer style to issue commands to remote helpers","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-09-04T02:13:51Z","receivedAt":"2009-09-04T02:13:51Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"This style is overkill for some commands, but it's worthwhile to use\nthe same style to issue all commands, and it's useful to avoid\nopen-coding string lengths.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\n transport-helper.c |   21 ++++++++++++++-------\n 1 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 4684877..b1ea7e6 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)\n \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n \tdata->helper = helper;\n \n-\twrite_in_full(data->helper->in, \"capabilities\\n\", 13);\n+\tstrbuf_addstr(&buf, \"capabilities\\n\");\n+\twrite_in_full(helper->in, buf.buf, buf.len);\n+\tstrbuf_reset(&buf);\n+\n \tfile = fdopen(helper->out, \"r\");\n \twhile (1) {\n \t\tif (strbuf_getline(&buf, file, '\\n') == EOF)\n@@ -78,11 +81,12 @@ static int fetch_with_fetch(struct transport *transport,\n \t\tconst struct ref *posn = to_fetch[i];\n \t\tif (posn->status & REF_STATUS_UPTODATE)\n \t\t\tcontinue;\n-\t\twrite_in_full(helper->in, \"fetch \", 6);\n-\t\twrite_in_full(helper->in, sha1_to_hex(posn->old_sha1), 40);\n-\t\twrite_in_full(helper->in, \" \", 1);\n-\t\twrite_in_full(helper->in, posn->name, strlen(posn->name));\n-\t\twrite_in_full(helper->in, \"\\n\", 1);\n+\n+\t\tstrbuf_addf(&buf, \"fetch %s %s\\n\",\n+\t\t\t    sha1_to_hex(posn->old_sha1), posn->name);\n+\t\twrite_in_full(helper->in, buf.buf, buf.len);\n+\t\tstrbuf_reset(&buf);\n+\n \t\tif (strbuf_getline(&buf, file, '\\n') == EOF)\n \t\t\texit(128); /* child died, message supplied already */\n \t}\n@@ -119,7 +123,10 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)\n \tFILE *file;\n \n \thelper = get_helper(transport);\n-\twrite_in_full(helper->in, \"list\\n\", 5);\n+\n+\tstrbuf_addstr(&buf, \"list\\n\");\n+\twrite_in_full(helper->in, buf.buf, buf.len);\n+\tstrbuf_reset(&buf);\n \n \tfile = fdopen(helper->out, \"r\");\n \twhile (1) {\n-- \n1.6.4.2.419.gc86f8\n"},{"id":"122409","messageId":"alpine.DEB.1.00.0909041236290.4605@intel-tinevez-2-302","threadId":"20840","inReplyTo":"alpine.LNX.2.00.0909032213200.28290@iabervon.org","subject":"Re: [PATCH 2/8] Use a clearer style to issue commands to remote helpers","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-09-04T10:40:05Z","receivedAt":"2009-09-04T10:40:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 3 Sep 2009, Daniel Barkalow wrote:\n\n> This style is overkill for some commands, but it's worthwhile to use\n> the same style to issue all commands, and it's useful to avoid\n> open-coding string lengths.\n\nWhy do I have to study the patch to find out what \"this style\" is?  And \nwhy do you not even _try_ to convince people that \"it's worthwhile\", say, \nby giving some example?\n\n> \n> diff --git a/transport-helper.c b/transport-helper.c\n> index 4684877..b1ea7e6 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -37,7 +37,10 @@ static struct child_process *get_helper(struct transport *transport)\n>  \t\tdie(\"Unable to run helper: git %s\", helper->argv[0]);\n>  \tdata->helper = helper;\n>  \n> -\twrite_in_full(data->helper->in, \"capabilities\\n\", 13);\n> +\tstrbuf_addstr(&buf, \"capabilities\\n\");\n> +\twrite_in_full(helper->in, buf.buf, buf.len);\n> +\tstrbuf_reset(&buf);\n> +\n\nIf you use that paradigm more often, why not rather introduce something \nlike\n\n\tvoid strbuf_flush(struct strbuf *buf, int fd) {\n\t\twrite_in_full(fd, buf->buf, buf->len);\n\t\tstrbuf_reset(buf);\n\t}\n\ninstead of violating the DRY principle?\n\nCiao,\nDscho\n"}]}