{"thread":{"id":"21658","subject":"[PATCH] Give the hunk comment its own color","startedAt":"2009-11-18T11:30:36Z","lastAt":"2009-11-30T09:26:22Z","messageCount":28,"participants":["Bert Wesarg","Tay Ray Chuan","Jeff King","Jason Sewall","Junio C Hamano","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"127832","messageId":"1258543836-799-1-git-send-email-bert.wesarg@googlemail.com","threadId":"21658","inReplyTo":null,"subject":"[PATCH] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-18T11:30:36Z","receivedAt":"2009-11-18T11:30:36Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Insired by the coloring of quilt.\n\nIntroduce a separate color for the hunk comment part, i.e. the current function.\nWhitespace between hunk header and hunk comment is now printed as plain.\n\nThe current default is magenta. But I'm not settled on this. My favorite would\nbe bold yellow.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\n Documentation/config.txt |    8 +++---\n combine-diff.c           |    5 +++-\n diff.c                   |   62 +++++++++++++++++++++++++++++++++++++++++++--\n diff.h                   |    1 +\n 4 files changed, 68 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex cb73d75..421cd50 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -598,10 +598,10 @@ color.diff.<slot>::\n \tUse customized color for diff colorization.  `<slot>` specifies\n \twhich part of the patch to use the specified color, and is one\n \tof `plain` (context text), `meta` (metainformation), `frag`\n-\t(hunk header), `old` (removed lines), `new` (added lines),\n-\t`commit` (commit headers), or `whitespace` (highlighting\n-\twhitespace errors). The values of these variables may be specified as\n-\tin color.branch.<slot>.\n+\t(hunk header), 'func' (function in hunk header), `old` (removed lines),\n+\t`new` (added lines), `commit` (commit headers), or `whitespace`\n+\t(highlighting whitespace errors). The values of these variables may be\n+\tspecified as in color.branch.<slot>.\n \n color.grep::\n \tWhen set to `always`, always highlight matches.  When `false` (or\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 5b63af1..6162691 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -524,6 +524,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \tint i;\n \tunsigned long lno = 0;\n \tconst char *c_frag = diff_get_color(use_color, DIFF_FRAGINFO);\n+\tconst char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);\n \tconst char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);\n \tconst char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);\n \tconst char *c_plain = diff_get_color(use_color, DIFF_PLAIN);\n@@ -588,7 +589,9 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \t\t\t\t    comment_end = i;\n \t\t\t}\n \t\t\tif (comment_end)\n-\t\t\t\tputchar(' ');\n+\t\t\t\tprintf(\"%s%s %s%s\", c_reset,\n+\t\t\t\t\t\t    c_plain, c_reset,\n+\t\t\t\t\t\t    c_func);\n \t\t\tfor (i = 0; i < comment_end; i++)\n \t\t\t\tputchar(hunk_comment[i]);\n \t\t}\ndiff --git a/diff.c b/diff.c\nindex 0d7f5ea..8a5ed1b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -39,6 +39,7 @@ static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_GREEN,\t/* NEW */\n \tGIT_COLOR_YELLOW,\t/* COMMIT */\n \tGIT_COLOR_BG_RED,\t/* WHITESPACE */\n+\tGIT_COLOR_MAGENTA,\t/* FUNCINFO */\n };\n \n static void diff_filespec_load_driver(struct diff_filespec *one);\n@@ -60,6 +61,8 @@ static int parse_diff_color_slot(const char *var, int ofs)\n \t\treturn DIFF_COMMIT;\n \tif (!strcasecmp(var+ofs, \"whitespace\"))\n \t\treturn DIFF_WHITESPACE;\n+\tif (!strcasecmp(var+ofs, \"func\"))\n+\t\treturn DIFF_FUNCINFO;\n \tdie(\"bad config variable '%s'\", var);\n }\n \n@@ -344,6 +347,61 @@ static void emit_add_line(const char *reset,\n \t}\n }\n \n+static void emit_hunk_line(struct emit_callback *ecbdata,\n+\t\t\t   const char *line, int len)\n+{\n+\tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n+\tconst char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n+\tconst char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n+\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n+\tconst char *part_end = NULL;\n+\tint part_len = 0;\n+\n+\t/* determine length of @ */\n+\twhile (part_len < len && line[part_len] == '@')\n+\t\tpart_len++;\n+\n+\t/* find end of frag, (Ie. find second @@) */\n+\tpart_end = memmem(line + part_len, len - part_len,\n+\t\t\t  line, part_len);\n+\tif (!part_end)\n+\t\treturn emit_line(ecbdata->file, frag, reset, line, len);\n+\t/* go to end of @@ */\n+\tpart_end += part_len;\n+\t/* calculate total length of frag */\n+\tpart_len = part_end - line;\n+\t/* emit frag */\n+\temit_line(ecbdata->file, frag, reset, line, part_len);\n+\n+\t/* consume hunk header */\n+\tlen -= part_len;\n+\tline += part_len;\n+\n+\t/* return early */\n+\tif (!len)\n+\t\treturn;\n+\n+\t/* determine length of sep space */\n+\tpart_len = 0;\n+\twhile (part_len < len && isspace(line[part_len]))\n+\t\tpart_len++;\n+\n+\t/* no whitespace sep => print reminder as FRAGINFO */\n+\tif (!part_len)\n+\t\treturn emit_line(ecbdata->file, frag, reset, line, len);\n+\n+\t/* print whitespace sep as PLAIN */\n+\temit_line(ecbdata->file, plain, reset, part_end, part_len);\n+\n+\t/* consume whitespace sep */\n+\tlen -= part_len;\n+\tline += part_len;\n+\n+\t/* print reminder as FUNCINFO */\n+\tif (len)\n+\t\temit_line(ecbdata->file, func, reset, line, len);\n+}\n+\n static struct diff_tempfile *claim_diff_tempfile(void) {\n \tint i;\n \tfor (i = 0; i < ARRAY_SIZE(diff_temp); i++)\n@@ -781,9 +839,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\t\tdiff_words_flush(ecbdata);\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n \t\tfind_lno(line, ecbdata);\n-\t\temit_line(ecbdata->file,\n-\t\t\t  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),\n-\t\t\t  reset, line, len);\n+\t\temit_hunk_line(ecbdata, line, len);\n \t\tif (line[len-1] != '\\n')\n \t\t\tputc('\\n', ecbdata->file);\n \t\treturn;\ndiff --git a/diff.h b/diff.h\nindex 2740421..15fcecd 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -130,6 +130,7 @@ enum color_diff {\n \tDIFF_FILE_NEW = 5,\n \tDIFF_COMMIT = 6,\n \tDIFF_WHITESPACE = 7,\n+\tDIFF_FUNCINFO = 8,\n };\n const char *diff_get_color(int diff_use_color, enum color_diff ix);\n #define diff_get_color_opt(o, ix) \\\n-- \ntg: (785c58e..) bw/func-color (depends on: master)\n"},{"id":"127834","messageId":"be6fef0d0911180344ld31237et533cfa8832ea0c6c@mail.gmail.com","threadId":"21658","inReplyTo":"1258543836-799-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-11-18T11:44:11Z","receivedAt":"2009-11-18T11:44:11Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Nov 18, 2009 at 7:30 PM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n> Insired by the coloring of quilt.\n\ns/Insired/Inspired/?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"127836","messageId":"36ca99e90911180357p929b642jada9f4afc81e99d8@mail.gmail.com","threadId":"21658","inReplyTo":"be6fef0d0911180344ld31237et533cfa8832ea0c6c@mail.gmail.com","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-18T11:57:33Z","receivedAt":"2009-11-18T11:57:33Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Nov 18, 2009 at 12:44, Tay Ray Chuan <rctay89@gmail.com> wrote:\n> Hi,\n>\n> On Wed, Nov 18, 2009 at 7:30 PM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n>> Insired by the coloring of quilt.\n>\n> s/Insired/Inspired/?\nSure.\n\nI also forgot to update the test suit.\n\nNew patch follows.\n\nBert\n>\n> --\n> Cheers,\n> Ray Chuan\n>\n"},{"id":"127850","messageId":"20091118142320.GA1220@coredump.intra.peff.net","threadId":"21658","inReplyTo":"1258543836-799-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-18T14:23:21Z","receivedAt":"2009-11-18T14:23:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 18, 2009 at 12:30:36PM +0100, Bert Wesarg wrote:\n\n> Insired by the coloring of quilt.\n> \n> Introduce a separate color for the hunk comment part, i.e. the current\n> function.  Whitespace between hunk header and hunk comment is now\n> printed as plain.\n> \n> The current default is magenta. But I'm not settled on this. My\n> favorite would be bold yellow.\n\nI don't see any reason not to add this, as it is simply introducing one\nextra knob to tweak for people who care. However, after some\nexperimentation, I found that I don't personally really like it. I ended\nup wanting it set to the same color as the hunk header.\n\nI wonder how hard it would be to make it backwards-compatible; that is,\nto inherit the color value of the hunk header (be it the original or one\nset by the user) unless the func color is set by the user. But maybe\nthat is over-engineering. It is not like we are breaking scripts, and it\nis not that hard for people to see the new behavior and then tweak their\nconfig if they don't like it.\n\n-Peff\n\nPS I almost complained about your default of \"magenta\" as the same as\nthe meta color before I remembered that magenta meta is a personal\nsetting I use. Personally I find the bold meta color to be distractingly\nugly. Blaming it, the default seems to come from Linus, who even in his\ncommit message (50f575f) seems to indicate that it is somewhat arbitrary\n(mostly just dropping the purple from the bold purple).\n\nI'm not sure what is the best way to arrive at a default color for\nsomething like this. Arguing about it really is almost the definition of\nbikeshedding.  Maybe next year's git survey should contain a special\nsection on colors, and majority should rule.  :)\n"},{"id":"127856","messageId":"1258557087-31540-1-git-send-email-bert.wesarg@googlemail.com","threadId":"21658","inReplyTo":"1258543836-799-1-git-send-email-bert.wesarg@googlemail.com","subject":"[PATCH v2] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-18T15:11:27Z","receivedAt":"2009-11-18T15:11:27Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Inspired by the coloring of quilt.\n\nIntroduce a separate color for the hunk comment part, i.e. the current function.\nWhitespace between hunk header and hunk comment is now printed as plain.\n\nThe current default is magenta. But I'm not settled on this. My favorite would\nbe bold yellow.\n\nNow with updated test suit.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\n Documentation/config.txt |    8 +++---\n combine-diff.c           |    5 +++-\n diff.c                   |   64 +++++++++++++++++++++++++++++++++++++++++++--\n diff.h                   |    1 +\n t/t4034-diff-words.sh    |    3 +-\n 5 files changed, 72 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex cb73d75..421cd50 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -598,10 +598,10 @@ color.diff.<slot>::\n \tUse customized color for diff colorization.  `<slot>` specifies\n \twhich part of the patch to use the specified color, and is one\n \tof `plain` (context text), `meta` (metainformation), `frag`\n-\t(hunk header), `old` (removed lines), `new` (added lines),\n-\t`commit` (commit headers), or `whitespace` (highlighting\n-\twhitespace errors). The values of these variables may be specified as\n-\tin color.branch.<slot>.\n+\t(hunk header), 'func' (function in hunk header), `old` (removed lines),\n+\t`new` (added lines), `commit` (commit headers), or `whitespace`\n+\t(highlighting whitespace errors). The values of these variables may be\n+\tspecified as in color.branch.<slot>.\n \n color.grep::\n \tWhen set to `always`, always highlight matches.  When `false` (or\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 5b63af1..6162691 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -524,6 +524,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \tint i;\n \tunsigned long lno = 0;\n \tconst char *c_frag = diff_get_color(use_color, DIFF_FRAGINFO);\n+\tconst char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);\n \tconst char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);\n \tconst char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);\n \tconst char *c_plain = diff_get_color(use_color, DIFF_PLAIN);\n@@ -588,7 +589,9 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \t\t\t\t    comment_end = i;\n \t\t\t}\n \t\t\tif (comment_end)\n-\t\t\t\tputchar(' ');\n+\t\t\t\tprintf(\"%s%s %s%s\", c_reset,\n+\t\t\t\t\t\t    c_plain, c_reset,\n+\t\t\t\t\t\t    c_func);\n \t\t\tfor (i = 0; i < comment_end; i++)\n \t\t\t\tputchar(hunk_comment[i]);\n \t\t}\ndiff --git a/diff.c b/diff.c\nindex 0d7f5ea..e210525 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -39,6 +39,7 @@ static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_GREEN,\t/* NEW */\n \tGIT_COLOR_YELLOW,\t/* COMMIT */\n \tGIT_COLOR_BG_RED,\t/* WHITESPACE */\n+\tGIT_COLOR_MAGENTA,\t/* FUNCINFO */\n };\n \n static void diff_filespec_load_driver(struct diff_filespec *one);\n@@ -60,6 +61,8 @@ static int parse_diff_color_slot(const char *var, int ofs)\n \t\treturn DIFF_COMMIT;\n \tif (!strcasecmp(var+ofs, \"whitespace\"))\n \t\treturn DIFF_WHITESPACE;\n+\tif (!strcasecmp(var+ofs, \"func\"))\n+\t\treturn DIFF_FUNCINFO;\n \tdie(\"bad config variable '%s'\", var);\n }\n \n@@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,\n \t}\n }\n \n+static void emit_hunk_line(struct emit_callback *ecbdata,\n+\t\t\t   const char *line, int len)\n+{\n+\tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n+\tconst char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n+\tconst char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n+\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n+\tconst char *orig_line = line;\n+\tint orig_len = len;\n+\tconst char *frag_start;\n+\tint frag_len;\n+\tconst char *part_end = NULL;\n+\tint part_len = 0;\n+\n+\t/* determine length of @ */\n+\twhile (part_len < len && line[part_len] == '@')\n+\t\tpart_len++;\n+\n+\t/* find end of frag, (Ie. find second @@) */\n+\tpart_end = memmem(line + part_len, len - part_len,\n+\t\t\t  line, part_len);\n+\tif (!part_end)\n+\t\treturn emit_line(ecbdata->file, frag, reset, line, len);\n+\t/* calculate total length of frag */\n+\tpart_len = (part_end + part_len) - line;\n+\n+\t/* remember frag part, we emit only if we find a space separator */\n+\tfrag_start = line;\n+\tfrag_len = part_len;\n+\n+\t/* consume hunk header */\n+\tlen -= part_len;\n+\tline += part_len;\n+\n+\t/*\n+\t * for empty reminder or empty space sequence (exclusive any newlines\n+\t * or carriage returns) emit complete original line as FRAGINFO\n+\t */\n+\tif (!len || !(part_len = strspn(line, \" \\t\")))\n+\t\treturn emit_line(ecbdata->file, frag, reset,\n+\t\t\t\t orig_line, orig_len);\n+\n+\t/* now emit the hunk header as FRAGINFO */\n+\temit_line(ecbdata->file, frag, reset, frag_start, frag_len);\n+\n+\t/* print whitespace sep as PLAIN */\n+\temit_line(ecbdata->file, plain, reset, line, part_len);\n+\n+\t/* consume whitespace sep */\n+\tlen -= part_len;\n+\tline += part_len;\n+\n+\t/* print reminder as FUNCINFO */\n+\tif (len)\n+\t\temit_line(ecbdata->file, func, reset, line, len);\n+}\n+\n static struct diff_tempfile *claim_diff_tempfile(void) {\n \tint i;\n \tfor (i = 0; i < ARRAY_SIZE(diff_temp); i++)\n@@ -781,9 +841,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\t\tdiff_words_flush(ecbdata);\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n \t\tfind_lno(line, ecbdata);\n-\t\temit_line(ecbdata->file,\n-\t\t\t  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),\n-\t\t\t  reset, line, len);\n+\t\temit_hunk_line(ecbdata, line, len);\n \t\tif (line[len-1] != '\\n')\n \t\t\tputc('\\n', ecbdata->file);\n \t\treturn;\ndiff --git a/diff.h b/diff.h\nindex 2740421..15fcecd 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -130,6 +130,7 @@ enum color_diff {\n \tDIFF_FILE_NEW = 5,\n \tDIFF_COMMIT = 6,\n \tDIFF_WHITESPACE = 7,\n+\tDIFF_FUNCINFO = 8,\n };\n const char *diff_get_color(int diff_use_color, enum color_diff ix);\n #define diff_get_color_opt(o, ix) \\\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 21db6e9..64a7c38 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -16,6 +16,7 @@ decrypt_color () {\n \t\t-e 's/.\\[1m/<WHITE>/g' \\\n \t\t-e 's/.\\[31m/<RED>/g' \\\n \t\t-e 's/.\\[32m/<GREEN>/g' \\\n+\t\t-e 's/.\\[35m/<MAGENTA>/g' \\\n \t\t-e 's/.\\[36m/<BROWN>/g' \\\n \t\t-e 's/.\\[m/<RESET>/g'\n }\n@@ -70,7 +71,7 @@ cat > expect <<\\EOF\n <WHITE>+++ b/post<RESET>\n <BROWN>@@ -1 +1 @@<RESET>\n <RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>\n-<BROWN>@@ -3,0 +4,4 @@ a = b + c<RESET>\n+<BROWN>@@ -3,0 +4,4 @@<RESET> <RESET><MAGENTA>a = b + c<RESET>\n \n <GREEN>aa = a<RESET>\n \n-- \ntg: (785c58e..) bw/func-color (depends on: master)\n"},{"id":"127857","messageId":"36ca99e90911180716v2bebffdeva7caa3abe7eb0115@mail.gmail.com","threadId":"21658","inReplyTo":"20091118142320.GA1220@coredump.intra.peff.net","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-18T15:16:15Z","receivedAt":"2009-11-18T15:16:15Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Nov 18, 2009 at 15:23, Jeff King <peff@peff.net> wrote:\n> PS I almost complained about your default of \"magenta\" as the same as\n> the meta color before I remembered that magenta meta is a personal\n> setting I use. Personally I find the bold meta color to be distractingly\n> ugly. Blaming it, the default seems to come from Linus, who even in his\n> commit message (50f575f) seems to indicate that it is somewhat arbitrary\n> (mostly just dropping the purple from the bold purple).\nI took magenta, because it is not used as any other default color\nvalue. I think choosing a color other than cyan would bring the\nattention to this new feature.\n\nBert\n"},{"id":"127858","messageId":"31e9dd080911180717i27c6ef3fp736b7f8d41e4c8be@mail.gmail.com","threadId":"21658","inReplyTo":"1258557087-31540-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH v2] Give the hunk comment its own color","fromName":"Jason Sewall","fromEmail":"jasonsewall@gmail.com","sentAt":"2009-11-18T15:17:01Z","receivedAt":"2009-11-18T15:17:01Z","isPatch":true,"sender":{"key":"jasonsewall@gmail.com","avatar":null},"body":"On Wed, Nov 18, 2009 at 10:11 AM, Bert Wesarg\n<bert.wesarg@googlemail.com> wrote:\n\n> Now with updated test suit.\n\nSpelling this as 'suite' would be sweet.\n"},{"id":"127859","messageId":"36ca99e90911180720yad3f08br1d1dd0d14f811ed5@mail.gmail.com","threadId":"21658","inReplyTo":"31e9dd080911180717i27c6ef3fp736b7f8d41e4c8be@mail.gmail.com","subject":"Re: [PATCH v2] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-18T15:20:43Z","receivedAt":"2009-11-18T15:20:43Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Wed, Nov 18, 2009 at 16:17, Jason Sewall <jasonsewall@gmail.com> wrote:\n> On Wed, Nov 18, 2009 at 10:11 AM, Bert Wesarg\n> <bert.wesarg@googlemail.com> wrote:\n>\n>> Now with updated test suit.\n>\n> Spelling this as 'suite' would be sweet.\nThanks. I re-submit only if you find a bug too.\n\nBert\n>\n"},{"id":"127874","messageId":"7vaayjebu5.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"20091118142320.GA1220@coredump.intra.peff.net","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-18T21:56:34Z","receivedAt":"2009-11-18T21:56:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> PS I almost complained about your default of \"magenta\" as the same as\n> the meta color before I remembered that magenta meta is a personal\n> setting I use.\n\nOn black-on-white terminals, cyan tends to be less visible, and I think\nthat is the whole point of painting the hunk header @@ .. @@ in that\ncolor--- make it less distracting). \n\nBut the function name on the line is not something that should be made\nless visible---if that part of the line were a meaningless cruft, we\nwouldn't have configurable funcname patterns after all.\n\nI would suggest \"normal\" as the neutral default.  After all, the purpose\nof the funcname in the hunk header is to give context to people who read\npatches.\n\n> I'm not sure what is the best way to arrive at a default color for\n> something like this. Arguing about it really is almost the definition of\n> bikeshedding.  Maybe next year's git survey should contain a special\n> section on colors, and majority should rule.  :)\n\nSorry, but this is no democracy ;-)\n"},{"id":"127877","messageId":"20091118224448.GA4616@coredump.intra.peff.net","threadId":"21658","inReplyTo":"7vaayjebu5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-18T22:44:49Z","receivedAt":"2009-11-18T22:44:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 18, 2009 at 01:56:34PM -0800, Junio C Hamano wrote:\n\n> On black-on-white terminals, cyan tends to be less visible, and I think\n> that is the whole point of painting the hunk header @@ .. @@ in that\n> color--- make it less distracting).\n\nHmm. I find cyan-on-white gratingly ugly instead of \"less distracting\",\nbut then again, I find black-on-white terminals to be eye-searing in\ngeneral.\n\n> I would suggest \"normal\" as the neutral default.  After all, the purpose\n> of the funcname in the hunk header is to give context to people who read\n> patches.\n\nI think that is sensible.\n\n-Peff\n"},{"id":"128455","messageId":"36ca99e90911260405y42a9a07cx419d2973ec673039@mail.gmail.com","threadId":"21658","inReplyTo":"7vaayjebu5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-26T12:05:53Z","receivedAt":"2009-11-26T12:05:53Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Junio,\n\nmay I kindly remind you of this patch. If it is only the nen-existing\nconsensus of the default color, than please use the die.\n\nKind regards,\nBert Wesarg\n"},{"id":"128521","messageId":"7v4oogzo74.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"36ca99e90911260405y42a9a07cx419d2973ec673039@mail.gmail.com","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-27T02:38:55Z","receivedAt":"2009-11-27T02:38:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> may I kindly remind you of this patch.\n\nYes you may ;-)  A more effective would have been a resend but it is\nalways appreciated.\n\n> ... If it is only the nen-existing\n> consensus of the default color, than please use the die.\n\nIf you are having me go find the mail and apply I would probably use\n\"plain\" as I suggested.\n"},{"id":"128528","messageId":"36ca99e90911262229o563ce504v6e3d6be15f4fa81f@mail.gmail.com","threadId":"21658","inReplyTo":"7v4oogzo74.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-27T06:29:26Z","receivedAt":"2009-11-27T06:29:26Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Fri, Nov 27, 2009 at 03:38, Junio C Hamano <gitster@pobox.com> wrote:\n> Bert Wesarg <bert.wesarg@googlemail.com> writes:\n>\n>> may I kindly remind you of this patch.\n>\n> Yes you may ;-)  A more effective would have been a resend but it is\n> always appreciated.\n>\n>> ... If it is only the nen-existing\n>> consensus of the default color, than please use the die.\n>\n> If you are having me go find the mail and apply I would probably use\n> \"plain\" as I suggested.\nSo I resend with plain as default. And therefore saved one resend ;-)\n\nBert\n>\n"},{"id":"128530","messageId":"20091127065202.GD20844@coredump.intra.peff.net","threadId":"21658","inReplyTo":"7v4oogzo74.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-11-27T06:52:03Z","receivedAt":"2009-11-27T06:52:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 26, 2009 at 06:38:55PM -0800, Junio C Hamano wrote:\n\n> > ... If it is only the nen-existing\n> > consensus of the default color, than please use the die.\n> \n> If you are having me go find the mail and apply I would probably use\n> \"plain\" as I suggested.\n\nAs the other person in the discussion, I'll just chime in that I also\nthink \"plain\" is the best of the suggested defaults.\n\n-Peff\n"},{"id":"128531","messageId":"1259304918-12600-1-git-send-email-bert.wesarg@googlemail.com","threadId":"21658","inReplyTo":"7v4oogzo74.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-27T06:55:18Z","receivedAt":"2009-11-27T06:55:18Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Inspired by the coloring of quilt.\n\nIntroduce a separate color for the hunk comment part, i.e. the current function.\nWhitespace between hunk header and hunk comment is now printed as plain.\n\nThe current default is plain.\n\nSigned-off-by: Bert Wesarg <bert.wesarg@googlemail.com>\n\n---\n Documentation/config.txt |    8 +++---\n combine-diff.c           |    5 +++-\n diff.c                   |   64 +++++++++++++++++++++++++++++++++++++++++++--\n diff.h                   |    1 +\n t/t4034-diff-words.sh    |    4 ++-\n 5 files changed, 73 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a8e0876..a1e36d7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -635,10 +635,10 @@ color.diff.<slot>::\n \tUse customized color for diff colorization.  `<slot>` specifies\n \twhich part of the patch to use the specified color, and is one\n \tof `plain` (context text), `meta` (metainformation), `frag`\n-\t(hunk header), `old` (removed lines), `new` (added lines),\n-\t`commit` (commit headers), or `whitespace` (highlighting\n-\twhitespace errors). The values of these variables may be specified as\n-\tin color.branch.<slot>.\n+\t(hunk header), 'func' (function in hunk header), `old` (removed lines),\n+\t`new` (added lines), `commit` (commit headers), or `whitespace`\n+\t(highlighting whitespace errors). The values of these variables may be\n+\tspecified as in color.branch.<slot>.\n \n color.grep::\n \tWhen set to `always`, always highlight matches.  When `false` (or\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 5b63af1..6162691 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -524,6 +524,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \tint i;\n \tunsigned long lno = 0;\n \tconst char *c_frag = diff_get_color(use_color, DIFF_FRAGINFO);\n+\tconst char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);\n \tconst char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);\n \tconst char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);\n \tconst char *c_plain = diff_get_color(use_color, DIFF_PLAIN);\n@@ -588,7 +589,9 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,\n \t\t\t\t    comment_end = i;\n \t\t\t}\n \t\t\tif (comment_end)\n-\t\t\t\tputchar(' ');\n+\t\t\t\tprintf(\"%s%s %s%s\", c_reset,\n+\t\t\t\t\t\t    c_plain, c_reset,\n+\t\t\t\t\t\t    c_func);\n \t\t\tfor (i = 0; i < comment_end; i++)\n \t\t\t\tputchar(hunk_comment[i]);\n \t\t}\ndiff --git a/diff.c b/diff.c\nindex 0d7f5ea..fd999fb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -39,6 +39,7 @@ static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_GREEN,\t/* NEW */\n \tGIT_COLOR_YELLOW,\t/* COMMIT */\n \tGIT_COLOR_BG_RED,\t/* WHITESPACE */\n+\tGIT_COLOR_NORMAL,\t/* FUNCINFO */\n };\n \n static void diff_filespec_load_driver(struct diff_filespec *one);\n@@ -60,6 +61,8 @@ static int parse_diff_color_slot(const char *var, int ofs)\n \t\treturn DIFF_COMMIT;\n \tif (!strcasecmp(var+ofs, \"whitespace\"))\n \t\treturn DIFF_WHITESPACE;\n+\tif (!strcasecmp(var+ofs, \"func\"))\n+\t\treturn DIFF_FUNCINFO;\n \tdie(\"bad config variable '%s'\", var);\n }\n \n@@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,\n \t}\n }\n \n+static void emit_hunk_line(struct emit_callback *ecbdata,\n+\t\t\t   const char *line, int len)\n+{\n+\tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n+\tconst char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n+\tconst char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n+\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n+\tconst char *orig_line = line;\n+\tint orig_len = len;\n+\tconst char *frag_start;\n+\tint frag_len;\n+\tconst char *part_end = NULL;\n+\tint part_len = 0;\n+\n+\t/* determine length of @ */\n+\twhile (part_len < len && line[part_len] == '@')\n+\t\tpart_len++;\n+\n+\t/* find end of frag, (Ie. find second @@) */\n+\tpart_end = memmem(line + part_len, len - part_len,\n+\t\t\t  line, part_len);\n+\tif (!part_end)\n+\t\treturn emit_line(ecbdata->file, frag, reset, line, len);\n+\t/* calculate total length of frag */\n+\tpart_len = (part_end + part_len) - line;\n+\n+\t/* remember frag part, we emit only if we find a space separator */\n+\tfrag_start = line;\n+\tfrag_len = part_len;\n+\n+\t/* consume hunk header */\n+\tlen -= part_len;\n+\tline += part_len;\n+\n+\t/*\n+\t * for empty reminder or empty space sequence (exclusive any newlines\n+\t * or carriage returns) emit complete original line as FRAGINFO\n+\t */\n+\tif (!len || !(part_len = strspn(line, \" \\t\")))\n+\t\treturn emit_line(ecbdata->file, frag, reset,\n+\t\t\t\t orig_line, orig_len);\n+\n+\t/* now emit the hunk header as FRAGINFO */\n+\temit_line(ecbdata->file, frag, reset, frag_start, frag_len);\n+\n+\t/* print whitespace sep as PLAIN */\n+\temit_line(ecbdata->file, plain, reset, line, part_len);\n+\n+\t/* consume whitespace sep */\n+\tlen -= part_len;\n+\tline += part_len;\n+\n+\t/* print reminder as FUNCINFO */\n+\tif (len)\n+\t\temit_line(ecbdata->file, func, reset, line, len);\n+}\n+\n static struct diff_tempfile *claim_diff_tempfile(void) {\n \tint i;\n \tfor (i = 0; i < ARRAY_SIZE(diff_temp); i++)\n@@ -781,9 +841,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\t\tdiff_words_flush(ecbdata);\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n \t\tfind_lno(line, ecbdata);\n-\t\temit_line(ecbdata->file,\n-\t\t\t  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),\n-\t\t\t  reset, line, len);\n+\t\temit_hunk_line(ecbdata, line, len);\n \t\tif (line[len-1] != '\\n')\n \t\t\tputc('\\n', ecbdata->file);\n \t\treturn;\ndiff --git a/diff.h b/diff.h\nindex 2740421..15fcecd 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -130,6 +130,7 @@ enum color_diff {\n \tDIFF_FILE_NEW = 5,\n \tDIFF_COMMIT = 6,\n \tDIFF_WHITESPACE = 7,\n+\tDIFF_FUNCINFO = 8,\n };\n const char *diff_get_color(int diff_use_color, enum color_diff ix);\n #define diff_get_color_opt(o, ix) \\\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 21db6e9..186eff8 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -8,6 +8,7 @@ test_expect_success setup '\n \n \tgit config diff.color.old red\n \tgit config diff.color.new green\n+\tgit config diff.color.func magenta\n \n '\n \n@@ -16,6 +17,7 @@ decrypt_color () {\n \t\t-e 's/.\\[1m/<WHITE>/g' \\\n \t\t-e 's/.\\[31m/<RED>/g' \\\n \t\t-e 's/.\\[32m/<GREEN>/g' \\\n+\t\t-e 's/.\\[35m/<MAGENTA>/g' \\\n \t\t-e 's/.\\[36m/<BROWN>/g' \\\n \t\t-e 's/.\\[m/<RESET>/g'\n }\n@@ -70,7 +72,7 @@ cat > expect <<\\EOF\n <WHITE>+++ b/post<RESET>\n <BROWN>@@ -1 +1 @@<RESET>\n <RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>\n-<BROWN>@@ -3,0 +4,4 @@ a = b + c<RESET>\n+<BROWN>@@ -3,0 +4,4 @@<RESET> <RESET><MAGENTA>a = b + c<RESET>\n \n <GREEN>aa = a<RESET>\n \n-- \ntg: (ad7ace7..) bw/func-color (depends on: master)\n"},{"id":"128536","messageId":"7v4oogwhoz.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"20091127065202.GD20844@coredump.intra.peff.net","subject":"Re: [PATCH] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-27T07:27:40Z","receivedAt":"2009-11-27T07:27:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Nov 26, 2009 at 06:38:55PM -0800, Junio C Hamano wrote:\n>\n>> > ... If it is only the nen-existing\n>> > consensus of the default color, than please use the die.\n>> \n>> If you are having me go find the mail and apply I would probably use\n>> \"plain\" as I suggested.\n>\n> As the other person in the discussion, I'll just chime in that I also\n> think \"plain\" is the best of the suggested defaults.\n\nOk, I tweaked the patch locally and applied.\n\nThanks.\n"},{"id":"128544","messageId":"7v3a40tl9t.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"1259304918-12600-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-27T08:38:38Z","receivedAt":"2009-11-27T08:38:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I was slightly surprised that this seems to have differences other than\nthe flipping of the default color since the last one, especially after you\nsounded like you would be sending with only that change.\n"},{"id":"128545","messageId":"36ca99e90911270044o68375902l3a0d2a4afa726a91@mail.gmail.com","threadId":"21658","inReplyTo":"7v3a40tl9t.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-27T08:44:01Z","receivedAt":"2009-11-27T08:44:01Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Fri, Nov 27, 2009 at 09:38, Junio C Hamano <gitster@pobox.com> wrote:\n> I was slightly surprised that this seems to have differences other than\n> the flipping of the default color since the last one, especially after you\n> sounded like you would be sending with only that change.\nv3 does only that and adopt the change to the t/t4034-diff-words.sh\ntest. There I set an explicit value of func to magenta for testing.\n\nMaybe you missed v2 (Message-Id:\n<1258557087-31540-1-git-send-email-bert.wesarg@googlemail.com>)? Which\nfixed the test and also a small bug.\n\nBert\n>\n"},{"id":"128548","messageId":"7vmy28s5q7.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"36ca99e90911270044o68375902l3a0d2a4afa726a91@mail.gmail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-27T08:59:44Z","receivedAt":"2009-11-27T08:59:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> Maybe you missed v2 (Message-Id:\n> <1258557087-31540-1-git-send-email-bert.wesarg@googlemail.com>)? Which\n> fixed the test and also a small bug.\n\nYeah, that was what happened.  Thanks for clarifying.\n"},{"id":"128614","messageId":"7vhbsfi4bz.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"1259304918-12600-1-git-send-email-bert.wesarg@googlemail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-28T05:52:16Z","receivedAt":"2009-11-28T05:52:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n>  diff.c                   |   64 +++++++++++++++++++++++++++++++++++++++++++--\n> ...\n> @@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,\n>  \t}\n>  }\n>  \n> +static void emit_hunk_line(struct emit_callback *ecbdata,\n> +\t\t\t   const char *line, int len)\n> +{\n> +\tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n> +\tconst char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n> +\tconst char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n> +\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n> +\tconst char *orig_line = line;\n> +\tint orig_len = len;\n> +\tconst char *frag_start;\n> +\tint frag_len;\n> +\tconst char *part_end = NULL;\n> +\tint part_len = 0;\n> +\n> +\t/* determine length of @ */\n> +\twhile (part_len < len && line[part_len] == '@')\n> +\t\tpart_len++;\n> +\n> +\t/* find end of frag, (Ie. find second @@) */\n> +\tpart_end = memmem(line + part_len, len - part_len,\n> +\t\t\t  line, part_len);\n\nThis is not incorrect per-se, but probably is overkill; this codepath only\ndeals with two-way diff and we know we are looking at \"@@ -..., +... @@\"\nat this point.\n\n\tpart_end = memmem(line + 2, len - 2, \"@@\", 2);\n\nwould be sufficient.\n\n> +\tif (!part_end)\n> +\t\treturn emit_line(ecbdata->file, frag, reset, line, len);\n> +\t/* calculate total length of frag */\n> +\tpart_len = (part_end + part_len) - line;\n> +\n> +\t/* remember frag part, we emit only if we find a space separator */\n> +\tfrag_start = line;\n> +\tfrag_len = part_len;\n> +\n> +\t/* consume hunk header */\n> +\tlen -= part_len;\n> +\tline += part_len;\n> +\n> +\t/*\n> +\t * for empty reminder or empty space sequence (exclusive any newlines\n> +\t * or carriage returns) emit complete original line as FRAGINFO\n> +\t */\n> +\tif (!len || !(part_len = strspn(line, \" \\t\")))\n\nSlightly worrisome is what guarantees this strspn() won't step outside\nlen.\n\nI would probably write the function like this instead.\n\n-- >8 --\n\nstatic void emit_hunk_header(struct emit_callback *ecbdata,\n\t\t\t     const char *line, int len)\n{\n\tconst char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n\tconst char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n\tconst char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n\tstatic const char atat[2] = { '@', '@' };\n\tconst char *cp, *ep;\n\n\t/*\n\t * As a hunk header must begin with \"@@ -<old>, +<new> @@\",\n\t * it always is at least 10 bytes long.\n\t */\n\tif (len < 10 ||\n\t    memcmp(line, atat, 2) ||\n\t    !(ep = memmem(line + 2, len - 2, atat, 2))) {\n\t\temit_line(ecbdata->file, plain, reset, line, len);\n\t\treturn;\n\t}\n\tep += 2; /* skip over the second @@ */\n\n\t/* The hunk header in fraginfo color */\n\temit_line(ecbdata->file, frag, reset, line, ep - line);\n\n\t/* blank before the func header */\n\tfor (cp = ep; ep - line < len; ep++)\n\t\tif (*ep != ' ' && *ep != 't')\n\t\t\tbreak;\n\tif (ep != cp)\n\t\temit_line(ecbdata->file, plain, reset, cp, ep - cp);\n\n\tif (ep < line + len)\n\t\temit_line(ecbdata->file, func, reset, ep, line + len - ep);\n}\n"},{"id":"128628","messageId":"36ca99e90911280408v186777f1h22254744fb61bf1f@mail.gmail.com","threadId":"21658","inReplyTo":"7vhbsfi4bz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-28T12:08:20Z","receivedAt":"2009-11-28T12:08:20Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Sat, Nov 28, 2009 at 06:52, Junio C Hamano <gitster@pobox.com> wrote:\n> Bert Wesarg <bert.wesarg@googlemail.com> writes:\n>\n>>  diff.c                   |   64 +++++++++++++++++++++++++++++++++++++++++++--\n>> ...\n>> @@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,\n>>       }\n>>  }\n>>\n>> +static void emit_hunk_line(struct emit_callback *ecbdata,\n>> +                        const char *line, int len)\n>> +{\n>> +     const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n>> +     const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n>> +     const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n>> +     const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n>> +     const char *orig_line = line;\n>> +     int orig_len = len;\n>> +     const char *frag_start;\n>> +     int frag_len;\n>> +     const char *part_end = NULL;\n>> +     int part_len = 0;\n>> +\n>> +     /* determine length of @ */\n>> +     while (part_len < len && line[part_len] == '@')\n>> +             part_len++;\n>> +\n>> +     /* find end of frag, (Ie. find second @@) */\n>> +     part_end = memmem(line + part_len, len - part_len,\n>> +                       line, part_len);\n>\n> This is not incorrect per-se, but probably is overkill; this codepath only\n> deals with two-way diff and we know we are looking at \"@@ -..., +... @@\"\n> at this point.\n>\n>        part_end = memmem(line + 2, len - 2, \"@@\", 2);\n>\n> would be sufficient.\nThats right, I made it generic by purpose.\n\n>\n>> +     if (!part_end)\n>> +             return emit_line(ecbdata->file, frag, reset, line, len);\n>> +     /* calculate total length of frag */\n>> +     part_len = (part_end + part_len) - line;\n>> +\n>> +     /* remember frag part, we emit only if we find a space separator */\n>> +     frag_start = line;\n>> +     frag_len = part_len;\n>> +\n>> +     /* consume hunk header */\n>> +     len -= part_len;\n>> +     line += part_len;\n>> +\n>> +     /*\n>> +      * for empty reminder or empty space sequence (exclusive any newlines\n>> +      * or carriage returns) emit complete original line as FRAGINFO\n>> +      */\n>> +     if (!len || !(part_len = strspn(line, \" \\t\")))\n>\n> Slightly worrisome is what guarantees this strspn() won't step outside\n> len.\nThats a valid concern and should be addressed.\n\n>\n> I would probably write the function like this instead.\n>\n> -- >8 --\n>\n> static void emit_hunk_header(struct emit_callback *ecbdata,\n>                             const char *line, int len)\n> {\n>        const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);\n>        const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n>        const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);\n>        const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n>        static const char atat[2] = { '@', '@' };\n>        const char *cp, *ep;\n>\n>        /*\n>         * As a hunk header must begin with \"@@ -<old>, +<new> @@\",\n>         * it always is at least 10 bytes long.\n>         */\n>        if (len < 10 ||\n>            memcmp(line, atat, 2) ||\n>            !(ep = memmem(line + 2, len - 2, atat, 2))) {\n>                emit_line(ecbdata->file, plain, reset, line, len);\n>                return;\n>        }\n>        ep += 2; /* skip over the second @@ */\n>\n>        /* The hunk header in fraginfo color */\n>        emit_line(ecbdata->file, frag, reset, line, ep - line);\n>\n>        /* blank before the func header */\n>        for (cp = ep; ep - line < len; ep++)\n>                if (*ep != ' ' && *ep != 't')\n>                        break;\n>        if (ep != cp)\n>                emit_line(ecbdata->file, plain, reset, cp, ep - cp);\n>\n>        if (ep < line + len)\n>                emit_line(ecbdata->file, func, reset, ep, line + len - ep);\n> }\nPlease check that its really an *ep != '\\t'. Its wrong in this mail, I\nsee only an *ep != 't'. Otherwise:\n\nAcked-by: Bert.Wesarg@googlemail.com\n>\n>\n"},{"id":"128728","messageId":"36ca99e90911292307w769913fdn1f610eeb065b41e@mail.gmail.com","threadId":"21658","inReplyTo":"36ca99e90911280408v186777f1h22254744fb61bf1f@mail.gmail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-30T07:07:22Z","receivedAt":"2009-11-30T07:07:22Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":">>        /* blank before the func header */\n>>        for (cp = ep; ep - line < len; ep++)\n>>                if (*ep != ' ' && *ep != 't')\n> Please check that its really an *ep != '\\t'. Its wrong in this mail, I\n> see only an *ep != 't'.\nObviously, you have not checked it. Please squash this in:\n\ndiff --git i/diff.c w/diff.c\nindex eaa1983..e126304 100644\n--- i/diff.c\n+++ w/diff.c\n@@ -376,7 +376,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,\n\n        /* blank before the func header */\n        for (cp = ep; ep - line < len; ep++)\n-               if (*ep != ' ' && *ep != 't')\n+               if (*ep != ' ' && *ep != '\\t')\n                        break;\n        if (ep != cp)\n                emit_line(ecbdata->file, plain, reset, cp, ep - cp);\n\nBert\n"},{"id":"128730","messageId":"7v4ooczdoe.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"36ca99e90911292307w769913fdn1f610eeb065b41e@mail.gmail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T07:15:13Z","receivedAt":"2009-11-30T07:15:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sorry, but I think the fix is already in 'next', no?\n"},{"id":"128737","messageId":"36ca99e90911292341o524840ebo47d79f06b1588d5c@mail.gmail.com","threadId":"21658","inReplyTo":"7v4ooczdoe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-11-30T07:41:49Z","receivedAt":"2009-11-30T07:41:49Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, Nov 30, 2009 at 08:15, Junio C Hamano <gitster@pobox.com> wrote:\n> Sorry, but I think the fix is already in 'next', no?\nYes it is, should have fetched first. sorry.\n"},{"id":"128738","messageId":"7vtywcwj1o.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"36ca99e90911292341o524840ebo47d79f06b1588d5c@mail.gmail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T07:47:31Z","receivedAt":"2009-11-30T07:47:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg <bert.wesarg@googlemail.com> writes:\n\n> On Mon, Nov 30, 2009 at 08:15, Junio C Hamano <gitster@pobox.com> wrote:\n>> Sorry, but I think the fix is already in 'next', no?\n> Yes it is, should have fetched first. sorry.\n\nDon't be sorry; thanks for catching it.\n\nThe current 'next' branch has the original commit with a botched rewrite\nby mine, and also a fixed commit, so unfortunately shortlog would list the\npatch twice.  I'll merge the updated (i.e. rewound and then rebuilt) tip\nof the topic branch when the topic graduates to the master (hopefully\nbefore 1.6.6-rc1), so we won't see the botched one in the end result.\n"},{"id":"128746","messageId":"fabb9a1e0911300009j1574c06cy500dde75fc68662f@mail.gmail.com","threadId":"21658","inReplyTo":"7vtywcwj1o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-11-30T08:09:22Z","receivedAt":"2009-11-30T08:09:22Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Nov 30, 2009 at 08:47, Junio C Hamano <gitster@pobox.com> wrote:\n> I'll merge the updated (i.e. rewound and then rebuilt) tip\n> of the topic branch when the topic graduates to the master (hopefully\n> before 1.6.6-rc1), so we won't see the botched one in the end result.\n\nI'm curious how you do this. Do you keep a list of replacements, that\nis \"when merging branch foo from next to master, instead merge bar\",\nor is it something the original author should remind you of when it's\ntime to merge to master?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"128754","messageId":"7vvdgstmic.fsf@alter.siamese.dyndns.org","threadId":"21658","inReplyTo":"fabb9a1e0911300009j1574c06cy500dde75fc68662f@mail.gmail.com","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-30T09:00:59Z","receivedAt":"2009-11-30T09:00:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> writes:\n\n> On Mon, Nov 30, 2009 at 08:47, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>> I'll merge the updated (i.e. rewound and then rebuilt) tip\n>> of the topic branch when the topic graduates to the master (hopefully\n>> before 1.6.6-rc1), so we won't see the botched one in the end result.\n>\n> I'm curious how you do this. Do you keep a list of replacements, that\n> is \"when merging branch foo from next to master, instead merge bar\",\n> or is it something the original author should remind you of when it's\n> time to merge to master?\n\nIf you run \"git log --oneline --first-parent master..pu\", you will notice\nthat there is no \"Merge ... into next\" at all.\n\nI maintain a private 'jch' branch that merges everything that has been\nmerged so far to 'next' and the branch always builds on top of 'master'\nwhenever 'pu' is pushed out.  The tree object recorded by the tip of 'jch'\nis designed to always match that of 'next'.  And 'pu' is built on top of\n'jch', instead of 'next', these days.\n\nThe Reintegrate script fron 'todo' (recall that I have a checkout of the\nbranch in \"Meta/\" directory) is used this way:\n\n    ... update 'jch' by merging topics that had new commits\n    $ git checkout jch && git merge xx/topic && ...\n    ... update the list of topics\n    $ Meta/Reintegrate master..jch >/var/tmp/redo-jch.sh\n\n    ... update 'master' with commits and merges\n    $ git checkout master\n    $ git am trivially-correct-patch.mbox\n    $ git merge yy/topic\n\n    ... update 'next'\n    $ git checkout next\n    $ git merge master\n    ... will merge new topics and topics that had new commits\n    $ sh /var/tmp/redo-jch.sh\n\n    ... rebuild 'jch' on top of the updated master\n    $ git checkout jch\n    $ git reset --hard master\n    $ sh /var/tmp/redo-jch.sh\n\n    ... then this shouldn't produce any output\n    $ git diff next\n\nThis time, what I did _after_ Bert noticed my typo was:\n\n    $ git checkout bw/diff-color-hunk-header\n    $ edit diff.c ;# fix my typo\n    $ git commit --amend -a\n    $ git diff HEAD@{1} >P.diff\n    $ git checkout next\n    $ git apply --index P.diff\n    $ git commit -m 'typofix'\n\nAfter this, while rebuilding 'jch' branch the next time, Reintegrate\nscript will notice that the bw/diff-color-hunk-header topic has been\nrebased.  I can simply edit that note out in the /var/tmp/redo-jch.sh\nscript and rebuild 'jch' branch---the result will exactly match 'next'.\n\nThe end result is that the commit merged in 'next' is not the tip of\nbw/diff-color-hunk-header anymore, but merging the _current_ tip of that\nbranch together with all the other topics on top of 'master' would produce\nthe desired result without \"oops---that was a stupid typo\" fixups.\n"},{"id":"128755","messageId":"fabb9a1e0911300126t48d767b9kbb31e9ac8e4a5d8e@mail.gmail.com","threadId":"21658","inReplyTo":"7vvdgstmic.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3] Give the hunk comment its own color","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-11-30T09:26:22Z","receivedAt":"2009-11-30T09:26:22Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Nov 30, 2009 at 10:00, Junio C Hamano <gitster@pobox.com> wrote:\n> I maintain a private 'jch' branch that merges everything that has been\n> merged so far to 'next' and the branch always builds on top of 'master'\n> whenever 'pu' is pushed out.  The tree object recorded by the tip of 'jch'\n> is designed to always match that of 'next'.  And 'pu' is built on top of\n> 'jch', instead of 'next', these days.\n\nAh, so _that''s_ how you pull of not rebasing and still maintaining a\nclean history in master: keeping a private shadow branch with the\ncleaned up history that is tree-identical to the no-rebase next\nbranch. Interesting.\n\n\n> The end result is that the commit merged in 'next' is not the tip of\n> bw/diff-color-hunk-header anymore, but merging the _current_ tip of that\n> branch together with all the other topics on top of 'master' would produce\n> the desired result without \"oops---that was a stupid typo\" fixups.\n\nThanks for explaining, git really does allow for a lot of interesting\nworkflows :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"}]}