{"thread":{"id":"44055","subject":"[PATCH 00/10] Another preparatory series for rename detection","startedAt":"2016-09-11T05:42:38Z","lastAt":"2016-09-16T23:04:34Z","messageCount":21,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"301605","messageId":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":null,"subject":"[PATCH 00/10] Another preparatory series for rename detection","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:00Z","receivedAt":"2016-09-11T05:42:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nThis applies on top of sb/diff-cleanup.\n\nThe long term strategy is to get the rename detection done, is to run\nany output through emit_line as there we can either write it to the\noutput file or buffer it internally across file pairs and work on the\nbuffered output.\n\nAs I got stuck with the word diff as well as the correct implementation\nof builtin_diff, I am sending this out as an intermediate state.\nAs the diff code is central to Git, I'd rather send these cleanups\nearly so I do not run into merge conflicts with other people down\nthe road.\n\nThanks,\nStefan\n\nStefan Beller (10):\n diff: move line ending check into emit_hunk_header\n diff: emit_{add, del, context}_line to increase {pre,post}image line\n count\n diff.c: drop tautologous condition in emit_line_0\n diff.c: rename diff_flush_patch to diff_flush_patch_filepair\n diff.c: reintroduce diff_flush_patch for all files\n diff.c: emit_line_0 can handle no color\n diff.c: convert fn_out_consume to use emit_line_*\n diff.c: convert emit_rewrite_diff to use emit_line_*\n diff.c: convert emit_rewrite_lines to use emit_line_0\n submodule.c: convert show_submodule_summary to use emit_line_fmt\n\n diff.c | 114 ++++++++++++++++++++++++++++++++++++------------------------\n diff.h | 3 ++\n submodule.c | 42 +++++++++-------------\n submodule.h | 3 +-\n 4 files changed, 89 insertions(+), 73 deletions(-)\n\n-- \n2.7.4\n\n"},{"id":"301606","messageId":"1473572530-25764-2-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 01/10] diff: move line ending check into emit_hunk_header","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:01Z","receivedAt":"2016-09-11T05:42:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nSigned-off-by: Stefan Beller \n---\n diff.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex cc8e812..aa50b2d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -610,6 +610,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,\n \t}\n \n \tstrbuf_add(&msgbuf, line + len, org_len - len);\n+\tif (line[org_len - 1] != '\\n')\n+\t\tstrbuf_addch(&msgbuf, '\\n');\n+\n \temit_line(ecbdata->opt, \"\", \"\", msgbuf.buf, msgbuf.len);\n \tstrbuf_release(&msgbuf);\n }\n@@ -1247,8 +1250,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n \t\tfind_lno(line, ecbdata);\n \t\temit_hunk_header(ecbdata, line, len);\n-\t\tif (line[len-1] != '\\n')\n-\t\t\tputc('\\n', o->file);\n \t\treturn;\n \t}\n \n-- \n2.7.4\n\n"},{"id":"301607","messageId":"1473572530-25764-3-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:02Z","receivedAt":"2016-09-11T05:42:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nAt all call sites of emit_{add, del, context}_line we increment the line\ncount, so move it into the respective functions to make the code at the\ncalling site a bit clearer.\n\nSigned-off-by: Stefan Beller \n---\n diff.c | 15 ++++++---------\n 1 file changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex aa50b2d..156c2aa 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -536,6 +536,7 @@ static void emit_add_line(const char *reset,\n \t\t\t struct emit_callback *ecbdata,\n \t\t\t const char *line, int len)\n {\n+\tecbdata->lno_in_postimage++;\n \temit_line_checked(reset, ecbdata, line, len,\n \t\t\t DIFF_FILE_NEW, WSEH_NEW, '+');\n }\n@@ -544,6 +545,7 @@ static void emit_del_line(const char *reset,\n \t\t\t struct emit_callback *ecbdata,\n \t\t\t const char *line, int len)\n {\n+\tecbdata->lno_in_preimage++;\n \temit_line_checked(reset, ecbdata, line, len,\n \t\t\t DIFF_FILE_OLD, WSEH_OLD, '-');\n }\n@@ -552,6 +554,8 @@ static void emit_context_line(const char *reset,\n \t\t\t struct emit_callback *ecbdata,\n \t\t\t const char *line, int len)\n {\n+\tecbdata->lno_in_postimage++;\n+\tecbdata->lno_in_preimage++;\n \temit_line_checked(reset, ecbdata, line, len,\n \t\t\t DIFF_CONTEXT, WSEH_CONTEXT, ' ');\n }\n@@ -662,13 +666,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,\n \n \t\tendp = memchr(data, '\\n', size);\n \t\tlen = endp ? (endp - data + 1) : size;\n-\t\tif (prefix != '+') {\n-\t\t\tecb->lno_in_preimage++;\n+\t\tif (prefix != '+')\n \t\t\temit_del_line(reset, ecb, data, len);\n-\t\t} else {\n-\t\t\tecb->lno_in_postimage++;\n+\t\telse\n \t\t\temit_add_line(reset, ecb, data, len);\n-\t\t}\n \t\tsize -= len;\n \t\tdata += len;\n \t}\n@@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \n \tswitch (line[0]) {\n \tcase '+':\n-\t\tecbdata->lno_in_postimage++;\n \t\temit_add_line(reset, ecbdata, line + 1, len - 1);\n \t\tbreak;\n \tcase '-':\n-\t\tecbdata->lno_in_preimage++;\n \t\temit_del_line(reset, ecbdata, line + 1, len - 1);\n \t\tbreak;\n \tcase ' ':\n-\t\tecbdata->lno_in_postimage++;\n-\t\tecbdata->lno_in_preimage++;\n \t\temit_context_line(reset, ecbdata, line + 1, len - 1);\n \t\tbreak;\n \tdefault:\n-- \n2.7.4\n\n"},{"id":"301608","messageId":"1473572530-25764-4-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 03/10] diff.c: drop tautologous condition in emit_line_0","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:03Z","receivedAt":"2016-09-11T05:42:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nWhen `first` equals '\\r', then it cannot equal '\\n', which makes\n`!has_trailing_newline` always true if `first` is '\\r'.\nDrop that check.\n\nSigned-off-by: Stefan Beller \n---\n diff.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 156c2aa..9d2e704 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n \n \tif (len == 0) {\n \t\thas_trailing_newline = (first == '\\n');\n-\t\thas_trailing_carriage_return = (!has_trailing_newline &&\n-\t\t\t\t\t\t(first == '\\r'));\n+\t\thas_trailing_carriage_return = (first == '\\r');\n \t\tnofirst = has_trailing_newline || has_trailing_carriage_return;\n \t} else {\n \t\thas_trailing_newline = (len > 0 && line[len-1] == '\\n');\n-- \n2.7.4\n\n"},{"id":"301609","messageId":"1473572530-25764-6-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:05Z","receivedAt":"2016-09-11T05:42:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\n---\n diff.c | 17 ++++++++++++-----\n 1 file changed, 12 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 85fb887..87b1bb2 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4608,6 +4608,17 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)\n \t\twarning(rename_limit_advice, varname, needed);\n }\n \n+static void diff_flush_patch(struct diff_options *o)\n+{\n+\tint i;\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p = q->queue[i];\n+\t\tif (check_pair_status(p))\n+\t\t\tdiff_flush_patch_filepair(p, o);\n+\t}\n+}\n+\n void diff_flush(struct diff_options *options)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n@@ -4702,11 +4713,7 @@ void diff_flush(struct diff_options *options)\n \t\t\t}\n \t\t}\n \n-\t\tfor (i = 0; i < q->nr; i++) {\n-\t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tif (check_pair_status(p))\n-\t\t\t\tdiff_flush_patch_filepair(p, options);\n-\t\t}\n+\t\tdiff_flush_patch(options);\n \t}\n \n \tif (output_format & DIFF_FORMAT_CALLBACK)\n-- \n2.7.4\n\n"},{"id":"301610","messageId":"1473572530-25764-5-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:04Z","receivedAt":"2016-09-11T05:42:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nSigned-off-by: Stefan Beller \n---\n diff.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 9d2e704..85fb887 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4176,7 +4176,7 @@ int diff_unmodified_pair(struct diff_filepair *p)\n \treturn 0;\n }\n \n-static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)\n+static void diff_flush_patch_filepair(struct diff_filepair *p, struct diff_options *o)\n {\n \tif (diff_unmodified_pair(p))\n \t\treturn;\n@@ -4672,7 +4672,7 @@ void diff_flush(struct diff_options *options)\n \t DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&\n \t DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {\n \t\t/*\n-\t\t * run diff_flush_patch for the exit status. setting\n+\t\t * run diff_flush_patch_filepair for the exit status. setting\n \t\t * options->file to /dev/null should be safe, because we\n \t\t * aren't supposed to produce any output anyway.\n \t\t */\n@@ -4685,7 +4685,7 @@ void diff_flush(struct diff_options *options)\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n \t\t\tif (check_pair_status(p))\n-\t\t\t\tdiff_flush_patch(p, options);\n+\t\t\t\tdiff_flush_patch_filepair(p, options);\n \t\t\tif (options->found_changes)\n \t\t\t\tbreak;\n \t\t}\n@@ -4705,7 +4705,7 @@ void diff_flush(struct diff_options *options)\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n \t\t\tif (check_pair_status(p))\n-\t\t\t\tdiff_flush_patch(p, options);\n+\t\t\t\tdiff_flush_patch_filepair(p, options);\n \t\t}\n \t}\n \n-- \n2.7.4\n\n"},{"id":"301611","messageId":"1473572530-25764-8-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:07Z","receivedAt":"2016-09-11T05:42:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nIn the coming patches we want to eliminate writes that do not go\nthough emit_line_* as we later want to hook into the emit_line_*\nfunctions to add a buffered output. To make the conversion easy,\nadd a function that takes a format string.\n\nSigned-off-by: Stefan Beller \n---\n diff.c | 24 +++++++++++++++++++-----\n 1 file changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 2aefd0f..7dcef73 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -493,6 +493,19 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset\n \temit_line_0(o, set, reset, line[0], line+1, len-1);\n }\n \n+static void emit_line_fmt(struct diff_options *o,\n+\t\t\t const char *set, const char *reset,\n+\t\t\t const char *fmt, ...)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tva_list ap;\n+\tva_start(ap, fmt);\n+\tstrbuf_vaddf(&sb, fmt, ap);\n+\tva_end(ap);\n+\n+\temit_line(o, set, reset, sb.buf, sb.len);\n+}\n+\n static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)\n {\n \tif (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&\n@@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \tconst char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);\n \tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n \tstruct diff_options *o = ecbdata->opt;\n-\tconst char *line_prefix = diff_line_prefix(o);\n \n \to->found_changes = 1;\n \n@@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\tname_a_tab = strchr(ecbdata->label_path[0], ' ') ? \"\\t\" : \"\";\n \t\tname_b_tab = strchr(ecbdata->label_path[1], ' ') ? \"\\t\" : \"\";\n \n-\t\tfprintf(o->file, \"%s%s--- %s%s%s\\n\",\n-\t\t\tline_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);\n-\t\tfprintf(o->file, \"%s%s+++ %s%s%s\\n\",\n-\t\t\tline_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);\n+\t\temit_line_fmt(o, meta, reset, \"--- %s%s\\n\",\n+\t\t\t ecbdata->label_path[0], name_a_tab);\n+\n+\t\temit_line_fmt(o, meta, reset, \"+++ %s%s\\n\",\n+\t\t\t ecbdata->label_path[1], name_b_tab);\n+\n \t\tecbdata->label_path[0] = ecbdata->label_path[1] = NULL;\n \t}\n \n-- \n2.7.4\n\n"},{"id":"301612","messageId":"1473572530-25764-11-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 10/10] submodule.c: convert show_submodule_summary to use emit_line_fmt","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:10Z","receivedAt":"2016-09-11T05:43:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\n---\n diff.c | 5 ++---\n diff.h | 3 +++\n submodule.c | 42 +++++++++++++++++-------------------------\n submodule.h | 3 +--\n 4 files changed, 23 insertions(+), 30 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 255fbf3..c00306b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -493,7 +493,7 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset\n \temit_line_0(o, set, reset, line[0], line+1, len-1);\n }\n \n-static void emit_line_fmt(struct diff_options *o,\n+void emit_line_fmt(struct diff_options *o,\n \t\t\t const char *set, const char *reset,\n \t\t\t const char *fmt, ...)\n {\n@@ -2310,8 +2310,7 @@ static void builtin_diff(const char *name_a,\n \t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n \t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n \t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n-\t\tshow_submodule_summary(o->file, one->path ? one->path : two->path,\n-\t\t\t\tline_prefix,\n+\t\tshow_submodule_summary(o, one->path ? one->path : two->path,\n \t\t\t\tone->oid.hash, two->oid.hash,\n \t\t\t\ttwo->dirty_submodule,\n \t\t\t\tmeta, del, add, reset);\ndiff --git a/diff.h b/diff.h\nindex 7883729..1699d9c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -180,6 +180,9 @@ struct diff_options {\n \tint diff_path_counter;\n };\n \n+void emit_line_fmt(struct diff_options *o, const char *set, const char *reset,\n+\t\t const char *fmt, ...);\n+\n enum color_diff {\n \tDIFF_RESET = 0,\n \tDIFF_CONTEXT = 1,\ndiff --git a/submodule.c b/submodule.c\nindex 1b5cdfb..c817b46 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -304,8 +304,8 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n \treturn prepare_revision_walk(rev);\n }\n \n-static void print_submodule_summary(struct rev_info *rev, FILE *f,\n-\t\tconst char *line_prefix,\n+static void print_submodule_summary(struct rev_info *rev,\n+\t\tstruct diff_options *o,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstatic const char format[] = \" %m %s\";\n@@ -317,24 +317,16 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,\n \t\tctx.date_mode = rev->date_mode;\n \t\tctx.output_encoding = get_log_output_encoding();\n \t\tstrbuf_setlen(&sb, 0);\n-\t\tstrbuf_addstr(&sb, line_prefix);\n-\t\tif (commit->object.flags & SYMMETRIC_LEFT) {\n-\t\t\tif (del)\n-\t\t\t\tstrbuf_addstr(&sb, del);\n-\t\t}\n-\t\telse if (add)\n-\t\t\tstrbuf_addstr(&sb, add);\n \t\tformat_commit_message(commit, format, &sb, &ctx);\n-\t\tif (reset)\n-\t\t\tstrbuf_addstr(&sb, reset);\n-\t\tstrbuf_addch(&sb, '\\n');\n-\t\tfprintf(f, \"%s\", sb.buf);\n+\t\tif (commit->object.flags & SYMMETRIC_LEFT)\n+\t\t\temit_line_fmt(o, del, reset, \"%s\\n\", sb.buf);\n+\t\telse if (add)\n+\t\t\temit_line_fmt(o, add, reset, \"%s\\n\", sb.buf);\n \t}\n \tstrbuf_release(&sb);\n }\n \n-void show_submodule_summary(FILE *f, const char *path,\n-\t\tconst char *line_prefix,\n+void show_submodule_summary(struct diff_options *o, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset)\n@@ -359,30 +351,30 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\tmessage = \"(revision walker failed)\";\n \n \tif (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n-\t\tfprintf(f, \"%sSubmodule %s contains untracked content\\n\",\n-\t\t\tline_prefix, path);\n+\t\temit_line_fmt(o, 0, 0,\n+\t\t\t \"Submodule %s contains untracked content\\n\", path);\n \tif (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n-\t\tfprintf(f, \"%sSubmodule %s contains modified content\\n\",\n-\t\t\tline_prefix, path);\n+\t\temit_line_fmt(o, 0, 0,\n+\t\t\t \"Submodule %s contains modified content\\n\", path);\n \n \tif (!hashcmp(one, two)) {\n \t\tstrbuf_release(&sb);\n \t\treturn;\n \t}\n \n-\tstrbuf_addf(&sb, \"%s%sSubmodule %s %s..\", line_prefix, meta, path,\n-\t\t\tfind_unique_abbrev(one, DEFAULT_ABBREV));\n+\tstrbuf_addf(&sb, \"Submodule %s %s..\", path,\n+\t\t find_unique_abbrev(one, DEFAULT_ABBREV));\n \tif (!fast_backward && !fast_forward)\n \t\tstrbuf_addch(&sb, '.');\n \tstrbuf_addf(&sb, \"%s\", find_unique_abbrev(two, DEFAULT_ABBREV));\n \tif (message)\n-\t\tstrbuf_addf(&sb, \" %s%s\\n\", message, reset);\n+\t\tstrbuf_addf(&sb, \" %s\\n\", message);\n \telse\n-\t\tstrbuf_addf(&sb, \"%s:%s\\n\", fast_backward ? \" (rewind)\" : \"\", reset);\n-\tfwrite(sb.buf, sb.len, 1, f);\n+\t\tstrbuf_addf(&sb, \"%s:\\n\", fast_backward ? \" (rewind)\" : \"\");\n+\temit_line_fmt(o, meta, reset, \"%s\", sb.buf);\n \n \tif (!message) /* only NULL if we succeeded in setting up the walk */\n-\t\tprint_submodule_summary(&rev, f, line_prefix, del, add, reset);\n+\t\tprint_submodule_summary(&rev, o, del, add, reset);\n \tif (left)\n \t\tclear_commit_marks(left, ~0);\n \tif (right)\ndiff --git a/submodule.h b/submodule.h\nindex 2af9390..fefc0ab 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -41,8 +41,7 @@ int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst);\n const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n-void show_submodule_summary(FILE *f, const char *path,\n-\t\tconst char *line_prefix,\n+void show_submodule_summary(struct diff_options *o, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset);\n-- \n2.7.4\n\n"},{"id":"301613","messageId":"1473572530-25764-10-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 09/10] diff.c: convert emit_rewrite_lines to use emit_line_0","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:09Z","receivedAt":"2016-09-11T05:43:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\n---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 98d6655..255fbf3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -690,7 +690,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb,\n \tif (!endp) {\n \t\tconst char *context = diff_get_color(ecb->color_diff,\n \t\t\t\t\t\t DIFF_CONTEXT);\n-\t\tputc('\\n', ecb->opt->file);\n+\t\temit_line_0(ecb->opt, 0, 0, 0, \"\\n\", 1);\n \t\temit_line_0(ecb->opt, context, reset, '\\\\',\n \t\t\t nneof, strlen(nneof));\n \t}\n-- \n2.7.4\n\n"},{"id":"301614","messageId":"1473572530-25764-9-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 08/10] diff.c: convert emit_rewrite_diff to use emit_line_*","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:08Z","receivedAt":"2016-09-11T05:43:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\nSigned-off-by: Stefan Beller \n---\n diff.c | 33 ++++++++++++++++++---------------\n 1 file changed, 18 insertions(+), 15 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 7dcef73..98d6655 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -653,17 +653,17 @@ static void remove_tempfile(void)\n \t}\n }\n \n-static void print_line_count(FILE *file, int count)\n+static void add_line_count(struct strbuf *out, int count)\n {\n \tswitch (count) {\n \tcase 0:\n-\t\tfprintf(file, \"0,0\");\n+\t\tstrbuf_addstr(out, \"0,0\");\n \t\tbreak;\n \tcase 1:\n-\t\tfprintf(file, \"1\");\n+\t\tstrbuf_addstr(out, \"1\");\n \t\tbreak;\n \tdefault:\n-\t\tfprintf(file, \"1,%d\", count);\n+\t\tstrbuf_addf(out, \"1,%d\", count);\n \t\tbreak;\n \t}\n }\n@@ -714,7 +714,7 @@ static void emit_rewrite_diff(const char *name_a,\n \tchar *data_one, *data_two;\n \tsize_t size_one, size_two;\n \tstruct emit_callback ecbdata;\n-\tconst char *line_prefix = diff_line_prefix(o);\n+\tstruct strbuf out = STRBUF_INIT;\n \n \tif (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {\n \t\ta_prefix = o->b_prefix;\n@@ -752,20 +752,23 @@ static void emit_rewrite_diff(const char *name_a,\n \tecbdata.lno_in_preimage = 1;\n \tecbdata.lno_in_postimage = 1;\n \n+\temit_line_fmt(o, metainfo, reset, \"--- %s%s\\n\", a_name.buf, name_a_tab);\n+\temit_line_fmt(o, metainfo, reset, \"+++ %s%s\\n\", b_name.buf, name_b_tab);\n+\n \tlc_a = count_lines(data_one, size_one);\n \tlc_b = count_lines(data_two, size_two);\n-\tfprintf(o->file,\n-\t\t\"%s%s--- %s%s%s\\n%s%s+++ %s%s%s\\n%s%s@@ -\",\n-\t\tline_prefix, metainfo, a_name.buf, name_a_tab, reset,\n-\t\tline_prefix, metainfo, b_name.buf, name_b_tab, reset,\n-\t\tline_prefix, fraginfo);\n+\n+\tstrbuf_addstr(&out, \"@@ -\");\n \tif (!o->irreversible_delete)\n-\t\tprint_line_count(o->file, lc_a);\n+\t\tadd_line_count(&out, lc_a);\n \telse\n-\t\tfprintf(o->file, \"?,?\");\n-\tfprintf(o->file, \" +\");\n-\tprint_line_count(o->file, lc_b);\n-\tfprintf(o->file, \" @@%s\\n\", reset);\n+\t\tstrbuf_addstr(&out, \"?,?\");\n+\tstrbuf_addstr(&out, \" +\");\n+\tadd_line_count(&out, lc_b);\n+\tstrbuf_addstr(&out, \" @@\\n\");\n+\temit_line(o, fraginfo, reset, out.buf, out.len);\n+\tstrbuf_release(&out);\n+\n \tif (lc_a && !o->irreversible_delete)\n \t\temit_rewrite_lines(&ecbdata, '-', data_one, size_one);\n \tif (lc_b)\n-- \n2.7.4\n\n"},{"id":"301615","messageId":"1473572530-25764-7-git-send-email-stefanbeller@gmail.com","threadId":"44055","inReplyTo":"1473572530-25764-1-git-send-email-stefanbeller@gmail.com","subject":"[PATCH 06/10] diff.c: emit_line_0 can handle no color","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2016-09-11T05:42:06Z","receivedAt":"2016-09-11T05:43:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Stefan Beller \n\n---\n diff.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 87b1bb2..2aefd0f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n \t}\n \n \tif (len || !nofirst) {\n-\t\tfputs(set, file);\n+\t\tif (set)\n+\t\t\tfputs(set, file);\n \t\tif (!nofirst)\n \t\t\tfputc(first, file);\n \t\tfwrite(line, len, 1, file);\n-\t\tfputs(reset, file);\n+\t\tif (reset)\n+\t\t\tfputs(reset, file);\n \t}\n \tif (has_trailing_carriage_return)\n \t\tfputc('\\r', file);\n-- \n2.7.4\n\n"},{"id":"301784","messageId":"xmqqa8fcy9m4.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-2-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 01/10] diff: move line ending check into emit_hunk_header","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-12T23:35:47Z","receivedAt":"2016-09-12T23:35:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Stefan Beller \n>\n> Signed-off-by: Stefan Beller \n> ---\n\nThe reason being...?\n\n\"Doing this would not change any behaviour and would not break\nanything\" may be true, but that is not a reason to do a change.\n\nHopefully it will become clear why this is needed once we look at a\nlater patch in the series.\n\n> diff.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index cc8e812..aa50b2d 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -610,6 +610,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,\n> \t}\n> \n> \tstrbuf_add(&msgbuf, line + len, org_len - len);\n> +\tif (line[org_len - 1] != '\\n')\n> +\t\tstrbuf_addch(&msgbuf, '\\n');\n> +\n> \temit_line(ecbdata->opt, \"\", \"\", msgbuf.buf, msgbuf.len);\n> \tstrbuf_release(&msgbuf);\n> }\n> @@ -1247,8 +1250,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n> \t\tlen = sane_truncate_line(ecbdata, line, len);\n> \t\tfind_lno(line, ecbdata);\n> \t\temit_hunk_header(ecbdata, line, len);\n> -\t\tif (line[len-1] != '\\n')\n> -\t\t\tputc('\\n', o->file);\n> \t\treturn;\n> \t}\n"},{"id":"301785","messageId":"xmqq60q0y93e.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-3-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-12T23:47:01Z","receivedAt":"2016-09-12T23:47:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Stefan Beller \n>\n> At all call sites of emit_{add, del, context}_line we increment the line\n> count, so move it into the respective functions to make the code at the\n> calling site a bit clearer.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> diff.c | 15 ++++++---------\n> 1 file changed, 6 insertions(+), 9 deletions(-)\n\nI am mostly in favor of this change, as the calls to these three\nfunctions are always preceded by increment of these fields, but it\nis \"mostly\" exactly because the reverse is not true. Namely ...\n\n> @@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n> \n> \tswitch (line[0]) {\n> \tcase '+':\n> -\t\tecbdata->lno_in_postimage++;\n> \t\temit_add_line(reset, ecbdata, line + 1, len - 1);\n> \t\tbreak;\n> \tcase '-':\n> -\t\tecbdata->lno_in_preimage++;\n> \t\temit_del_line(reset, ecbdata, line + 1, len - 1);\n> \t\tbreak;\n> \tcase ' ':\n> -\t\tecbdata->lno_in_postimage++;\n> -\t\tecbdata->lno_in_preimage++;\n> \t\temit_context_line(reset, ecbdata, line + 1, len - 1);\n> \t\tbreak;\n> \tdefault:\n\n... there still needs an increment in the context lines, not shown\nin the patch, just after this \"default:\". I think the patch is OK\nas the comment after this \"default:\" (also not shown in the patch)\nmakes it clear what is going on.\n\nThanks.\n\n\n"},{"id":"301786","messageId":"xmqq1t0oy8t8.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-4-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-12T23:53:07Z","receivedAt":"2016-09-12T23:53:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/diff.c b/diff.c\n> index 156c2aa..9d2e704 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n> \n> \tif (len == 0) {\n> \t\thas_trailing_newline = (first == '\\n');\n> -\t\thas_trailing_carriage_return = (!has_trailing_newline &&\n> -\t\t\t\t\t\t(first == '\\r'));\n> +\t\thas_trailing_carriage_return = (first == '\\r');\n> \t\tnofirst = has_trailing_newline || has_trailing_carriage_return;\n> \t} else {\n> \t\thas_trailing_newline = (len > 0 && line[len-1] == '\\n');\n\nInteresting.\n\nThis may be a mis-conversion at 250f7993 (\"diff.c: split emit_line()\nfrom the first char and the rest of the line\", 2009-09-14), I\nsuspect. The original took line[] with length and peeked for '\\n',\nand when it saw one, it decremented length before checking\nline[len-1] for '\\r'.\n\nBut of course if there is only one byte on the line (i.e. len == 0\nafter first is stripped off), it cannot be both '\\n' or '\\r' at the\nsame time.\n\nThanks for spotting.\n\n\n\n\n"},{"id":"301787","messageId":"xmqqwpigwu7q.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-5-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-12T23:53:45Z","receivedAt":"2016-09-12T23:53:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Stefan Beller \n>\n> Signed-off-by: Stefan Beller \n> ---\n\nThe reason being...?\n\n"},{"id":"301788","messageId":"xmqqsht4wtns.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-6-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-13T00:05:43Z","receivedAt":"2016-09-13T00:05:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Stefan Beller \n>\n> ---\n\nThis shows why 04/10 should have had the overall plan for these two\nsteps. We want a short-and-sweet name \"diff-flush-patch\" to mean\n\"flush all the queued diff elements\" so rename the singleton one\nfrom diff-flush-patch to diff-flush-patch-filepair to make room in\n04/10 and then introduce the \"diff-flush-patch-all\" in 05/10.\n\nI just said with the above explanation the changes in 04/10 and\n05/10 become undertandable, which is a bit different from being\njustifiable. \"diff_flush_raw()\", \"diff_flush_stat()\", etc. are\n_all_ about a single filepair. I'd rather see diff_flush_patch()\nto be also about a single filepair.\n\nIt may be helpful to have a helper that calls diff_flush_patch() for\nall filepairs in the queue, but can't that function get a new name\ninstead? By definition, it will be called much less often than a\npair-wise one, so it can afford to have a longer name.\n\ndiff_flush_patch_queue() or something, perhaps? I am not sure if it\nshould be diff_flush_queue_patch(), or even\n\n diff_flush_queue(struct diff_options *o, diff_flush_fn fn);\n\nwhere diff_flush_fn is \n\n typedef void (*diff_flush_fn)(struct diff_filepair *p,\n \t\t\tstruct diff_options *o, void *other_data)\n\nthat can be used to flush the queue by calling any of these\nfilepair-wise flush functions like diff_flush_{raw,stat,checkdiff,patch}.\n\nThis last approach might be overkill, but if you want to try it,\nyou'd need a preparatory step to give an unused \"void *other\" to\ndiff_flush_{raw,patch,checkdiff} as diff_flush_stat() is an oddball\nthat needs an extra \"accumulator\" pointer. I actually wonder if\nthat \"diffstat\" pointer should become part of \"struct diff_options\",\nthough. Anyway.\n\n> diff.c | 17 ++++++++++++-----\n> 1 file changed, 12 insertions(+), 5 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index 85fb887..87b1bb2 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -4608,6 +4608,17 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)\n> \t\twarning(rename_limit_advice, varname, needed);\n> }\n> \n> +static void diff_flush_patch(struct diff_options *o)\n> +{\n> +\tint i;\n> +\tstruct diff_queue_struct *q = &diff_queued_diff;\n> +\tfor (i = 0; i < q->nr; i++) {\n> +\t\tstruct diff_filepair *p = q->queue[i];\n> +\t\tif (check_pair_status(p))\n> +\t\t\tdiff_flush_patch_filepair(p, o);\n> +\t}\n> +}\n> +\n> void diff_flush(struct diff_options *options)\n> {\n> \tstruct diff_queue_struct *q = &diff_queued_diff;\n> @@ -4702,11 +4713,7 @@ void diff_flush(struct diff_options *options)\n> \t\t\t}\n> \t\t}\n> \n> -\t\tfor (i = 0; i < q->nr; i++) {\n> -\t\t\tstruct diff_filepair *p = q->queue[i];\n> -\t\t\tif (check_pair_status(p))\n> -\t\t\t\tdiff_flush_patch_filepair(p, options);\n> -\t\t}\n> +\t\tdiff_flush_patch(options);\n> \t}\n> \n> \tif (output_format & DIFF_FORMAT_CALLBACK)\n"},{"id":"301789","messageId":"xmqqoa3swteo.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-7-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 06/10] diff.c: emit_line_0 can handle no color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-13T00:11:11Z","receivedAt":"2016-09-13T00:11:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Stefan Beller \n>\n> ---\n\n\"X can do Y\" can be taken as a statement of fact (to which \"so\nwhat?\" is an appropriate response), a desire (to which \"then please\nsay 'make X do Y' instead\" is an appropriate response), or a report\nof a bug (to which \"please explain why X should be forbidden from\ndoing Y\" is an appropriate response).\n\nThis is way under-explained. I think this is \"make X do Y\" kind,\nand if so, please say so and possibly why it is a good idea to teach\nX how to do Y.\n\nThanks.\n\n\n\n> diff.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index 87b1bb2..2aefd0f 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n> \t}\n> \n> \tif (len || !nofirst) {\n> -\t\tfputs(set, file);\n> +\t\tif (set)\n> +\t\t\tfputs(set, file);\n> \t\tif (!nofirst)\n> \t\t\tfputc(first, file);\n> \t\tfwrite(line, len, 1, file);\n> -\t\tfputs(reset, file);\n> +\t\tif (reset)\n> +\t\t\tfputs(reset, file);\n> \t}\n> \tif (has_trailing_carriage_return)\n> \t\tfputc('\\r', file);\n"},{"id":"301790","messageId":"CAGZ79kbP3k0pa62xpv3SCnTvp-qSzfGKjSy8infcx=EexnBOTA@mail.gmail.com","threadId":"44055","inReplyTo":"xmqqoa3swteo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 06/10] diff.c: emit_line_0 can handle no color","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-13T00:25:37Z","receivedAt":"2016-09-13T00:25:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Sep 12, 2016 at 5:11 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> From: Stefan Beller \n>>\n>> ---\n>\n> \"X can do Y\" can be taken as a statement of fact (to which \"so\n> what?\" is an appropriate response), a desire (to which \"then please\n> say 'make X do Y' instead\" is an appropriate response), or a report\n> of a bug (to which \"please explain why X should be forbidden from\n> doing Y\" is an appropriate response).\n>\n> This is way under-explained. I think this is \"make X do Y\" kind,\n> and if so, please say so and possibly why it is a good idea to teach\n> X how to do Y.\n>\n> Thanks.\n\nOk, I see the general pattern of your answers: Add more explanations.\n\nAnswering for\npatch 01/10 as well as this one here:\n\n I want to propose an option to detect&color moved lines in a patch,\n which cannot be done in a one-pass over the diff. Instead we need to go\n over the whole diff twice, because we cannot detect the first line of a\n line pair that got moved in the first pass. So I aim for\n * collecting all output into a buffer as a first pass,\n * as the second pass output the buffer.\n\nSo in a later patch I will split up the emit_line_* machinery to either\nemitting to options->file or buffering if we do the 2 pass thing.\n\nTo make sure the 2 passes work correctly, we need to make sure all output\nis routed through the emit_line functions, and there will be no direct writes.\n\nNow that we will be using the emit_lines functions for non colored\noutput as well,\nwe want to pass in \"no color\" which I think is best done via NULL and then not\ncalling the output of the color writes.\n"},{"id":"301793","messageId":"xmqqk2egwsq3.fsf@gitster.mtv.corp.google.com","threadId":"44055","inReplyTo":"1473572530-25764-8-git-send-email-stefanbeller@gmail.com","subject":"Re: [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-13T00:25:56Z","receivedAt":"2016-09-13T00:26:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/diff.c b/diff.c\n> index 2aefd0f..7dcef73 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -493,6 +493,19 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset\n> \temit_line_0(o, set, reset, line[0], line+1, len-1);\n> }\n> \n> +static void emit_line_fmt(struct diff_options *o,\n> +\t\t\t const char *set, const char *reset,\n> +\t\t\t const char *fmt, ...)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tva_list ap;\n> +\tva_start(ap, fmt);\n> +\tstrbuf_vaddf(&sb, fmt, ap);\n> +\tva_end(ap);\n> +\n> +\temit_line(o, set, reset, sb.buf, sb.len);\n> +}\n> +\n> static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)\n> {\n> \tif (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&\n> @@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n> \tconst char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);\n> \tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n> \tstruct diff_options *o = ecbdata->opt;\n> -\tconst char *line_prefix = diff_line_prefix(o);\n> \n> \to->found_changes = 1;\n> \n> @@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n> \t\tname_a_tab = strchr(ecbdata->label_path[0], ' ') ? \"\\t\" : \"\";\n> \t\tname_b_tab = strchr(ecbdata->label_path[1], ' ') ? \"\\t\" : \"\";\n> \n> -\t\tfprintf(o->file, \"%s%s--- %s%s%s\\n\",\n> -\t\t\tline_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);\n> -\t\tfprintf(o->file, \"%s%s+++ %s%s%s\\n\",\n> -\t\t\tline_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);\n> +\t\temit_line_fmt(o, meta, reset, \"--- %s%s\\n\",\n> +\t\t\t ecbdata->label_path[0], name_a_tab);\n> +\n> +\t\temit_line_fmt(o, meta, reset, \"+++ %s%s\\n\",\n> +\t\t\t ecbdata->label_path[1], name_b_tab);\n\nHmph, the original showed the following for the name-a line:\n\n\tdiff_line_prefix(o) META \"--- \" label_path RESET name_a_tab LF\n\nThe updated one calls emit_line_fmt() with o, meta, reset, fmt and\nargs, and then\n\n * strbuf_vaddf(&buf, \"--- %s%s\\n\", label_path, name_a_tab) creates\n a string \"--- \" + label_path + LF\n\n * emit_line() is called on the whole thing with META and RESET\n\n * which is emit_line_0() that encloses the whole thing between META\n and RESET but knows the trailing LF should come after RESET.\n\nSo the coloring seems to be correct, but I am not sure where the\nline-prefix went.\n\n\n\n"},{"id":"301798","messageId":"CAGZ79kZbuVsj+hqc1CE05Nx+NqGYp9bRqKr+oKu-Rmx87A1C-Q@mail.gmail.com","threadId":"44055","inReplyTo":"xmqqk2egwsq3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-13T00:41:53Z","receivedAt":"2016-09-13T00:42:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Sep 12, 2016 at 5:25 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> diff --git a/diff.c b/diff.c\n>> index 2aefd0f..7dcef73 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -493,6 +493,19 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset\n>> emit_line_0(o, set, reset, line[0], line+1, len-1);\n>> }\n>>\n>> +static void emit_line_fmt(struct diff_options *o,\n>> + const char *set, const char *reset,\n>> + const char *fmt, ...)\n>> +{\n>> + struct strbuf sb = STRBUF_INIT;\n>> + va_list ap;\n>> + va_start(ap, fmt);\n>> + strbuf_vaddf(&sb, fmt, ap);\n>> + va_end(ap);\n>> +\n>> + emit_line(o, set, reset, sb.buf, sb.len);\n>> +}\n>> +\n>> static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)\n>> {\n>> if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&\n>> @@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n>> const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);\n>> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n>> struct diff_options *o = ecbdata->opt;\n>> - const char *line_prefix = diff_line_prefix(o);\n>>\n>> o->found_changes = 1;\n>>\n>> @@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n>> name_a_tab = strchr(ecbdata->label_path[0], ' ') ? \"\\t\" : \"\";\n>> name_b_tab = strchr(ecbdata->label_path[1], ' ') ? \"\\t\" : \"\";\n>>\n>> - fprintf(o->file, \"%s%s--- %s%s%s\\n\",\n>> - line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);\n>> - fprintf(o->file, \"%s%s+++ %s%s%s\\n\",\n>> - line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);\n>> + emit_line_fmt(o, meta, reset, \"--- %s%s\\n\",\n>> + ecbdata->label_path[0], name_a_tab);\n>> +\n>> + emit_line_fmt(o, meta, reset, \"+++ %s%s\\n\",\n>> + ecbdata->label_path[1], name_b_tab);\n>\n> Hmph, the original showed the following for the name-a line:\n>\n> diff_line_prefix(o) META \"--- \" label_path RESET name_a_tab LF\n>\n> The updated one calls emit_line_fmt() with o, meta, reset, fmt and\n> args, and then\n>\n> * strbuf_vaddf(&buf, \"--- %s%s\\n\", label_path, name_a_tab) creates\n> a string \"--- \" + label_path + LF\n>\n> * emit_line() is called on the whole thing with META and RESET\n\nOh right, that is a real issue, i.e. name_b_tab is not colored. I'll\nhave to think about that.\n\n>\n> * which is emit_line_0() that encloses the whole thing between META\n> and RESET but knows the trailing LF should come after RESET.\n>\n> So the coloring seems to be correct, but I am not sure where the\n> line-prefix went.\n\nemit_line_0 puts the line_prefix by default in front, i.e.\nit emits\n\n line_prefix (SET) (first char) line (RESET) (CR/LF)\n\nwith things in parenthesis optional.\n\nAs said in 6/10 I think the emit_line_0 function is a great starting point\nto build up a buffer when we do a two passes output. For that I want to channel\nall output through emit_line_*.\n\n\n---\nI am done converting all diff output to emit_line for a rought first series,\nand now all tests pass when asking for a buffered output.\n\nSo I started building the --color-moved on top of that, which seems\nto work as well. The diff stat is ~700 insertions and ~300 deletions\ncompared to 2.10, however we call out to the xdl stuff only once.\n\nComparing that to the original quick-hack-patch\nhttps://public-inbox.org/git/20160906070151.15163-1-stefanbeller@gmail.com/\nwhich has just 280 additions, 10 deletions;\nwe seem to need about ~350 refactor lines and\nslightly slower emissions in the non-color-moved case\n(all the calls to emit_line instead of directly using fprintf/fputs/fputc)\n"},{"id":"302087","messageId":"CAGZ79kbRBqEMib=ym9xE2E_Tw4r_nA+wvU9NDfjnJRLKXk8ZzQ@mail.gmail.com","threadId":"44055","inReplyTo":"xmqq1t0oy8t8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-16T23:04:29Z","receivedAt":"2016-09-16T23:04:34Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Sep 12, 2016 at 4:53 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> diff --git a/diff.c b/diff.c\n>> index 156c2aa..9d2e704 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res\n>>\n>> if (len == 0) {\n>> has_trailing_newline = (first == '\\n');\n>> - has_trailing_carriage_return = (!has_trailing_newline &&\n>> - (first == '\\r'));\n>> + has_trailing_carriage_return = (first == '\\r');\n>> nofirst = has_trailing_newline || has_trailing_carriage_return;\n>> } else {\n>> has_trailing_newline = (len > 0 && line[len-1] == '\\n');\n>\n> Interesting.\n>\n> This may be a mis-conversion at 250f7993 (\"diff.c: split emit_line()\n> from the first char and the rest of the line\", 2009-09-14), I\n> suspect. The original took line[] with length and peeked for '\\n',\n> and when it saw one, it decremented length before checking\n> line[len-1] for '\\r'.\n>\n> But of course if there is only one byte on the line (i.e. len == 0\n> after first is stripped off), it cannot be both '\\n' or '\\r' at the\n> same time.\n>\n> Thanks for spotting.\n\nOh, right, it used to be possible to remove \\r\\n completely and that information\nwas then kept as has_trailing_newline = has_trailing_carriage_return = 1;\nand the resulting line is kept completely without ending line.\n\nAfter some thought I don't think I can use this mis-conversion\nto trigger a bug though, because the len=0 can only ever happen\nif first is '\\n' alone essentially.\n\nAnother thing I noticed when playing around with diffs:\n\n $ printf \"\\r\\n\" >crlf\n $ git commit crlf -m \"add file crlf, empty line\"\n $ printf \"non zero length\\r\\n\" >crlf\n $ diff --git a/crlf b/crlf\n $ index d3f5a12..ece7140 100644\n --- a/crlf\n +++ b/crlf\n @@ -1 +1 @@\n -\n +non zero length^M\n $ # The - line is missing a ^M ?\n"}]}