{"thread":{"id":"23688","subject":"[PATCH] diff: fix coloring of extended diff headers","startedAt":"2010-05-03T22:38:07Z","lastAt":"2010-05-09T18:43:07Z","messageCount":2,"participants":["Bert Wesarg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"140858","messageId":"591c96c90ca049400a8c0df70f8b44061aeacba5.1272926169.git.bert.wesarg@googlemail.com","threadId":"23688","inReplyTo":null,"subject":"[PATCH] diff: fix coloring of extended diff headers","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-05-03T22:38:07Z","receivedAt":"2010-05-03T22:38:07Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Coloring the extended headers where done as a whole not per line. less with\noption -R (which is the default from git) does not support this coloring\nmode because of performance reasons. The -r option would be an alternative\nbut has problems with lines that are longer than the screen. Therefore\nstick to the idiom to color each line separately. The problem is, that the\nresult of ill_metainfo() will also be used as an parameter to an external\ndiff driver, so we need to disable coloring in this case.\n\nBecause coloring is now done inside fill_metainfo() we can simply add this\nstring to the diff header and therefore keep the last newline in the\nextended header. This results also into the fact that the external diff\ndriver now gets this last newline too. Which is a change in behavior\nbut a good one.\n---\n diff.c |   61 +++++++++++++++++++++++++++++++++++--------------------------\n 1 files changed, 35 insertions(+), 26 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d0ecbc3..7e508dd 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1650,21 +1650,21 @@ static void builtin_diff(const char *name_a,\n \tif (lbl[0][0] == '/') {\n \t\t/* /dev/null */\n \t\tstrbuf_addf(&header, \"%snew file mode %06o%s\\n\", set, two->mode, reset);\n-\t\tif (xfrm_msg && xfrm_msg[0])\n-\t\t\tstrbuf_addf(&header, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\t\tif (xfrm_msg)\n+\t\t\tstrbuf_addstr(&header, xfrm_msg);\n \t}\n \telse if (lbl[1][0] == '/') {\n \t\tstrbuf_addf(&header, \"%sdeleted file mode %06o%s\\n\", set, one->mode, reset);\n-\t\tif (xfrm_msg && xfrm_msg[0])\n-\t\t\tstrbuf_addf(&header, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\t\tif (xfrm_msg)\n+\t\t\tstrbuf_addstr(&header, xfrm_msg);\n \t}\n \telse {\n \t\tif (one->mode != two->mode) {\n \t\t\tstrbuf_addf(&header, \"%sold mode %06o%s\\n\", set, one->mode, reset);\n \t\t\tstrbuf_addf(&header, \"%snew mode %06o%s\\n\", set, two->mode, reset);\n \t\t}\n-\t\tif (xfrm_msg && xfrm_msg[0])\n-\t\t\tstrbuf_addf(&header, \"%s%s%s\\n\", set, xfrm_msg, reset);\n+\t\tif (xfrm_msg)\n+\t\t\tstrbuf_addstr(&header, xfrm_msg);\n \n \t\t/*\n \t\t * we do not run diff between different kind\n@@ -2323,30 +2323,36 @@ static void fill_metainfo(struct strbuf *msg,\n \t\t\t  struct diff_filespec *one,\n \t\t\t  struct diff_filespec *two,\n \t\t\t  struct diff_options *o,\n-\t\t\t  struct diff_filepair *p)\n+\t\t\t  struct diff_filepair *p,\n+\t\t\t  int use_color)\n {\n+\tconst char *set = diff_get_color(use_color, DIFF_METAINFO);\n+\tconst char *reset = diff_get_color(use_color, DIFF_RESET);\n+\n \tstrbuf_init(msg, PATH_MAX * 2 + 300);\n \tswitch (p->status) {\n \tcase DIFF_STATUS_COPIED:\n-\t\tstrbuf_addf(msg, \"similarity index %d%%\", similarity_index(p));\n-\t\tstrbuf_addstr(msg, \"\\ncopy from \");\n+\t\tstrbuf_addf(msg, \"%ssimilarity index %d%%\",\n+\t\t\t    set, similarity_index(p));\n+\t\tstrbuf_addf(msg, \"%s\\n%scopy from \", reset, set);\n \t\tquote_c_style(name, msg, NULL, 0);\n-\t\tstrbuf_addstr(msg, \"\\ncopy to \");\n+\t\tstrbuf_addf(msg, \"%s\\n%scopy to \", reset, set);\n \t\tquote_c_style(other, msg, NULL, 0);\n-\t\tstrbuf_addch(msg, '\\n');\n+\t\tstrbuf_addf(msg, \"%s\\n\", reset);\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n-\t\tstrbuf_addf(msg, \"similarity index %d%%\", similarity_index(p));\n-\t\tstrbuf_addstr(msg, \"\\nrename from \");\n+\t\tstrbuf_addf(msg, \"%ssimilarity index %d%%\",\n+\t\t\t    set, similarity_index(p));\n+\t\tstrbuf_addf(msg, \"%s\\n%srename from \", reset, set);\n \t\tquote_c_style(name, msg, NULL, 0);\n-\t\tstrbuf_addstr(msg, \"\\nrename to \");\n+\t\tstrbuf_addf(msg, \"%s\\n%srename to \", reset, set);\n \t\tquote_c_style(other, msg, NULL, 0);\n-\t\tstrbuf_addch(msg, '\\n');\n+\t\tstrbuf_addf(msg, \"%s\\n\", reset);\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n \t\tif (p->score) {\n-\t\t\tstrbuf_addf(msg, \"dissimilarity index %d%%\\n\",\n-\t\t\t\t    similarity_index(p));\n+\t\t\tstrbuf_addf(msg, \"%sdissimilarity index %d%%%s\\n\",\n+\t\t\t\t    set, similarity_index(p), reset);\n \t\t\tbreak;\n \t\t}\n \t\t/* fallthru */\n@@ -2363,15 +2369,13 @@ static void fill_metainfo(struct strbuf *msg,\n \t\t\t    (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))\n \t\t\t\tabbrev = 40;\n \t\t}\n-\t\tstrbuf_addf(msg, \"index %.*s..%.*s\",\n+\t\tstrbuf_addf(msg, \"%sindex %.*s..%.*s\", set,\n \t\t\t    abbrev, sha1_to_hex(one->sha1),\n \t\t\t    abbrev, sha1_to_hex(two->sha1));\n \t\tif (one->mode == two->mode)\n \t\t\tstrbuf_addf(msg, \" %06o\", one->mode);\n-\t\tstrbuf_addch(msg, '\\n');\n+\t\tstrbuf_addf(msg, \"%s\\n\", reset);\n \t}\n-\tif (msg->len)\n-\t\tstrbuf_setlen(msg, msg->len - 1);\n }\n \n static void run_diff_cmd(const char *pgm,\n@@ -2387,11 +2391,6 @@ static void run_diff_cmd(const char *pgm,\n \tconst char *xfrm_msg = NULL;\n \tint complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;\n \n-\tif (msg) {\n-\t\tfill_metainfo(msg, name, other, one, two, o, p);\n-\t\txfrm_msg = msg->len ? msg->buf : NULL;\n-\t}\n-\n \tif (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))\n \t\tpgm = NULL;\n \telse {\n@@ -2400,6 +2399,16 @@ static void run_diff_cmd(const char *pgm,\n \t\t\tpgm = drv->external;\n \t}\n \n+\tif (msg) {\n+\t\t/*\n+\t\t * don't use colors when the header is intended for an\n+\t\t * external diff driver\n+\t\t */\n+\t\tfill_metainfo(msg, name, other, one, two, o, p,\n+\t\t\t      DIFF_OPT_TST(o, COLOR_DIFF) && !pgm);\n+\t\txfrm_msg = msg->len ? msg->buf : NULL;\n+\t}\n+\n \tif (pgm) {\n \t\trun_external_diff(pgm, name, other, one, two, xfrm_msg,\n \t\t\t\t  complete_rewrite);\n-- \n1.7.1.rc2.620.g49a9d\n"},{"id":"141325","messageId":"AANLkTilFfkadPZd56OSGr1yDLVeSQ_G2jOxga1dK5NBD@mail.gmail.com","threadId":"23688","inReplyTo":"591c96c90ca049400a8c0df70f8b44061aeacba5.1272926169.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH] diff: fix coloring of extended diff headers","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-05-09T18:43:07Z","receivedAt":"2010-05-09T18:43:07Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, May 4, 2010 at 00:38, Bert Wesarg <bert.wesarg@googlemail.com> wrote:\n> Coloring the extended headers where done as a whole not per line. less with\n> option -R (which is the default from git) does not support this coloring\n> mode because of performance reasons. The -r option would be an alternative\n> but has problems with lines that are longer than the screen. Therefore\n> stick to the idiom to color each line separately. The problem is, that the\n> result of ill_metainfo() will also be used as an parameter to an external\n> diff driver, so we need to disable coloring in this case.\n>\n> Because coloring is now done inside fill_metainfo() we can simply add this\n> string to the diff header and therefore keep the last newline in the\n> extended header. This results also into the fact that the external diff\n> driver now gets this last newline too. Which is a change in behavior\n> but a good one.\n\nPing.\n\nRegards,\nBert\n"}]}