{"thread":{"id":"34568","subject":"[PATCH 0/3] Remove sq_quote_print() in favor of *_buf","startedAt":"2013-07-30T08:31:24Z","lastAt":"2013-07-30T23:20:35Z","messageCount":7,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"224286","messageId":"1375173087-2164-1-git-send-email-artagnon@gmail.com","threadId":"34568","inReplyTo":null,"subject":"[PATCH 0/3] Remove sq_quote_print() in favor of *_buf","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-07-30T08:31:24Z","receivedAt":"2013-07-30T08:31:24Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nWhile going through the for-each-ref-pretty series that Duy and I were\ndeveloping, I noticed that this cleanup was independent and good\nas-it-is.\n\nSo here it is.\n\nNguyễn Thái Ngọc Duy (1):\n for-each-ref, quote: convert *_quote_print -> *_quote_buf\n\nRamkumar Ramachandra (2):\n tar-tree: remove dependency on sq_quote_print()\n quote: remove sq_quote_print()\n\n builtin/for-each-ref.c | 13 +++++++----\n builtin/tar-tree.c | 11 +++++----\n quote.c | 61 ++++++++++++++++++--------------------------------\n quote.h | 8 +++----\n 4 files changed, 39 insertions(+), 54 deletions(-)\n\n-- \n1.8.4.rc0.4.g4634265\n"},{"id":"224287","messageId":"1375173087-2164-2-git-send-email-artagnon@gmail.com","threadId":"34568","inReplyTo":"1375173087-2164-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/3] for-each-ref, quote: convert *_quote_print -> *_quote_buf","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-07-30T08:31:25Z","receivedAt":"2013-07-30T08:31:25Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"From: Nguyễn Thái Ngọc Duy \n\nfor-each-ref.c:print_value() currently prints values to stdout\nimmediately using {sq|perl|python|tcl}_quote_print, giving us no\nopportunity to do any further processing. In preparation for getting\nprint_value() to accept an additional strbuf argument to write to,\nconvert the *_quote_print functions and callers to *_quote_buf.\n\n[rr: commit message, minor modifications]\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: Ramkumar Ramachandra \n---\n builtin/for-each-ref.c | 13 +++++++++----\n quote.c | 44 ++++++++++++++++++++++----------------------\n quote.h | 6 +++---\n 3 files changed, 34 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 7f059c3..1d4083c 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -867,24 +867,29 @@ static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs\n static void print_value(struct refinfo *ref, int atom, int quote_style)\n {\n \tstruct atom_value *v;\n+\tstruct strbuf sb = STRBUF_INIT;\n \tget_value(ref, atom, &v);\n \tswitch (quote_style) {\n \tcase QUOTE_NONE:\n \t\tfputs(v->s, stdout);\n \t\tbreak;\n \tcase QUOTE_SHELL:\n-\t\tsq_quote_print(stdout, v->s);\n+\t\tsq_quote_buf(&sb, v->s);\n \t\tbreak;\n \tcase QUOTE_PERL:\n-\t\tperl_quote_print(stdout, v->s);\n+\t\tperl_quote_buf(&sb, v->s);\n \t\tbreak;\n \tcase QUOTE_PYTHON:\n-\t\tpython_quote_print(stdout, v->s);\n+\t\tpython_quote_buf(&sb, v->s);\n \t\tbreak;\n \tcase QUOTE_TCL:\n-\t\ttcl_quote_print(stdout, v->s);\n+\t\ttcl_quote_buf(&sb, v->s);\n \t\tbreak;\n \t}\n+\tif (quote_style != QUOTE_NONE) {\n+\t\tfputs(sb.buf, stdout);\n+\t\tstrbuf_release(&sb);\n+\t}\n }\n \n static int hex1(char ch)\ndiff --git a/quote.c b/quote.c\nindex 5c88081..9fd66c6 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -408,72 +408,72 @@ int unquote_c_style(struct strbuf *sb, const char *quoted, const char **endp)\n \n /* quoting as a string literal for other languages */\n \n-void perl_quote_print(FILE *stream, const char *src)\n+void perl_quote_buf(struct strbuf *sb, const char *src)\n {\n \tconst char sq = '\\'';\n \tconst char bq = '\\\\';\n \tchar c;\n \n-\tfputc(sq, stream);\n+\tstrbuf_addch(sb, sq);\n \twhile ((c = *src++)) {\n \t\tif (c == sq || c == bq)\n-\t\t\tfputc(bq, stream);\n-\t\tfputc(c, stream);\n+\t\t\tstrbuf_addch(sb, bq);\n+\t\tstrbuf_addch(sb, c);\n \t}\n-\tfputc(sq, stream);\n+\tstrbuf_addch(sb, sq);\n }\n \n-void python_quote_print(FILE *stream, const char *src)\n+void python_quote_buf(struct strbuf *sb, const char *src)\n {\n \tconst char sq = '\\'';\n \tconst char bq = '\\\\';\n \tconst char nl = '\\n';\n \tchar c;\n \n-\tfputc(sq, stream);\n+\tstrbuf_addch(sb, sq);\n \twhile ((c = *src++)) {\n \t\tif (c == nl) {\n-\t\t\tfputc(bq, stream);\n-\t\t\tfputc('n', stream);\n+\t\t\tstrbuf_addch(sb, bq);\n+\t\t\tstrbuf_addch(sb, 'n');\n \t\t\tcontinue;\n \t\t}\n \t\tif (c == sq || c == bq)\n-\t\t\tfputc(bq, stream);\n-\t\tfputc(c, stream);\n+\t\t\tstrbuf_addch(sb, bq);\n+\t\tstrbuf_addch(sb, c);\n \t}\n-\tfputc(sq, stream);\n+\tstrbuf_addch(sb, sq);\n }\n \n-void tcl_quote_print(FILE *stream, const char *src)\n+void tcl_quote_buf(struct strbuf *sb, const char *src)\n {\n \tchar c;\n \n-\tfputc('\"', stream);\n+\tstrbuf_addch(sb, '\"');\n \twhile ((c = *src++)) {\n \t\tswitch (c) {\n \t\tcase '[': case ']':\n \t\tcase '{': case '}':\n \t\tcase '$': case '\\\\': case '\"':\n-\t\t\tfputc('\\\\', stream);\n+\t\t\tstrbuf_addch(sb, '\\\\');\n \t\tdefault:\n-\t\t\tfputc(c, stream);\n+\t\t\tstrbuf_addch(sb, c);\n \t\t\tbreak;\n \t\tcase '\\f':\n-\t\t\tfputs(\"\\\\f\", stream);\n+\t\t\tstrbuf_addstr(sb, \"\\\\f\");\n \t\t\tbreak;\n \t\tcase '\\r':\n-\t\t\tfputs(\"\\\\r\", stream);\n+\t\t\tstrbuf_addstr(sb, \"\\\\r\");\n \t\t\tbreak;\n \t\tcase '\\n':\n-\t\t\tfputs(\"\\\\n\", stream);\n+\t\t\tstrbuf_addstr(sb, \"\\\\n\");\n \t\t\tbreak;\n \t\tcase '\\t':\n-\t\t\tfputs(\"\\\\t\", stream);\n+\t\t\tstrbuf_addstr(sb, \"\\\\t\");\n \t\t\tbreak;\n \t\tcase '\\v':\n-\t\t\tfputs(\"\\\\v\", stream);\n+\t\t\tstrbuf_addstr(sb, \"\\\\v\");\n \t\t\tbreak;\n \t\t}\n \t}\n-\tfputc('\"', stream);\n+\tstrbuf_addch(sb, '\"');\n }\ndiff --git a/quote.h b/quote.h\nindex ed110a5..6996ebd 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -68,8 +68,8 @@ extern char *quote_path_relative(const char *in, const char *prefix,\n \t\t\t struct strbuf *out);\n \n /* quoting as a string literal for other languages */\n-extern void perl_quote_print(FILE *stream, const char *src);\n-extern void python_quote_print(FILE *stream, const char *src);\n-extern void tcl_quote_print(FILE *stream, const char *src);\n+extern void perl_quote_buf(struct strbuf *sb, const char *src);\n+extern void python_quote_buf(struct strbuf *sb, const char *src);\n+extern void tcl_quote_buf(struct strbuf *sb, const char *src);\n \n #endif\n-- \n1.8.4.rc0.4.g4634265\n"},{"id":"224289","messageId":"1375173087-2164-3-git-send-email-artagnon@gmail.com","threadId":"34568","inReplyTo":"1375173087-2164-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/3] tar-tree: remove dependency on sq_quote_print()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-07-30T08:31:26Z","receivedAt":"2013-07-30T08:31:26Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, there is exactly one caller of sq_quote_print(), namely\ncmd_tar_tree(). In the interest of removing sq_quote_print() and\nsimplification, replace it with an equivalent call to sq_quote_argv().\nNo functional changes intended.\n\nSigned-off-by: Ramkumar Ramachandra \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/tar-tree.c | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/tar-tree.c b/builtin/tar-tree.c\nindex 3f1e701..ba3ffe6 100644\n--- a/builtin/tar-tree.c\n+++ b/builtin/tar-tree.c\n@@ -26,8 +26,8 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)\n \t * $0 tree-ish basedir ==>\n \t * \tgit archive --format-tar --prefix=basedir tree-ish\n \t */\n-\tint i;\n \tconst char **nargv = xcalloc(sizeof(*nargv), argc + 3);\n+\tstruct strbuf sb = STRBUF_INIT;\n \tchar *basedir_arg;\n \tint nargc = 0;\n \n@@ -65,11 +65,10 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)\n \tfprintf(stderr,\n \t\t\"*** \\\"git tar-tree\\\" is now deprecated.\\n\"\n \t\t\"*** Running \\\"git archive\\\" instead.\\n***\");\n-\tfor (i = 0; i < nargc; i++) {\n-\t\tfputc(' ', stderr);\n-\t\tsq_quote_print(stderr, nargv[i]);\n-\t}\n-\tfputc('\\n', stderr);\n+\tsq_quote_argv(&sb, nargv, 0);\n+\tstrbuf_addch(&sb, '\\n');\n+\tfputs(sb.buf, stderr);\n+\tstrbuf_release(&sb);\n \treturn cmd_archive(nargc, nargv, prefix);\n }\n \n-- \n1.8.4.rc0.4.g4634265\n"},{"id":"224288","messageId":"1375173087-2164-4-git-send-email-artagnon@gmail.com","threadId":"34568","inReplyTo":"1375173087-2164-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/3] quote: remove sq_quote_print()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-07-30T08:31:27Z","receivedAt":"2013-07-30T08:31:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Remove sq_quote_print() since it has no callers. A nicer alternative\nsq_quote_buf() exists: its callers aren't forced to print immediately.\n\nFor historical context, sq_quote_print() was first introduced in\n575ba9d6 (GIT_TRACE: show which built-in/external commands are executed,\n2006-06-25) for the purpose of printing argv for $GIT_TRACE. Today, we\nachieve this using trace_argv_printf() -> sq_quote_argv() ->\nsq_quote_buf(), which ultimately fills in a strbuf.\n\nSigned-off-by: Ramkumar Ramachandra \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n quote.c | 17 -----------------\n quote.h | 2 --\n 2 files changed, 19 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex 9fd66c6..45e3db1 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -42,23 +42,6 @@ void sq_quote_buf(struct strbuf *dst, const char *src)\n \tfree(to_free);\n }\n \n-void sq_quote_print(FILE *stream, const char *src)\n-{\n-\tchar c;\n-\n-\tfputc('\\'', stream);\n-\twhile ((c = *src++)) {\n-\t\tif (need_bs_quote(c)) {\n-\t\t\tfputs(\"'\\\\\", stream);\n-\t\t\tfputc(c, stream);\n-\t\t\tfputc('\\'', stream);\n-\t\t} else {\n-\t\t\tfputc(c, stream);\n-\t\t}\n-\t}\n-\tfputc('\\'', stream);\n-}\n-\n void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)\n {\n \tint i;\ndiff --git a/quote.h b/quote.h\nindex 6996ebd..71dcc3a 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -27,8 +27,6 @@ struct strbuf;\n * excluding the final null regardless of the buffer size.\n */\n \n-extern void sq_quote_print(FILE *stream, const char *src);\n-\n extern void sq_quote_buf(struct strbuf *, const char *src);\n extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen);\n \n-- \n1.8.4.rc0.4.g4634265\n"},{"id":"224297","messageId":"7viozsxfcd.fsf@alter.siamese.dyndns.org","threadId":"34568","inReplyTo":"1375173087-2164-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 0/3] Remove sq_quote_print() in favor of *_buf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-30T15:00:34Z","receivedAt":"2013-07-30T15:00:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> While going through the for-each-ref-pretty series that Duy and I were\n> developing, I noticed that this cleanup was independent and good\n> as-it-is.\n>\n> So here it is.\n\nYou always can first allocate a piece of memory and write into it\ninstead of writing things out directly. The patch shows it _can_ be\ndone, but that is not a news.\n\nAnd such a change is hardly a \"clean-up\". It just wastes more\nmemory you do not have to waste, in order to do what you are doing.\n\nWhen there is a reason why you need an in-memory representation,\nthis change starts to become the first step refactoring for an\nenhancement.\n\n> Nguyễn Thái Ngọc Duy (1):\n> for-each-ref, quote: convert *_quote_print -> *_quote_buf\n\nThe log message for this one explains it very well. This change by\nitself is not useful, but it will become necessary once you start\nneeding to access an in-memory result.\n"},{"id":"224300","messageId":"7va9l4xe3k.fsf@alter.siamese.dyndns.org","threadId":"34568","inReplyTo":"7viozsxfcd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] Remove sq_quote_print() in favor of *_buf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-30T15:27:27Z","receivedAt":"2013-07-30T15:27:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ramkumar Ramachandra writes:\n>\n>> While going through the for-each-ref-pretty series that Duy and I were\n>> developing, I noticed that this cleanup was independent and good\n>> as-it-is.\n>>\n>> So here it is.\n>\n> You always can first allocate a piece of memory and write into it\n> instead of writing things out directly. The patch shows it _can_ be\n> done, but that is not a news.\n>\n> And such a change is hardly a \"clean-up\". It just wastes more\n> memory you do not have to waste, in order to do what you are doing.\n>\n> When there is a reason why you need an in-memory representation,\n> this change starts to become the first step refactoring for an\n> enhancement.\n\nHaving said all that, the patch texts all look OK, so I'll queue\nthem with updated log messages. It was the usual me reacting to\nunjustified value judgement made in log messages and cover letters.\n\nIt would save me a lot of work if people stopped doing that and\ninstead stuck to facts. For example, between print_to_buf() and\nprint_to_stdout(), the former is *not* necessarily \"nicer\". It is\nmore flexible but that flexibility comes with a price, and the\ncaller needs a demonstrative need for that flexibility to justify\nthe cost.\n\nThanks.\n"},{"id":"224331","messageId":"CALkWK0mQx+5J_tGka6psemckvnqzp+jQF9+csN4HWBuwY=eoQA@mail.gmail.com","threadId":"34568","inReplyTo":"7va9l4xe3k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/3] Remove sq_quote_print() in favor of *_buf","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-07-30T23:20:35Z","receivedAt":"2013-07-30T23:20:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Having said all that, the patch texts all look OK, so I'll queue\n> them with updated log messages. It was the usual me reacting to\n> unjustified value judgement made in log messages and cover letters.\n\nThanks. I'll look at how the log messages are different in the queued\nversion, and adapt accordingly next time.\n"}]}