{"thread":{"id":"30556","subject":"Add comment lines to patch format","startedAt":"2012-05-18T13:22:28Z","lastAt":"2012-05-19T04:54:51Z","messageCount":5,"participants":["Nguyen Thai Ngoc Duy","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"191661","messageId":"20120518132228.GA27970@do","threadId":"30556","inReplyTo":null,"subject":"Add comment lines to patch format","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-18T13:22:28Z","receivedAt":"2012-05-18T13:22:28Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Hi,\n\nThis is just an idea to help spot subtle changes in patch form\n(e.g. visible over email). A line starting with '=' (or something\nelse) is treated like a comment line and ignored by \"git apply\". We\ncould use this line to mark special things in the previous line.\n\nOne thing is trailing space, like demonstrated in the patch below,\nbecause trailing spaces may be intentional sometimes. But I'd like to\nincorporate some word-diff goodness in patch format using this comment\nline to spot few/single character addition/removal.\n\nFor example, instead of showing\n\n- \"this is a string\"\n+ N_(\"this is a string\")\n\nwe could show\n\n- \"this is a string\"\n= ..... .. . ^.......\n+ N_(\"this is a string\")\n= ^^^..... .. . .......^\n\nIf anyone knows a tool with similar feature, I'd greatly appreciate it\n(as the Internet taught me, everything I think of is already thought\nof/implemented by someone)\n\n-- 8< --\ndiff --git a/diff.c b/diff.c\nindex 77edd50..09d8c19 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1110,6 +1110,30 @@ static void find_lno(const char *line, struct emit_callback *ecbdata)\n \tecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);\n }\n \n+static void annotate_line(struct emit_callback *ecbdata,\n+\t\t\t const char *line, unsigned long len)\n+{\n+\tchar *buf, *s;\n+\tint trailing = 1;\n+\tif (!isspace(line[len-2])) \n=\t.. .......................^^^\n+\t\treturn;\n+\tbuf = malloc(len);\n+\tmemcpy(buf, line, len);\n+\tbuf[0] = '=';\n+\tfor (s = buf + len - 2; s > buf; s--) {\n+\t\tif (trailing && isspace(*s))\n+\t\t\t*s = '^';\n+\t\telse\n+\t\t\ttrailing = 0;\n+\t\tif (!trailing && !isspace(*s))\n+\t\t\t*s = '.';\n+\t}\n+\temit_line(ecbdata->opt,\n+\t\t diff_get_color(ecbdata->color_diff, DIFF_PLAIN),\n+\t\t diff_get_color(ecbdata->color_diff, DIFF_RESET), buf, len);\n+\tfree(buf);\n+}\n+\n static void fn_out_consume(void *priv, char *line, unsigned long len)\n {\n \tstruct emit_callback *ecbdata = priv;\n@@ -1208,17 +1232,26 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\treturn;\n \t}\n \n-\tif (line[0] != '+') {\n-\t\tconst char *color =\n-\t\t\tdiff_get_color(ecbdata->color_diff,\n-\t\t\t\t line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);\n+\tswitch (line[0]) {\n+\tcase '-':\n \t\tecbdata->lno_in_preimage++;\n-\t\tif (line[0] == ' ')\n-\t\t\tecbdata->lno_in_postimage++;\n-\t\temit_line(ecbdata->opt, color, reset, line, len);\n-\t} else {\n+\t\temit_line(ecbdata->opt,\n+\t\t\t diff_get_color(ecbdata->color_diff, DIFF_FILE_OLD),\n+\t\t\t reset, line, len);\n+\t\tannotate_line(ecbdata, line, len);\n+\t\tbreak;\n+\tcase '+':\n \t\tecbdata->lno_in_postimage++;\n \t\temit_add_line(reset, ecbdata, line + 1, len - 1);\n+\t\tannotate_line(ecbdata, line, len);\n+\t\tbreak;\n+\tcase ' ':\n+\t\tecbdata->lno_in_preimage++;\n+\t\t\tecbdata->lno_in_postimage++;\n+\t\temit_line(ecbdata->opt, plain, reset, line, len);\n+\t\tbreak;\n+\tdefault:\n+\t\tdie(\"huh? a diff line starting with '%c'??\", line[0]);\n \t}\n }\n \n-- 8< --\n-- \nDuy\n\n-- \nDuy\n"},{"id":"191665","messageId":"xmqq62bt4ida.fsf@junio.mtv.corp.google.com","threadId":"30556","inReplyTo":"20120518132228.GA27970@do","subject":"Re: Add comment lines to patch format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-18T15:30:09Z","receivedAt":"2012-05-18T15:30:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> If anyone knows a tool with similar feature, I'd greatly appreciate it\n> (as the Internet taught me, everything I think of is already thought\n> of/implemented by someone)\n\nLook in the archive and I think you will find a patch by me that\nimplemented this not with '=' but with some other character, possibly\nwith a matching patch to 'apply' without which its output is not usable.\nIt was back when I was still a contributor, I think, so I do not\nremember the details.\n"},{"id":"191704","messageId":"20120519001410.GC765@sigill.intra.peff.net","threadId":"30556","inReplyTo":"20120518132228.GA27970@do","subject":"Re: Add comment lines to patch format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-19T00:14:10Z","receivedAt":"2012-05-19T00:14:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 18, 2012 at 08:22:28PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> One thing is trailing space, like demonstrated in the patch below,\n> because trailing spaces may be intentional sometimes. But I'd like to\n> incorporate some word-diff goodness in patch format using this comment\n> line to spot few/single character addition/removal.\n> [...]\n> If anyone knows a tool with similar feature, I'd greatly appreciate it\n> (as the Internet taught me, everything I think of is already thought\n> of/implemented by someone)\n\nHave you looked at contrib/diff-highlight?\n\nI think your approach is interesting because it can annotate much more\nthan just \"this part is interesting\". Sometimes when I send a patch, I'd\nlike to be able to comment in-line, like:\n\n + if (bar > 3)\n + foo(bar);\n = Yeah, this magic \"3\" is ugly, and we should handle it\n = through $whatever in the re-roll.\n + else\n + something_else();\n\nI usually just write something in the cover letter, or reply to myself\nwith comments inline. I don't know how much I would use it in practice,\nbut it might be a neat thing for \"git apply\" to simply ignore comment\nlines in the middle of a hunk.\n\nOf course, this feature might not be worth breaking compatibility with\nevery existing version of \"git apply\" and \"patch\" out there.\n\n-Peff\n"},{"id":"191718","messageId":"CACsJy8BMKwyMhREe-VyKNu-G9V-hMhMjWdty3QHHppY329vTbA@mail.gmail.com","threadId":"30556","inReplyTo":"xmqq62bt4ida.fsf@junio.mtv.corp.google.com","subject":"Re: Add comment lines to patch format","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-19T04:50:03Z","receivedAt":"2012-05-19T04:50:03Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, May 18, 2012 at 10:30 PM, Junio C Hamano wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n>> If anyone knows a tool with similar feature, I'd greatly appreciate it\n>> (as the Internet taught me, everything I think of is already thought\n>> of/implemented by someone)\n>\n> Look in the archive and I think you will find a patch by me that\n> implemented this not with '=' but with some other character, possibly\n> with a matching patch to 'apply' without which its output is not usable.\n> It was back when I was still a contributor, I think, so I do not\n> remember the details.\n\nNothing stands out from gmane interface up until you became\nmaintainer. But I might overlook because you exceeded 1000 mails just\nin three months. That might be Linus' secret criteria for a new\nmaintainer: pick the first one that passes 1000 mails limit ;)\n\nAnyway don't waste your time looking back in your mail archive.\n-- \nDuy\n"},{"id":"191719","messageId":"CACsJy8C-+VwpR4XEBqxhpwhtfZ=cxo6uKSp6Ov7K6aLGxqR=Sw@mail.gmail.com","threadId":"30556","inReplyTo":"20120519001410.GC765@sigill.intra.peff.net","subject":"Re: Add comment lines to patch format","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-19T04:54:51Z","receivedAt":"2012-05-19T04:54:51Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 19, 2012 at 7:14 AM, Jeff King wrote:\n> On Fri, May 18, 2012 at 08:22:28PM +0700, Nguyen Thai Ngoc Duy wrote:\n>\n>> One thing is trailing space, like demonstrated in the patch below,\n>> because trailing spaces may be intentional sometimes. But I'd like to\n>> incorporate some word-diff goodness in patch format using this comment\n>> line to spot few/single character addition/removal.\n>> [...]\n>> If anyone knows a tool with similar feature, I'd greatly appreciate it\n>> (as the Internet taught me, everything I think of is already thought\n>> of/implemented by someone)\n>\n> Have you looked at contrib/diff-highlight?\n\nI didn't. Its output looks too packed though.\n\n> I think your approach is interesting because it can annotate much more\n> than just \"this part is interesting\". Sometimes when I send a patch, I'd\n> like to be able to comment in-line, like:\n>\n>  +       if (bar > 3)\n>  +               foo(bar);\n>  = Yeah, this magic \"3\" is ugly, and we should handle it\n>  = through $whatever in the re-roll.\n>  +       else\n>  +               something_else();\n>\n> I usually just write something in the cover letter, or reply to myself\n> with comments inline. I don't know how much I would use it in practice,\n> but it might be a neat thing for \"git apply\" to simply ignore comment\n> lines in the middle of a hunk.\n\nYeah. That's a good thing about in-patch comments.\n\nOne thing that's still bugging me is that we don't have a way to send\nmachine-readable conflict resolutions in text form. But that's\nprobably something else.\n\n> Of course, this feature might not be worth breaking compatibility with\n> every existing version of \"git apply\" and \"patch\" out there.\n\nIt's opt-in. And it's quite easy to convert back. A simple \"sed -i\n'/^=/d'\" would do.\n-- \nDuy\n"}]}