{"thread":{"id":"17058","subject":"[RFC PATCH] make diff --color-words customizable","startedAt":"2009-01-09T00:05:05Z","lastAt":"2009-01-13T18:50:29Z","messageCount":34,"participants":["Thomas Rast","Johannes Schindelin","Jeff King","Teemu Likonen","Jakub Narebski","Davide Libenzi","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"99750","messageId":"1231459505-14395-1-git-send-email-trast@student.ethz.ch","threadId":"17058","inReplyTo":null,"subject":"[RFC PATCH] make diff --color-words customizable","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-09T00:05:05Z","receivedAt":"2009-01-09T00:05:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Allows for user-configurable word splits when using --color-words.\nThis can make the diff more readable if the regex is configured\naccording to the language of the file.\n\nFor now the (POSIX extended) regex must be set via the environment\nGIT_DIFF_WORDS_REGEX. Each (non-overlapping) match of the regex is\nconsidered a word. Anything characters not matched are considered\nwhitespace. For example, for C try\n\n GIT_DIFF_WORDS_REGEX='[0-9]+|[a-zA-Z_][a-zA-Z0-9_]*|(\\+|-|&|\\|){1,2}|\\S'\n\nand for TeX try\n\n GIT_DIFF_WORDS_REGEX='\\\\[a-zA-Z@]+ *|\\{|\\}|\\\\.|[^\\{} [:space:]]+'\n\nSigned-off-by: Thomas Rast \n\n---\n\nWord diff becomes much more useful especially with TeX, where it is\ncommon to run together \\sequences\\of\\commands\\like\\this that the\ncurrent --color-words treats as a single word.\n\nApart from possible bugs, the main issue is: where should I put the\nconfiguration for this?\n\n\n diff.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 files changed, 127 insertions(+), 15 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d235482..c1e24de 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -321,6 +321,7 @@ struct diff_words_buffer {\n \tlong alloc;\n \tlong current; /* output pointer */\n \tint suppressed_newline;\n+\tenum diff_word_boundaries *boundaries;\n };\n \n static void diff_words_append(char *line, unsigned long len,\n@@ -336,21 +337,35 @@ static void diff_words_append(char *line, unsigned long len,\n \tbuffer->text.size += len;\n }\n \n+enum diff_word_boundaries {\n+\tDIFF_WORD_CONT,\n+\tDIFF_WORD_START,\n+\tDIFF_WORD_SPACE\n+};\n+\n+\n struct diff_words_data {\n \tstruct diff_words_buffer minus, plus;\n \tFILE *file;\n+\tenum diff_word_boundaries *minus_boundaries, *plus_boundaries;\n };\n \n-static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n+static int print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n \t\tint suppress_newline)\n {\n \tconst char *ptr;\n \tint eol = 0;\n \n \tif (len == 0)\n-\t\treturn;\n+\t\treturn len;\n \n \tptr = buffer->text.ptr + buffer->current;\n+\n+\tif (buffer->boundaries[buffer->current+len-1] == DIFF_WORD_START) {\n+\t\tbuffer->boundaries[buffer->current+len-1] = DIFF_WORD_CONT;\n+\t\tlen--;\n+\t}\n+\n \tbuffer->current += len;\n \n \tif (ptr[len - 1] == '\\n') {\n@@ -368,6 +383,8 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n \t\telse\n \t\t\tputc('\\n', file);\n \t}\n+\n+\treturn len;\n }\n \n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n@@ -391,13 +408,79 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\t\t &diff_words->plus, len, DIFF_FILE_NEW, 0);\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tprint_word(diff_words->file,\n-\t\t\t\t &diff_words->plus, len, DIFF_PLAIN, 0);\n+\t\t\tlen = print_word(diff_words->file,\n+\t\t\t\t\t &diff_words->plus, len, DIFF_PLAIN, 0);\n \t\t\tdiff_words->minus.current += len;\n \t\t\tbreak;\n \t}\n }\n \n+static char *worddiff_default = \"\\\\S+\";\n+static regex_t worddiff_regex;\n+static int worddiff_regex_compiled = 0;\n+\n+static int scan_word_boundaries(struct diff_words_buffer *buf)\n+{\n+\tenum diff_word_boundaries *boundaries = buf->boundaries;\n+\tchar *text = buf->text.ptr;\n+\tint len = buf->text.size;\n+\n+\tint i = 0;\n+\tint count = 0;\n+\tint ret;\n+\tregmatch_t matches[1];\n+\tint offset, wordlen;\n+\tchar *strz;\n+\n+\tif (!text)\n+\t\treturn 0;\n+\n+\tif (!worddiff_regex_compiled) {\n+\t\tchar *wd_pat = getenv(\"GIT_DIFF_WORDS_REGEX\");\n+\t\tif (!wd_pat)\n+\t\t\twd_pat = worddiff_default;\n+\t\tret = regcomp(&worddiff_regex, wd_pat, REG_EXTENDED);\n+\t\tif (ret) {\n+\t\t\tchar errbuf[1024];\n+\t\t\tregerror(ret, &worddiff_regex, errbuf, 1024);\n+\t\t\tdie(\"word diff regex failed to compile: '%s': %s\",\n+\t\t\t wd_pat, errbuf);\n+\t\t}\n+\t\tworddiff_regex_compiled = 1;\n+\t}\n+\n+\tstrz = xmalloc(len+1);\n+\tmemcpy(strz, text, len);\n+\tstrz[len] = '\\0';\n+\n+\twhile (i < len) {\n+\t\tret = regexec(&worddiff_regex, strz+i, 1, matches, 0);\n+\t\tif (ret == REG_NOMATCH) {\n+\t\t\t/* the rest is whitespace */\n+\t\t\twhile (i < len)\n+\t\t\t\tboundaries[i++] = DIFF_WORD_SPACE;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\toffset = matches[0].rm_so;\n+\t\twhile (offset-- > 0 && i < len)\n+\t\t\tboundaries[i++] = DIFF_WORD_SPACE;\n+\n+\t\twordlen = matches[0].rm_eo - matches[0].rm_so;\n+\t\tif (wordlen-- > 0 && i < len) {\n+\t\t\tboundaries[i++] = DIFF_WORD_START;\n+\t\t\tcount++;\n+\t\t}\n+\t\twhile (wordlen-- > 0 && i < len)\n+\t\t\tboundaries[i++] = DIFF_WORD_CONT;\n+\t}\n+\n+\tfree(strz);\n+\n+\treturn count;\n+}\n+\n+\n /* this executes the word diff on the accumulated buffers */\n static void diff_words_show(struct diff_words_data *diff_words)\n {\n@@ -406,23 +489,50 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \txdemitcb_t ecb;\n \tmmfile_t minus, plus;\n \tint i;\n+\tchar *p;\n+\tint bcount;\n \n \tmemset(&xpp, 0, sizeof(xpp));\n \tmemset(&xecfg, 0, sizeof(xecfg));\n-\tminus.size = diff_words->minus.text.size;\n-\tminus.ptr = xmalloc(minus.size);\n-\tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n-\tfor (i = 0; i < minus.size; i++)\n-\t\tif (isspace(minus.ptr[i]))\n-\t\t\tminus.ptr[i] = '\\n';\n+\n+\tdiff_words->minus.boundaries = xmalloc(diff_words->minus.text.size * sizeof(enum diff_word_boundaries));\n+\tbcount = scan_word_boundaries(&diff_words->minus);\n+\tminus.size = diff_words->minus.text.size + bcount;\n+\tminus.ptr = xmalloc(minus.size + bcount);\n+\tp = minus.ptr;\n+\tfor (i = 0; i < diff_words->minus.text.size; i++) {\n+\t\tswitch (diff_words->minus.boundaries[i]) {\n+\t\tcase DIFF_WORD_START:\n+\t\t\t*p++ = '\\n';\n+\t\t\t/* fall through */\n+\t\tcase DIFF_WORD_CONT:\n+\t\t\t*p++ = diff_words->minus.text.ptr[i];\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_SPACE:\n+\t\t\t*p++ = '\\n';\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \tdiff_words->minus.current = 0;\n \n-\tplus.size = diff_words->plus.text.size;\n+\tdiff_words->plus.boundaries = xmalloc(diff_words->plus.text.size * sizeof(enum diff_word_boundaries));\n+\tbcount = scan_word_boundaries(&diff_words->plus);\n+\tplus.size = diff_words->plus.text.size + bcount;\n \tplus.ptr = xmalloc(plus.size);\n-\tmemcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);\n-\tfor (i = 0; i < plus.size; i++)\n-\t\tif (isspace(plus.ptr[i]))\n-\t\t\tplus.ptr[i] = '\\n';\n+\tp = plus.ptr;\n+\tfor (i = 0; i < diff_words->plus.text.size; i++) {\n+\t\tswitch (diff_words->plus.boundaries[i]) {\n+\t\tcase DIFF_WORD_START:\n+\t\t\t*p++ = '\\n';\n+\t\t\t/* fall through */\n+\t\tcase DIFF_WORD_CONT:\n+\t\t\t*p++ = diff_words->plus.text.ptr[i];\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_SPACE:\n+\t\t\t*p++ = '\\n';\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \tdiff_words->plus.current = 0;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n@@ -432,6 +542,8 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n+\tfree(diff_words->minus.boundaries);\n+\tfree(diff_words->plus.boundaries);\n \n \tif (diff_words->minus.suppressed_newline) {\n \t\tputc('\\n', diff_words->file);\n-- \ntg: (c123b7c..) t/word-diff-regex (depends on: origin/master)\n"},{"id":"99755","messageId":"alpine.DEB.1.00.0901090121432.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"1231459505-14395-1-git-send-email-trast@student.ethz.ch","subject":"Re: [RFC PATCH] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-09T00:25:16Z","receivedAt":"2009-01-09T00:25:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Jan 2009, Thomas Rast wrote:\n\n> Allows for user-configurable word splits when using --color-words. This \n> can make the diff more readable if the regex is configured according to \n> the language of the file.\n> \n> For now the (POSIX extended) regex must be set via the environment\n> GIT_DIFF_WORDS_REGEX. Each (non-overlapping) match of the regex is\n> considered a word. Anything characters not matched are considered\n> whitespace. For example, for C try\n> \n> GIT_DIFF_WORDS_REGEX='[0-9]+|[a-zA-Z_][a-zA-Z0-9_]*|(\\+|-|&|\\|){1,2}|\\S'\n> \n> and for TeX try\n> \n> GIT_DIFF_WORDS_REGEX='\\\\[a-zA-Z@]+ *|\\{|\\}|\\\\.|[^\\{} [:space:]]+'\n\nInteresting idea. However, I think it would be better to do the opposite, \nhave _word_ patterns. And even better to have _one_ pattern.\n\nThen we could have a --color-words-regex= option.\n\nBTW I think you could do what you intended to do with a _way_ smaller \nand more intuitive patch.\n\nCiao,\nDscho\n"},{"id":"99758","messageId":"200901090151.10880.trast@student.ethz.ch","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901090121432.30769@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH] make diff --color-words customizable","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-09T00:50:50Z","receivedAt":"2009-01-09T00:50:50Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> On Fri, 9 Jan 2009, Thomas Rast wrote:\n> \n> > Allows for user-configurable word splits when using --color-words. This \n> > can make the diff more readable if the regex is configured according to \n> > the language of the file.\n> > \n> > For now the (POSIX extended) regex must be set via the environment\n> > GIT_DIFF_WORDS_REGEX. Each (non-overlapping) match of the regex is\n> > considered a word. Anything characters not matched are considered\n> > whitespace. For example, for C try\n> > \n> > GIT_DIFF_WORDS_REGEX='[0-9]+|[a-zA-Z_][a-zA-Z0-9_]*|(\\+|-|&|\\|){1,2}|\\S'\n[...]\n> Interesting idea. However, I think it would be better to do the opposite, \n> have _word_ patterns. And even better to have _one_ pattern.\n\nI'm not sure I understand. It _is_ a single pattern. The examples\njust have several cases to distinguish various semantic groups that\ncan occur, as a sort of \"half tokenizer\". (The C example isn't very\ncomplete however.)\n\n> BTW I think you could do what you intended to do with a _way_ smaller \n> and more intuitive patch.\n\nHow?\n\nI don't think the existing mechanism, which just replaces all\nwhitespace with newlines and does a line diff to find out which words\nchanged, can \"just\" be adapted. We will have to insert extra newlines\nat points where the regex said to split a word, but where there was no\nwhitespace in the original content. If there's a significantly easier\nway to do that than I hacked up, please share.\n\nOr maybe I got your original code all wrong?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n\n"},{"id":"99776","messageId":"20090109095300.GA4099@coredump.intra.peff.net","threadId":"17058","inReplyTo":"1231459505-14395-1-git-send-email-trast@student.ethz.ch","subject":"Re: [RFC PATCH] make diff --color-words customizable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-09T09:53:00Z","receivedAt":"2009-01-09T09:53:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 09, 2009 at 01:05:05AM +0100, Thomas Rast wrote:\n\n> Word diff becomes much more useful especially with TeX, where it is\n> common to run together \\sequences\\of\\commands\\like\\this that the\n> current --color-words treats as a single word.\n\nI have run into this, as well, and it would be nice to have configurable\nword boundaries.\n\n> Apart from possible bugs, the main issue is: where should I put the\n> configuration for this?\n\nIt's a per-file thing, so probably in the diff driver that is triggered\nvia attributes. See userdiff.[ch]; you'll need to add an entry to the\nuserdiff_driver struct. You can look at the funcname pattern stuff as a\ntemplate, as this is very similar.\n\n-Peff\n"},{"id":"99784","messageId":"alpine.DEB.1.00.0901091202250.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"200901090151.10880.trast@student.ethz.ch","subject":"Re: [RFC PATCH] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-09T11:15:43Z","receivedAt":"2009-01-09T11:15:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Jan 2009, Thomas Rast wrote:\n\n> Johannes Schindelin wrote:\n> > On Fri, 9 Jan 2009, Thomas Rast wrote:\n> > \n> > > Allows for user-configurable word splits when using --color-words. This \n> > > can make the diff more readable if the regex is configured according to \n> > > the language of the file.\n> > > \n> > > For now the (POSIX extended) regex must be set via the environment\n> > > GIT_DIFF_WORDS_REGEX. Each (non-overlapping) match of the regex is\n> > > considered a word. Anything characters not matched are considered\n> > > whitespace. For example, for C try\n> > > \n> > > GIT_DIFF_WORDS_REGEX='[0-9]+|[a-zA-Z_][a-zA-Z0-9_]*|(\\+|-|&|\\|){1,2}|\\S'\n> [...]\n> > Interesting idea. However, I think it would be better to do the opposite, \n> > have _word_ patterns. And even better to have _one_ pattern.\n> \n> I'm not sure I understand. It _is_ a single pattern. The examples\n> just have several cases to distinguish various semantic groups that\n> can occur, as a sort of \"half tokenizer\". (The C example isn't very\n> complete however.)\n\nOh, I was fooled by your use of an array of enums whose purpose I did not \nunderstand at all.\n\n> > BTW I think you could do what you intended to do with a _way_ smaller \n> > and more intuitive patch.\n> \n> How?\n\nIntuitively, all you would have to do is to replace this part in \ndiff_words_show()\n\n for (i = 0; i < minus.size; i++)\n if (isspace(minus.ptr[i]))\n minus.ptr[i] = '\\n';\n\nby a loop finding the next word boundary. I would suggest making that a \nfunction, say,\n\n\tint find_word_boundary(struct diff_words_data *data, char *minus);\n\nThis function would also be responsible to initialize the regexp.\n\nHowever, as I said, I think it would be much more intuitive to \ncharacterize the _words_ instead of the _word boundaries_.\n\nAnd I would like to keep the default as-is (together _with_ the \nperformance. IOW if the user did not specify a regexp, it should fall \nback to what it does now, which is slow enough).\n\nCiao,\nDscho\n"},{"id":"99785","messageId":"alpine.DEB.1.00.0901091215590.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"20090109095300.GA4099@coredump.intra.peff.net","subject":"Re: [RFC PATCH] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-09T11:18:37Z","receivedAt":"2009-01-09T11:18:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 9 Jan 2009, Jeff King wrote:\n\n> On Fri, Jan 09, 2009 at 01:05:05AM +0100, Thomas Rast wrote:\n> \n> > Apart from possible bugs, the main issue is: where should I put the \n> > configuration for this?\n> \n> It's a per-file thing, so probably in the diff driver that is triggered \n> via attributes. See userdiff.[ch]; you'll need to add an entry to the \n> userdiff_driver struct. You can look at the funcname pattern stuff as a \n> template, as this is very similar.\n\nI am not sure I would want that in the config or the attributes. For me, \nit always has been a question of \"oh, that LaTeX diff looks ugly, let's \nsee what words actually changed\".\n\nOnly rarely did I wish for a different word boundary detection algorithm.\n\nSo I'd rather have an alias than a config/attribute setting.\n\nCiao,\nDscho\n"},{"id":"99786","messageId":"20090109112239.GA11466@coredump.intra.peff.net","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901091215590.30769@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH] make diff --color-words customizable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-09T11:22:39Z","receivedAt":"2009-01-09T11:22:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 09, 2009 at 12:18:37PM +0100, Johannes Schindelin wrote:\n\n> > It's a per-file thing, so probably in the diff driver that is triggered \n> > via attributes. See userdiff.[ch]; you'll need to add an entry to the \n> > userdiff_driver struct. You can look at the funcname pattern stuff as a \n> > template, as this is very similar.\n> \n> I am not sure I would want that in the config or the attributes. For me, \n> it always has been a question of \"oh, that LaTeX diff looks ugly, let's \n> see what words actually changed\".\n> \n> Only rarely did I wish for a different word boundary detection algorithm.\n> \n> So I'd rather have an alias than a config/attribute setting.\n\nI am not sure what you are saying.\n\nIf it is \"I do not want color-words on by default for LaTeX\", then I\nagree. I meant merely that _if_ color-words is enabled, the word\nboundaries would be taken from the diff driver config (just like we do\nfor matching the funcname header).\n\nIf it is \"I want to specify the color-words boundary on a per-run basis\nrather than a per-file basis\", then I want the opposite. However, there\nis no reason that both cannot be supported (with command line or\nenvironment taking precedence over what's in the config).\n\n-Peff\n"},{"id":"99787","messageId":"alpine.DEB.1.00.0901091255230.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901091202250.30769@pacific.mpi-cbg.de","subject":"[ILLUSTRATION PATCH] color-words: take an optional regular expression describing words","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-09T11:59:03Z","receivedAt":"2009-01-09T11:59:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIn some applications, words are not delimited by white space. To\nallow for that, you can specify a regular expression describing\nwhat makes a word with\n\n\tgit diff --color-words='^[A-Za-z0-9]*'\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tOn Fri, 9 Jan 2009, Johannes Schindelin wrote:\n\n\t> Intuitively, all you would have to do is to replace this part in \n\t> diff_words_show()\n\t> \n\t> for (i = 0; i < minus.size; i++)\n\t> if (isspace(minus.ptr[i]))\n\t> minus.ptr[i] = '\\n';\n\t> \n\t> by a loop finding the next word boundary. I would suggest making that a \n\t> function, say,\n\t> \n\t> \tint find_word_boundary(struct diff_words_data *data, char *minus);\n\t> \n\t> This function would also be responsible to initialize the regexp.\n\t> \n\t> However, as I said, I think it would be much more intuitive to \n\t> characterize the _words_ instead of the _word boundaries_.\n\t> \n\t> And I would like to keep the default as-is (together _with_ the \n\t> performance. IOW if the user did not specify a regexp, it should fall \n\t> back to what it does now, which is slow enough).\n\n\tAnd this patch does all that, and it _is_ substantially more \n\tcompact, as promised.\n\n\tIt lacks testing, a test script and documentation, as well as \n\tconfigurability via config and/or attributes, but that's your\n\tjob, as I am not really _that_ interested in the feature myself.\n\n diff.c | 45 +++++++++++++++++++++++++++++++++++++++------\n diff.h | 1 +\n 2 files changed, 40 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4643ffc..c7ddb60 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -339,6 +339,7 @@ static void diff_words_append(char *line, unsigned long len,\n struct diff_words_data {\n \tstruct diff_words_buffer minus, plus;\n \tFILE *file;\n+\tregex_t *word_regex;\n };\n \n static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n@@ -398,6 +399,25 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static int find_word_boundary(struct diff_words_data *diff_words,\n+\t\tmmfile_t *buffer, int i)\n+{\n+\tif (i >= buffer->size)\n+\t\treturn i;\n+\n+\tif (diff_words->word_regex) {\n+\t\tregmatch_t match[1];\n+\t\tif (!regexec(diff_words->word_regex, buffer->ptr + i,\n+\t\t\t\t1, match, 0))\n+\t\t\ti += match[0].rm_eo;\n+\t}\n+\telse\n+\t\twhile (i < buffer->size && !isspace(i))\n+\t\t\ti++;\n+\n+\treturn i;\n+}\n+\n /* this executes the word diff on the accumulated buffers */\n static void diff_words_show(struct diff_words_data *diff_words)\n {\n@@ -412,17 +432,17 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tminus.size = diff_words->minus.text.size;\n \tminus.ptr = xmalloc(minus.size);\n \tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n-\tfor (i = 0; i < minus.size; i++)\n-\t\tif (isspace(minus.ptr[i]))\n-\t\t\tminus.ptr[i] = '\\n';\n+\tfor (i = 0; (i = find_word_boundary(diff_words, &minus, i))\n+\t\t\t< minus.size; i++)\n+\t\tminus.ptr[i] = '\\n';\n \tdiff_words->minus.current = 0;\n \n \tplus.size = diff_words->plus.text.size;\n \tplus.ptr = xmalloc(plus.size);\n \tmemcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);\n-\tfor (i = 0; i < plus.size; i++)\n-\t\tif (isspace(plus.ptr[i]))\n-\t\t\tplus.ptr[i] = '\\n';\n+\tfor (i = 0; (i = find_word_boundary(diff_words, &plus, i))\n+\t\t\t< plus.size; i++)\n+\t\tplus.ptr[i] = '\\n';\n \tdiff_words->plus.current = 0;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n@@ -461,6 +481,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \n \t\tfree (ecbdata->diff_words->minus.text.ptr);\n \t\tfree (ecbdata->diff_words->plus.text.ptr);\n+\t\tfree(ecbdata->diff_words->word_regex);\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\n@@ -1483,6 +1504,14 @@ static void builtin_diff(const char *name_a,\n \t\t\tecbdata.diff_words =\n \t\t\t\txcalloc(1, sizeof(struct diff_words_data));\n \t\t\tecbdata.diff_words->file = o->file;\n+\t\t\tif (o->word_regex) {\n+\t\t\t\tecbdata.diff_words->word_regex = (regex_t *)\n+\t\t\t\t\txmalloc(sizeof(regex_t));\n+\t\t\t\tif (regcomp(ecbdata.diff_words->word_regex,\n+\t\t\t\t\t\to->word_regex, REG_EXTENDED))\n+\t\t\t\t\tdie (\"Invalid regular expression: %s\",\n+\t\t\t\t\t\t\to->word_regex);\n+\t\t\t}\n \t\t}\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n \t\t\t &xpp, &xecfg, &ecb);\n@@ -2496,6 +2525,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_CLR(options, COLOR_DIFF);\n \telse if (!strcmp(arg, \"--color-words\"))\n \t\toptions->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;\n+\telse if (!prefixcmp(arg, \"--color-words=\")) {\n+\t\toptions->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;\n+\t\toptions->word_regex = arg + 14;\n+\t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \telse if (!strcmp(arg, \"--quiet\"))\ndiff --git a/diff.h b/diff.h\nindex 4d5a327..23cd90c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -98,6 +98,7 @@ struct diff_options {\n \n \tint stat_width;\n \tint stat_name_width;\n+\tconst char *word_regex;\n \n \t/* this is set by diffcore for DIFF_FORMAT_PATCH */\n \tint found_changes;\n-- \n1.6.1.203.gc8be3\n"},{"id":"99788","messageId":"200901091324.40583.trast@student.ethz.ch","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901091255230.30769@pacific.mpi-cbg.de","subject":"Re: [ILLUSTRATION PATCH] color-words: take an optional regular expression describing words","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-09T12:24:33Z","receivedAt":"2009-01-09T12:24:33Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> \n> In some applications, words are not delimited by white space. To\n> allow for that, you can specify a regular expression describing\n> what makes a word with\n> \n> \tgit diff --color-words='^[A-Za-z0-9]*'\n[...]\n> \t> Intuitively, all you would have to do is to replace this part in \n> \t> diff_words_show()\n> \t> \n> \t> for (i = 0; i < minus.size; i++)\n> \t> if (isspace(minus.ptr[i]))\n> \t> minus.ptr[i] = '\\n';\n> \t> \n> \t> by a loop finding the next word boundary.\n[...]\n> \t> However, as I said, I think it would be much more intuitive to \n> \t> characterize the _words_ instead of the _word boundaries_.\n\nThat doesn't work. You cannot overwrite actual content in the strings\nto be diffed with newlines. The current --color-words exploits the\nfact that we don't care about spaces anyway, so we might as well\nreplace them with newlines, but we _do_ care about the words and in\nthe regexed version, you have no guarantees about where they might start.\n\nTo wit:\n\n thomas@thomas:~/tmp/foo(master)$ cat >foo\n foo_bar_baz\n quux\n thomas@thomas:~/tmp/foo(master)$ git add foo\n thomas@thomas:~/tmp/foo(master)$ git ci -m initial\n [master (root-commit)]: created f110c6c: \"initial\"\n 1 files changed, 2 insertions(+), 0 deletions(-)\n create mode 100644 foo\n thomas@thomas:~/tmp/foo(master)$ cat >foo\n foo_\n ar_\n az\n quux\n thomas@thomas:~/tmp/foo(master)$ git diff\n diff --git i/foo w/foo\n index 5b34f11..a2762c6 100644\n --- i/foo\n +++ w/foo\n @@ -1,2 +1,4 @@\n -foo_bar_baz\n +foo_\n +ar_\n +az\n quux\n thomas@thomas:~/tmp/foo(master)$ git diff --color-words\n diff --git i/foo w/foo\n index 5b34f11..a2762c6 100644\n --- i/foo\n +++ w/foo\n @@ -1,2 +1,4 @@\n foo_bar_bafoo_\n ar_\n az\n quux\n thomas@thomas:~/tmp/foo(master)$ git diff --color-words='[a-zA-Z]+_?'\n diff --git i/foo w/foo\n index 5b34f11..a2762c6 100644\n --- i/foo\n +++ w/foo\n @@ -1,2 +1,4 @@\n quux\n\nEven without the colours, you can see that it has a blind spot for\nchanges around a newline. Perhaps there is an easier way to remember\nthem, but we definitely cannot *forget* about the word boundaries.\n\nThat being said, even though my patch correctly sees the changes, the\nabove test case also exposes some sort of string overrun :-(\n\n> \t> And I would like to keep the default as-is (together _with_ the \n> \t> performance. IOW if the user did not specify a regexp, it should fall \n> \t> back to what it does now, which is slow enough).\n\nThat's definitely a valid request.\n\nI'll come up with a fixed patch, and probably make it both\nfuncname-like (Jeff's idea) and command line configurable.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n\n"},{"id":"99789","messageId":"87wsd48wam.fsf@iki.fi","threadId":"17058","inReplyTo":"200901091324.40583.trast@student.ethz.ch","subject":"Re: [ILLUSTRATION PATCH] color-words: take an optional regular expression describing words","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2009-01-09T13:05:05Z","receivedAt":"2009-01-09T13:05:05Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Thomas Rast (2009-01-09 13:24 +0100) wrote:\n\n> Johannes Schindelin wrote:\n>> > And I would like to keep the default as-is (together _with_ the\n>> > performance. IOW if the user did not specify a regexp, it should\n>> > fall back to what it does now, which is slow enough).\n>\n> That's definitely a valid request.\n\nI agree with that too. A good thing about the current --color-words is\nthat it automatically works with UTF-8 encoded text. This is _very_\nimportant as --color-words is usually the best diff tool for\nhuman-language texts.\n"},{"id":"99831","messageId":"1231549039-5236-1-git-send-email-trast@student.ethz.ch","threadId":"17058","inReplyTo":"87wsd48wam.fsf@iki.fi","subject":"[PATCH v2] make diff --color-words customizable","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-10T00:57:19Z","receivedAt":"2009-01-10T00:57:19Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Allows for user-configurable word splits via a regular expression when\nusing --color-words. This can make the diff more readable if the\nregex is configured according to the language of the file.\n\nThe regex can be specified either through an optional argument\n--color-words= or through the attributes mechanism, similar to\nthe funcname pattern.\n\nEach non-overlapping match of the regex is a word; everything in\nbetween is whitespace. We disallow matching the empty string (because\nit results in an endless loop) or a newline (breaks color escapes and\ninteracts badly with the input coming from the usual line diff). To\nhelp the user, we set REG_NEWLINE so that [^...] and . do not match\nnewlines.\n\n--color-words works (and always worked) by splitting words onto one\nline each, and using the normal line-diff machinery to get a word\ndiff. Since we cannot reuse the current approach of simply\noverwriting uninteresting characters with '\\n', we insert an\nartificial '\\n' at the end of each detected word. Its presence must\nbe tracked so that we can distinguish artificial from source newlines.\n\nInsertion of spaces is somewhat subtle. We echo a \"context\" space\ntwice (once on each side of the diff) if it follows directly after a\nword, by \"skipping\" it during the translation (instead of generating a\n'\\n'). While this loses a tiny bit of accuracy, it runs together long\nsequences of changed words into one removed and one added block,\nmaking the diff much more readable. As a side-effect, the splitting\nregex '\\S+' currently results in the exact same output as the original\ncode. The existing code still stays in place in case no regex is\nprovided, for performance.\n\nWe also build in patterns for some of the languages that already had\nfuncname regexes. They are designed to group UTF-8 sequences into a\nsingle word to make sure they remain readable.\n\nThanks to Johannes Schindelin for the option handling code.\n\nSigned-off-by: Thomas Rast \n\n---\n\nThomas Rast wrote:\n> I'll come up with a fixed patch, and probably make it both\n> funcname-like (Jeff's idea) and command line configurable.\n\nI think this should do. Getting the spaces right was harder than I\nthought; originally it only tracked _END and _BODY, but then a changed\nsentence will look like a lot of separate word changes, making it\nextremely confusing.\n\nTeemu Likonen wrote:\n> I agree with that too. A good thing about the current --color-words is\n> that it automatically works with UTF-8 encoded text. This is _very_\n> important as --color-words is usually the best diff tool for\n> human-language texts.\n\nThanks for pointing this out. I put a [\\x80-\\xff]+ clause in the\nbuilt-in patterns that do not already match high-bit characters, so\nthat they will keep them together no matter what. Unfortunately it's\nrather hard to get the same effect \"by hand\", as neither shell, nor\ngit-config, nor regex.c, seem to expand \\xNN or \\NNN. You'll need $''\nin bash (is this POSIX?) or 'echo -e' or a very large keyboard, or a\npattern that can be written in terms of a negated class.\n\n(I briefly considered forcing \"|[\\x80-\\xff]+|\\S\" into the regular\nexpression, but the former is very encoding-specific. Maybe at least\n\"|\\S\" would be a good addition.)\n\n\n\n Documentation/diff-options.txt | 18 +++-\n Documentation/gitattributes.txt | 21 ++++\n diff.c | 199 +++++++++++++++++++++++++++++++++++----\n diff.h | 1 +\n t/t4033-diff-color-words.sh | 90 ++++++++++++++++++\n userdiff.c | 27 ++++--\n userdiff.h | 1 +\n 7 files changed, 330 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 671f533..d22c06b 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -91,8 +91,22 @@ endif::git-format-patch[]\n \tTurn off colored diff, even when the configuration file\n \tgives the default to color output.\n \n---color-words::\n-\tShow colored word diff, i.e. color words which have changed.\n+--color-words[=]::\n+\tShow colored word diff, i.e., color words which have changed.\n+\tBy default, a new word only starts at whitespace, so that a\n+\t'word' is defined as a maximal sequence of non-whitespace\n+\tcharacters. The optional argument can be used to\n+\tconfigure this. It can also be set via a diff driver, see\n+\tlinkgit:gitattributes[1]; if a is given explicitly, it\n+\toverrides any diff driver setting.\n++\n+The must be an (extended) regular expression. When set, every\n+non-overlapping match of the is considered a word. (Regular\n+expression semantics ensure that quantifiers grab a maximal sequence\n+of characters.) Anything between these matches is considered\n+whitespace and ignored for the purposes of finding differences. You\n+may want to append `|\\S` to your regular expression to make sure that\n+it matches all non-whitespace characters.\n \n --no-renames::\n \tTurn off rename detection, even when the configuration\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 8af22ec..67f5522 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -334,6 +334,27 @@ patterns are available:\n - `tex` suitable for source code for LaTeX documents.\n \n \n+Customizing word diff\n+^^^^^^^^^^^^^^^^^^^^^\n+\n+You can customize the rules that `git diff --color-words` uses to\n+split words in a line, by specifying an appropriate regular expression\n+in the \"diff.*.wordregex\" configuration variable. For example, in TeX\n+a backslash followed by a sequence of letters forms a command, but\n+several such commands can be run together without intervening\n+whitespace. To separate them, use a regular expression such as\n+\n+------------------------\n+[diff \"tex\"]\n+\twordregex = \"\\\\\\\\[a-zA-Z]+|[{}]|\\\\\\\\.|[^\\\\{} \\t]+\"\n+------------------------\n+\n+Similar to 'xfuncname', a built in value is provided for the drivers\n+`bibtex`, `html`, `java`, `php`, `python` and `tex`. See the\n+documentation of --color-words in linkgit:git-diff[1] for the precise\n+semantics.\n+\n+\n Performing text diffs of binary files\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n \ndiff --git a/diff.c b/diff.c\nindex d235482..620911e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -321,6 +321,7 @@ struct diff_words_buffer {\n \tlong alloc;\n \tlong current; /* output pointer */\n \tint suppressed_newline;\n+\tenum diff_word_boundaries *boundaries;\n };\n \n static void diff_words_append(char *line, unsigned long len,\n@@ -336,23 +337,55 @@ static void diff_words_append(char *line, unsigned long len,\n \tbuffer->text.size += len;\n }\n \n+/*\n+ * We use these to save the word boundaries. WORD_BODY and WORD_END\n+ * signal a word, meaning that after the WORD_END character an\n+ * artificial newline will be inserted.\n+ */\n+enum diff_word_boundaries {\n+\tDIFF_WORD_UNDEF,\n+\tDIFF_WORD_BODY,\n+\tDIFF_WORD_END,\n+\tDIFF_WORD_SPACE,\n+\tDIFF_WORD_SKIP\n+};\n+\n struct diff_words_data {\n \tstruct diff_words_buffer minus, plus;\n \tFILE *file;\n+\tregex_t *word_regex;\n+\tenum diff_word_boundaries *minus_boundaries, *plus_boundaries;\n };\n \n-static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n+static int print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n \t\tint suppress_newline)\n {\n \tconst char *ptr;\n \tint eol = 0;\n \n \tif (len == 0)\n-\t\treturn;\n+\t\treturn len;\n \n \tptr = buffer->text.ptr + buffer->current;\n+\n+\tif (buffer->boundaries\n+\t && (buffer->boundaries[buffer->current] == DIFF_WORD_BODY\n+\t\t|| buffer->boundaries[buffer->current] == DIFF_WORD_END)) {\n+\t\t/* account for the artificial newline */\n+\t\tlen--;\n+\t\t/* we still have len>0 because it is a word */\n+\t}\n+\n \tbuffer->current += len;\n \n+\tif (buffer->boundaries\n+\t && buffer->boundaries[buffer->current] == DIFF_WORD_SKIP) {\n+\t\t/* we had an artificial newline, but the next whitespace\n+\t\t * character right after was skipped because of it */\n+\t\tbuffer->current++;\n+\t\tlen++;\n+\t}\n+\n \tif (ptr[len - 1] == '\\n') {\n \t\teol = 1;\n \t\tlen--;\n@@ -368,6 +401,10 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n \t\telse\n \t\t\tputc('\\n', file);\n \t}\n+\n+\t/* we need to return how many chars to skip on the other side,\n+\t * so account for the (held off) \\n */\n+\treturn len+eol;\n }\n \n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n@@ -391,13 +428,106 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\t\t &diff_words->plus, len, DIFF_FILE_NEW, 0);\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tprint_word(diff_words->file,\n-\t\t\t\t &diff_words->plus, len, DIFF_PLAIN, 0);\n+\t\t\tlen = print_word(diff_words->file,\n+\t\t\t\t\t &diff_words->plus, len, DIFF_PLAIN, 0);\n \t\t\tdiff_words->minus.current += len;\n \t\t\tbreak;\n \t}\n }\n \n+static void scan_word_boundaries(regex_t *pattern, struct diff_words_buffer *buf,\n+\t\t\t\t mmfile_t *mmfile)\n+{\n+\tchar *text = buf->text.ptr;\n+\tint len = buf->text.size;\n+\tint i = 0;\n+\tint count = 0;\n+\tint ret;\n+\tregmatch_t matches[1];\n+\tint offset, wordlen;\n+\tchar *strz, *p;\n+\n+\t/* overallocate by 1 so we can safely peek past the end for a SKIP */\n+\tbuf->boundaries = xmalloc((len+1) * sizeof(enum diff_word_boundaries));\n+\tbuf->boundaries[len] = DIFF_WORD_UNDEF;\n+\n+\tif (!text) {\n+\t\tmmfile->ptr = NULL;\n+\t\tmmfile->size = 0;\n+\t\treturn;\n+\t}\n+\n+\tstrz = xmalloc(len+1);\n+\tmemcpy(strz, text, len);\n+\tstrz[len] = '\\0';\n+\n+\twhile (i < len) {\n+\t\tret = regexec(pattern, strz+i, 1, matches, 0);\n+\t\tif (ret == REG_NOMATCH) {\n+\t\t\t/* the rest is whitespace */\n+\t\t\tif (i > 0 && i < len) {\n+\t\t\t\tbuf->boundaries[i++] = DIFF_WORD_SKIP;\n+\t\t\t\tcount--;\n+\t\t\t}\n+\t\t\twhile (i < len)\n+\t\t\t\tbuf->boundaries[i++] = DIFF_WORD_SPACE;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\toffset = matches[0].rm_so;\n+\t\tif (offset > 0 && i > 0) {\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_SKIP;\n+\t\t\tcount--;\n+\t\t\toffset--;\n+\t\t}\n+\t\twhile (offset-- > 0)\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_SPACE;\n+\n+\t\twordlen = matches[0].rm_eo - matches[0].rm_so;\n+\t\twhile (wordlen > 1) {\n+\t\t\tif (strz[i] == '\\n')\n+\t\t\t\tdie(\"word regex matched a newline near '%s'\",\n+\t\t\t\t strz+i);\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_BODY;\n+\t\t\twordlen--;\n+\t\t}\n+\t\tif (wordlen > 0) {\n+\t\t\tif (strz[i] == '\\n')\n+\t\t\t\tdie(\"word regex matched a newline near '%s'\",\n+\t\t\t\t strz+i);\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_END;\n+\t\t\tcount++;\n+\t\t} else {\n+\t\t\tdie(\"word regex matched the empty string at '%s'\",\n+\t\t\t strz+i);\n+\t\t}\n+\t}\n+\n+\tfree(strz);\n+\n+\tmmfile->size = len + count;\n+\tmmfile->ptr = xmalloc(mmfile->size);\n+\tp = mmfile->ptr;\n+\tfor (i = 0; i < len; i++) {\n+\t\tswitch (buf->boundaries[i]) {\n+\t\tcase DIFF_WORD_BODY:\n+\t\t\t*p++ = text[i];\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_END:\n+\t\t\t*p++ = text[i];\n+\t\t\t*p++ = '\\n'; /* insert an artificial newline */\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_SPACE:\n+\t\t\t*p++ = '\\n';\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_SKIP:\n+\t\t\t/* nothing */\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+\n /* this executes the word diff on the accumulated buffers */\n static void diff_words_show(struct diff_words_data *diff_words)\n {\n@@ -409,22 +539,31 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \n \tmemset(&xpp, 0, sizeof(xpp));\n \tmemset(&xecfg, 0, sizeof(xecfg));\n-\tminus.size = diff_words->minus.text.size;\n-\tminus.ptr = xmalloc(minus.size);\n-\tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n-\tfor (i = 0; i < minus.size; i++)\n-\t\tif (isspace(minus.ptr[i]))\n-\t\t\tminus.ptr[i] = '\\n';\n-\tdiff_words->minus.current = 0;\n \n-\tplus.size = diff_words->plus.text.size;\n-\tplus.ptr = xmalloc(plus.size);\n-\tmemcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);\n-\tfor (i = 0; i < plus.size; i++)\n-\t\tif (isspace(plus.ptr[i]))\n-\t\t\tplus.ptr[i] = '\\n';\n+\tif (!diff_words->word_regex) {\n+\t\tminus.size = diff_words->minus.text.size;\n+\t\tminus.ptr = xmalloc(minus.size);\n+\t\tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n+\t\tfor (i = 0; i < minus.size; i++)\n+\t\t\tif (isspace(minus.ptr[i]))\n+\t\t\t\tminus.ptr[i] = '\\n';\n+\n+\t\tplus.size = diff_words->plus.text.size;\n+\t\tplus.ptr = xmalloc(plus.size);\n+\t\tmemcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);\n+\t\tfor (i = 0; i < plus.size; i++)\n+\t\t\tif (isspace(plus.ptr[i]))\n+\t\t\t\tplus.ptr[i] = '\\n';\n+\t} else {\n+\t\tscan_word_boundaries(diff_words->word_regex,\n+\t\t\t\t &diff_words->minus, &minus);\n+\t\tscan_word_boundaries(diff_words->word_regex,\n+\t\t\t\t &diff_words->plus, &plus);\n+\t}\n+\tdiff_words->minus.current = 0;\n \tdiff_words->plus.current = 0;\n \n+\n \txpp.flags = XDF_NEED_MINIMAL;\n \txecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;\n \txdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,\n@@ -432,6 +571,8 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n+\tfree(diff_words->minus.boundaries);\n+\tfree(diff_words->plus.boundaries);\n \n \tif (diff_words->minus.suppressed_newline) {\n \t\tputc('\\n', diff_words->file);\n@@ -461,6 +602,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \n \t\tfree (ecbdata->diff_words->minus.text.ptr);\n \t\tfree (ecbdata->diff_words->plus.text.ptr);\n+\t\tfree(ecbdata->diff_words->word_regex);\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\n@@ -1323,6 +1465,12 @@ static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespe\n \treturn one->driver->funcname.pattern ? &one->driver->funcname : NULL;\n }\n \n+static const char *userdiff_word_regex(struct diff_filespec *one)\n+{\n+\tdiff_filespec_load_driver(one);\n+\treturn one->driver->word_regex;\n+}\n+\n void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)\n {\n \tif (!options->a_prefix)\n@@ -1483,6 +1631,19 @@ static void builtin_diff(const char *name_a,\n \t\t\tecbdata.diff_words =\n \t\t\t\txcalloc(1, sizeof(struct diff_words_data));\n \t\t\tecbdata.diff_words->file = o->file;\n+\t\t\tif (!o->word_regex)\n+\t\t\t\to->word_regex = userdiff_word_regex(one);\n+\t\t\tif (!o->word_regex)\n+\t\t\t\to->word_regex = userdiff_word_regex(two);\n+\t\t\tif (o->word_regex) {\n+\t\t\t\tecbdata.diff_words->word_regex = (regex_t *)\n+\t\t\t\t\txmalloc(sizeof(regex_t));\n+\t\t\t\tif (regcomp(ecbdata.diff_words->word_regex,\n+\t\t\t\t\t o->word_regex,\n+\t\t\t\t\t REG_EXTENDED|REG_NEWLINE))\n+\t\t\t\t\tdie (\"Invalid regular expression: %s\",\n+\t\t\t\t\t o->word_regex);\n+\t\t\t}\n \t\t}\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n \t\t\t &xpp, &xecfg, &ecb);\n@@ -2494,6 +2655,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_CLR(options, COLOR_DIFF);\n \telse if (!strcmp(arg, \"--color-words\"))\n \t\toptions->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;\n+\telse if (!prefixcmp(arg, \"--color-words=\")) {\n+\t\toptions->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;\n+\t\toptions->word_regex = arg + 14;\n+\t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \telse if (!strcmp(arg, \"--quiet\"))\ndiff --git a/diff.h b/diff.h\nindex 4d5a327..23cd90c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -98,6 +98,7 @@ struct diff_options {\n \n \tint stat_width;\n \tint stat_name_width;\n+\tconst char *word_regex;\n \n \t/* this is set by diffcore for DIFF_FORMAT_PATCH */\n \tint found_changes;\ndiff --git a/t/t4033-diff-color-words.sh b/t/t4033-diff-color-words.sh\nnew file mode 100755\nindex 0000000..536cdac\n--- /dev/null\n+++ b/t/t4033-diff-color-words.sh\n@@ -0,0 +1,90 @@\n+#!/bin/sh\n+\n+\n+test_description='diff --color-words'\n+. ./test-lib.sh\n+\n+cat < test_a\n+foo_bar_baz\n+a qu_ux b c\n+alpha beta gamma delta\n+EOF\n+\n+cat < test_b\n+foo_baz_baz\n+a qu_new_ux b c\n+alpha 4 2 delta\n+EOF\n+\n+# t4026-diff-color.sh tests the color escapes, so we assume they do\n+# not change\n+\n+munge () {\n+ tail -n +5 | tr '\\033' '!'\n+}\n+\n+cat < expect-plain\n+![36m@@ -1,3 +1,3 @@![m\n+![31mfoo_bar_baz![m![32mfoo_baz_baz![m\n+a ![m![31mqu_ux ![m![32mqu_new_ux ![mb ![mc![m\n+alpha ![m![31mbeta ![m![31mgamma ![m![32m4 ![m![32m2 ![mdelta![m\n+EOF\n+\n+test_expect_success 'default settings' '\n+\tgit diff --no-index --color-words test_a test_b |\n+\t\tmunge > actual-plain &&\n+\ttest_cmp expect-plain actual-plain\n+'\n+\n+test_expect_success 'trivial regex yields same as default' '\n+\tgit diff --no-index --color-words=\"\\\\S+\" test_a test_b |\n+\t\tmunge > actual-trivial &&\n+\ttest_cmp expect-plain actual-trivial\n+'\n+\n+cat < expect-chars\n+![36m@@ -1,3 +1,3 @@![m\n+f![mo![mo![m_![mb![ma![m![31mr![m![32mz![m_![mb![ma![mz![m\n+a ![mq![mu![m_![m![32mn![m![32me![m![32mw![m![32m_![mu![mx ![mb ![mc![m\n+a![ml![mp![mh![ma ![m![31mb![m![31me![m![31mt![m![31ma ![m![31mg![m![31ma![m![31mm![m![31mm![m![31ma ![m![32m4 ![m![32m2 ![md![me![ml![mt![ma![m\n+EOF\n+\n+test_expect_success 'character by character regex' '\n+\tgit diff --no-index --color-words=\"\\\\S\" test_a test_b |\n+\t\tmunge > actual-chars &&\n+\ttest_cmp expect-chars actual-chars\n+'\n+\n+cat < expect-nontrivial\n+![36m@@ -1,3 +1,3 @@![m\n+foo![m_![m![31mbar![m![32mbaz![m_![mbaz![m\n+a ![mqu![m_![m![32mnew![m![32m_![mux ![mb ![mc![m\n+alpha ![m![31mbeta ![m![31mgamma ![m![32m4![m![32m ![m![32m2![m![32m ![mdelta![m\n+EOF\n+\n+test_expect_success 'nontrivial regex' '\n+\tgit diff --no-index --color-words=\"[a-z]+|_\" test_a test_b |\n+\t\tmunge > actual-nontrivial &&\n+\ttest_cmp expect-nontrivial actual-nontrivial\n+'\n+\n+test_expect_success 'set a diff driver' '\n+\tgit config diff.testdriver.wordregex \"\\\\S\" &&\n+\tcat < .gitattributes\n+test_* diff=testdriver\n+EOF\n+'\n+\n+test_expect_success 'use default supplied by driver' '\n+\tgit diff --no-index --color-words test_a test_b |\n+\t\tmunge > actual-chars-2 &&\n+\ttest_cmp expect-chars actual-chars-2\n+'\n+\n+test_expect_success 'option overrides default' '\n+\tgit diff --no-index --color-words=\"[a-z]+|_\" test_a test_b |\n+\t\tmunge > actual-nontrivial-2 &&\n+\ttest_cmp expect-nontrivial actual-nontrivial-2\n+'\n+\n+test_done\ndiff --git a/userdiff.c b/userdiff.c\nindex 3681062..7fd9a07 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -6,13 +6,17 @@ static struct userdiff_driver *drivers;\n static int ndrivers;\n static int drivers_alloc;\n \n-#define FUNCNAME(name, pattern) \\\n+#define FUNCNAME(name, pattern)\t\t\t\\\n \t{ name, NULL, -1, { pattern, REG_EXTENDED } }\n+#define PATTERNS(name, pattern, wordregex)\t\t\t\\\n+\t{ name, NULL, -1, { pattern, REG_EXTENDED }, NULL, wordregex }\n static struct userdiff_driver builtin_drivers[] = {\n-FUNCNAME(\"html\", \"^[ \\t]*(<[Hh][1-6][ \\t].*>.*)$\"),\n-FUNCNAME(\"java\",\n+PATTERNS(\"html\", \"^[ \\t]*(<[Hh][1-6][ \\t].*>.*)$\",\n+\t \"[^<>= \\t]+|\\\\S\"),\n+PATTERNS(\"java\",\n \t \"!^[ \\t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\\n\"\n-\t \"^[ \\t]*(([ \\t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \\t]*\\\\([^;]*)$\"),\n+\t \"^[ \\t]*(([ \\t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \\t]*\\\\([^;]*)$\",\n+\t \"[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[-+*/]=|\\\\+\\\\+|--|\\\\S|[\\x80-\\xff]+\"),\n FUNCNAME(\"objc\",\n \t /* Negate C statements that can look like functions */\n \t \"!^[ \\t]*(do|for|if|else|return|switch|while)\\n\"\n@@ -27,14 +31,19 @@ FUNCNAME(\"pascal\",\n \t\t\"implementation|initialization|finalization)[ \\t]*.*)$\"\n \t \"\\n\"\n \t \"^(.*=[ \\t]*(class|record).*)$\"),\n-FUNCNAME(\"php\", \"^[\\t ]*((function|class).*)\"),\n-FUNCNAME(\"python\", \"^[ \\t]*((class|def)[ \\t].*)$\"),\n+PATTERNS(\"php\", \"^[\\t ]*((function|class).*)\",\n+\t \"\\\\$?[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[-+*/]=|\\\\+\\\\+|--|->|\\\\S|[\\x80-\\xff]+\"),\n+PATTERNS(\"python\", \"^[ \\t]*((class|def)[ \\t].*)$\",\n+\t \"[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[-+*/]=|//|\\\\S|[\\x80-\\xff]+\"),\n FUNCNAME(\"ruby\", \"^[ \\t]*((class|module|def)[ \\t].*)$\"),\n-FUNCNAME(\"bibtex\", \"(@[a-zA-Z]{1,}[ \\t]*\\\\{{0,1}[ \\t]*[^ \\t\\\"@',\\\\#}{~%]*).*$\"),\n-FUNCNAME(\"tex\", \"^(\\\\\\\\((sub)*section|chapter|part)\\\\*{0,1}\\\\{.*)$\"),\n+PATTERNS(\"bibtex\", \"(@[a-zA-Z]{1,}[ \\t]*\\\\{{0,1}[ \\t]*[^ \\t\\\"@',\\\\#}{~%]*).*$\",\n+\t \"[={}\\\"]|[^={}\\\" \\t]+\"),\n+PATTERNS(\"tex\", \"^(\\\\\\\\((sub)*section|chapter|part)\\\\*{0,1}\\\\{.*)$\",\n+\t \"\\\\\\\\[a-zA-Z@]+|[{}]|\\\\\\\\.|[^\\\\{} \\t]+\"),\n { \"default\", NULL, -1, { NULL, 0 } },\n };\n #undef FUNCNAME\n+#undef PATTERNS\n \n static struct userdiff_driver driver_true = {\n \t\"diff=true\",\n@@ -134,6 +143,8 @@ int userdiff_config(const char *k, const char *v)\n \t\treturn parse_string(&drv->external, k, v);\n \tif ((drv = parse_driver(k, v, \"textconv\")))\n \t\treturn parse_string(&drv->textconv, k, v);\n+\tif ((drv = parse_driver(k, v, \"wordregex\")))\n+\t\treturn parse_string(&drv->word_regex, k, v);\n \n \treturn 0;\n }\ndiff --git a/userdiff.h b/userdiff.h\nindex ba29457..2aab13e 100644\n--- a/userdiff.h\n+++ b/userdiff.h\n@@ -12,6 +12,7 @@ struct userdiff_driver {\n \tint binary;\n \tstruct userdiff_funcname funcname;\n \tconst char *textconv;\n+\tconst char *word_regex;\n };\n \n int userdiff_config(const char *k, const char *v);\n-- \ntg: (c123b7c..) t/word-diff-regex (depends on: origin/master)\n"},{"id":"99842","messageId":"gk8usj$slh$1@ger.gmane.org","threadId":"17058","inReplyTo":"1231549039-5236-1-git-send-email-trast@student.ethz.ch","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-10T01:50:15Z","receivedAt":"2009-01-10T01:50:15Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Thomas Rast wrote:\n\n> --color-words works (and always worked) by splitting words onto one\n> line each, and using the normal line-diff machinery to get a word\n> diff. \n\nCannot we generalize diff machinery / use underlying LCS diff engine\ninstead of going through line diff?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"99868","messageId":"alpine.DEB.1.00.0901101146230.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"1231549039-5236-1-git-send-email-trast@student.ethz.ch","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-10T10:49:52Z","receivedAt":"2009-01-10T10:49:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Jan 2009, Thomas Rast wrote:\n\n> diff.c | 199 +++++++++++++++++++++++++++++++++++----\n\n!!!\n\nBTW I did not really think about the issue you raised about the newlines, \nas I seemed to remember that the idea was to substitute all non-word \ncharacters with newlines, so that the offsets in the substituted text are \nthe same as in the original text.\n\nSo I still find your patch way too large, and it keeps growing, \nunfortunately.\n\nCiao,\nDscho\n"},{"id":"99871","messageId":"200901101225.10719.trast@student.ethz.ch","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901101146230.30769@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-10T11:25:06Z","receivedAt":"2009-01-10T11:25:06Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> BTW I did not really think about the issue you raised about the newlines, \n> as I seemed to remember that the idea was to substitute all non-word \n> characters with newlines, so that the offsets in the substituted text are \n> the same as in the original text.\n\nOk, so here's a very simple example: Suppose you have the word regex\n'x+|y+' and compare these two lines:\n\nA: xxyyxy\nB: xyxyy\n\nThere are *no* non-word characters between consecutive words in this\ncase, so you *cannot* replace them with newlines. You cannot replace\nsome word character either, as should be obvious from the case of\none-letter words, as you would lose actual content. My counterexample\nto your illustration patch exploited a similar border case: suppose\nyou decide to overwrite the first (instead of last) character of each\nword, then you won't be able to tell \"foo\" from \"\\noo\" in the input.\n\nUnfortunately the space adjustement makes things even worse. The\nexisting method has the side-effect that it only inserts a single\nnewline between words separated by exactly one space, which runs them\ntogether in the resulting line diff, for example\n\nA: foo bar\nB: baz quux\n\nwould result in\n\n -foo\n -bar\n +baz\n +quux\n\ninstead of (as my original attempts did) the arguably more correct,\nbut less readable\n\n -foo\n +bar\n \n -baz\n +quux\n\nwhere the middle line is a context line for the space. So in addition\nto the word-ending adjustments for the inserted newlines, I also have\nto track the status of the space right after the word.\n\n> So I still find your patch way too large\n\nI can't think of a simpler way to do it, and yours unfortunately\ndoesn't work.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n\n\n"},{"id":"99874","messageId":"alpine.DEB.1.00.0901101237050.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"gk8usj$slh$1@ger.gmane.org","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-10T11:37:57Z","receivedAt":"2009-01-10T11:37:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Jan 2009, Jakub Narebski wrote:\n\n> Thomas Rast wrote:\n> \n> > --color-words works (and always worked) by splitting words onto one\n> > line each, and using the normal line-diff machinery to get a word\n> > diff. \n> \n> Cannot we generalize diff machinery / use underlying LCS diff engine\n> instead of going through line diff?\n\nWhat do you think we're doing? libxdiff is pretty hardcoded to newlines. \nThat's why we're substituting non-word characters with newlines.\n\nCiao,\nDscho\n"},{"id":"99876","messageId":"alpine.DEB.1.00.0901101239550.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"200901101225.10719.trast@student.ethz.ch","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-10T11:45:31Z","receivedAt":"2009-01-10T11:45:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Jan 2009, Thomas Rast wrote:\n\n> Johannes Schindelin wrote:\n> > BTW I did not really think about the issue you raised about the newlines, \n> > as I seemed to remember that the idea was to substitute all non-word \n> > characters with newlines, so that the offsets in the substituted text are \n> > the same as in the original text.\n> \n> Ok, so here's a very simple example: Suppose you have the word regex\n> 'x+|y+' and compare these two lines:\n> \n> A: xxyyxy\n> B: xyxyy\n\nAh, I see.\n\n> > So I still find your patch way too large\n> \n> I can't think of a simpler way to do it, and yours unfortunately doesn't \n> work.\n\nWell, the thing I tried to hint at: it is not good to have a monster \npatch, as nobody will review it.\n\nIn your case, I imagine it would be much easier to get reviewers if you \nhad\n\n\tpatch 1/4 refactor color-words to allow for 0-character word \n\t\tboundaries\n\tpatch 2/4 allow regular expressions to define what makes a word\n\tpatch 3/4 add option to specify word boundary regexps via\n\t\tattributes\n\tpatch 4/4 test word boundary regexps\n\nAnd I admit that I documented the code lousily, but that does not mean \nthat you should repeat that mistake.\n\nCiao,\nDscho\n"},{"id":"99882","messageId":"200901101436.48149.jnareb@gmail.com","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901101237050.30769@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-10T13:36:46Z","receivedAt":"2009-01-10T13:36:46Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n> On Sat, 10 Jan 2009, Jakub Narebski wrote:\n> > Thomas Rast wrote:\n> > \n> > > --color-words works (and always worked) by splitting words onto one\n> > > line each, and using the normal line-diff machinery to get a word\n> > > diff. \n> > \n> > Cannot we generalize diff machinery / use underlying LCS diff engine\n> > instead of going through line diff?\n> \n> What do you think we're doing? libxdiff is pretty hardcoded to newlines. \n> That's why we're substituting non-word characters with newlines.\n\nIsn't Meyers algorithm used by libxdiff based on LCS, largest common\nsubsequence, and doesn't it generate from the mathematical point of\nview \"diff\" between two sequences (two arrays) which just happen to\nbe lines? It is a bit strange that libxdiff doesn't export its low\nlevel algorithm...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"99890","messageId":"alpine.DEB.1.00.0901101507590.30769@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"200901101436.48149.jnareb@gmail.com","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-10T14:08:42Z","receivedAt":"2009-01-10T14:08:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 Jan 2009, Jakub Narebski wrote:\n\n> On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n> > On Sat, 10 Jan 2009, Jakub Narebski wrote:\n> > > Thomas Rast wrote:\n> > > \n> > > > --color-words works (and always worked) by splitting words onto one\n> > > > line each, and using the normal line-diff machinery to get a word\n> > > > diff. \n> > > \n> > > Cannot we generalize diff machinery / use underlying LCS diff engine\n> > > instead of going through line diff?\n> > \n> > What do you think we're doing? libxdiff is pretty hardcoded to newlines. \n> > That's why we're substituting non-word characters with newlines.\n> \n> Isn't Meyers algorithm used by libxdiff based on LCS, largest common\n> subsequence, and doesn't it generate from the mathematical point of\n> view \"diff\" between two sequences (two arrays) which just happen to\n> be lines? It is a bit strange that libxdiff doesn't export its low\n> level algorithm...\n\nUmm.\n\nIt _is_ Myers' algorithm. It just so happens that libxdiff hardcodes \nnewline to be the separator.\n\nCiao,\nDscho\n"},{"id":"99913","messageId":"alpine.DEB.1.10.0901100950230.21891@alien.or.mcafeemobile.com","threadId":"17058","inReplyTo":"200901101436.48149.jnareb@gmail.com","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Davide Libenzi","fromEmail":"davidel@xmailserver.org","sentAt":"2009-01-10T17:53:56Z","receivedAt":"2009-01-10T17:53:56Z","isPatch":true,"sender":{"key":"davidel@xmailserver.org","avatar":null},"body":"On Sat, 10 Jan 2009, Jakub Narebski wrote:\n\n> On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n> > On Sat, 10 Jan 2009, Jakub Narebski wrote:\n> > > Thomas Rast wrote:\n> > > \n> > > > --color-words works (and always worked) by splitting words onto one\n> > > > line each, and using the normal line-diff machinery to get a word\n> > > > diff. \n> > > \n> > > Cannot we generalize diff machinery / use underlying LCS diff engine\n> > > instead of going through line diff?\n> > \n> > What do you think we're doing? libxdiff is pretty hardcoded to newlines. \n> > That's why we're substituting non-word characters with newlines.\n> \n> Isn't Meyers algorithm used by libxdiff based on LCS, largest common\n> subsequence, and doesn't it generate from the mathematical point of\n> view \"diff\" between two sequences (two arrays) which just happen to\n> be lines? It is a bit strange that libxdiff doesn't export its low\n> level algorithm...\n\nThe core doesn't know anything about lines. Only pre-processing (setting \nup the hash by tokenizing the input) and post-processing (adding '\\n' to \nthe end of each token), knows about newlines. Memory consumption would \nincrease significantly though, since there is a per-token cost, and a \nword-based diff will create more of them WRT the same input.\n\n\n- Davide\n"},{"id":"99930","messageId":"7vr63atykr.fsf@gitster.siamese.dyndns.org","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901101239550.30769@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-11T01:34:44Z","receivedAt":"2009-01-11T01:34:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Well, the thing I tried to hint at: it is not good to have a monster \n> patch, as nobody will review it.\n>\n> In your case, I imagine it would be much easier to get reviewers if you \n> had\n>\n> \tpatch 1/4 refactor color-words to allow for 0-character word \n> \t\tboundaries\n> \tpatch 2/4 allow regular expressions to define what makes a word\n> \tpatch 3/4 add option to specify word boundary regexps via\n> \t\tattributes\n> \tpatch 4/4 test word boundary regexps\n>\n> And I admit that I documented the code lousily, but that does not mean \n> that you should repeat that mistake.\n\nSounds like a reasonable request. Also I am seeing:\n\n diff.c: In function 'scan_word_boundaries':\n diff.c:512: warning: enumeration value 'DIFF_WORD_UNDEF' not handled in switch\n\nfrom this part of the code:\n\n\tfor (i = 0; i < len; i++) {\n\t\tswitch (buf->boundaries[i]) {\n\t\tcase DIFF_WORD_BODY:\n\t\t\t*p++ = text[i];\n\t\t\tbreak;\n\t\tcase DIFF_WORD_END:\n\t\t\t*p++ = text[i];\n\t\t\t*p++ = '\\n'; /* insert an artificial newline */\n\t\t\tbreak;\n\t\tcase DIFF_WORD_SPACE:\n\t\t\t*p++ = '\\n';\n\t\t\tbreak;\n\t\tcase DIFF_WORD_SKIP:\n\t\t\t/* nothing */\n\t\t\tbreak;\n\t\t}\n\t}\n"},{"id":"99939","messageId":"cover.1231669012.git.trast@student.ethz.ch","threadId":"17058","inReplyTo":"7vr63atykr.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v3 0/4] customizable --color-words","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-11T10:27:10Z","receivedAt":"2009-01-11T10:27:10Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> On Sat, 10 Jan 2009, Thomas Rast wrote:\n> > Johannes Schindelin wrote:\n> > > So I still find your patch way too large\n\nThe bad news is... it just got bigger ;-)\n\n> In your case, I imagine it would be much easier to get reviewers if you \n> had\n> \n> \tpatch 1/4 refactor color-words to allow for 0-character word \n> \t\tboundaries\n> \tpatch 2/4 allow regular expressions to define what makes a word\n\nSo here's a 4-patch series. I put the first split in a different\nplace than you suggested, however. I couldn't see a good way to\nseparate empty boundaries from regex splitting in such a way that the\nfirst half can be exercised (is not just dead code). 1/4 basically\njust rearranges code a bit and should be a real no-op patch.\n\nJunio C Hamano wrote:\n> diff.c: In function 'scan_word_boundaries':\n> diff.c:512: warning: enumeration value 'DIFF_WORD_UNDEF' not handled in sw\n\nThanks, added a case to test for this.\n\nThere is one other minor semantic change in 2/4: the error reporting\nin case your regex matched \"foo\\nbar\" now says \"before 'bar'\" instead\nof \"near '\\nbar'\". Other than that, there are only a bunch of added\ncomments when comparing the result of all four patches with v2.\n\n\nThomas Rast (4):\n word diff: comments, preparations for regex customization\n word diff: customizable word splits\n word diff: make regex configurable via attributes\n word diff: test customizable word splits\n\n Documentation/diff-options.txt | 18 +++-\n Documentation/gitattributes.txt | 21 +++\n diff.c | 282 ++++++++++++++++++++++++++++++++++++---\n diff.h | 1 +\n t/t4033-diff-color-words.sh | 90 +++++++++++++\n userdiff.c | 27 +++-\n userdiff.h | 1 +\n 7 files changed, 413 insertions(+), 27 deletions(-)\n create mode 100755 t/t4033-diff-color-words.sh\n"},{"id":"99940","messageId":"4aea85caafd38a058145c5769fe8a30ffdbd4d13.1231669012.git.trast@student.ethz.ch","threadId":"17058","inReplyTo":"cover.1231669012.git.trast@student.ethz.ch","subject":"[PATCH v3 1/4] word diff: comments, preparations for regex customization","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-11T10:27:11Z","receivedAt":"2009-01-11T10:27:11Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This reorganizes the code for diff --color-words in a way that will be\nconvenient for the next patch, without changing any of the semantics.\nThe new variables are not used yet except for their default state.\n\nWe also add some comments on the workings of diff_words_show() and\nassociated helper routines.\n\nSigned-off-by: Thomas Rast \n---\n diff.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 69 insertions(+), 17 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d235482..f274bf5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -316,11 +316,21 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)\n \treturn 0;\n }\n \n+/* unused right now */\n+enum diff_word_boundaries {\n+\tDIFF_WORD_UNDEF,\n+\tDIFF_WORD_BODY,\n+\tDIFF_WORD_END,\n+\tDIFF_WORD_SPACE,\n+\tDIFF_WORD_SKIP\n+};\n+\n struct diff_words_buffer {\n \tmmfile_t text;\n \tlong alloc;\n \tlong current; /* output pointer */\n \tint suppressed_newline;\n+\tenum diff_word_boundaries *boundaries;\n };\n \n static void diff_words_append(char *line, unsigned long len,\n@@ -339,16 +349,23 @@ static void diff_words_append(char *line, unsigned long len,\n struct diff_words_data {\n \tstruct diff_words_buffer minus, plus;\n \tFILE *file;\n+\tregex_t *word_regex; /* currently unused */\n };\n \n-static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n+/*\n+ * Print 'len' characters from the \"real\" diff data in 'buffer'. Also\n+ * returns how many characters were printed (currently always 'len').\n+ * With 'suppress_newline', we remember a final newline instead of\n+ * printing it.\n+ */\n+static int print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n \t\tint suppress_newline)\n {\n \tconst char *ptr;\n \tint eol = 0;\n \n \tif (len == 0)\n-\t\treturn;\n+\t\treturn len;\n \n \tptr = buffer->text.ptr + buffer->current;\n \tbuffer->current += len;\n@@ -368,18 +385,30 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n \t\telse\n \t\t\tputc('\\n', file);\n \t}\n+\n+\t/* we need to return how many chars to skip on the other side,\n+\t * so account for the (held off) \\n */\n+\treturn len+eol;\n }\n \n+/*\n+ * Callback for word diff output\n+ */\n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n {\n \tstruct diff_words_data *diff_words = priv;\n \n \tif (diff_words->minus.suppressed_newline) {\n+\t\t/* We completely drop a suppressed newline on the\n+\t\t * minus side, if it is immediately followed by a plus\n+\t\t * side output. This formats a word change right\n+\t\t * before the end of line correctly */\n \t\tif (line[0] != '+')\n \t\t\tputc('\\n', diff_words->file);\n \t\tdiff_words->minus.suppressed_newline = 0;\n \t}\n \n+\t/* account for the [+- ] inserted by the line diff */\n \tlen--;\n \tswitch (line[0]) {\n \t\tcase '-':\n@@ -391,8 +420,10 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\t\t &diff_words->plus, len, DIFF_FILE_NEW, 0);\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tprint_word(diff_words->file,\n-\t\t\t\t &diff_words->plus, len, DIFF_PLAIN, 0);\n+\t\t\tlen = print_word(diff_words->file,\n+\t\t\t\t\t &diff_words->plus, len, DIFF_PLAIN, 0);\n+\t\t\t/* skip the characters that were printed on\n+\t\t\t * the other side, too */\n \t\t\tdiff_words->minus.current += len;\n \t\t\tbreak;\n \t}\n@@ -409,22 +440,37 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \n \tmemset(&xpp, 0, sizeof(xpp));\n \tmemset(&xecfg, 0, sizeof(xecfg));\n-\tminus.size = diff_words->minus.text.size;\n-\tminus.ptr = xmalloc(minus.size);\n-\tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n-\tfor (i = 0; i < minus.size; i++)\n-\t\tif (isspace(minus.ptr[i]))\n-\t\t\tminus.ptr[i] = '\\n';\n-\tdiff_words->minus.current = 0;\n \n-\tplus.size = diff_words->plus.text.size;\n-\tplus.ptr = xmalloc(plus.size);\n-\tmemcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);\n-\tfor (i = 0; i < plus.size; i++)\n-\t\tif (isspace(plus.ptr[i]))\n-\t\t\tplus.ptr[i] = '\\n';\n+\t/* currently always true */\n+\tif (!diff_words->word_regex) {\n+\t\t/*\n+\t\t * \"Simple\" word diff: replace all space characters\n+\t\t * with a newline.\n+\t\t *\n+\t\t * This groups together \"words\" of nonspaces on a line\n+\t\t * each, which we then diff using the normal line-diff\n+\t\t * mechanism. It also has the nice property that\n+\t\t * character counts/offsets stay the same.\n+\t\t */\n+\t\tminus.size = diff_words->minus.text.size;\n+\t\tminus.ptr = xmalloc(minus.size);\n+\t\tmemcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);\n+\t\tfor (i = 0; i < minus.size; i++)\n+\t\t\tif (isspace(minus.ptr[i]))\n+\t\t\t\tminus.ptr[i] = '\\n';\n+\n+\t\tplus.size = diff_words->plus.text.size;\n+\t\tplus.ptr = xmalloc(plus.size);\n+\t\tmemcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);\n+\t\tfor (i = 0; i < plus.size; i++)\n+\t\t\tif (isspace(plus.ptr[i]))\n+\t\t\t\tplus.ptr[i] = '\\n';\n+\t}\n+\tdiff_words->minus.current = 0;\n \tdiff_words->plus.current = 0;\n \n+\t/* we want a minimal diff with enough context to run\n+\t * everything into a single hunk */\n \txpp.flags = XDF_NEED_MINIMAL;\n \txecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;\n \txdi_diff_outf(&minus, &plus, fn_out_diff_words_aux, diff_words,\n@@ -432,7 +478,12 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n+\t/* these two are currently free(NULL) */\n+\tfree(diff_words->minus.boundaries);\n+\tfree(diff_words->plus.boundaries);\n \n+\t/* do not forget about a possible final newline that was held\n+\t * back */\n \tif (diff_words->minus.suppressed_newline) {\n \t\tputc('\\n', diff_words->file);\n \t\tdiff_words->minus.suppressed_newline = 0;\n@@ -461,6 +512,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \n \t\tfree (ecbdata->diff_words->minus.text.ptr);\n \t\tfree (ecbdata->diff_words->plus.text.ptr);\n+\t\tfree(ecbdata->diff_words->word_regex);\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\n-- \n1.6.1.269.g0769\n"},{"id":"99941","messageId":"529cd830908f018f796dbc46d3b055c1f8ba9c1b.1231669012.git.trast@student.ethz.ch","threadId":"17058","inReplyTo":"4aea85caafd38a058145c5769fe8a30ffdbd4d13.1231669012.git.trast@student.ethz.ch","subject":"[PATCH v3 2/4] word diff: customizable word splits","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-11T10:27:12Z","receivedAt":"2009-01-11T10:27:12Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Allows for user-configurable word splits when using --color-words.\nThis can make the diff more readable if the regex is configured\naccording to the language of the file.\n\nEach non-overlapping match of the regex is a word; everything in\nbetween is whitespace. We disallow matching the empty string (because\nit results in an endless loop) or a newline (breaks color escapes and\ninteracts badly with the input coming from the usual line diff). To\nhelp the user, we set REG_NEWLINE so that [^...] and . do not match\nnewlines.\n\n--color-words works (and always worked) by splitting words onto one\nline each, and using the normal line-diff machinery to get a word\ndiff. Since we cannot reuse the current approach of simply\noverwriting uninteresting characters with '\\n', we insert an\nartificial '\\n' at the end of each detected word. Its presence must\nbe tracked so that we can distinguish artificial from source newlines.\n\nInsertion of spaces is somewhat subtle. We echo a \"context\" space\ntwice (once on each side of the diff) if it follows directly after a\nword. While this loses a tiny bit of accuracy, it runs together long\nsequences of changed word into one removed and one added block, making\nthe diff much more readable. This feature also means that the\nsplitting regex '\\S+' results in the same output as the original code.\nThe existing code still stays in place in case no regex is provided,\nfor performance.\n\nSigned-off-by: Thomas Rast \n---\n Documentation/diff-options.txt | 16 +++-\n diff.c | 196 +++++++++++++++++++++++++++++++++++++++-\n diff.h | 1 +\n 3 files changed, 206 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 671f533..6152d5b 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -91,8 +91,20 @@ endif::git-format-patch[]\n \tTurn off colored diff, even when the configuration file\n \tgives the default to color output.\n \n---color-words::\n-\tShow colored word diff, i.e. color words which have changed.\n+--color-words[=]::\n+\tShow colored word diff, i.e., color words which have changed.\n+\tBy default, a new word only starts at whitespace, so that a\n+\t'word' is defined as a maximal sequence of non-whitespace\n+\tcharacters. The optional argument can be used to\n+\tconfigure this.\n++\n+The must be an (extended) regular expression. When set, every\n+non-overlapping match of the is considered a word. (Regular\n+expression semantics ensure that quantifiers grab a maximal sequence\n+of characters.) Anything between these matches is considered\n+whitespace and ignored for the purposes of finding differences. You\n+may want to append `|\\S` to your regular expression to make sure that\n+it matches all non-whitespace characters.\n \n --no-renames::\n \tTurn off rename detection, even when the configuration\ndiff --git a/diff.c b/diff.c\nindex f274bf5..badaea6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -316,7 +316,21 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)\n \treturn 0;\n }\n \n-/* unused right now */\n+/*\n+ * We use these to save the word boundaries:\n+ *\n+ * - BODY and END track the extent of a word; after END, an artificial\n+ * newline will be inserted.\n+ *\n+ * - SPACE means the character is a space, and will be replaced by a\n+ * newline. If a space follows immediately after END, it is flagged\n+ * SKIP instead, so that two words with exactly one space in between\n+ * end up with only one newline between them.\n+ *\n+ * - The boundary array is overallocated by one, and the spare element\n+ * is flagged UNDEF to allow peeking over the end of a word to see\n+ * if the next element is a SKIP.\n+ */\n enum diff_word_boundaries {\n \tDIFF_WORD_UNDEF,\n \tDIFF_WORD_BODY,\n@@ -349,12 +363,12 @@ static void diff_words_append(char *line, unsigned long len,\n struct diff_words_data {\n \tstruct diff_words_buffer minus, plus;\n \tFILE *file;\n-\tregex_t *word_regex; /* currently unused */\n+\tregex_t *word_regex;\n };\n \n /*\n * Print 'len' characters from the \"real\" diff data in 'buffer'. Also\n- * returns how many characters were printed (currently always 'len').\n+ * returns how many characters were printed.\n * With 'suppress_newline', we remember a final newline instead of\n * printing it.\n */\n@@ -368,8 +382,27 @@ static int print_word(FILE *file, struct diff_words_buffer *buffer, int len, int\n \t\treturn len;\n \n \tptr = buffer->text.ptr + buffer->current;\n+\n+\tif (buffer->boundaries\n+\t && (buffer->boundaries[buffer->current] == DIFF_WORD_BODY\n+\t\t|| buffer->boundaries[buffer->current] == DIFF_WORD_END)) {\n+\t\t/* drop the artificial newline */\n+\t\tlen--;\n+\t\t/* we still have len>0 because it is a word, and\n+\t\t * scan_word_boundaries() disallows words of length 0. */\n+\t}\n+\n \tbuffer->current += len;\n \n+\t/* Peek past the end (this is safe because we overallocated)\n+\t * to check if the next character was a skipped space. If so,\n+\t * we put it together with the word. */\n+\tif (buffer->boundaries\n+\t && buffer->boundaries[buffer->current] == DIFF_WORD_SKIP) {\n+\t\tbuffer->current++;\n+\t\tlen++;\n+\t}\n+\n \tif (ptr[len - 1] == '\\n') {\n \t\teol = 1;\n \t\tlen--;\n@@ -429,6 +462,141 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+/*\n+ * Fancy word splitting by regex\n+ *\n+ * We search for all non-overlapping matches of 'pattern' in the\n+ * 'buf', and define every match as a (separate) word and all\n+ * unmatched characters as whitespace.\n+ *\n+ * Then we build a new mmfile where each word is on a line of its own.\n+ * Two of these can then be line-diffed to find the word differences\n+ * between the original buffers. Unlike the normal word diff (see\n+ * diff_words_show() below), the transformation does not preserve\n+ * character counts, so we need to keep tracking information in\n+ * buf->boundaries for later use by print_word().\n+ */\n+static void scan_word_boundaries(regex_t *pattern, struct diff_words_buffer *buf,\n+\t\t\t\t mmfile_t *mmfile)\n+{\n+\tchar *text = buf->text.ptr;\n+\tint len = buf->text.size;\n+\tint i = 0;\n+\t/* counts how many extra characters will be inserted into the\n+\t * mmfile */\n+\tint count = 0;\n+\tint ret;\n+\tregmatch_t matches[1];\n+\tint offset, wordlen;\n+\tchar *strz, *p;\n+\n+\t/* overallocate by 1 so we can safely peek past the end for a\n+\t * SKIP, see print_word() */\n+\tbuf->boundaries = xmalloc((len+1) * sizeof(enum diff_word_boundaries));\n+\tbuf->boundaries[len] = DIFF_WORD_UNDEF;\n+\n+\tif (!text) {\n+\t\tmmfile->ptr = NULL;\n+\t\tmmfile->size = 0;\n+\t\treturn;\n+\t}\n+\n+\t/* we unfortunately need a null-terminated copy for regexec */\n+\tstrz = xmalloc(len+1);\n+\tmemcpy(strz, text, len);\n+\tstrz[len] = '\\0';\n+\n+\twhile (i < len) {\n+\t\t/* iteratively match the regex against the rest of the\n+\t\t * input string to find the next word */\n+\t\tret = regexec(pattern, strz+i, 1, matches, 0);\n+\t\tif (ret == REG_NOMATCH) {\n+\t\t\t/* The rest is whitespace. The first space\n+\t\t\t * character is flagged SKIP unless there was\n+\t\t\t * no preceding text at all */\n+\t\t\tif (i > 0 && i < len) {\n+\t\t\t\tbuf->boundaries[i++] = DIFF_WORD_SKIP;\n+\t\t\t\tcount--; /* SKIP characters have no output */\n+\t\t\t}\n+\t\t\twhile (i < len)\n+\t\t\t\tbuf->boundaries[i++] = DIFF_WORD_SPACE;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\toffset = matches[0].rm_so;\n+\n+\t\t/* everything up to the next word is whitespace, using\n+\t\t * the same SKIP rule as above */\n+\t\tif (offset > 0 && i > 0) {\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_SKIP;\n+\t\t\tcount--;\n+\t\t\toffset--;\n+\t\t}\n+\t\twhile (offset-- > 0)\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_SPACE;\n+\n+\t\t/* rm_eo is the first character after the match, so\n+\t\t * this is indeed the number of characters matched */\n+\t\twordlen = matches[0].rm_eo - matches[0].rm_so;\n+\n+\t\t/* all but the last character are BODY */\n+\t\twhile (wordlen > 1) {\n+\t\t\tif (strz[i] == '\\n')\n+\t\t\t\tdie(\"word regex matched a newline before '%s'\",\n+\t\t\t\t strz+i+1);\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_BODY;\n+\t\t\twordlen--;\n+\t\t}\n+\t\t/* the last character is END, we will insert an extra\n+\t\t * '\\n' after it */\n+\t\tif (wordlen > 0) {\n+\t\t\tif (strz[i] == '\\n')\n+\t\t\t\tdie(\"word regex matched a newline before '%s'\",\n+\t\t\t\t strz+i+1);\n+\t\t\tbuf->boundaries[i++] = DIFF_WORD_END;\n+\t\t\tcount++;\n+\t\t} else {\n+\t\t\t/* this would cause an endless loop, so panic */\n+\t\t\tdie(\"word regex matched the empty string at '%s'\",\n+\t\t\t strz+i);\n+\t\t}\n+\t}\n+\n+\tfree(strz);\n+\n+\t/* now build the mmfile. there will be 'count' more characters\n+\t * than in the original */\n+\tmmfile->size = len + count;\n+\tmmfile->ptr = xmalloc(mmfile->size);\n+\tp = mmfile->ptr;\n+\tfor (i = 0; i < len; i++) {\n+\t\tswitch (buf->boundaries[i]) {\n+\t\tcase DIFF_WORD_BODY: /* copy over */\n+\t\t\t*p++ = text[i];\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_END: /* copy and insert an artificial newline */\n+\t\t\t*p++ = text[i];\n+\t\t\t*p++ = '\\n';\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_SPACE: /* replace by '\\n' */\n+\t\t\t*p++ = '\\n';\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_SKIP:\n+\t\t\t/* Ignore. Since the character right before\n+\t\t\t * this one is always an END, another way to\n+\t\t\t * look at it is that we avoid duplicate END\n+\t\t\t * newlines that are already provided by\n+\t\t\t * SPACE */\n+\t\t\tbreak;\n+\t\tcase DIFF_WORD_UNDEF:\n+\t\t\t/* can't happen, but silences a warning */\n+\t\t\tdie(\"can't happen, send test case to git@vger.kernel.org\");\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+\n /* this executes the word diff on the accumulated buffers */\n static void diff_words_show(struct diff_words_data *diff_words)\n {\n@@ -441,7 +609,6 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tmemset(&xpp, 0, sizeof(xpp));\n \tmemset(&xecfg, 0, sizeof(xecfg));\n \n-\t/* currently always true */\n \tif (!diff_words->word_regex) {\n \t\t/*\n \t\t * \"Simple\" word diff: replace all space characters\n@@ -465,6 +632,13 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \t\tfor (i = 0; i < plus.size; i++)\n \t\t\tif (isspace(plus.ptr[i]))\n \t\t\t\tplus.ptr[i] = '\\n';\n+\t} else {\n+\t\t/* Configurable word diff with a regex. See\n+\t\t * scan_word_boundaries() above. */\n+\t\tscan_word_boundaries(diff_words->word_regex,\n+\t\t\t\t &diff_words->minus, &minus);\n+\t\tscan_word_boundaries(diff_words->word_regex,\n+\t\t\t\t &diff_words->plus, &plus);\n \t}\n \tdiff_words->minus.current = 0;\n \tdiff_words->plus.current = 0;\n@@ -478,7 +652,6 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n-\t/* these two are currently free(NULL) */\n \tfree(diff_words->minus.boundaries);\n \tfree(diff_words->plus.boundaries);\n \n@@ -1535,6 +1708,15 @@ static void builtin_diff(const char *name_a,\n \t\t\tecbdata.diff_words =\n \t\t\t\txcalloc(1, sizeof(struct diff_words_data));\n \t\t\tecbdata.diff_words->file = o->file;\n+\t\t\tif (o->word_regex) {\n+\t\t\t\tecbdata.diff_words->word_regex = (regex_t *)\n+\t\t\t\t\txmalloc(sizeof(regex_t));\n+\t\t\t\tif (regcomp(ecbdata.diff_words->word_regex,\n+\t\t\t\t\t o->word_regex,\n+\t\t\t\t\t REG_EXTENDED|REG_NEWLINE))\n+\t\t\t\t\tdie (\"Invalid regular expression: %s\",\n+\t\t\t\t\t o->word_regex);\n+\t\t\t}\n \t\t}\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n \t\t\t &xpp, &xecfg, &ecb);\n@@ -2546,6 +2728,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_CLR(options, COLOR_DIFF);\n \telse if (!strcmp(arg, \"--color-words\"))\n \t\toptions->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;\n+\telse if (!prefixcmp(arg, \"--color-words=\")) {\n+\t\toptions->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;\n+\t\toptions->word_regex = arg + 14;\n+\t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \telse if (!strcmp(arg, \"--quiet\"))\ndiff --git a/diff.h b/diff.h\nindex 4d5a327..23cd90c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -98,6 +98,7 @@ struct diff_options {\n \n \tint stat_width;\n \tint stat_name_width;\n+\tconst char *word_regex;\n \n \t/* this is set by diffcore for DIFF_FORMAT_PATCH */\n \tint found_changes;\n-- \n1.6.1.269.g0769\n"},{"id":"99942","messageId":"72242bd75fa8d55c2afc723f8539ef56f2569d3e.1231669012.git.trast@student.ethz.ch","threadId":"17058","inReplyTo":"529cd830908f018f796dbc46d3b055c1f8ba9c1b.1231669012.git.trast@student.ethz.ch","subject":"[PATCH v3 3/4] word diff: make regex configurable via attributes","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-11T10:27:13Z","receivedAt":"2009-01-11T10:27:13Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Make the --color-words splitting regular expression configurable via\nthe diff driver's 'wordregex' attribute. The user can then set the\ndriver on a file in .gitattributes. If a regex is given on the\ncommand line, it overrides the driver's setting.\n\nWe also provide built-in regexes for some of the languages that\nalready had funcname patterns. (They are designed to run UTF-8\nsequences into a single chunk to make sure they remain readable.)\n\nSigned-off-by: Thomas Rast \n---\n Documentation/diff-options.txt | 4 +++-\n Documentation/gitattributes.txt | 21 +++++++++++++++++++++\n diff.c | 10 ++++++++++\n userdiff.c | 27 +++++++++++++++++++--------\n userdiff.h | 1 +\n 5 files changed, 54 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 6152d5b..d22c06b 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -96,7 +96,9 @@ endif::git-format-patch[]\n \tBy default, a new word only starts at whitespace, so that a\n \t'word' is defined as a maximal sequence of non-whitespace\n \tcharacters. The optional argument can be used to\n-\tconfigure this.\n+\tconfigure this. It can also be set via a diff driver, see\n+\tlinkgit:gitattributes[1]; if a is given explicitly, it\n+\toverrides any diff driver setting.\n +\n The must be an (extended) regular expression. When set, every\n non-overlapping match of the is considered a word. (Regular\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 8af22ec..67f5522 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -334,6 +334,27 @@ patterns are available:\n - `tex` suitable for source code for LaTeX documents.\n \n \n+Customizing word diff\n+^^^^^^^^^^^^^^^^^^^^^\n+\n+You can customize the rules that `git diff --color-words` uses to\n+split words in a line, by specifying an appropriate regular expression\n+in the \"diff.*.wordregex\" configuration variable. For example, in TeX\n+a backslash followed by a sequence of letters forms a command, but\n+several such commands can be run together without intervening\n+whitespace. To separate them, use a regular expression such as\n+\n+------------------------\n+[diff \"tex\"]\n+\twordregex = \"\\\\\\\\[a-zA-Z]+|[{}]|\\\\\\\\.|[^\\\\{} \\t]+\"\n+------------------------\n+\n+Similar to 'xfuncname', a built in value is provided for the drivers\n+`bibtex`, `html`, `java`, `php`, `python` and `tex`. See the\n+documentation of --color-words in linkgit:git-diff[1] for the precise\n+semantics.\n+\n+\n Performing text diffs of binary files\n ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n \ndiff --git a/diff.c b/diff.c\nindex badaea6..c1cc426 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1548,6 +1548,12 @@ static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespe\n \treturn one->driver->funcname.pattern ? &one->driver->funcname : NULL;\n }\n \n+static const char *userdiff_word_regex(struct diff_filespec *one)\n+{\n+\tdiff_filespec_load_driver(one);\n+\treturn one->driver->word_regex;\n+}\n+\n void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)\n {\n \tif (!options->a_prefix)\n@@ -1708,6 +1714,10 @@ static void builtin_diff(const char *name_a,\n \t\t\tecbdata.diff_words =\n \t\t\t\txcalloc(1, sizeof(struct diff_words_data));\n \t\t\tecbdata.diff_words->file = o->file;\n+\t\t\tif (!o->word_regex)\n+\t\t\t\to->word_regex = userdiff_word_regex(one);\n+\t\t\tif (!o->word_regex)\n+\t\t\t\to->word_regex = userdiff_word_regex(two);\n \t\t\tif (o->word_regex) {\n \t\t\t\tecbdata.diff_words->word_regex = (regex_t *)\n \t\t\t\t\txmalloc(sizeof(regex_t));\ndiff --git a/userdiff.c b/userdiff.c\nindex 3681062..7fd9a07 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -6,13 +6,17 @@ static struct userdiff_driver *drivers;\n static int ndrivers;\n static int drivers_alloc;\n \n-#define FUNCNAME(name, pattern) \\\n+#define FUNCNAME(name, pattern)\t\t\t\\\n \t{ name, NULL, -1, { pattern, REG_EXTENDED } }\n+#define PATTERNS(name, pattern, wordregex)\t\t\t\\\n+\t{ name, NULL, -1, { pattern, REG_EXTENDED }, NULL, wordregex }\n static struct userdiff_driver builtin_drivers[] = {\n-FUNCNAME(\"html\", \"^[ \\t]*(<[Hh][1-6][ \\t].*>.*)$\"),\n-FUNCNAME(\"java\",\n+PATTERNS(\"html\", \"^[ \\t]*(<[Hh][1-6][ \\t].*>.*)$\",\n+\t \"[^<>= \\t]+|\\\\S\"),\n+PATTERNS(\"java\",\n \t \"!^[ \\t]*(catch|do|for|if|instanceof|new|return|switch|throw|while)\\n\"\n-\t \"^[ \\t]*(([ \\t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \\t]*\\\\([^;]*)$\"),\n+\t \"^[ \\t]*(([ \\t]*[A-Za-z_][A-Za-z_0-9]*){2,}[ \\t]*\\\\([^;]*)$\",\n+\t \"[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[-+*/]=|\\\\+\\\\+|--|\\\\S|[\\x80-\\xff]+\"),\n FUNCNAME(\"objc\",\n \t /* Negate C statements that can look like functions */\n \t \"!^[ \\t]*(do|for|if|else|return|switch|while)\\n\"\n@@ -27,14 +31,19 @@ FUNCNAME(\"pascal\",\n \t\t\"implementation|initialization|finalization)[ \\t]*.*)$\"\n \t \"\\n\"\n \t \"^(.*=[ \\t]*(class|record).*)$\"),\n-FUNCNAME(\"php\", \"^[\\t ]*((function|class).*)\"),\n-FUNCNAME(\"python\", \"^[ \\t]*((class|def)[ \\t].*)$\"),\n+PATTERNS(\"php\", \"^[\\t ]*((function|class).*)\",\n+\t \"\\\\$?[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[-+*/]=|\\\\+\\\\+|--|->|\\\\S|[\\x80-\\xff]+\"),\n+PATTERNS(\"python\", \"^[ \\t]*((class|def)[ \\t].*)$\",\n+\t \"[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[-+*/]=|//|\\\\S|[\\x80-\\xff]+\"),\n FUNCNAME(\"ruby\", \"^[ \\t]*((class|module|def)[ \\t].*)$\"),\n-FUNCNAME(\"bibtex\", \"(@[a-zA-Z]{1,}[ \\t]*\\\\{{0,1}[ \\t]*[^ \\t\\\"@',\\\\#}{~%]*).*$\"),\n-FUNCNAME(\"tex\", \"^(\\\\\\\\((sub)*section|chapter|part)\\\\*{0,1}\\\\{.*)$\"),\n+PATTERNS(\"bibtex\", \"(@[a-zA-Z]{1,}[ \\t]*\\\\{{0,1}[ \\t]*[^ \\t\\\"@',\\\\#}{~%]*).*$\",\n+\t \"[={}\\\"]|[^={}\\\" \\t]+\"),\n+PATTERNS(\"tex\", \"^(\\\\\\\\((sub)*section|chapter|part)\\\\*{0,1}\\\\{.*)$\",\n+\t \"\\\\\\\\[a-zA-Z@]+|[{}]|\\\\\\\\.|[^\\\\{} \\t]+\"),\n { \"default\", NULL, -1, { NULL, 0 } },\n };\n #undef FUNCNAME\n+#undef PATTERNS\n \n static struct userdiff_driver driver_true = {\n \t\"diff=true\",\n@@ -134,6 +143,8 @@ int userdiff_config(const char *k, const char *v)\n \t\treturn parse_string(&drv->external, k, v);\n \tif ((drv = parse_driver(k, v, \"textconv\")))\n \t\treturn parse_string(&drv->textconv, k, v);\n+\tif ((drv = parse_driver(k, v, \"wordregex\")))\n+\t\treturn parse_string(&drv->word_regex, k, v);\n \n \treturn 0;\n }\ndiff --git a/userdiff.h b/userdiff.h\nindex ba29457..2aab13e 100644\n--- a/userdiff.h\n+++ b/userdiff.h\n@@ -12,6 +12,7 @@ struct userdiff_driver {\n \tint binary;\n \tstruct userdiff_funcname funcname;\n \tconst char *textconv;\n+\tconst char *word_regex;\n };\n \n int userdiff_config(const char *k, const char *v);\n-- \n1.6.1.269.g0769\n"},{"id":"99943","messageId":"0da6ba6dd66a2de84be34f58566d0d6ccbd7e949.1231669012.git.trast@student.ethz.ch","threadId":"17058","inReplyTo":"72242bd75fa8d55c2afc723f8539ef56f2569d3e.1231669012.git.trast@student.ethz.ch","subject":"[PATCH v3 4/4] word diff: test customizable word splits","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-01-11T10:27:14Z","receivedAt":"2009-01-11T10:27:14Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Several tests for regex-configured word splits via command line and\ngitattributes. For good measure we also do a basic test of the\ndefault --color-words since it was so far not covered at all.\n\nSigned-off-by: Thomas Rast \n---\n t/t4033-diff-color-words.sh | 90 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 90 insertions(+), 0 deletions(-)\n create mode 100755 t/t4033-diff-color-words.sh\n\ndiff --git a/t/t4033-diff-color-words.sh b/t/t4033-diff-color-words.sh\nnew file mode 100755\nindex 0000000..536cdac\n--- /dev/null\n+++ b/t/t4033-diff-color-words.sh\n@@ -0,0 +1,90 @@\n+#!/bin/sh\n+\n+\n+test_description='diff --color-words'\n+. ./test-lib.sh\n+\n+cat < test_a\n+foo_bar_baz\n+a qu_ux b c\n+alpha beta gamma delta\n+EOF\n+\n+cat < test_b\n+foo_baz_baz\n+a qu_new_ux b c\n+alpha 4 2 delta\n+EOF\n+\n+# t4026-diff-color.sh tests the color escapes, so we assume they do\n+# not change\n+\n+munge () {\n+ tail -n +5 | tr '\\033' '!'\n+}\n+\n+cat < expect-plain\n+![36m@@ -1,3 +1,3 @@![m\n+![31mfoo_bar_baz![m![32mfoo_baz_baz![m\n+a ![m![31mqu_ux ![m![32mqu_new_ux ![mb ![mc![m\n+alpha ![m![31mbeta ![m![31mgamma ![m![32m4 ![m![32m2 ![mdelta![m\n+EOF\n+\n+test_expect_success 'default settings' '\n+\tgit diff --no-index --color-words test_a test_b |\n+\t\tmunge > actual-plain &&\n+\ttest_cmp expect-plain actual-plain\n+'\n+\n+test_expect_success 'trivial regex yields same as default' '\n+\tgit diff --no-index --color-words=\"\\\\S+\" test_a test_b |\n+\t\tmunge > actual-trivial &&\n+\ttest_cmp expect-plain actual-trivial\n+'\n+\n+cat < expect-chars\n+![36m@@ -1,3 +1,3 @@![m\n+f![mo![mo![m_![mb![ma![m![31mr![m![32mz![m_![mb![ma![mz![m\n+a ![mq![mu![m_![m![32mn![m![32me![m![32mw![m![32m_![mu![mx ![mb ![mc![m\n+a![ml![mp![mh![ma ![m![31mb![m![31me![m![31mt![m![31ma ![m![31mg![m![31ma![m![31mm![m![31mm![m![31ma ![m![32m4 ![m![32m2 ![md![me![ml![mt![ma![m\n+EOF\n+\n+test_expect_success 'character by character regex' '\n+\tgit diff --no-index --color-words=\"\\\\S\" test_a test_b |\n+\t\tmunge > actual-chars &&\n+\ttest_cmp expect-chars actual-chars\n+'\n+\n+cat < expect-nontrivial\n+![36m@@ -1,3 +1,3 @@![m\n+foo![m_![m![31mbar![m![32mbaz![m_![mbaz![m\n+a ![mqu![m_![m![32mnew![m![32m_![mux ![mb ![mc![m\n+alpha ![m![31mbeta ![m![31mgamma ![m![32m4![m![32m ![m![32m2![m![32m ![mdelta![m\n+EOF\n+\n+test_expect_success 'nontrivial regex' '\n+\tgit diff --no-index --color-words=\"[a-z]+|_\" test_a test_b |\n+\t\tmunge > actual-nontrivial &&\n+\ttest_cmp expect-nontrivial actual-nontrivial\n+'\n+\n+test_expect_success 'set a diff driver' '\n+\tgit config diff.testdriver.wordregex \"\\\\S\" &&\n+\tcat < .gitattributes\n+test_* diff=testdriver\n+EOF\n+'\n+\n+test_expect_success 'use default supplied by driver' '\n+\tgit diff --no-index --color-words test_a test_b |\n+\t\tmunge > actual-chars-2 &&\n+\ttest_cmp expect-chars actual-chars-2\n+'\n+\n+test_expect_success 'option overrides default' '\n+\tgit diff --no-index --color-words=\"[a-z]+|_\" test_a test_b |\n+\t\tmunge > actual-nontrivial-2 &&\n+\ttest_cmp expect-nontrivial actual-nontrivial-2\n+'\n+\n+test_done\n-- \n1.6.1.269.g0769\n"},{"id":"99960","messageId":"alpine.DEB.1.00.0901111440410.3586@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"4aea85caafd38a058145c5769fe8a30ffdbd4d13.1231669012.git.trast@student.ethz.ch","subject":"Re: [PATCH v3 1/4] word diff: comments, preparations for regex customization","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-11T13:41:19Z","receivedAt":"2009-01-11T13:41:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Jan 2009, Thomas Rast wrote:\n\n> This reorganizes the code for diff --color-words in a way that will be\n> convenient for the next patch, without changing any of the semantics.\n> The new variables are not used yet except for their default state.\n> \n> We also add some comments on the workings of diff_words_show() and\n> associated helper routines.\n> \n> Signed-off-by: Thomas Rast \n> ---\n\nIt not only got bigger, it still fails to explain the idea to me.\n\nI'll work on it myself, then,\nDscho\n"},{"id":"99983","messageId":"alpine.DEB.1.00.0901111707110.3586@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"4aea85caafd38a058145c5769fe8a30ffdbd4d13.1231669012.git.trast@student.ethz.ch","subject":"Re: [PATCH v3 1/4] word diff: comments, preparations for regex customization","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-11T19:49:30Z","receivedAt":"2009-01-11T19:49:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Jan 2009, Thomas Rast wrote:\n\n\n> +/* unused right now */\n> +enum diff_word_boundaries {\n> +\tDIFF_WORD_UNDEF,\n> +\tDIFF_WORD_BODY,\n> +\tDIFF_WORD_END,\n> +\tDIFF_WORD_SPACE,\n> +\tDIFF_WORD_SKIP\n> +};\n> +\n\nJust to illustrate why I reacted so harshly to this patch series: this \nchange is utterly useless.\n\nYou do not use the word boundaries at all throughout the complete patch. \nThis would have been the only way you could have possibly illustrated how \nthe enum is to be used at all, given that you did not document what the \nenum should do eventually.\n\nWith such a patch that leaves me as clueless as to the way you want to fix \nthe issues as before, I regret having spent the time to look at it at all.\n\nWhat you _should_ have done:\n\n- _not_ introduce the regex. That is not what the first patch should be \n about.\n\n- _use_ the enum for the existing functionality.\n\n- _explain_ what the different values of the enum are about.\n\n- _say_ what the enum pointer points at (is it a single value? an array? \n if it is an array, is it per letter? per word?)\n\nIf you are honest, you'll admit that this patch would leave you puzzled \n_yourself_, 6 months from now.\n\nFWIW I think I have a better patch series, to be sent in a few minutes.\n\nCiao,\nDscho\n"},{"id":"100017","messageId":"7vljthpjsi.fsf@gitster.siamese.dyndns.org","threadId":"17058","inReplyTo":"4aea85caafd38a058145c5769fe8a30ffdbd4d13.1231669012.git.trast@student.ethz.ch","subject":"Re: [PATCH v3 1/4] word diff: comments, preparations for regex customization","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-11T22:19:57Z","receivedAt":"2009-01-11T22:19:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> +/* unused right now */\n> +enum diff_word_boundaries {\n> +\tDIFF_WORD_UNDEF,\n> +\tDIFF_WORD_BODY,\n> +\tDIFF_WORD_END,\n> +\tDIFF_WORD_SPACE,\n> +\tDIFF_WORD_SKIP\n> +};\n\nDon't do this. Please introduce them when you start using them.\n\n> struct diff_words_buffer {\n> \tmmfile_t text;\n> \tlong alloc;\n> \tlong current; /* output pointer */\n> \tint suppressed_newline;\n> +\tenum diff_word_boundaries *boundaries;\n> };\n\nLikewise. Especially because this raises eyebrows \"Huh, a pointer to an\nenum, or perhaps he allocates an array of enums?\" without allowing the\nreader to figure it out much later when the field is actually used.\n\n> static void diff_words_append(char *line, unsigned long len,\n> @@ -339,16 +349,23 @@ static void diff_words_append(char *line, unsigned long len,\n> struct diff_words_data {\n> \tstruct diff_words_buffer minus, plus;\n> \tFILE *file;\n> +\tregex_t *word_regex; /* currently unused */\n> };\n\nI see having this here and keeping it NULL in this patch makes the later\npatch to diff_words_show() more readable, so this probably should stay\nhere.\n\n> -static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n> +/*\n> + * Print 'len' characters from the \"real\" diff data in 'buffer'. Also\n> + * returns how many characters were printed (currently always 'len').\n> + * With 'suppress_newline', we remember a final newline instead of\n> + * printing it.\n> + */\n\n\"... Even in such a case, 'len' that is returned counts the suppressed\nnewline\", or something like that? If you can concisely describe why the\ncaller wants the returned count not match the number of actually printed\nchars (i.e. it includes the suppressed newline), it would help the reader\nunderstand the logic. I am _guessing_ it is because this is called to\nprint matching words from the preimage buffer, and the return value is\nused to skip over the same part in the postimage buffer, and by definition\nthey have to be of the same length (otherwise they won't be matching).\n\n> +static int print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n> \t\tint suppress_newline)\n> {\n> \tconst char *ptr;\n> \tint eol = 0;\n> \n> \tif (len == 0)\n> -\t\treturn;\n> +\t\treturn len;\n> \n> \tptr = buffer->text.ptr + buffer->current;\n> \tbuffer->current += len;\n> @@ -368,18 +385,30 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n> \t\telse\n> \t\t\tputc('\\n', file);\n> \t}\n> +\n> +\t/* we need to return how many chars to skip on the other side,\n> +\t * so account for the (held off) \\n */\n\nMulti-line comment style? I won't repeat this but you have many...\n\n> +\treturn len+eol;\n> }\n> \n> +/*\n> + * Callback for word diff output\n> + */\n\nWithout saying \"to do what\", the comment adds more noise than signal.\n\"Called to parse diff between pre- and post- image files converted into\none-word-per-line format and concatenate them to into lines by dropping\nsome of the end-of-lines but keeping some others\", or something like that?\n\nThanks.\n"},{"id":"100016","messageId":"7vfxjppjs8.fsf@gitster.siamese.dyndns.org","threadId":"17058","inReplyTo":"529cd830908f018f796dbc46d3b055c1f8ba9c1b.1231669012.git.trast@student.ethz.ch","subject":"Re: [PATCH v3 2/4] word diff: customizable word splits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-11T22:20:07Z","receivedAt":"2009-01-11T22:20:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Allows for user-configurable word splits when using --color-words.\n> This can make the diff more readable if the regex is configured\n> according to the language of the file.\n>\n> Each non-overlapping match of the regex is a word; everything in\n> between is whitespace.\n\nWhat happens if the input \"language\" does not have any inter-word spacing\nbut its words can still be expressed by regexp patterns?\n\nImagineALanguageThatAllowsYouToWriteSomethingLikeThis. Does the mechanism\nhelp users who want to do word-diff files written in such a language by\noutputting:\n\n\tImagineALanguageThatWhichAllowsYou...\n\nwhen '[A-Z][a-z]*' is given by the word pattern?\n\n> We disallow matching the empty string (because\n> it results in an endless loop) or a newline (breaks color escapes and\n> interacts badly with the input coming from the usual line diff). To\n> help the user, we set REG_NEWLINE so that [^...] and . do not match\n> newlines.\n\nAndImagineALanguageWhoseWordStruc\ntureDoesNotCareAboutLineBreak\n\nCan you help users with such payload?\n\n\tSide note. Yes, I am coming from Japanese background.\n\n Side note 2. No, I am not saying your code must support both of\n the above to be acceptable. I am just gauging the design\n assumptions and limitations.\n\n> Insertion of spaces is somewhat subtle. We echo a \"context\" space\n> twice (once on each side of the diff) if it follows directly after a\n> word. While this loses a tiny bit of accuracy, it runs together long\n> sequences of changed word into one removed and one added block, making\n> the diff much more readable.\n\nI guess this part can be later enhanced to be more precise, so that it\nkeeps the original context space more faithfully (i.e. does not lose two\nconsecutive spaces in the original occidental script, and does not insert\nany extra space to the oriental script), if we were to support the second\nexample I gave above in the future as a follow-up patch.\n\n> +--color-words[=]::\n> +\tShow colored word diff, i.e., color words which have changed.\n> +\tBy default, a new word only starts at whitespace, so that a\n> +\t'word' is defined as a maximal sequence of non-whitespace\n> +\tcharacters. The optional argument can be used to\n> +\tconfigure this.\n> ++\n> +The must be an (extended) regular expression. When set, every\n> +non-overlapping match of the is considered a word. (Regular\n> +expression semantics ensure that quantifiers grab a maximal sequence\n> +of characters.) Anything between these matches is considered\n> +whitespace and ignored for the purposes of finding differences. You\n> +may want to append `|\\S` to your regular expression to make sure that\n> +it matches all non-whitespace characters.\n\nWhose regexp library do we assume here? Traditionally we limited\nourselves to POSIX BRE, and I do not think anybody minds using POSIX ERE\nhere, but we need to be clear. In either case \\S is a pcre outside\nPOSIX.\n\nThe rest I only skimmed but did not spot anything glaringly wrong; thanks.\n"},{"id":"100031","messageId":"7veiz9o2ek.fsf@gitster.siamese.dyndns.org","threadId":"17058","inReplyTo":"72242bd75fa8d55c2afc723f8539ef56f2569d3e.1231669012.git.trast@student.ethz.ch","subject":"Re: [PATCH v3 3/4] word diff: make regex configurable via attributes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-11T23:20:51Z","receivedAt":"2009-01-11T23:20:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Make the --color-words splitting regular expression configurable via\n> the diff driver's 'wordregex' attribute. The user can then set the\n> driver on a file in .gitattributes. If a regex is given on the\n> command line, it overrides the driver's setting.\n> ...\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 6152d5b..d22c06b 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -96,7 +96,9 @@ endif::git-format-patch[]\n> \tBy default, a new word only starts at whitespace, so that a\n> \t'word' is defined as a maximal sequence of non-whitespace\n> \tcharacters. The optional argument can be used to\n> -\tconfigure this.\n> +\tconfigure this. It can also be set via a diff driver, see\n> +\tlinkgit:gitattributes[1]; if a is given explicitly, it\n> +\toverrides any diff driver setting.\n> +\n> The must be an (extended) regular expression. When set, every\n> non-overlapping match of the is considered a word. (Regular\n\nOne bikeshedding I think is better to get over with is that this probably\nshould be called xwordregex for consistency with xfuncname where 'x'\nvariant means POSIX ERE and the one without means POSIX BRE, even if we\nare not going to support diff.wordregex that uses BRE.\n\nI am assuming [3/4] can be trivially adjusted if we were to adopt the\nclean-up approach Dscho is taking?\n"},{"id":"100198","messageId":"200901130059.19511.jnareb@gmail.com","threadId":"17058","inReplyTo":"alpine.DEB.1.00.0901101507590.30769@pacific.mpi-cbg.de","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-12T23:59:19Z","receivedAt":"2009-01-12T23:59:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Hello!\n\nOn Sat, 10 Jan 2009, Johannes Schindelin wrote:\n> On Sat, 10 Jan 2009, Jakub Narebski wrote:\n>> On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n>>> On Sat, 10 Jan 2009, Jakub Narebski wrote:\n>>>> Thomas Rast wrote:\n>>>> \n>>>>> --color-words works (and always worked) by splitting words onto one\n>>>>> line each, and using the normal line-diff machinery to get a word\n>>>>> diff. \n>>>> \n>>>> Cannot we generalize diff machinery / use underlying LCS diff engine\n>>>> instead of going through line diff?\n>>> \n>>> What do you think we're doing? libxdiff is pretty hardcoded to newlines. \n>>> That's why we're substituting non-word characters with newlines.\n>> \n>> Isn't Meyers algorithm used by libxdiff based on LCS, largest common\n>> subsequence, and doesn't it generate from the mathematical point of\n>> view \"diff\" between two sequences (two arrays) which just happen to\n>> be lines? It is a bit strange that libxdiff doesn't export its low\n>> level algorithm...\n> \n> Umm.\n> \n> It _is_ Myers' algorithm. It just so happens that libxdiff hardcodes \n> newline to be the separator.\n\nSo amd I to understand that _exported_ functions hardcode separator\nto be newline (most probably for performance), and there is no function\nin libxdiff which calculates LCS, or returns diff for arrays\n(sequences)?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"100204","messageId":"alpine.DEB.1.00.0901130140360.3586@pacific.mpi-cbg.de","threadId":"17058","inReplyTo":"200901130059.19511.jnareb@gmail.com","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-13T00:40:49Z","receivedAt":"2009-01-13T00:40:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Jan 2009, Jakub Narebski wrote:\n\n> On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n> > On Sat, 10 Jan 2009, Jakub Narebski wrote:\n> >> On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n> >>> On Sat, 10 Jan 2009, Jakub Narebski wrote:\n> >>>> Thomas Rast wrote:\n> >>>> \n> >>>>> --color-words works (and always worked) by splitting words onto one\n> >>>>> line each, and using the normal line-diff machinery to get a word\n> >>>>> diff. \n> >>>> \n> >>>> Cannot we generalize diff machinery / use underlying LCS diff engine\n> >>>> instead of going through line diff?\n> >>> \n> >>> What do you think we're doing? libxdiff is pretty hardcoded to newlines. \n> >>> That's why we're substituting non-word characters with newlines.\n> >> \n> >> Isn't Meyers algorithm used by libxdiff based on LCS, largest common\n> >> subsequence, and doesn't it generate from the mathematical point of\n> >> view \"diff\" between two sequences (two arrays) which just happen to\n> >> be lines? It is a bit strange that libxdiff doesn't export its low\n> >> level algorithm...\n> > \n> > Umm.\n> > \n> > It _is_ Myers' algorithm. It just so happens that libxdiff hardcodes \n> > newline to be the separator.\n> \n> So amd I to understand that _exported_ functions hardcode separator\n> to be newline (most probably for performance), and there is no function\n> in libxdiff which calculates LCS, or returns diff for arrays\n> (sequences)?\n\nThat is my understanding, yes.\n\nCiao,\nDscho\n"},{"id":"100207","messageId":"200901130152.24401.jnareb@gmail.com","threadId":"17058","inReplyTo":"alpine.DEB.1.10.0901100950230.21891@alien.or.mcafeemobile.com","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-01-13T00:52:23Z","receivedAt":"2009-01-13T00:52:23Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 10 Jan 2009, Davide Libenzi wrote:\n> On Sat, 10 Jan 2009, Jakub Narebski wrote:\n>> On Sat, 10 Jan 2009, Johannes Schindelin wrote:\n>>> On Sat, 10 Jan 2009, Jakub Narebski wrote:\n>>>> Thomas Rast wrote:\n>>>> \n>>>>> --color-words works (and always worked) by splitting words onto one\n>>>>> line each, and using the normal line-diff machinery to get a word\n>>>>> diff. \n>>>> \n>>>> Cannot we generalize diff machinery / use underlying LCS diff engine\n>>>> instead of going through line diff?\n>>> \n>>> What do you think we're doing? libxdiff is pretty hardcoded to newlines. \n>>> That's why we're substituting non-word characters with newlines.\n>> \n>> Isn't Meyers algorithm used by libxdiff based on LCS, largest common\n>> subsequence, and doesn't it generate from the mathematical point of\n>> view \"diff\" between two sequences (two arrays) which just happen to\n>> be lines? It is a bit strange that libxdiff doesn't export its low\n>> level algorithm...\n> \n> The core doesn't know anything about lines. Only pre-processing (setting \n> up the hash by tokenizing the input) and post-processing (adding '\\n' to \n> the end of each token), knows about newlines. Memory consumption would \n> increase significantly though, since there is a per-token cost, and a \n> word-based diff will create more of them WRT the same input.\n\nIs this core algorithm available as some exported function in libxdiff?\nI mean would it be easy to replace default line tokenizer (per-line\npre-processing) and post-processing to better deal with word diff?\n\nThe other side would be to generate per-paragraph diffs (with empty\nline being separator)...\n-- \nJakub Narebski\nPoland\n"},{"id":"100297","messageId":"alpine.DEB.1.10.0901131045470.14865@alien.or.mcafeemobile.com","threadId":"17058","inReplyTo":"200901130152.24401.jnareb@gmail.com","subject":"Re: [PATCH v2] make diff --color-words customizable","fromName":"Davide Libenzi","fromEmail":"davidel@xmailserver.org","sentAt":"2009-01-13T18:50:29Z","receivedAt":"2009-01-13T18:50:29Z","isPatch":true,"sender":{"key":"davidel@xmailserver.org","avatar":null},"body":"On Tue, 13 Jan 2009, Jakub Narebski wrote:\n\n> Is this core algorithm available as some exported function in libxdiff?\n> I mean would it be easy to replace default line tokenizer (per-line\n> pre-processing) and post-processing to better deal with word diff?\n\nIn libxdiff, no. I hadn't thought to export the raw Meyer algo, and nobody \never asked before.\n\n\n> The other side would be to generate per-paragraph diffs (with empty\n> line being separator)...\n\nIn Git I guess you can use it to generate other kind of diffs. I don't see \nany problems with that. Just requires more memory WRT a line based one.\n\n\n- Davide\n"}]}