{"thread":{"id":"13350","subject":"[PATCH] Make words boundary for --color-words configurable","startedAt":"2008-05-02T03:39:24Z","lastAt":"2008-05-13T01:42:55Z","messageCount":92,"participants":["Ping Yin","Junio C Hamano","Johannes Schindelin","Teemu Likonen","Jakub Narebski","Dirk Süsserott","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"75797","messageId":"1209699564-2800-1-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":null,"subject":"[PATCH] Make words boundary for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T03:39:24Z","receivedAt":"2008-05-02T03:39:24Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Previously --color-words only allow spaces as words boundary. However,\njust space is not enough. For example, when i rename a function from\nfoo to bar, following example doesn't show as expected when using\n--color-words.\n\n------------------\n- if (foo(arg))\n+ if (bar(arg))\n------------------\n\nIt shows as \"if <r>(foo(arg))</r><g>(foo(arg))</g>\". Actually, it's the\nbest to show as \"if (<r>foo</r><g>bar</g>(arg))\". Here \"r\" and \"g\"\nrepresent \"red\" and \"green\" separately.\n\nThis patch introduces a configuration diff.wordsboundary to make\n--color-words treat both spaces and characters in diff.wordsboundary as\nboundary characters.\n\nIf we set diff.wordsboundary to \"()\", the example above will show as\n\"if (<r>foo(</r><g>bar(</g>arg))\". It's much better, athough not the best,\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   14 ++++++++++++--\n 1 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 3632b55..2c37bb6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -23,6 +23,7 @@ static int diff_rename_limit_default = 100;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n+static const char *diff_words_boundary = \"\";\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \t\"\\033[m\",\t/* reset */\n@@ -159,6 +160,10 @@ int git_diff_ui_config(const char *var, const char *value)\n \t\texternal_diff_cmd_cfg = xstrdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"diff.wordsboundary\")) {\n+\t\tdiff_words_boundary = value ? xstrdup(value) : \"\";\n+\t\treturn 0;\n+\t}\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \n@@ -434,6 +439,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static int iswordsboundary(char c)\n+{\n+\treturn isspace(c) || !!strchr(diff_words_boundary, c);\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@@ -448,7 +458,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (iswordsboundary(minus.ptr[i]))\n \t\t\tminus.ptr[i] = '\\n';\n \tdiff_words->minus.current = 0;\n \n@@ -456,7 +466,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (iswordsboundary(plus.ptr[i]))\n \t\t\tplus.ptr[i] = '\\n';\n \tdiff_words->plus.current = 0;\n \n-- \n1.5.5.1.116.ge4b9c.dirty\n"},{"id":"75798","messageId":"7vprs5xsvi.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"1209699564-2800-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-02T03:54:57Z","receivedAt":"2008-05-02T03:54:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> Previously --color-words only allow spaces as words boundary. However,\n> just space is not enough. For example, when i rename a function from\n> foo to bar, following example doesn't show as expected when using\n> --color-words.\n>\n> ------------------\n> - if (foo(arg))\n> + if (bar(arg))\n> ------------------\n>\n> It shows as \"if <r>(foo(arg))</r><g>(foo(arg))</g>\". Actually, it's the\n> best to show as \"if (<r>foo</r><g>bar</g>(arg))\". Here \"r\" and \"g\"\n> represent \"red\" and \"green\" separately.\n>\n> This patch introduces a configuration diff.wordsboundary to make\n> --color-words treat both spaces and characters in diff.wordsboundary as\n> boundary characters.\n\nJust an idle thought.\n\nI suspect a more natural definition of word boundary is between a run of\nword characters and a run of non-word characters.  IOW, instead of saying\n\" \" and \"(these other)\" characters are boundary, you would say\n\n\tif (foo(arg))\n\nbetween f and space, open paren and f, second o and open paren, that open\nparen and a, ... are boundaries.\n\nIf you go that route, you would make the definition of \"what is the set of\nword characters\" configurable.\n"},{"id":"75799","messageId":"46dff0320805012128l6cb15e1ekd40f84a9eac724d1@mail.gmail.com","threadId":"13350","inReplyTo":"7vprs5xsvi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T04:28:38Z","receivedAt":"2008-05-02T04:28:38Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Fri, May 2, 2008 at 11:54 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ping Yin <pkufranky@gmail.com> writes:\n>\n\n>  I suspect a more natural definition of word boundary is between a run of\n>  word characters and a run of non-word characters.  IOW, instead of saying\n>  \" \" and \"(these other)\" characters are boundary, you would say\n>\n>         if (foo(arg))\n>\n>  between f and space, open paren and f, second o and open paren, that open\n>  paren and a, ... are boundaries.\n>\n>  If you go that route, you would make the definition of \"what is the set of\n>  word characters\" configurable.\n\nI prefer nonwordcharacters to wordcharacters. With\ndiff.wordcharacters, to maintain the same behaviour as before, i have\nto define a long list of characters. While with\ndiff.nonwordcharacters, i need only define it to \"\".\n\nSo how about just renaming wordsboundary to nonwordcharacters?\n\n\n\n\n\n-- \nPing Yin\n"},{"id":"75809","messageId":"alpine.DEB.1.00.0805020839200.2691@eeepc-johanness","threadId":"13350","inReplyTo":"1209699564-2800-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-02T07:45:33Z","receivedAt":"2008-05-02T07:45:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 2 May 2008, Ping Yin wrote:\n\n> Previously --color-words only allow spaces as words boundary. However, \n> just space is not enough. For example, when i rename a function from foo \n> to bar, following example doesn't show as expected when using \n> --color-words.\n\nThanks for starting this.\n\nHowever, as Junio pointed out, it is easier to specify word-characters, \nrather than non-word characters (think TAB), and...\n\n> +static int iswordsboundary(char c)\n> +{\n> +\treturn isspace(c) || !!strchr(diff_words_boundary, c);\n> +}\n\nthis will be called quite some times.  So it would make more sense to have \nan \"unsigned char word_characters[256]\" and set those entries to 1 that \nare to be interpreted as word characters.\n\nThis would allow you also to interpret \"0-9A-Za-z\" correctly.\n\nOh, and maybe having \"::default\" and \"::alnum\" suffixes interpreted, so \nthat I can say \"_::alnum\" to have C identifiers interpreted as words?\n\nThanks,\nDscho\n"},{"id":"75811","messageId":"20080502081408.GA11420@mithlond.arda.local","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805020839200.2691@eeepc-johanness","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-02T08:14:08Z","receivedAt":"2008-05-02T08:14:08Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Johannes Schindelin wrote (2008-05-02 08:45 +0100):\n\n> On Fri, 2 May 2008, Ping Yin wrote:\n> \n> > Previously --color-words only allow spaces as words boundary.\n> > However, just space is not enough. For example, when i rename\n> > a function from foo to bar, following example doesn't show as\n> > expected when using --color-words.\n> \n> Thanks for starting this.\n> \n> However, as Junio pointed out, it is easier to specify\n> word-characters, rather than non-word characters (think TAB), and...\n\nJust a quick note from someone who is not so much a programmer but who\nuses Git to track text/LaTex/etc. files with human languages: Please\ndon't make this kind of things too Ascii-specific and too much\nbyte-is-interpreted-as-character type thing. \n\nIn general, my opinion is that with international text it's better to\ndefine word boundary characters than trying to maintain a _huge_ list of\ncharacters used within words in different human languages.\n"},{"id":"75814","messageId":"46dff0320805020223g497bee82o7bd21d530df9f6de@mail.gmail.com","threadId":"13350","inReplyTo":"20080502081408.GA11420@mithlond.arda.local","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T09:23:10Z","receivedAt":"2008-05-02T09:23:10Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Fri, May 2, 2008 at 4:14 PM, Teemu Likonen <tlikonen@iki.fi> wrote:\n> Johannes Schindelin wrote (2008-05-02 08:45 +0100):\n>\n\n>  In general, my opinion is that with international text it's better to\n>  define word boundary characters than trying to maintain a _huge_ list of\n>  characters used within words in different human languages.\n>\n\nAgreed, there is no easy way to designate such a huge list of\nnon-ascii characters. Even if we figure out one way, the user may\nstill be scared by such a huge list or the character class syntax.\n\nI think doing this complex both the implementation and the representation.\n\nInstead, if using non word characters, the user only needs specify a\nsmall list of characters (ascii or wide character). instead of the\nnearly whole set.\n\n\n-- \nPing Yin\n"},{"id":"75816","messageId":"46dff0320805020228v61c452f4y7f6c2e92cb9b4ea8@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805020839200.2691@eeepc-johanness","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T09:28:29Z","receivedAt":"2008-05-02T09:28:29Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Fri, May 2, 2008 at 3:45 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n\n>\n>  However, as Junio pointed out, it is easier to specify word-characters,\n>  rather than non-word characters (think TAB), and...\n>\n\nWe don't have to designate TAB. i think space characters should always\nbe word boundary.\n\n\n\n-- \nPing Yin\n"},{"id":"75819","messageId":"20080502100154.GB11420@mithlond.arda.local","threadId":"13350","inReplyTo":"20080502081408.GA11420@mithlond.arda.local","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-02T10:01:54Z","receivedAt":"2008-05-02T10:01:54Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Teemu Likonen wrote (2008-05-02 11:14 +0300):\n\n> > On Fri, 2 May 2008, Ping Yin wrote:\n> > \n> > > Previously --color-words only allow spaces as words boundary.\n> > > However, just space is not enough. For example, when i rename\n> > > a function from foo to bar, following example doesn't show as\n> > > expected when using --color-words.\n\n> Just a quick note from someone who is not so much a programmer but who\n> uses Git to track text/LaTex/etc. files with human languages: \n\nAnd let me add a sidenote that, at least to me, --color-words is the\ntool for doing diffs of human language text files. I think it's a lot\nmore useful than normal diff.\n"},{"id":"75842","messageId":"1209736766-8029-1-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"46dff0320805012128l6cb15e1ekd40f84a9eac724d1@mail.gmail.com","subject":"[PATCH] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T13:59:26Z","receivedAt":"2008-05-02T13:59:26Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Previously --color-words only allow spaces as boundary characters.\nHowever, just space is not enough. For example, when i rename a function\nfrom foo to bar, following example doesn't show as expected when using\n--color-words.\n\n------------------\n- if (foo(arg))\n+ if (bar(arg))\n------------------\n\nIt shows as \"if <r>(foo(arg))</r><g>(foo(arg))</g>\". Actually, it's the\nbest to show as \"if (<r>foo</r><g>bar</g>(arg))\". Here \"r\" and \"g\"\nrepresent \"red\" and \"green\" separately.\n\nThis patch introduces a configuration diff.nonwordchars to make\n--color-words treat both spaces and characters in diff.nonwordchars as\nboundary characters.\n\nIf we set diff.nonwordchars to \"()\", the example above will show as\n\"if (<r>foo(</r><g>bar(</g>arg))\". It's much better, athough not the best,\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n>>  In general, my opinion is that with international text it's better to\n>>  define word boundary characters than trying to maintain a _huge_ list of\n>>  characters used within words in different human languages.\n>>\n>\n> Agreed, there is no easy way to designate such a huge list of\n> non-ascii characters. Even if we figure out one way, the user may\n> still be scared by such a huge list or the character class syntax.\n> \n> I think doing this complex both the implementation and the representation.\n> \n> Instead, if using non word characters, the user only needs specify a\n> small list of characters (ascii or wide character). instead of the\n> nearly whole set.\n\nTwo changes:\n\n* Rename diff.wordsboundary to diff.nonwordchars\n* Add documentation\n\n Documentation/config.txt |    4 ++++\n diff.c                   |   14 ++++++++++++--\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 824e416..eb05592 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -546,6 +546,10 @@ diff.renames::\n \twill enable basic rename detection.  If set to \"copies\" or\n \t\"copy\", it will detect copies, as well.\n \n+diff.nonwordchars::\n+    Specify additional boundary characters other than spaces for\n+    --color-words.\n+\n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\n \ttransfer is below this\ndiff --git a/diff.c b/diff.c\nindex 3632b55..e48466d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -23,6 +23,7 @@ static int diff_rename_limit_default = 100;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n+static const char *diff_non_word_chars = \"\";\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \t\"\\033[m\",\t/* reset */\n@@ -159,6 +160,10 @@ int git_diff_ui_config(const char *var, const char *value)\n \t\texternal_diff_cmd_cfg = xstrdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"diff.nonwordchars\")) {\n+\t\tdiff_non_word_chars = value ? xstrdup(value) : \"\";\n+\t\treturn 0;\n+\t}\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \n@@ -434,6 +439,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static int is_non_word_char(char c)\n+{\n+\treturn isspace(c) || !!strchr(diff_non_word_chars, c);\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@@ -448,7 +458,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (is_non_word_char(minus.ptr[i]))\n \t\t\tminus.ptr[i] = '\\n';\n \tdiff_words->minus.current = 0;\n \n@@ -456,7 +466,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (is_non_word_char(plus.ptr[i]))\n \t\t\tplus.ptr[i] = '\\n';\n \tdiff_words->plus.current = 0;\n \n-- \n1.5.5.1.117.g73010\n"},{"id":"75845","messageId":"46dff0320805020726y2592732cj9aef0111e5b2288a@mail.gmail.com","threadId":"13350","inReplyTo":"1209736766-8029-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T14:26:45Z","receivedAt":"2008-05-02T14:26:45Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Fri, May 2, 2008 at 9:59 PM, Ping Yin <pkufranky@gmail.com> wrote:\n> Previously --color-words only allow spaces as boundary characters.\n>  However, just space is not enough. For example, when i rename a function\n>  from foo to bar, following example doesn't show as expected when using\n>  --color-words.\n>\n>  ------------------\n>  - if (foo(arg))\n>  + if (bar(arg))\n>  ------------------\n>\n>  It shows as \"if <r>(foo(arg))</r><g>(foo(arg))</g>\". Actually, it's the\n>  best to show as \"if (<r>foo</r><g>bar</g>(arg))\". Here \"r\" and \"g\"\n>  represent \"red\" and \"green\" separately.\n>\n>  This patch introduces a configuration diff.nonwordchars to make\n>  --color-words treat both spaces and characters in diff.nonwordchars as\n>  boundary characters.\n>\n>  If we set diff.nonwordchars to \"()\", the example above will show as\n>  \"if (<r>foo(</r><g>bar(</g>arg))\". It's much better, athough not the best,\n>\n\nOh, there are some problems, assuming \"{}\" are set as diff.nonwordchars\n\n1. Trailing boundary character lost, for example\n----------------------------\n$ git diff-\n- foo{\n+ foo\n$ git diff --color-words\nfoo\n----------------------------\nWith --color-words,  i can't know the trailing '{' is removed. This\nproblem exists even without my patch. In that case, only trainling\nspaces are  lost.\n\n2. Trailing removed words shows at new line instead of the same line\n----------------------------\n$ git diff\n- foo bar\n+ foo\n(note: no space after foo)\n$ git diff --color-words\nfoo\n<red>bar</red>\n--------------------------------\nbar should show in the same line with bar. This is not related to my patch.\n\n\n-- \nPing Yin\n"},{"id":"75846","messageId":"46dff0320805020727x7609826cm75d91f5c525dd5a2@mail.gmail.com","threadId":"13350","inReplyTo":"46dff0320805020726y2592732cj9aef0111e5b2288a@mail.gmail.com","subject":"Re: [PATCH] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-02T14:27:43Z","receivedAt":"2008-05-02T14:27:43Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Fri, May 2, 2008 at 10:26 PM, Ping Yin <pkufranky@gmail.com> wrote:\n\n>  bar should show in the same line with bar. This is not related to my patch.\n\ns/with bar/with foo/\n\n\n-- \nPing Yin\n"},{"id":"75848","messageId":"20080502143650.GB3079@mithlond.arda.local","threadId":"13350","inReplyTo":"1209736766-8029-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Make boundary characters for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-02T14:36:50Z","receivedAt":"2008-05-02T14:36:50Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Ping Yin wrote (2008-05-02 21:59 +0800):\n\n> Previously --color-words only allow spaces as boundary characters.\n> However, just space is not enough. For example, when i rename\n> a function from foo to bar, following example doesn't show as expected\n> when using --color-words.\n\n> Two changes:\n> \n> * Rename diff.wordsboundary to diff.nonwordchars\n> * Add documentation\n\nI think config variables should be in alphabetical order in config.txt.\nHence your diff.nonwordchars is not in the right place. Other than that\nthis patch seems to work and is really useful to me. Thanks.\n"},{"id":"75895","messageId":"m34p9g6y09.fsf@localhost.localdomain","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805020839200.2691@eeepc-johanness","subject":"Re: [PATCH] Make words boundary for --color-words configurable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-03T00:18:31Z","receivedAt":"2008-05-03T00:18:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Oh, and maybe having \"::default\" and \"::alnum\" suffixes interpreted, so \n> that I can say \"_::alnum\" to have C identifiers interpreted as words?\n\nI'd rather use POSIX classes like \"[:alnum:]\" for that\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"75896","messageId":"1209774178-26552-1-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"20080502143650.GB3079@mithlond.arda.local","subject":"Re: [PATCH] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T00:22:58Z","receivedAt":"2008-05-03T00:22:58Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n> I think config variables should be in alphabetical order in config.txt.\n> Hence your diff.nonwordchars is not in the right place\n\nTHX, this is fixing patch\n\n Documentation/config.txt |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex eb05592..812ec2c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -537,6 +537,10 @@ diff.external::\n \tprogram only on a subset of your files, you might want to\n \tuse linkgit:gitattributes[5] instead.\n \n+diff.nonwordchars::\n+\tSpecify additional boundary characters other than spaces for\n+\t--color-words.\n+\n diff.renameLimit::\n \tThe number of files to consider when performing the copy/rename\n \tdetection; equivalent to the git diff option '-l'.\n@@ -546,10 +550,6 @@ diff.renames::\n \twill enable basic rename detection.  If set to \"copies\" or\n \t\"copy\", it will detect copies, as well.\n \n-diff.nonwordchars::\n-    Specify additional boundary characters other than spaces for\n-    --color-words.\n-\n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\n \ttransfer is below this\n-- \n1.5.5.1.117.g73010\n"},{"id":"75911","messageId":"1209815828-6548-1-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"46dff0320805020726y2592732cj9aef0111e5b2288a@mail.gmail.com","subject":"[PATCH v2 0/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T11:57:03Z","receivedAt":"2008-05-03T11:57:03Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Ping Yin (5):\n      diff.c: Remove code redundancy in diff_words_show\n      diff.c: Use show variable name in fn_out_diff_words_aux\n      diff.c: Fix --color-words showing trailing deleted words at another line\n      Make boundary characters for --color-words configurable\n      fn_out_diff_words_aux: Handle common diff line more carefully\n\n Documentation/config.txt       |    4 ++\n Documentation/diff-options.txt |    1 +\n diff.c                         |   83 +++++++++++++++++++++++++++-------------\n 3 files changed, 61 insertions(+), 27 deletions(-)\n\nThe first two patches are just code refactor\n\nThe 3rd patch fixes following problem 2\nThe 4th patch introduces diff.nonwordchars\nThe 5th patch fixes following problem 1\n\n> Oh, there are some problems, assuming \"{}\" are set as diff.nonwordchars\n> \n> 1. Trailing boundary character lost, for example\n> ----------------------------\n> $ git diff-\n> - foo{\n> + foo\n> $ git diff --color-words\n> foo\n> ----------------------------\n> With --color-words,  i can't know the trailing '{' is removed. This\n> problem exists even without my patch. In that case, only trainling\n> spaces are  lost.\n> \n> 2. Trailing removed words shows at new line instead of the same line\n> ----------------------------\n> $ git diff\n> - foo bar\n> + foo\n> (note: no space after foo)\n> $ git diff --color-words\n> foo\n> <red>bar</red>\n> --------------------------------\n> bar should show in the same line with bar. This is not related to my patch.\n"},{"id":"75914","messageId":"1209815828-6548-2-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH v2 1/5] diff.c: Remove code redundancy in diff_words_show","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T11:57:04Z","receivedAt":"2008-05-03T11:57:04Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Introduce mmfile_copy_set_boundary to do the repeated work.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   26 +++++++++++++-------------\n 1 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 3632b55..acef138 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -434,6 +434,17 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n+\tint i;\n+\n+\tdest->size = src->size;\n+\tdest->ptr = xmalloc(dest->size);\n+\tmemcpy(dest->ptr, src->ptr, dest->size);\n+\tfor (i = 0; i < dest->size; i++)\n+\t\tif (isspace(dest->ptr[i]))\n+\t\t\tdest->ptr[i] = '\\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@@ -444,20 +455,9 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tint i;\n \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+\tmmfile_copy_set_boundary(&minus, &(diff_words->minus.text));\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+\tmmfile_copy_set_boundary(&plus, &(diff_words->plus.text));\n \tdiff_words->plus.current = 0;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75910","messageId":"1209815828-6548-3-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-2-git-send-email-pkufranky@gmail.com","subject":"[PATCH v2 2/5] diff.c: Use show variable name in fn_out_diff_words_aux","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T11:57:05Z","receivedAt":"2008-05-03T11:57:05Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   26 +++++++++++++++-----------\n 1 files changed, 15 insertions(+), 11 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex acef138..b5f7141 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -408,28 +408,32 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\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+\tstruct diff_words_data *diff_words;\n+\tstruct diff_words_buffer *dm, *dp;\n+\tFILE *df;\n \n-\tif (diff_words->minus.suppressed_newline) {\n+\tdiff_words = priv;\n+\tdm = &(diff_words->minus);\n+\tdp = &(diff_words->plus);\n+\tdf = diff_words->file;\n+\n+\tif (dm->suppressed_newline) {\n \t\tif (line[0] != '+')\n-\t\t\tputc('\\n', diff_words->file);\n-\t\tdiff_words->minus.suppressed_newline = 0;\n+\t\t\tputc('\\n', df);\n+\t\tdm->suppressed_newline = 0;\n \t}\n \n \tlen--;\n \tswitch (line[0]) {\n \t\tcase '-':\n-\t\t\tprint_word(diff_words->file,\n-\t\t\t\t   &diff_words->minus, len, DIFF_FILE_OLD, 1);\n+\t\t\tprint_word(df, dm, len, DIFF_FILE_OLD, 1);\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_FILE_NEW, 0);\n+\t\t\tprint_word(df, dp, 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\tdiff_words->minus.current += len;\n+\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 0);\n+\t\t\tdm->current += len;\n \t\t\tbreak;\n \t}\n }\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75915","messageId":"1209815828-6548-4-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-3-git-send-email-pkufranky@gmail.com","subject":"[PATCH v2 3/5] diff.c: Fix --color-words showing trailing deleted words at another line","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T11:57:06Z","receivedAt":"2008-05-03T11:57:06Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"With --color-words, following example will show deleted word \"bar\" at\nanother line. (<r> represents red)\n------------------\n$ git diff\n- foo bar\n+ foo\n$ git diff --color-words\nfoo\n<r>bar</r>\n------------------\n\nThis wrong behaviour is a bug in fn_out_diff_words_aux which always\noutputs a newline after handling the diff line beginning with \"+\" and\nending with a newline.\n\nInstead, we always supress the newline when using print_word, and in\nfn_out_diff_words_aux, a newline is shown only in following cases:\n\n  - true minus.suppressed_newline followd by a line beginning with\n    '-', ' ' or '@' (i.e. not '+')\n  - true plus.suppressed_newline followd by a line beginning with\n    '+', ' ' or '@' (i.e. not '-')\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   19 +++++++++++++------\n 1 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex b5f7141..11316fe 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -409,6 +409,7 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n {\n \tstruct diff_words_data *diff_words;\n+\tchar cm;\n \tstruct diff_words_buffer *dm, *dp;\n \tFILE *df;\n \n@@ -417,10 +418,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \tdp = &(diff_words->plus);\n \tdf = diff_words->file;\n \n-\tif (dm->suppressed_newline) {\n-\t\tif (line[0] != '+')\n-\t\t\tputc('\\n', df);\n+\tif ((dm->suppressed_newline && line[0] != '+') ||\n+\t\t\t(dp->suppressed_newline && line[0] != '-')) {\n+\t\tputc('\\n', df);\n \t\tdm->suppressed_newline = 0;\n+\t\tdp->suppressed_newline = 0;\n \t}\n \n \tlen--;\n@@ -429,11 +431,14 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\tprint_word(df, dm, len, DIFF_FILE_OLD, 1);\n \t\t\tbreak;\n \t\tcase '+':\n-\t\t\tprint_word(df, dp, len, DIFF_FILE_NEW, 0);\n+\t\t\tprint_word(df, dp, len, DIFF_FILE_NEW, 1);\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 0);\n+\t\t\tcm = dm->text.ptr[dm->current + len - 1];\n+\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 1);\n \t\t\tdm->current += len;\n+\t\t\tif (cm == '\\n')\n+\t\t\t\tdm->suppressed_newline = 1;\n \t\t\tbreak;\n \t}\n }\n@@ -475,9 +480,11 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n \n-\tif (diff_words->minus.suppressed_newline) {\n+\tif (diff_words->minus.suppressed_newline ||\n+\t\t\tdiff_words->plus.suppressed_newline) {\n \t\tputc('\\n', diff_words->file);\n \t\tdiff_words->minus.suppressed_newline = 0;\n+\t\tdiff_words->plus.suppressed_newline = 0;\n \t}\n }\n \n-- \n1.5.5.1.121.g26b3\n"},{"id":"75912","messageId":"1209815828-6548-5-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-4-git-send-email-pkufranky@gmail.com","subject":"[PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T11:57:07Z","receivedAt":"2008-05-03T11:57:07Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Previously --color-words only allow spaces as boundary characters.\nHowever, just space is not enough. For example, when i rename a function\nfrom foo to bar, following example doesn't show as expected when using\n--color-words.\n\n------------------\n- if (foo(arg))\n+ if (bar(arg))\n------------------\n\nIt shows as \"if <r>(foo(arg))</r><g>(foo(arg))</g>\". Actually, it's the\nbest to show as \"if (<r>foo</r><g>bar</g>(arg))\". Here \"r\" and \"g\"\nrepresent \"red\" and \"green\" separately.\n\nThis patch introduces a configuration diff.nonwordchars to make\n--color-words treat both spaces and characters in diff.nonwordchars as\nboundary characters.\n\nIf we set diff.nonwordchars to \"()\", the example above will show as\n\"if (<r>foo(</r><g>bar(</g>arg))\". It's much better, athough not the best,\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n Documentation/config.txt       |    4 ++++\n Documentation/diff-options.txt |    1 +\n diff.c                         |   12 +++++++++++-\n 3 files changed, 16 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 824e416..812ec2c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -537,6 +537,10 @@ diff.external::\n \tprogram only on a subset of your files, you might want to\n \tuse linkgit:gitattributes[5] instead.\n \n+diff.nonwordchars::\n+\tSpecify additional boundary characters other than spaces for\n+\t--color-words.\n+\n diff.renameLimit::\n \tThe number of files to consider when performing the copy/rename\n \tdetection; equivalent to the git diff option '-l'.\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 13234fa..60dd5e6 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -95,6 +95,7 @@ endif::git-format-patch[]\n \n --color-words::\n \tShow colored word diff, i.e. color words which have changed.\n+\tThe boundary characters can be configured with diff.nonwordchars.\n \n --no-renames::\n \tTurn off rename detection, even when the configuration\ndiff --git a/diff.c b/diff.c\nindex 11316fe..50d7fa7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -23,6 +23,7 @@ static int diff_rename_limit_default = 100;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n+static const char *diff_non_word_chars = \"\";\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \t\"\\033[m\",\t/* reset */\n@@ -159,6 +160,10 @@ int git_diff_ui_config(const char *var, const char *value)\n \t\texternal_diff_cmd_cfg = xstrdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"diff.nonwordchars\")) {\n+\t\tdiff_non_word_chars = value ? xstrdup(value) : \"\";\n+\t\treturn 0;\n+\t}\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \n@@ -443,6 +448,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static int is_non_word_char(char c)\n+{\n+\treturn isspace(c) || !!strchr(diff_non_word_chars, c);\n+}\n+\n static mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n \tint i;\n \n@@ -450,7 +460,7 @@ static mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n \tdest->ptr = xmalloc(dest->size);\n \tmemcpy(dest->ptr, src->ptr, dest->size);\n \tfor (i = 0; i < dest->size; i++)\n-\t\tif (isspace(dest->ptr[i]))\n+\t\tif (is_non_word_char(dest->ptr[i]))\n \t\t\tdest->ptr[i] = '\\n';\n }\n \n-- \n1.5.5.1.121.g26b3\n"},{"id":"75913","messageId":"1209815828-6548-6-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-5-git-send-email-pkufranky@gmail.com","subject":"[PATCH v2 5/5] fn_out_diff_words_aux: Handle common diff line more carefully","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T11:57:08Z","receivedAt":"2008-05-03T11:57:08Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Before feeding minus and plus lines into xdi_diff, we replace non word\ncharacters with '\\n'. So we need recover the replaced character (always\nthe last character) in the callback fn_out_diff_words_aux.\n\nTherefore, a common diff line beginning with ' ' is not always a real\ncommon line. And we should check the last characters of the common diff\nline. If they are different, we should output the first len-1 characters\nas the common part and then the last characters in minus and plus\nseparately.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   18 +++++++++++++-----\n 1 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 50d7fa7..72fe804 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -414,7 +414,7 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n {\n \tstruct diff_words_data *diff_words;\n-\tchar cm;\n+\tchar cm, cp;\n \tstruct diff_words_buffer *dm, *dp;\n \tFILE *df;\n \n@@ -440,10 +440,18 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\tbreak;\n \t\tcase ' ':\n \t\t\tcm = dm->text.ptr[dm->current + len - 1];\n-\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 1);\n-\t\t\tdm->current += len;\n-\t\t\tif (cm == '\\n')\n-\t\t\t\tdm->suppressed_newline = 1;\n+\t\t\tcp = dp->text.ptr[dp->current + len - 1];\n+\t\t\tif (cm == cp) {\n+\t\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 1);\n+\t\t\t\tdm->current += len;\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tlen--;\n+\t\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 1);\n+\t\t\t\tdm->current += len;\n+\t\t\t\tprint_word(df, dm, 1, DIFF_FILE_OLD, 1);\n+\t\t\t\tprint_word(df, dp, 1, DIFF_FILE_NEW, 1);\n+\t\t\t}\n \t\t\tbreak;\n \t}\n }\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75916","messageId":"46dff0320805030501s4bf2c68dh336efd8ba375d207@mail.gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-3-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v2 2/5] diff.c: Use show variable name in fn_out_diff_words_aux","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T12:01:15Z","receivedAt":"2008-05-03T12:01:15Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sat, May 3, 2008 at 7:57 PM, Ping Yin <pkufranky@gmail.com> wrote:\n> Signed-off-by: Ping Yin <pkufranky@gmail.com>\n>  ---\n\nSorry, the wrong title, s/show/short/\n-- \nPing Yin\n"},{"id":"75920","messageId":"481C6707.5060308@dirk.my1.cc","threadId":"13350","inReplyTo":"1209774178-26552-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH] Make boundary characters for --color-words configurable","fromName":"Dirk Süsserott","fromEmail":"newsletter@dirk.my1.cc","sentAt":"2008-05-03T13:22:15Z","receivedAt":"2008-05-03T13:22:15Z","isPatch":true,"sender":{"key":"newsletter@dirk.my1.cc","avatar":null},"body":"Hi Ping,\n\nI highly appreciate your effort into \"diff --color-words\"\nand hope it makes it into the the next release. I wanted\nto change the behaviour as well, but when I saw that\ngit-diff is a builtin, I had to give up. I hoped it was\na perl script and could insert some \"\\b\" regexes somewhere,\nbut I was wrong. I'm using Git for Windows, you know.\n\nHowever, I'd like to ask you whether you've done any research\nin how to use \"--color-words\" in gitk? gitk seems to color\nthe lines only by means of a '+' or '-' sign in the first\ncolumn. Hardcoded. I managed to add a checkbox to gitk that\nadds the '--color-words' switch to git diff, but when checked\nthe output is just muddled up. All of those ^] characters\nwhithin the code. :-(\n\nDirk\n\nPing Yin schrieb:\n> Signed-off-by: Ping Yin <pkufranky@gmail.com>\n> ---\n>   \n>> I think config variables should be in alphabetical order in config.txt.\n>> Hence your diff.nonwordchars is not in the right place\n>>     \n>\n> THX, this is fixing patch\n>\n>  Documentation/config.txt |    8 ++++----\n>  1 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index eb05592..812ec2c 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -537,6 +537,10 @@ diff.external::\n>  \tprogram only on a subset of your files, you might want to\n>  \tuse linkgit:gitattributes[5] instead.\n>  \n> +diff.nonwordchars::\n> +\tSpecify additional boundary characters other than spaces for\n> +\t--color-words.\n> +\n>  diff.renameLimit::\n>  \tThe number of files to consider when performing the copy/rename\n>  \tdetection; equivalent to the git diff option '-l'.\n> @@ -546,10 +550,6 @@ diff.renames::\n>  \twill enable basic rename detection.  If set to \"copies\" or\n>  \t\"copy\", it will detect copies, as well.\n>  \n> -diff.nonwordchars::\n> -    Specify additional boundary characters other than spaces for\n> -    --color-words.\n> -\n>  fetch.unpackLimit::\n>  \tIf the number of objects fetched over the git native\n>  \ttransfer is below this\n>   \n"},{"id":"75929","messageId":"46dff0320805030657w1c9bef0dr346a51b2678c97c0@mail.gmail.com","threadId":"13350","inReplyTo":"481C6707.5060308@dirk.my1.cc","subject":"Re: [PATCH] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T13:57:59Z","receivedAt":"2008-05-03T13:57:59Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sat, May 3, 2008 at 9:22 PM, Dirk Süsserott <newsletter@dirk.my1.cc> wrote:\n> Hi Ping,\n>\n>  I highly appreciate your effort into \"diff --color-words\"\n>  and hope it makes it into the the next release.\n\nGlad to hear that.\n\n>\n>  However, I'd like to ask you whether you've done any research\n>  in how to use \"--color-words\" in gitk?\n\noh, i havn' t even used gtk yet. I don't have the X environment\nbecause i use windows and then use securecrt to ssh to  remote server.\n\n\n-- \nPing Yin\n"},{"id":"75930","messageId":"alpine.DEB.1.00.0805031501290.30431@racer","threadId":"13350","inReplyTo":"1209736766-8029-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH] --color-words: Make the word characters configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-03T14:03:17Z","receivedAt":"2008-05-03T14:03:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nNow, you can specify which characters are to be interpreted as word \ncharacters with \"--color-words=A-Za-z\", or by setting the config variable \ndiff.wordCharacters.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tI would have preferred an approach like this.\n\n Documentation/config.txt       |    6 ++++\n Documentation/diff-options.txt |    8 ++++-\n README                         |    2 +-\n diff.c                         |   64 +++++++++++++++++++++++++++++++++++++++-\n 4 files changed, 77 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 05bf2df..663d82b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -546,6 +546,12 @@ diff.renames::\n \twill enable basic rename detection.  If set to \"copies\" or\n \t\"copy\", it will detect copies, as well.\n \n+diff.wordcharacters::\n+\tThis config setting overrides which characters are interpreted as\n+\tword characters by the --color-words option of linkgit:git-diff[1].\n++\n+The default is: all ASCII characters excluding NUL to SPACE.\n+\n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\n \ttransfer is below this\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 13234fa..88ea5d4 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -93,8 +93,14 @@ 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+--color-words[=<set>]::\n \tShow colored word diff, i.e. color words which have changed.\n++\n+If a set of characters is specified, it is interpreted as the range of\n+word characters.  Example: \"0-9A-Fa-f\".  As a convenience, \"[:alnum:]\"\n+and \"[:alpha:]\" expand to alpha-numeric and alpha characters,\n+respectively.  This argument overrides the config setting\n+'diff.wordCharacters'.\n \n --no-renames::\n \tTurn off rename detection, even when the configuration\ndiff --git a/README b/README\nindex 548142c..0e325e2 100644\n--- a/README\n+++ b/README\n@@ -4,7 +4,7 @@\n \n ////////////////////////////////////////////////////////////////\n \n-\"git\" can mean anything, depending on your mood.\n+\"git\" cann mean anything, depending on your mood.\n \n  - random three-letter combination that is pronounceable, and not\n    actually used by any common UNIX command.  The fact that it is a\ndiff --git a/diff.c b/diff.c\nindex 3632b55..3e8719c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -23,6 +23,20 @@ static int diff_rename_limit_default = 100;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n+static char word_character[256] = {\n+\t0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,\n+\t0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,\n+\t0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+\t1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,\n+};\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \t\"\\033[m\",\t/* reset */\n@@ -123,6 +137,44 @@ static int parse_funcname_pattern(const char *var, const char *ep, const char *v\n \treturn 0;\n }\n \n+static void set_word_character_range(char start, char end)\n+{\n+\tint i;\n+\tfor (i = (unsigned char)start; i <= (unsigned char)end; i++)\n+\t\tword_character[i] = 1;\n+}\n+\n+static int set_word_characters(const char *set)\n+{\n+\tint previous_character = -1;\n+\n+\tmemset(word_character, 0, sizeof(word_character));\n+\n+\t/* parse values like \"0-9[:alnum:]\" */\n+\tfor (; *set; set++)\n+\t\tif (!prefixcmp(set, \"[:alpha:]\")) {\n+\t\t\tset_word_character_range('A', 'Z');\n+\t\t\tset_word_character_range('a', 'z');\n+\t\t\tprevious_character = -1;\n+\t\t\tset += 8;\n+\t\t} else if (!prefixcmp(set, \"[:alnum:]\")) {\n+\t\t\tset_word_character_range('A', 'Z');\n+\t\t\tset_word_character_range('a', 'z');\n+\t\t\tset_word_character_range('0', '9');\n+\t\t\tprevious_character = -1;\n+\t\t\tset += 8;\n+\t\t} else if (*set == '-' && previous_character >= 0) {\n+\t\t\tset++;\n+\t\t\tset_word_character_range(previous_character, *set);\n+\t\t\tprevious_character = -1;\n+\t\t} else {\n+\t\t\tword_character[(unsigned int)*set] = 1;\n+\t\t\tprevious_character = *set;\n+\t\t}\n+\n+\treturn 0;\n+}\n+\n /*\n  * These are to give UI layer defaults.\n  * The core-level commands such as git-diff-files should\n@@ -179,6 +231,12 @@ int git_diff_basic_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"diff.wordcharacters\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\treturn set_word_characters(value);\n+\t}\n+\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \t\tif (ep != var + 4) {\n@@ -456,7 +514,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (!word_character[(unsigned char)plus.ptr[i]])\n \t\t\tplus.ptr[i] = '\\n';\n \tdiff_words->plus.current = 0;\n \n@@ -2489,6 +2547,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\tset_word_characters(arg + 13);\n+\t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \telse if (!strcmp(arg, \"--quiet\"))\n-- \n1.5.5.1.266.g7cbb\n"},{"id":"75935","messageId":"46dff0320805030713r673ea479u37018333c32131bb@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805031501290.30431@racer","subject":"Re: [PATCH] --color-words: Make the word characters configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-03T14:13:12Z","receivedAt":"2008-05-03T14:13:12Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sat, May 3, 2008 at 10:03 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n>  Now, you can specify which characters are to be interpreted as word\n>  characters with \"--color-words=A-Za-z\", or by setting the config variable\n>  diff.wordCharacters.\n>\n>  Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>  ---\n>\n>         I would have preferred an approach like this.\n>\n>   Documentation/config.txt       |    6 ++++\n>   Documentation/diff-options.txt |    8 ++++-\n>   README                         |    2 +-\n>   diff.c                         |   64 +++++++++++++++++++++++++++++++++++++++-\n>   4 files changed, 77 insertions(+), 3 deletions(-)\n>\n>  diff --git a/Documentation/config.txt b/Documentation/config.txt\n>  index 05bf2df..663d82b 100644\n>  --- a/Documentation/config.txt\n>  +++ b/Documentation/config.txt\n>  @@ -546,6 +546,12 @@ diff.renames::\n>         will enable basic rename detection.  If set to \"copies\" or\n>         \"copy\", it will detect copies, as well.\n>\n>  +diff.wordcharacters::\n>  +       This config setting overrides which characters are interpreted as\n>  +       word characters by the --color-words option of linkgit:git-diff[1].\n\nI think a-zA-Z0-9_  should always be word characters. We can't\noverride them, instead, we just extend them.\n\n>\n>  -\"git\" can mean anything, depending on your mood.\n>  +\"git\" cann mean anything, depending on your mood.\n\nWhy replacing can as cann?\n\n\n-- \nPing Yin\n"},{"id":"75937","messageId":"alpine.DEB.1.00.0805031522360.30431@racer","threadId":"13350","inReplyTo":"46dff0320805030713r673ea479u37018333c32131bb@mail.gmail.com","subject":"Re: [PATCH] --color-words: Make the word characters configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-03T14:23:40Z","receivedAt":"2008-05-03T14:23:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 3 May 2008, Ping Yin wrote:\n\n> On Sat, May 3, 2008 at 10:03 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> >  Now, you can specify which characters are to be interpreted as word\n> >  characters with \"--color-words=A-Za-z\", or by setting the config variable\n> >  diff.wordCharacters.\n> >\n> >  Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >  ---\n> >\n> >         I would have preferred an approach like this.\n> >\n> >   Documentation/config.txt       |    6 ++++\n> >   Documentation/diff-options.txt |    8 ++++-\n> >   README                         |    2 +-\n> >   diff.c                         |   64 +++++++++++++++++++++++++++++++++++++++-\n> >   4 files changed, 77 insertions(+), 3 deletions(-)\n> >\n> >  diff --git a/Documentation/config.txt b/Documentation/config.txt\n> >  index 05bf2df..663d82b 100644\n> >  --- a/Documentation/config.txt\n> >  +++ b/Documentation/config.txt\n> >  @@ -546,6 +546,12 @@ diff.renames::\n> >         will enable basic rename detection.  If set to \"copies\" or\n> >         \"copy\", it will detect copies, as well.\n> >\n> >  +diff.wordcharacters::\n> >  +       This config setting overrides which characters are interpreted as\n> >  +       word characters by the --color-words option of linkgit:git-diff[1].\n> \n> I think a-zA-Z0-9_ should always be word characters. We can't override \n> them, instead, we just extend them.\n\nNo.  That is exactly the artificial-limitation-by-design I do not want.\n\n> >  -\"git\" can mean anything, depending on your mood.\n> >  +\"git\" cann mean anything, depending on your mood.\n> \n> Why replacing can as cann?\n\nBecause I am stupid and committed my test case.  Of course, this patch was \ndone under time pressure, because I have to leave for the day, like, right \nnow.\n\nCiao,\nDscho\n"},{"id":"75939","messageId":"20080503144337.GA7987@mithlond.arda.local","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805031501290.30431@racer","subject":"Re: [PATCH] --color-words: Make the word characters configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-03T14:43:37Z","receivedAt":"2008-05-03T14:43:37Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Johannes Schindelin wrote (2008-05-03 15:03 +0100):\n\n> Now, you can specify which characters are to be interpreted as word\n> characters with \"--color-words=A-Za-z\", or by setting the config\n> variable diff.wordCharacters.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> \n> \tI would have preferred an approach like this.\n\nUnfortunately this does not work at all with other than Ascii\ncharacters. It makes --color-words completely unusable for anything\nother than Ascii text. Sorry.\n\nPing Yin's version has also the problem that UTF-8 multibyte characters\nU+0080..U+10FFFF don't work in diff.nonwordchars. Fortunately the most\nimportant word delimiters are in U+0000..U+007F (=Ascii) area so Ping's\nversion is perfectly usable with Unicode text. (Even the old\n--color-words behaviour with only SPACE as non-word char was perfectly\nusable with Unicode text.) I, too, would like to see Ping's patch series\nmerged in.\n"},{"id":"75946","messageId":"7vmyn7uvut.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805031501290.30431@racer","subject":"Re: [PATCH] --color-words: Make the word characters configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T17:43:22Z","receivedAt":"2008-05-03T17:43:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Now, you can specify which characters are to be interpreted as word \n> characters with \"--color-words=A-Za-z\", or by setting the config variable \n> diff.wordCharacters.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>\n> \tI would have preferred an approach like this.\n\nHmmm...\n\n> diff --git a/README b/README\n> index 548142c..0e325e2 100644\n> --- a/README\n> +++ b/README\n> @@ -4,7 +4,7 @@\n>  \n>  ////////////////////////////////////////////////////////////////\n>  \n> -\"git\" can mean anything, depending on your mood.\n> +\"git\" cann mean anything, depending on your mood.\n\nHeh.\n\n> @@ -456,7 +514,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (!word_character[(unsigned char)plus.ptr[i]])\n>  \t\t\tplus.ptr[i] = '\\n';\n>  \tdiff_words->plus.current = 0;\n\nI do not think there is much difference between specifying the set of word\ncharacters and the set of non-word characters, especially as long as your\ndefinition of \"character\" is limited to 8-bit bytes.  By enumerating word\ncharacters, your patch is letting the user specify non word characters\nthat are remainder from the 256-element set.  By the way, I think you\nmeant to do the same for the \"minus\" side a few lines above this hunk.\n\nI commented on the patch from Ping earier about a quite different issue.\nI was wondering if we can avoid losing the non-word character information.\nThe original code replaces any isspace byte with LF, but a whitespace is a\nwhitespace is a whitespace so there won't be much loss of information, but\nmaking the above isspace() configurable means that now you are going to\ndrop non-space non-word characters from the output set.\n\nInstead of dropping the original character and replacing it with LF,\nI thought a more sensible approach would be to _insert_ a line break\nbetween runs of word characters and non-word characters (while probably\ndropping a LF in the original).  That is, instead of what the current\nimplementation of the above loop does to \"ab  c d\" (i.e. rewrite it to\n\"ab\\n\\nc\\nd\"), rewrite it to \"ab\\n  \\nc\\n \\nd\".  Which feels more consistent\nwith the way how \\b should work.\n"},{"id":"75948","messageId":"7vbq3nuvoj.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"1209815828-6548-3-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v2 2/5] diff.c: Use show variable name in fn_out_diff_words_aux","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T17:47:08Z","receivedAt":"2008-05-03T17:47:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Cannot parse the summary line.  Try again.\n"},{"id":"75951","messageId":"7v7iebuv0s.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"1209815828-6548-4-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v2 3/5] diff.c: Fix --color-words showing trailing deleted words at another line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T18:01:23Z","receivedAt":"2008-05-03T18:01:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> With --color-words, following example will show deleted word \"bar\" at\n> another line. (<r> represents red)\n> ------------------\n> $ git diff\n> - foo bar\n> + foo\n> $ git diff --color-words\n> foo\n> <r>bar</r>\n> ------------------\n\nMinor style nit, but commit log is not asiidoc and the horizontal bars are\ndistracting.  Just use a blank line either ends to separate the example\nfrom the descriptive text, and indent the example by a few places.\n\n> This wrong behaviour is a bug in fn_out_diff_words_aux which always\n\n\"This wrong behaviour is a bug\" is a very roundabout way to say it. \"This\nis caused by a bug in ...\"?\n\n> Instead, we always supress the newline when using print_word, and in\n> fn_out_diff_words_aux, a newline is shown only in following cases:\n>\n>   - true minus.suppressed_newline followd by a line beginning with\n>     '-', ' ' or '@' (i.e. not '+')\n>   - true plus.suppressed_newline followd by a line beginning with\n>     '+', ' ' or '@' (i.e. not '-')\n\nHmm, that may describe _what_ the updated code _does_, but does not\nexplain why/how it is an improvement.\n\nFor this kind of change we really would need tests to illustrate various\ninputs and desired output for them.\n\n> diff --git a/diff.c b/diff.c\n> index b5f7141..11316fe 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -409,6 +409,7 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n>  static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n>  {\n>  \tstruct diff_words_data *diff_words;\n> +\tchar cm;\n>  \tstruct diff_words_buffer *dm, *dp;\n>  \tFILE *df;\n\nThese nondescriptive two letter variable names make the logic very hard to\nfollow.  What does cm represent in the new code?  Let's see....  (spends a\nfew minutes to follow the code)... Ah, it is just a randomly named\ntemporary variable of \"char\" type and do not have long-term meaning of any\nsignificance, and could have been named \"c\" or \"ch\" or whatever.  Heck,\nthe code fooled me because it looked like it was named similarly to the dm\nand dp nearby.\n\nNot a demonstration of \"cm\" being named poorly, but this wasted few\nminutes shows that dm and dp are named very poorly.\n"},{"id":"75953","messageId":"7vy76rtfns.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"1209815828-6548-5-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T18:18:31Z","receivedAt":"2008-05-03T18:18:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Has this series been ever tested?\n\n$ git diff one two\ndiff --git a/one b/two\nindex f9facd3..10c6195 100644\n--- a/one\n+++ b/two\n@@ -1 +1 @@\n-A quick(brown) fox\n+A quick(yellow) fox\n\n$ tail -n 2 .git/config\n[diff]\n        nonwordchars = \"()\"\n\n$ git diff --color-words one two\ndiff --git a/one b/two\nindex f9facd3..10c6195 100644\n--- a/one\n+++ b/two\n@@ -1 +1 @@\nA quick(<red>brown)</red><green>yellow)</green> fox\n"},{"id":"75954","messageId":"7vr6cjtfjt.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"1209815828-6548-2-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v2 1/5] diff.c: Remove code redundancy in diff_words_show","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-03T18:20:54Z","receivedAt":"2008-05-03T18:20:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> +static mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n\ncc1: warnings being treated as errors\ndiff.c:464: warning: return type defaults int\n\n> +\tint i;\n> +\n> +\tdest->size = src->size;\n> +\tdest->ptr = xmalloc(dest->size);\n> +\tmemcpy(dest->ptr, src->ptr, dest->size);\n> +\tfor (i = 0; i < dest->size; i++)\n> +\t\tif (isspace(dest->ptr[i]))\n> +\t\t\tdest->ptr[i] = '\\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> @@ -444,20 +455,9 @@ static void diff_words_show(struct diff_words_data *diff_words)\n>  \tint i;\n\ndiff.c:482: warning: unused variable 'i'\n"},{"id":"75956","messageId":"20080503184126.GA21187@mithlond.arda.local","threadId":"13350","inReplyTo":"7vy76rtfns.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-03T18:41:26Z","receivedAt":"2008-05-03T18:41:26Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Junio C Hamano wrote (2008-05-03 11:18 -0700):\n\n> Has this series been ever tested?\n> \n> $ git diff one two\n> diff --git a/one b/two\n> index f9facd3..10c6195 100644\n> --- a/one\n> +++ b/two\n> @@ -1 +1 @@\n> -A quick(brown) fox\n> +A quick(yellow) fox\n> \n> $ tail -n 2 .git/config\n> [diff]\n>         nonwordchars = \"()\"\n> \n> $ git diff --color-words one two\n> diff --git a/one b/two\n> index f9facd3..10c6195 100644\n> --- a/one\n> +++ b/two\n> @@ -1 +1 @@\n> A quick(<red>brown)</red><green>yellow)</green> fox\n\nI've been testing but not quite sure what to think about the above\noutput. Is this more natural and expected output:\n\n  A quick(<red>brown</red><green>yellow</green>) fox\n\ni.e. no ()'s between the changed words? Although this\n\n  -A quick brown fox\n  +A quick yellow fox\n\nhas always became this:\n\n  A quick <red>brown</red> <green>yellow</green> fox\n                    ------^\n  (Notice space here)           \n\nSo there is kind of \"added space\" but I guess technically it's actually\nlike this:\n\n  A quick <red>brown </red><green>yellow </green>fox\n\nSo I think space is consistent with parentheses in your example.\n"},{"id":"75966","messageId":"46dff0320805031732x25286707r991358162046c07c@mail.gmail.com","threadId":"13350","inReplyTo":"7vy76rtfns.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T00:32:54Z","receivedAt":"2008-05-04T00:32:54Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 2:18 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Has this series been ever tested?\n>\n>  $ git diff one two\n>  diff --git a/one b/two\n>  index f9facd3..10c6195 100644\n>  --- a/one\n>  +++ b/two\n>  @@ -1 +1 @@\n>  -A quick(brown) fox\n>  +A quick(yellow) fox\n>\n>  $ tail -n 2 .git/config\n>  [diff]\n>         nonwordchars = \"()\"\n>\n>  $ git diff --color-words one two\n>  diff --git a/one b/two\n>  index f9facd3..10c6195 100644\n>  --- a/one\n>  +++ b/two\n>  @@ -1 +1 @@\n>  A quick(<red>brown)</red><green>yellow)</green> fox\n\nYeah, i tested it. It's a better improvement, although not the best.\nAs i said in the commit msg\n\nIf we set diff.nonwordchars to \"()\", the example above will show as\n\"if (<r>foo(</r><g>bar(</g>arg))\". It's much better, athough not the best.\n\nAs you said in another thread, unless we insert LF between run of word\nchars and run of nonword chars, the is no way to achieve the best\nresult.\n\nI have try my best to achieve a better output in current\nimplementation (say, replace nonword chars with LF)\n\n\n\n-- \nPing Yin\n"},{"id":"75975","messageId":"1209874815-14411-1-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209815828-6548-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 0/6] --color-words improvement","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:09Z","receivedAt":"2008-05-04T04:20:09Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Ping Yin (6):\n      diff.c: Remove code redundancy in diff_words_show\n      fn_out_diff_words_aux: Use short variable name\n      --color-words: Fix showing trailing deleted words at another line\n      --color-words: Make non-word characters configurable\n      fn_out_diff_words_aux: Handle common diff line more carefully\n      --color-words: Add test t4030\n\nRelated to last series\n \n - add a test patch (the last one)\n - refine commit message\n - use more meaningful varaible name (dm -> dwb_minus etc.)\n - refine some words (boundary -> non-word etc.)\n - some compiling warning given by junio (add void, remove int i)\n\nThe diff between previous patch series and current series\nexcept the last patch\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 60dd5e6..70acc14 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -95,7 +95,7 @@ endif::git-format-patch[]\n \n --color-words::\n \tShow colored word diff, i.e. color words which have changed.\n-\tThe boundary characters can be configured with diff.nonwordchars.\n+\tThe non-word characters can be configured with diff.nonwordchars.\n \n --no-renames::\n \tTurn off rename detection, even when the configuration\ndiff --git a/diff.c b/diff.c\nindex 72fe804..08048e4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -414,43 +414,43 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n {\n \tstruct diff_words_data *diff_words;\n-\tchar cm, cp;\n-\tstruct diff_words_buffer *dm, *dp;\n-\tFILE *df;\n+\tchar lastchar_minus, lastchar_plus;\n+\tstruct diff_words_buffer *dwb_minus, *dwb_plus;\n+\tFILE *outfile;\n \n \tdiff_words = priv;\n-\tdm = &(diff_words->minus);\n-\tdp = &(diff_words->plus);\n-\tdf = diff_words->file;\n+\tdwb_minus = &(diff_words->minus);\n+\tdwb_plus = &(diff_words->plus);\n+\toutfile = diff_words->file;\n \n-\tif ((dm->suppressed_newline && line[0] != '+') ||\n-\t\t\t(dp->suppressed_newline && line[0] != '-')) {\n-\t\tputc('\\n', df);\n-\t\tdm->suppressed_newline = 0;\n-\t\tdp->suppressed_newline = 0;\n+\tif ((dwb_minus->suppressed_newline && line[0] != '+') ||\n+\t\t\t(dwb_plus->suppressed_newline && line[0] != '-')) {\n+\t\tputc('\\n', outfile);\n+\t\tdwb_minus->suppressed_newline = 0;\n+\t\tdwb_plus->suppressed_newline = 0;\n \t}\n \n \tlen--;\n \tswitch (line[0]) {\n \t\tcase '-':\n-\t\t\tprint_word(df, dm, len, DIFF_FILE_OLD, 1);\n+\t\t\tprint_word(outfile, dwb_minus, len, DIFF_FILE_OLD, 1);\n \t\t\tbreak;\n \t\tcase '+':\n-\t\t\tprint_word(df, dp, len, DIFF_FILE_NEW, 1);\n+\t\t\tprint_word(outfile, dwb_plus, len, DIFF_FILE_NEW, 1);\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tcm = dm->text.ptr[dm->current + len - 1];\n-\t\t\tcp = dp->text.ptr[dp->current + len - 1];\n-\t\t\tif (cm == cp) {\n-\t\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 1);\n-\t\t\t\tdm->current += len;\n+\t\t\tlastchar_minus = dwb_minus->text.ptr[dwb_minus->current + len - 1];\n+\t\t\tlastchar_plus = dwb_plus->text.ptr[dwb_plus->current + len - 1];\n+\t\t\tif (lastchar_minus == lastchar_plus) {\n+\t\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 1);\n+\t\t\t\tdwb_minus->current += len;\n \t\t\t}\n \t\t\telse {\n \t\t\t\tlen--;\n-\t\t\t\tprint_word(df, dp, len, DIFF_PLAIN, 1);\n-\t\t\t\tdm->current += len;\n-\t\t\t\tprint_word(df, dm, 1, DIFF_FILE_OLD, 1);\n-\t\t\t\tprint_word(df, dp, 1, DIFF_FILE_NEW, 1);\n+\t\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 1);\n+\t\t\t\tdwb_minus->current += len;\n+\t\t\t\tprint_word(outfile, dwb_minus, 1, DIFF_FILE_OLD, 1);\n+\t\t\t\tprint_word(outfile, dwb_plus, 1, DIFF_FILE_NEW, 1);\n \t\t\t}\n \t\t\tbreak;\n \t}\n@@ -461,7 +461,7 @@ static int is_non_word_char(char c)\n \treturn isspace(c) || !!strchr(diff_non_word_chars, c);\n }\n \n-static mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n+static void mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n \tint i;\n \n \tdest->size = src->size;\n@@ -479,7 +479,6 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \txdemitconf_t xecfg;\n \txdemitcb_t ecb;\n \tmmfile_t minus, plus;\n-\tint i;\n \n \tmemset(&xecfg, 0, sizeof(xecfg));\n \tmmfile_copy_set_boundary(&minus, &(diff_words->minus.text));\n\n\n Documentation/config.txt       |    4 ++\n Documentation/diff-options.txt |    1 +\n diff.c                         |   84 ++++++++++++++++++++++++++-------------\n t/t4030-diff-color-words.sh    |   42 ++++++++++++++++++++\n t/t4030/expect1                |    1 +\n t/t4030/expect10               |    1 +\n t/t4030/expect2                |    1 +\n t/t4030/expect3                |    1 +\n t/t4030/expect4                |    1 +\n t/t4030/expect5                |    1 +\n t/t4030/expect6                |    1 +\n t/t4030/expect7                |    1 +\n t/t4030/expect8                |    1 +\n t/t4030/expect9                |    1 +\n t/t4030/gen-expect.sh          |   35 ++++++++++++++++\n 15 files changed, 148 insertions(+), 28 deletions(-)\n"},{"id":"75971","messageId":"1209874815-14411-2-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209874815-14411-1-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 1/6] diff.c: Remove code redundancy in diff_words_show","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:10Z","receivedAt":"2008-05-04T04:20:10Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Introduce mmfile_copy_set_boundary to do the repeated work.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   27 +++++++++++++--------------\n 1 files changed, 13 insertions(+), 14 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 3632b55..6633c9c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -434,6 +434,17 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static void mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n+\tint i;\n+\n+\tdest->size = src->size;\n+\tdest->ptr = xmalloc(dest->size);\n+\tmemcpy(dest->ptr, src->ptr, dest->size);\n+\tfor (i = 0; i < dest->size; i++)\n+\t\tif (isspace(dest->ptr[i]))\n+\t\t\tdest->ptr[i] = '\\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,23 +452,11 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \txdemitconf_t xecfg;\n \txdemitcb_t ecb;\n \tmmfile_t minus, plus;\n-\tint i;\n \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+\tmmfile_copy_set_boundary(&minus, &(diff_words->minus.text));\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+\tmmfile_copy_set_boundary(&plus, &(diff_words->plus.text));\n \tdiff_words->plus.current = 0;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75972","messageId":"1209874815-14411-3-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209874815-14411-2-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 2/6] fn_out_diff_words_aux: Use short variable name","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:11Z","receivedAt":"2008-05-04T04:20:11Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Signed-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   26 +++++++++++++++-----------\n 1 files changed, 15 insertions(+), 11 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 6633c9c..05f7c35 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -408,28 +408,32 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\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+\tstruct diff_words_data *diff_words;\n+\tstruct diff_words_buffer *dwb_minus, *dwb_plus;\n+\tFILE *outfile;\n \n-\tif (diff_words->minus.suppressed_newline) {\n+\tdiff_words = priv;\n+\tdwb_minus = &(diff_words->minus);\n+\tdwb_plus = &(diff_words->plus);\n+\toutfile = diff_words->file;\n+\n+\tif (dwp_minus->suppressed_newline) {\n \t\tif (line[0] != '+')\n-\t\t\tputc('\\n', diff_words->file);\n-\t\tdiff_words->minus.suppressed_newline = 0;\n+\t\t\tputc('\\n', outfile);\n+\t\tdwp_minus->suppressed_newline = 0;\n \t}\n \n \tlen--;\n \tswitch (line[0]) {\n \t\tcase '-':\n-\t\t\tprint_word(diff_words->file,\n-\t\t\t\t   &diff_words->minus, len, DIFF_FILE_OLD, 1);\n+\t\t\tprint_word(outfile, dwb_minus, len, DIFF_FILE_OLD, 1);\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_FILE_NEW, 0);\n+\t\t\tprint_word(outfile, dwb_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\tdiff_words->minus.current += len;\n+\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 0);\n+\t\t\tdwb_minus->current += len;\n \t\t\tbreak;\n \t}\n }\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75973","messageId":"1209874815-14411-4-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209874815-14411-3-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 3/6] --color-words: Fix showing trailing deleted words at another line","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:12Z","receivedAt":"2008-05-04T04:20:12Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"With --color-words, following example will show deleted word \"bar\" at\nanother line. (<r> represents red)\n\n  $ git diff\n  - foo bar\n  + foo\n  $ git diff --color-words\n  foo\n  <r>bar</r>\n\nThis is caused by the unsymmetrical handling of LF in the plus and minus\nbuffer in fn_out_diff_words_aux.\n\nAssume \"trailing minus (or plus) word\" represents the last word\n(with real LF following it) in a line of the minus (or plus) buffer.\n\nFollowing is original unsymmetrical handling rules where LF represents\na LF will be shown there.\n\n  - trailing minus word, [plus word, ...], LF, non plus word\n  - trailing plus word, LF, any word\n\nThe second rule causes any word following the trailing plus word will\nbe shown in a different line.\n\nThis patch tries to implement the symmetrical handling rules:\n\n  - trailing minus word, [plus word, ...], LF, non plus word\n  - trailing plus word, [minus word, ...], LF, non minus word\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   21 ++++++++++++++-------\n 1 files changed, 14 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 05f7c35..c7a0d77 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -409,6 +409,7 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n {\n \tstruct diff_words_data *diff_words;\n+\tchar lastchar_minus;\n \tstruct diff_words_buffer *dwb_minus, *dwb_plus;\n \tFILE *outfile;\n \n@@ -417,10 +418,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \tdwb_plus = &(diff_words->plus);\n \toutfile = diff_words->file;\n \n-\tif (dwp_minus->suppressed_newline) {\n-\t\tif (line[0] != '+')\n-\t\t\tputc('\\n', outfile);\n-\t\tdwp_minus->suppressed_newline = 0;\n+\tif ((dwb_minus->suppressed_newline && line[0] != '+') ||\n+\t\t\t(dwb_plus->suppressed_newline && line[0] != '-')) {\n+\t\tputc('\\n', outfile);\n+\t\tdwb_minus->suppressed_newline = 0;\n+\t\tdwb_plus->suppressed_newline = 0;\n \t}\n \n \tlen--;\n@@ -429,11 +431,14 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\tprint_word(outfile, dwb_minus, len, DIFF_FILE_OLD, 1);\n \t\t\tbreak;\n \t\tcase '+':\n-\t\t\tprint_word(outfile, dwb_plus, len, DIFF_FILE_NEW, 0);\n+\t\t\tprint_word(outfile, dwb_plus, len, DIFF_FILE_NEW, 1);\n \t\t\tbreak;\n \t\tcase ' ':\n-\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 0);\n+\t\t\tlastchar_minus = dwb_minus->text.ptr[dwb_minus->current + len - 1];\n+\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 1);\n \t\t\tdwb_minus->current += len;\n+\t\t\tif (lastchar_minus == '\\n')\n+\t\t\t\tdwb_minus->suppressed_newline = 1;\n \t\t\tbreak;\n \t}\n }\n@@ -474,9 +479,11 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \tfree(plus.ptr);\n \tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n \n-\tif (diff_words->minus.suppressed_newline) {\n+\tif (diff_words->minus.suppressed_newline ||\n+\t\t\tdiff_words->plus.suppressed_newline) {\n \t\tputc('\\n', diff_words->file);\n \t\tdiff_words->minus.suppressed_newline = 0;\n+\t\tdiff_words->plus.suppressed_newline = 0;\n \t}\n }\n \n-- \n1.5.5.1.121.g26b3\n"},{"id":"75970","messageId":"1209874815-14411-5-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209874815-14411-4-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 4/6] --color-words: Make non-word characters configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:13Z","receivedAt":"2008-05-04T04:20:13Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Previously --color-words only allow spaces as non-word characters.\nHowever, just space is not enough. For example, when i rename a function\nfrom foo to bar, following example doesn't show as expected when using\n--color-words.\n\n  - if (foo(arg))\n  + if (bar(arg))\n\nAssuming \"r\" and \"g\" represent \"red\" and \"green\" separately. It shows as\n\n  if <r>(foo(arg))</r><g>(foo(arg))</g>\n\nActually, it's the best to show as\n\n  if (<r>foo</r><g>bar</g>(arg))\n\nThis patch introduces a configuration diff.nonwordchars to allow configurable\nnon-word characters (both spaces and chars in diff.nonwordchars)\nfor --color-words.\n\nNow, with diff.nonwordchars set to \"()\", the example above will show as\n\n  if (<r>foo(</r><g>bar(</g>arg))\n\nIt's much better, athough not the best,\n\nNOTE:\n\nWith current implementation (i.e. to replace non word characters with\nLF before feeding into xdi_diff), we can't get the best output.\n\nA more sensible implementation is to use 'insert' instead of 'replace'.\nSay, to insert a line break between runs of word characters and non-word\ncharacters or between non-word characters.\n\nThat is, \"foo>=bar\" will be rewritten as \"foo\\n>\\n=\\nbar\" instead of\n\"foo\\n\\nbar\".\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n Documentation/config.txt       |    4 ++++\n Documentation/diff-options.txt |    1 +\n diff.c                         |   12 +++++++++++-\n 3 files changed, 16 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 824e416..812ec2c 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -537,6 +537,10 @@ diff.external::\n \tprogram only on a subset of your files, you might want to\n \tuse linkgit:gitattributes[5] instead.\n \n+diff.nonwordchars::\n+\tSpecify additional boundary characters other than spaces for\n+\t--color-words.\n+\n diff.renameLimit::\n \tThe number of files to consider when performing the copy/rename\n \tdetection; equivalent to the git diff option '-l'.\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 13234fa..70acc14 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -95,6 +95,7 @@ endif::git-format-patch[]\n \n --color-words::\n \tShow colored word diff, i.e. color words which have changed.\n+\tThe non-word characters can be configured with diff.nonwordchars.\n \n --no-renames::\n \tTurn off rename detection, even when the configuration\ndiff --git a/diff.c b/diff.c\nindex c7a0d77..eb7c086 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -23,6 +23,7 @@ static int diff_rename_limit_default = 100;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n+static const char *diff_non_word_chars = \"\";\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \t\"\\033[m\",\t/* reset */\n@@ -159,6 +160,10 @@ int git_diff_ui_config(const char *var, const char *value)\n \t\texternal_diff_cmd_cfg = xstrdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"diff.nonwordchars\")) {\n+\t\tdiff_non_word_chars = value ? xstrdup(value) : \"\";\n+\t\treturn 0;\n+\t}\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \n@@ -443,6 +448,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t}\n }\n \n+static int is_non_word_char(char c)\n+{\n+\treturn isspace(c) || !!strchr(diff_non_word_chars, c);\n+}\n+\n static void mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n \tint i;\n \n@@ -450,7 +460,7 @@ static void mmfile_copy_set_boundary(mmfile_t *dest, mmfile_t *src) {\n \tdest->ptr = xmalloc(dest->size);\n \tmemcpy(dest->ptr, src->ptr, dest->size);\n \tfor (i = 0; i < dest->size; i++)\n-\t\tif (isspace(dest->ptr[i]))\n+\t\tif (is_non_word_char(dest->ptr[i]))\n \t\t\tdest->ptr[i] = '\\n';\n }\n \n-- \n1.5.5.1.121.g26b3\n"},{"id":"75976","messageId":"1209874815-14411-6-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209874815-14411-5-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 5/6] fn_out_diff_words_aux: Handle common diff line more carefully","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:14Z","receivedAt":"2008-05-04T04:20:14Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Before feeding minus and plus lines into xdi_diff, we replace non word\ncharacters with '\\n'. So we need recover the replaced character (always\nthe last character) in the callback fn_out_diff_words_aux.\n\nTherefore, a common diff line beginning with ' ' is not always a real\ncommon line. And we should check the last characters of the common diff\nline. If they are different, we should output the first len-1 characters\nas the common part and then the last characters in minus and plus\nseparately.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n diff.c |   18 +++++++++++++-----\n 1 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex eb7c086..08048e4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -414,7 +414,7 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in\n static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n {\n \tstruct diff_words_data *diff_words;\n-\tchar lastchar_minus;\n+\tchar lastchar_minus, lastchar_plus;\n \tstruct diff_words_buffer *dwb_minus, *dwb_plus;\n \tFILE *outfile;\n \n@@ -440,10 +440,18 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n \t\t\tbreak;\n \t\tcase ' ':\n \t\t\tlastchar_minus = dwb_minus->text.ptr[dwb_minus->current + len - 1];\n-\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 1);\n-\t\t\tdwb_minus->current += len;\n-\t\t\tif (lastchar_minus == '\\n')\n-\t\t\t\tdwb_minus->suppressed_newline = 1;\n+\t\t\tlastchar_plus = dwb_plus->text.ptr[dwb_plus->current + len - 1];\n+\t\t\tif (lastchar_minus == lastchar_plus) {\n+\t\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 1);\n+\t\t\t\tdwb_minus->current += len;\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tlen--;\n+\t\t\t\tprint_word(outfile, dwb_plus, len, DIFF_PLAIN, 1);\n+\t\t\t\tdwb_minus->current += len;\n+\t\t\t\tprint_word(outfile, dwb_minus, 1, DIFF_FILE_OLD, 1);\n+\t\t\t\tprint_word(outfile, dwb_plus, 1, DIFF_FILE_NEW, 1);\n+\t\t\t}\n \t\t\tbreak;\n \t}\n }\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75974","messageId":"1209874815-14411-7-git-send-email-pkufranky@gmail.com","threadId":"13350","inReplyTo":"1209874815-14411-6-git-send-email-pkufranky@gmail.com","subject":"[PATCH v3 6/6] --color-words: Add test t4030","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T04:20:15Z","receivedAt":"2008-05-04T04:20:15Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"Also add a script gen-expect.sh to generate the expected\ndiff output.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n t/t4030-diff-color-words.sh |   42 ++++++++++++++++++++++++++++++++++++++++++\n t/t4030/expect1             |    1 +\n t/t4030/expect10            |    1 +\n t/t4030/expect2             |    1 +\n t/t4030/expect3             |    1 +\n t/t4030/expect4             |    1 +\n t/t4030/expect5             |    1 +\n t/t4030/expect6             |    1 +\n t/t4030/expect7             |    1 +\n t/t4030/expect8             |    1 +\n t/t4030/expect9             |    1 +\n t/t4030/gen-expect.sh       |   35 +++++++++++++++++++++++++++++++++++\n 12 files changed, 87 insertions(+), 0 deletions(-)\n create mode 100755 t/t4030-diff-color-words.sh\n create mode 100644 t/t4030/expect1\n create mode 100644 t/t4030/expect10\n create mode 100644 t/t4030/expect2\n create mode 100644 t/t4030/expect3\n create mode 100644 t/t4030/expect4\n create mode 100644 t/t4030/expect5\n create mode 100644 t/t4030/expect6\n create mode 100644 t/t4030/expect7\n create mode 100644 t/t4030/expect8\n create mode 100644 t/t4030/expect9\n create mode 100644 t/t4030/gen-expect.sh\n\ndiff --git a/t/t4030-diff-color-words.sh b/t/t4030-diff-color-words.sh\nnew file mode 100755\nindex 0000000..e1c8e8e\n--- /dev/null\n+++ b/t/t4030-diff-color-words.sh\n@@ -0,0 +1,42 @@\n+#!/bin/sh\n+\n+test_description='diff --color-words'\n+\n+. ./test-lib.sh\n+. ../diff-lib.sh\n+\n+dotest() {\n+\ttest_expect_success \"$1\" \"\n+\techo '$1' >t &&\n+\tgit diff --color-words | tail -1 >actual &&\n+\tcat actual &&\n+\ttest_cmp ../t4030/$2 actual\n+\"\t\n+}\n+\n+test_expect_success 'setup for foo bar(_baz' '\n+\tgit config diff.nonwordchars \"_()\" &&\n+\techo \"foo bar(_baz\" > t &&\n+\tgit add t  &&\n+\tgit commit -m \"add t\"\n+'\n+\n+dotest \"foo bar(_\" expect1\n+dotest \"foo bar(\" expect2\n+dotest \"foo bar\" expect3\n+dotest \"foo (_baz\" expect4\n+dotest \"foo _baz\" expect5\n+dotest \"foo baz\" expect6\n+dotest \"bar(_baz\" expect7\n+\n+test_expect_success 'setup for foo bar(_' '\n+\techo \"foo bar(_\" > t &&\n+\tgit add t  &&\n+\tgit commit -m \"add t\"\n+'\n+\n+dotest \"foo bar(\" expect8\n+dotest \"foo bar\" expect9\n+dotest \"foo bar_baz\" expect10\n+\n+test_done\ndiff --git a/t/t4030/expect1 b/t/t4030/expect1\nnew file mode 100644\nindex 0000000..fb75467\n--- /dev/null\n+++ b/t/t4030/expect1\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[mbar(\u001b[m_\u001b[m\u001b[31mbaz\u001b[m\u001b[32m\u001b[m\ndiff --git a/t/t4030/expect10 b/t/t4030/expect10\nnew file mode 100644\nindex 0000000..b36e5db\n--- /dev/null\n+++ b/t/t4030/expect10\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[mbar\u001b[m\u001b[31m(\u001b[m\u001b[32m_\u001b[m\u001b[31m_\u001b[m\u001b[31m\u001b[m\u001b[32mbaz\u001b[m\ndiff --git a/t/t4030/expect2 b/t/t4030/expect2\nnew file mode 100644\nindex 0000000..6a9ca69\n--- /dev/null\n+++ b/t/t4030/expect2\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[mbar(\u001b[m\u001b[31m_\u001b[m\u001b[32m\u001b[m\u001b[31mbaz\u001b[m\ndiff --git a/t/t4030/expect3 b/t/t4030/expect3\nnew file mode 100644\nindex 0000000..d7d9885\n--- /dev/null\n+++ b/t/t4030/expect3\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[mbar\u001b[m\u001b[31m(\u001b[m\u001b[32m\u001b[m\u001b[31m_\u001b[m\u001b[31mbaz\u001b[m\ndiff --git a/t/t4030/expect4 b/t/t4030/expect4\nnew file mode 100644\nindex 0000000..449fd6d\n--- /dev/null\n+++ b/t/t4030/expect4\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[m\u001b[31mbar(\u001b[m\u001b[32m(\u001b[m_\u001b[mbaz\u001b[m\ndiff --git a/t/t4030/expect5 b/t/t4030/expect5\nnew file mode 100644\nindex 0000000..eb184f7\n--- /dev/null\n+++ b/t/t4030/expect5\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[m\u001b[31mbar(\u001b[m_\u001b[mbaz\u001b[m\ndiff --git a/t/t4030/expect6 b/t/t4030/expect6\nnew file mode 100644\nindex 0000000..58591ad\n--- /dev/null\n+++ b/t/t4030/expect6\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[m\u001b[31mbar(\u001b[m\u001b[31m_\u001b[mbaz\u001b[m\ndiff --git a/t/t4030/expect7 b/t/t4030/expect7\nnew file mode 100644\nindex 0000000..c68a1f1\n--- /dev/null\n+++ b/t/t4030/expect7\n@@ -0,0 +1 @@\n+\u001b[m\u001b[31mfoo \u001b[mbar(\u001b[m_\u001b[mbaz\u001b[m\ndiff --git a/t/t4030/expect8 b/t/t4030/expect8\nnew file mode 100644\nindex 0000000..f48152f\n--- /dev/null\n+++ b/t/t4030/expect8\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[mbar(\u001b[m\u001b[31m_\u001b[m\u001b[32m\u001b[m\u001b[31m\u001b[m\ndiff --git a/t/t4030/expect9 b/t/t4030/expect9\nnew file mode 100644\nindex 0000000..4f42b6c\n--- /dev/null\n+++ b/t/t4030/expect9\n@@ -0,0 +1 @@\n+\u001b[mfoo \u001b[mbar\u001b[m\u001b[31m(\u001b[m\u001b[32m\u001b[m\u001b[31m_\u001b[m\u001b[31m\u001b[m\ndiff --git a/t/t4030/gen-expect.sh b/t/t4030/gen-expect.sh\nnew file mode 100644\nindex 0000000..da07535\n--- /dev/null\n+++ b/t/t4030/gen-expect.sh\n@@ -0,0 +1,35 @@\n+#!/bin/bash\n+\n+git config diff.nonwordchars \"_()\"\n+\n+echo \"foo bar(_baz\" >&2 &&\n+echo \"foo bar(_baz\" > t\n+\n+git add t  &&\n+git commit -m \"add t\"\n+\n+dotest() {\n+\techo \"$1\" >&2 &&\n+\techo \"$1\" >t  &&\n+\tgit diff --color-words |\n+\ttail -1 > $2\n+}\n+\n+dotest \"foo bar(_\" expect1\n+dotest \"foo bar(\" expect2\n+dotest \"foo bar\" expect3\n+dotest \"foo (_baz\" expect4\n+dotest \"foo _baz\" expect5\n+dotest \"foo baz\" expect6\n+dotest \"bar(_baz\" expect7\n+\n+\n+echo \"foo bar(_\" >&2 &&\n+echo \"foo bar(_\" >t\n+\n+git add t  &&\n+git commit -m \"add t\"\n+\n+dotest \"foo bar(\" expect8\n+dotest \"foo bar\" expect9\n+dotest \"foo bar_baz\" expect10\n-- \n1.5.5.1.121.g26b3\n"},{"id":"75991","messageId":"7vk5iar2it.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"1209874815-14411-5-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v3 4/6] --color-words: Make non-word characters configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-04T06:45:14Z","receivedAt":"2008-05-04T06:45:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ping Yin <pkufranky@gmail.com> writes:\n\n> A more sensible implementation is to use 'insert' instead of 'replace'.\n> Say, to insert a line break between runs of word characters and non-word\n> characters or between non-word characters.\n>\n> That is, \"foo>=bar\" will be rewritten as \"foo\\n>\\n=\\nbar\" instead of\n> \"foo\\n\\nbar\".\n\nHmmmm.\n\nI am not sure if/why you would want a separator between '>' and '=' in\nthat example.  Shouldn't that be \"foo <sep> >= <sep> bar\"?\n"},{"id":"75993","messageId":"46dff0320805040004g2f38494fx33e062ede4204bd4@mail.gmail.com","threadId":"13350","inReplyTo":"7vk5iar2it.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3 4/6] --color-words: Make non-word characters configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T07:04:03Z","receivedAt":"2008-05-04T07:04:03Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ping Yin <pkufranky@gmail.com> writes:\n>\n>  > A more sensible implementation is to use 'insert' instead of 'replace'.\n>  > Say, to insert a line break between runs of word characters and non-word\n>  > characters or between non-word characters.\n>  >\n>  > That is, \"foo>=bar\" will be rewritten as \"foo\\n>\\n=\\nbar\" instead of\n>  > \"foo\\n\\nbar\".\n>\n>  Hmmmm.\n>\n>  I am not sure if/why you would want a separator between '>' and '=' in\n>  that example.  Shouldn't that be \"foo <sep> >= <sep> bar\"?\n\nThat's intentional. If i change >= to >, i want it to be highlighted\nas <r>=</r> instead of <r> >= </r> <g> > </g> (spaced added for\nreadability). So i tend to consider each non-word character as a\nsingle word.\n\n\n-- \nPing Yin\n"},{"id":"75995","messageId":"alpine.DEB.1.00.0805041010000.30431@racer","threadId":"13350","inReplyTo":"20080503144337.GA7987@mithlond.arda.local","subject":"Re: [PATCH] --color-words: Make the word characters configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:18:21Z","receivedAt":"2008-05-04T09:18:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 3 May 2008, Teemu Likonen wrote:\n\n> Johannes Schindelin wrote (2008-05-03 15:03 +0100):\n> \n> > Now, you can specify which characters are to be interpreted as word\n> > characters with \"--color-words=A-Za-z\", or by setting the config\n> > variable diff.wordCharacters.\n> > \n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> > \n> > \tI would have preferred an approach like this.\n> \n> Unfortunately this does not work at all with other than Ascii \n> characters. It makes --color-words completely unusable for anything \n> other than Ascii text. Sorry.\n\nSorry, but the original way was also only meant for ASCII.  The fact that \nisspace() happens to work with UTF-8 does _not_ mean that it was any more \nuseful with non-ASCII: think UTF-16.\n\nSo no, I do not buy into your ASCII argument at all.\n\n> Ping Yin's version has also the problem that UTF-8 multibyte characters\n> U+0080..U+10FFFF don't work in diff.nonwordchars. Fortunately the most\n> important word delimiters are in U+0000..U+007F (=Ascii) area so Ping's\n> version is perfectly usable with Unicode text.\n>\n> (Even the old --color-words behaviour with only SPACE as non-word char \n> was perfectly usable with Unicode text.)\n\nSee above.\n\n> I, too, would like to see Ping's patch series merged in.\n\nI have no problems with the intention.  But I have problems with the \ndesign.  It is no less ASCII-bound than what I proposed, it wants you to \nspecify what does _not_ make a word character (making every newbie, and \nme, too, going \"Huh?\").\n\nAnd I also commented on the artificial limitations by design: I think it \nis a big mistake to limit the user's options when it would be easy not to, \njust because the designer could not think of useful applications.\n\nCiao,\nDscho\n"},{"id":"75996","messageId":"alpine.DEB.1.00.0805041018280.30431@racer","threadId":"13350","inReplyTo":"7vmyn7uvut.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] --color-words: Make the word characters configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:25:39Z","receivedAt":"2008-05-04T09:25:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 3 May 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Now, you can specify which characters are to be interpreted as word \n> > characters with \"--color-words=A-Za-z\", or by setting the config variable \n> > diff.wordCharacters.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >\n> > \tI would have preferred an approach like this.\n> \n> Hmmm...\n\nJust to clarify: specifying word characters, and allowing sets (as \nspecifyable for tr(1)).\n\n> > diff --git a/README b/README\n> > index 548142c..0e325e2 100644\n> > --- a/README\n> > +++ b/README\n> > @@ -4,7 +4,7 @@\n> >  \n> >  ////////////////////////////////////////////////////////////////\n> >  \n> > -\"git\" can mean anything, depending on your mood.\n> > +\"git\" cann mean anything, depending on your mood.\n> \n> Heh.\n\nYeah, I already said I am a moron.  I can repeat it if it makes you \nhappier ;-)\n\n> > @@ -456,7 +514,7 @@ static void diff_words_show(struct diff_words_data *diff_words)\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\tif (!word_character[(unsigned char)plus.ptr[i]])\n> >  \t\t\tplus.ptr[i] = '\\n';\n> >  \tdiff_words->plus.current = 0;\n> \n> I do not think there is much difference between specifying the set of \n> word characters and the set of non-word characters, especially as long \n> as your definition of \"character\" is limited to 8-bit bytes.  By \n> enumerating word characters, your patch is letting the user specify non \n> word characters that are remainder from the 256-element set.  By the \n> way, I think you meant to do the same for the \"minus\" side a few lines \n> above this hunk.\n\nI just imitated Ping's patch, but you're right, I forgot that.\n\n> I commented on the patch from Ping earier about a quite different issue. \n> I was wondering if we can avoid losing the non-word character \n> information. The original code replaces any isspace byte with LF, but a \n> whitespace is a whitespace is a whitespace so there won't be much loss \n> of information, but making the above isspace() configurable means that \n> now you are going to drop non-space non-word characters from the output \n> set.\n> \n> Instead of dropping the original character and replacing it with LF, I \n> thought a more sensible approach would be to _insert_ a line break \n> between runs of word characters and non-word characters (while probably \n> dropping a LF in the original).  That is, instead of what the current \n> implementation of the above loop does to \"ab c d\" (i.e. rewrite it to \n> \"ab\\n\\nc\\nd\"), rewrite it to \"ab\\n \\nc\\n \\nd\".  Which feels more \n> consistent with the way how \\b should work.\n\nThe conversion to \"\\n\" is done only because of limitations in libxdiff \n(did I not just rant about artificial limitations in another mail?), \nbecause it is married to the notion that LF ends a line.\n\nNow, there are two options:\n\n- try to reconstruct the original text from what libxdiff returns.  This \n  is potentially memory-efficient, but tricky, and therefore easy to get \n  wrong.\n\n- go with your approach.  You will have to duplicate all the text, so this \n  is something quite heavy on memory consumption.  But you have to do \n  something special for _real_ LFs so that they are not stripped away when \n  displaying the result.\n\nI like your idea (I was trying to come up with something sensible for the \nfirst option, but as I said, it is too tricky).\n\nBut the LF issue is a real one.\n\nCiao,\nDscho\n"},{"id":"76000","messageId":"alpine.DEB.1.00.0805041040560.30431@racer","threadId":"13350","inReplyTo":"46dff0320805031732x25286707r991358162046c07c@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:44:41Z","receivedAt":"2008-05-04T09:44:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 May 2008, Ping Yin wrote:\n\n> If we set diff.nonwordchars to \"()\", the example above will show as \"if \n> (<r>foo(</r><g>bar(</g>arg))\". It's much better, athough not the best.\n\nOkay, let's use the power of Open Source, and come up with the best \nsolution.\n\nThe problem: given two chunks of text, where a word was changed, and a\nnon-word-character was moved to the next line.  Example:\n\n\tThe quick,\n\tbrown fox\n\nvs\n\n\tThe fast\n\t, brown fox\n\nIMHO the layout of the new version should be retained, i.e.\n\n\tThe /quick/fast/\n\t, brown fox\n\nshould be shown.\n\nIf everybody is fine with that, I'll try to come up with an \nimplementation. \n\nCiao,\nDscho\n"},{"id":"76001","messageId":"alpine.DEB.1.00.0805041046040.30431@racer","threadId":"13350","inReplyTo":"1209874815-14411-2-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v3 1/6] diff.c: Remove code redundancy in diff_words_show","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:46:51Z","receivedAt":"2008-05-04T09:46:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 May 2008, Ping Yin wrote:\n\n> Introduce mmfile_copy_set_boundary to do the repeated work.\n\nIt the name supposed to be descriptive?  Because it sure is not for me.  \nReading the patch, I know what it should do, but the commit message could \nbe a big blank space then, to save me time.\n\nCiao,\nDscho\n"},{"id":"76002","messageId":"alpine.DEB.1.00.0805041047220.30431@racer","threadId":"13350","inReplyTo":"1209874815-14411-3-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v3 2/6] fn_out_diff_words_aux: Use short variable name","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:47:50Z","receivedAt":"2008-05-04T09:47:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nthe use of this patch evades me.  Care to defend why it is necessary?  \nPreferably in the commit message?\n\nThanks,\nDscho\n"},{"id":"76003","messageId":"alpine.DEB.1.00.0805041049150.30431@racer","threadId":"13350","inReplyTo":"1209874815-14411-4-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v3 3/6] --color-words: Fix showing trailing deleted words at another line","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:52:26Z","receivedAt":"2008-05-04T09:52:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 May 2008, Ping Yin wrote:\n\n> With --color-words, following example will show deleted word \"bar\" at\n> another line.\n\n\"will\", or \"used to\"?\n\n> This is caused by the unsymmetrical handling of LF in the plus and minus \n> buffer in fn_out_diff_words_aux.\n\nIs it not rather caused by the need to replace non-word-characters with \nLF?\n\n> Following is original unsymmetrical handling rules where LF represents\n> a LF will be shown there.\n\nI cannot parse this sentence.\n\n> The second rule causes any word following the trailing plus word will\n> be shown in a different line.\n\nI cannot parse this sentence.\n\n> @@ -417,10 +418,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n>  \tdwb_plus = &(diff_words->plus);\n>  \toutfile = diff_words->file;\n>  \n> -\tif (dwp_minus->suppressed_newline) {\n> -\t\tif (line[0] != '+')\n> -\t\t\tputc('\\n', outfile);\n> -\t\tdwp_minus->suppressed_newline = 0;\n> +\tif ((dwb_minus->suppressed_newline && line[0] != '+') ||\n\nIf the previous version had dwp_minus, and the new version has dwb_minus, \nI wonder if both compile and pass the test suite.\n\nCiao,\nDscho\n"},{"id":"76004","messageId":"alpine.DEB.1.00.0805041053380.30431@racer","threadId":"13350","inReplyTo":"1209874815-14411-6-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v3 5/6] fn_out_diff_words_aux: Handle common diff line more carefully","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-04T09:54:19Z","receivedAt":"2008-05-04T09:54:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 May 2008, Ping Yin wrote:\n\n> Before feeding minus and plus lines into xdi_diff, we replace non word\n> characters with '\\n'. So we need recover the replaced character (always\n> the last character) in the callback fn_out_diff_words_aux.\n> \n> Therefore, a common diff line beginning with ' ' is not always a real\n> common line.\n\nUmm, why?\n\n> And we should check the last characters of the common diff line. If they \n> are different, we should output the first len-1 characters as the common \n> part and then the last characters in minus and plus separately.\n\nUmm, why?\n\nCiao,\nDscho\n"},{"id":"76028","messageId":"46dff0320805040935n22354e1bta85b3f3fe7c16cad@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805041040560.30431@racer","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T16:35:57Z","receivedAt":"2008-05-04T16:35:57Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 5:44 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n>  The problem: given two chunks of text, where a word was changed, and a\n>  non-word-character was moved to the next line.  Example:\n>\n>         The quick,\n>         brown fox\n>\n>  vs\n>\n>         The fast\n>         , brown fox\n>\n>  IMHO the layout of the new version should be retained, i.e.\n>\n>         The /quick/fast/\n>         , brown fox\n>\n>  should be shown.\n\nWhy not\n\n  The <r>quick</r><g>fast</g><r>,</r>\n  <g>,</g>brown fox\n\n\n\n-- \nPing Yin\n"},{"id":"76029","messageId":"46dff0320805040939t6290ed1dsad79d8de99c7cdde@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805041047220.30431@racer","subject":"Re: [PATCH v3 2/6] fn_out_diff_words_aux: Use short variable name","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T16:39:22Z","receivedAt":"2008-05-04T16:39:22Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 5:47 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n>  the use of this patch evades me.  Care to defend why it is necessary?\n>  Preferably in the commit message?\n>\n\nIn fn_out_diff_words_aux, we use diff_words->plus in so many places.\nSo just use shorter name to save typing and avoid typo.\n\n-- \nPing Yin\n"},{"id":"76030","messageId":"46dff0320805040948g2956d724wb41f3eb8651443@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805041049150.30431@racer","subject":"Re: [PATCH v3 3/6] --color-words: Fix showing trailing deleted words at another line","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T16:48:15Z","receivedAt":"2008-05-04T16:48:15Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 5:52 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n>\n>  On Sun, 4 May 2008, Ping Yin wrote:\n>\n>  > With --color-words, following example will show deleted word \"bar\" at\n>  > another line.\n>\n>  \"will\", or \"used to\"?\n\nshould be 'used to'\n\n>  > This is caused by the unsymmetrical handling of LF in the plus and minus\n>  > buffer in fn_out_diff_words_aux.\n>\n>  Is it not rather caused by the need to replace non-word-characters with\n>  LF?\n\nNo, i think this has nothing to do with the replacing\nnon-word-characters with LF.\n\n>\n>  > Following is original unsymmetrical handling rules where LF represents\n>  > a LF will be shown there.\n>\n>  I cannot parse this sentence.\n\nFollowing is the original unsymmetrical handling rules\n\n>\n>  > The second rule causes any word following the trailing plus word will\n>  > be shown in a different line.\n>\n>  I cannot parse this sentence.\n>\n>\n\nThe second rule causes any word following the trailing plus word to show\nin a different line with the trailing plus word.\n\n>\n>  > @@ -417,10 +418,11 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)\n>  >       dwb_plus = &(diff_words->plus);\n>  >       outfile = diff_words->file;\n>  >\n>  > -     if (dwp_minus->suppressed_newline) {\n>  > -             if (line[0] != '+')\n>  > -                     putc('\\n', outfile);\n>  > -             dwp_minus->suppressed_newline = 0;\n>  > +     if ((dwb_minus->suppressed_newline && line[0] != '+') ||\n>\n>  If the previous version had dwp_minus, and the new version has dwb_minus,\n>  I wonder if both compile and pass the test suite.\n>\n\nOh, it's a typo in the former patch.\n\n\n-- \nPing Yin\n"},{"id":"76031","messageId":"46dff0320805040953i4230a686j8e8d63eaa6728c2f@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805041053380.30431@racer","subject":"Re: [PATCH v3 5/6] fn_out_diff_words_aux: Handle common diff line more carefully","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-04T16:53:17Z","receivedAt":"2008-05-04T16:53:17Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sun, May 4, 2008 at 5:54 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n>\n>  On Sun, 4 May 2008, Ping Yin wrote:\n>\n>  > Before feeding minus and plus lines into xdi_diff, we replace non word\n>  > characters with '\\n'. So we need recover the replaced character (always\n>  > the last character) in the callback fn_out_diff_words_aux.\n>  >\n>  > Therefore, a common diff line beginning with ' ' is not always a real\n>  > common line.\n>\n>  Umm, why?\n\nBecause we need recover the replaced character.\n\nSay, for a common diff line \" foo\", after restoring the replaced\ncharacter, the corresponding line in minus and plus may be different.\nFor example, \"foo(\" and \"foo)\".\n\n>  > And we should check the last characters of the common diff line. If they\n>  > are different, we should output the first len-1 characters as the common\n>  > part and then the last characters in minus and plus separately.\n>\n>  Umm, why?\n\nExplained.\n\n\n\n-- \nPing Yin\n"},{"id":"76040","messageId":"7v63ttq0y8.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"46dff0320805040935n22354e1bta85b3f3fe7c16cad@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"junio@pobox.com","sentAt":"2008-05-04T20:16:47Z","receivedAt":"2008-05-04T20:16:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Let's step back a bit and try to clarify the problem with a bit of\nillustration.\n\nThe motivation behind \"word diff\" is because line oriented diff is\nsometimes unwieldy.\n\n    -Hello world.\n    +Hi, world.\n\nA naïve strategy to solve this would be to convert the input into one\ncharacter a line while changing the representation of characters into\ntheir codepoints, take the diff between them, and synthesize the result\nback, like this:\n\n    preimage        postimage       char-diff\n    48 H            48 H             48 H\n    65 e                            -65 e\n    6c l                            -6c l\n    6c l                            -6c l\n    6f o                            -6f o\n                    69 i            +69 i\n                    2c ,            +2c ,\n    20 ' '          20 ' '           20 ' ' \n    77 w            77 w             77 w   \n    6f o            6f o             6f o   \n    72 r            72 r             72 r   \n    6c l            6c l             6c l   \n    64 d            64 d             64 d   \n    2e .            2e .             2e .   \n    0a '\\n'         0a '\\n'          0a '\\n'\n\nThat would produce \"H/ello/i,/ world.\\n\" which is very suboptimal for\nhuman consumption because it chomps a word \"Hello\" and \"Hi\" in the middle.\nWe instead can do this word by word (note that I am doing this as a\nthought experiment, to illustrate what the problem is and what should\nconceptually happen, not suggesting this particular implementation):\n\n    preimage        postimage       word-diff\n    48656c6c6f                      -48656c6c6f Hello\n                    4869            +4869       Hi\n                    2c              +2c         ,\n    20              20               20         ' '\n    776f726c64      776f726c64       776f726c64 world      \n    2e              2e               2e         .\n    0a              0a               0a         '\\n'\n\nWhich would give you \"/Hello/Hi,/ world.\\n\".\n\nAnother my favorite example:\n\n    -if (i > 1)\n    +while (i >= 0)\n        \n    preimage       postimage        word-diff\n    6966                            -6966       if\n                   7768696c65       +7768696c65 while\n    20             20                20         ' '\n    28             28                28         (  \n    69             69                69         i  \n    20             20                20         ' '\n    3e                              -3e         >\n                   3e3d             +3e3d       >=\n    20             20                20         ' '\n    31                              -31         1  \n                   30               +30         0  \n    29             29                29         )\n\nwhich should yield \"/if/while/ (i />/>=/ /1/0/)\".\n\nSo the overall algorithm I think should be is:\n\n - make the input into stream of tokens, where a token is either a run of\n   word characters only, non-word punct characters only, or whitespaces\n   only;\n\n - compute the diff over the stream of tokens;\n\n - emit common tokens in white, deleted in red and added in green.\n\nNotice that you do not have to special case LF in any way if you go this\nroute.\n\nYou could do this with only two classes, and use a different tokenization\nrule: a token is either a run of word characters only, or each byte of non\nword character becomes individual token.  This however would yield a\nsuboptimal result:\n\n    -if (i > 1)\n    +while (i >= 0)\n        \n    preimage       postimage        word-diff\n    6966                            -6966       if\n                   7768696c65       +7768696c65 while\n    20             20                20         ' '\n    28             28                28         (  \n    69             69                69         i  \n    20             20                20         ' '\n    3e             3e                3e         >\n                   3d               +3d         =\n    20             20                20         ' '\n    31                              -31         1  \n                   30               +30         0  \n    29             29                29         )\n\nThis would give \"/if/while/ (i >//=/ /1/0/)\".  A logical unit \">=\" is\nchomped into two tokens, which is suboptimal for the same reason why the\noutput \"H/ello/i,/\" from the original char-diff based one was suboptimal.\n"},{"id":"76042","messageId":"m3ve1t6bli.fsf@localhost.localdomain","threadId":"13350","inReplyTo":"7v63ttq0y8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-04T20:47:28Z","receivedAt":"2008-05-04T20:47:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano <junio@pobox.com> writes:\n\n> Let's step back a bit and try to clarify the problem with a bit of\n> illustration.\n> \n> The motivation behind \"word diff\" is because line oriented diff is\n> sometimes unwieldy.\n> \n>     -Hello world.\n>     +Hi, world.\n[...]\n> We instead can do this word by word (note that I am doing this as a\n> thought experiment, to illustrate what the problem is and what should\n> conceptually happen, not suggesting this particular implementation):\n> \n>     preimage        postimage       word-diff\n>     48656c6c6f                      -48656c6c6f Hello\n>                     4869            +4869       Hi\n>                     2c              +2c         ,\n>     20              20               20         ' '\n>     776f726c64      776f726c64       776f726c64 world      \n>     2e              2e               2e         .\n>     0a              0a               0a         '\\n'\n> \n> Which would give you \"/Hello/Hi,/ world.\\n\".\n\nWould it be possible instead of in-line word diff, use word coloring\nto enhance traditional diff format?  Something like\n\n     -/Hello/ world.\n     +/Hi,/ world.\n\n(We could use bold, or reverse for marking changed fragment, or use\ncolor only for changed fragment).\n\nIMHO current output is nice, unless you have long lines and not very\nwide screen...\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"76044","messageId":"20080504212708.GA5660@mithlond.arda.local","threadId":"13350","inReplyTo":"m3ve1t6bli.fsf@localhost.localdomain","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-04T21:27:08Z","receivedAt":"2008-05-04T21:27:08Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Jakub Narebski wrote (2008-05-04 13:47 -0700):\n\n> Would it be possible instead of in-line word diff, use word coloring\n> to enhance traditional diff format?  Something like\n> \n>      -/Hello/ world.\n>      +/Hi,/ world.\n> \n> (We could use bold, or reverse for marking changed fragment, or use\n> color only for changed fragment).\n\nThat would be helpful too, no doubt. I'm advocating the word diff\nbecause it's extremely useful with human languages. Lines don't have\n(usually) any special meaning there so in this context words are the\nmost useful units.\n"},{"id":"76058","messageId":"46dff0320805041840g1b9362d3u138b9d40cde160f2@mail.gmail.com","threadId":"13350","inReplyTo":"7v63ttq0y8.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-05T01:40:47Z","receivedAt":"2008-05-05T01:40:47Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On 5/5/08, Junio C Hamano <junio@pobox.com> wrote:\n>\n> So the overall algorithm I think should be is:\n>\n>  - make the input into stream of tokens, where a token is either a run of\n>   word characters only, non-word punct characters only, or whitespaces\n>   only;\n>\n>  - compute the diff over the stream of tokens;\n>\n>  - emit common tokens in white, deleted in red and added in green.\n>\n> Notice that you do not have to special case LF in any way if you go this\n> route.\n>\n> You could do this with only two classes, and use a different tokenization\n> rule: a token is either a run of word characters only, or each byte of non\n> word character becomes individual token.  This however would yield a\n> suboptimal result:\n>\n>    -if (i > 1)\n>    +while (i >= 0)\n>\n>    preimage       postimage        word-diff\n>    6966                            -6966       if\n>                   7768696c65       +7768696c65 while\n>    20             20                20         ' '\n>    28             28                28         (\n>    69             69                69         i\n>    20             20                20         ' '\n>    3e             3e                3e         >\n>                   3d               +3d         =\n>    20             20                20         ' '\n>    31                              -31         1\n>                   30               +30         0\n>    29             29                29         )\n>\n> This would give \"/if/while/ (i >//=/ /1/0/)\".  A logical unit \">=\" is\n> chomped into two tokens, which is suboptimal for the same reason why the\n> output \"H/ello/i,/\" from the original char-diff based one was suboptimal.\n>\n\nFor this example,both \"/if/while/ (i />/>=/ /1/0/)\" and  \"/if/while/\n(i >//=/ /1/0/)\" are fine to me. However, the run of non-word\ncharacters shouldn't always be considered as a single token.\n\nFor example\n\n  - **************\n  + ************\n\nIf  just a '+' is removed, surely \"************/*//\" is better.\n\nAnd when designing, i think it's better to take multi-byte characters\ninto account. For multi-byte characters (especially CJK), every\ncharacter should be considered as a token. if we consider either a run\nof word characters or a run of non-word characters as a single token,\nthere is no way to specify every character as a token.\n\nSo from this viewpoint, is it better to use single-token character or\nsomething else instead of non-word character?\n\nAnother consideration: Space information is also important for me when\nusing --color-words. However, i can't distinguish between the removed\nspaces and added spaces in current implementaion. So how about use\nred/green background color for removed/added spaces?\n\n-- \nPing Yin\n"},{"id":"76073","messageId":"7vprs1ny5e.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"46dff0320805041840g1b9362d3u138b9d40cde160f2@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-05T05:00:13Z","receivedAt":"2008-05-05T05:00:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n> For this example,both \"/if/while/ (i />/>=/ /1/0/)\" and  \"/if/while/\n> (i >//=/ /1/0/)\" are fine to me.\n\nFor the particular example, both are Ok, but for this other example:\n\n\t-if (i > 1...\n        +if ((i > 1...\n\nit probably is better to treat each non-word character as a separate\ntoken, that is, it would be easier to read if we said \"( stayed intact,\nand another ( was added\", instead of saying \"( is changed to ((\".\n\nSo \"a run of punct chars\" rule only sometimes produces better output but\notherwise worse output, and to make it produce better output consistently,\nwe would need to know the syntax of the target language for tokenization,\ni.e. \">=\" and \">\" are comparison operators, while \"(\" is a token and \"((\"\nis better split into two open-paren tokens.\n\nSo as a very longer term subproject, we may want to teach the mechanism\nlanguage specific tokenization rules, just like we can specify the hunk\nheader pattern via gitattributes(5) to the diff output layer.\n\nOf course, I do not expect you to do that during this round --- and if we\nchoose to keep the rule simple, I think it is probably better to use\none-char-one-token rule for now.\n\n> And when designing, i think it's better to take multi-byte characters\n> into account. For multi-byte characters (especially CJK), every\n> character should be considered as a token.\n\nIf we take an idealistic view for the longer term, we should be tokenizing\neven CJK sensibly, but unlike Occidental scripts, we cannot even use\ninter-word spacing for tokenizing hint, so unless we are willing to learn\nmorphological analysis (which we are not for now), the best we can do is\nto use one-char-one-token rule.\n\n\tSide Note.  For Japanese we could cheat and often do a slightly\n\tbetter job than simple one-char-one-token without having full\n\tmorphological analysis by splicing between Kanji and Kana\n\tboundaries, but I'd prefer not to go there and keep the rules we\n\twould use to the minimum.\n\nI should stress that I said \"character\" in the above \"punct\" and \"CJK\"\ndiscussions, not \"byte\".\n"},{"id":"76113","messageId":"alpine.DEB.1.00.0805051249520.30431@racer","threadId":"13350","inReplyTo":"46dff0320805040935n22354e1bta85b3f3fe7c16cad@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-05T11:51:21Z","receivedAt":"2008-05-05T11:51:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 5 May 2008, Ping Yin wrote:\n\n> On Sun, May 4, 2008 at 5:44 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> >  The problem: given two chunks of text, where a word was changed, and a\n> >  non-word-character was moved to the next line.  Example:\n> >\n> >         The quick,\n> >         brown fox\n> >\n> >  vs\n> >\n> >         The fast\n> >         , brown fox\n> >\n> >  IMHO the layout of the new version should be retained, i.e.\n> >\n> >         The /quick/fast/\n> >         , brown fox\n> >\n> >  should be shown.\n> \n> Why not\n> \n>   The <r>quick</r><g>fast</g><r>,</r>\n>   <g>,</g>brown fox\n\nI might well be a complete idiot, but your <r></r><something> example is \nway harder for me to read than my example.\n\nAnd of course your example would still be wrong: the \"quick\" and the comma \nwould not be separated at all.\n\nCiao,\nDscho\n"},{"id":"76114","messageId":"46dff0320805050502l5a456b69oe621ebad28b8eb63@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805051249520.30431@racer","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-05T12:02:46Z","receivedAt":"2008-05-05T12:02:46Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Mon, May 5, 2008 at 7:51 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>  > >\n>  > >         The quick,\n>  > >         brown fox\n>  > >\n>  > >  vs\n>  > >\n>  > >         The fast\n>  > >         , brown fox\n>  > >\n>  > >  IMHO the layout of the new version should be retained, i.e.\n>  > >\n>  > >         The /quick/fast/\n>  > >         , brown fox\n>  > >\n>  > >  should be shown.\n>  >\n>  > Why not\n>  >\n>  >   The <r>quick</r><g>fast</g><r>,</r>\n>  >   <g>,</g>brown fox\n>\n>  I might well be a complete idiot, but your <r></r><something> example is\n>  way harder for me to read than my example.\n>\n>  And of course your example would still be wrong: the \"quick\" and the comma\n>  would not be separated at all.\n\nSo i am an idiot too -:). Should be\n\n The /quick,/fast/\n //, /brown fox\n\n\n-- \nPing Yin\n"},{"id":"76115","messageId":"alpine.DEB.1.00.0805051305280.30431@racer","threadId":"13350","inReplyTo":"46dff0320805040939t6290ed1dsad79d8de99c7cdde@mail.gmail.com","subject":"Re: [PATCH v3 2/6] fn_out_diff_words_aux: Use short variable name","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-05T12:05:49Z","receivedAt":"2008-05-05T12:05:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 5 May 2008, Ping Yin wrote:\n\n> On Sun, May 4, 2008 at 5:47 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > Hi,\n> >\n> >  the use of this patch evades me.  Care to defend why it is necessary?\n> >  Preferably in the commit message?\n> >\n> \n> In fn_out_diff_words_aux, we use diff_words->plus in so many places.\n> So just use shorter name to save typing and avoid typo.\n\nMe, on the other hand, I find \"diff_words->plus\" so much better to read.\n\nCiao,\nDscho\n"},{"id":"76116","messageId":"46dff0320805050510t3bc5fd0eq44e0d58d1bb57629@mail.gmail.com","threadId":"13350","inReplyTo":"7vprs1ny5e.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-05T12:10:11Z","receivedAt":"2008-05-05T12:10:11Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Mon, May 5, 2008 at 1:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>\n>  > For this example,both \"/if/while/ (i />/>=/ /1/0/)\" and  \"/if/while/\n>  > (i >//=/ /1/0/)\" are fine to me.\n>\n>  For the particular example, both are Ok, but for this other example:\n>\n>         -if (i > 1...\n>         +if ((i > 1...\n>\n>  it probably is better to treat each non-word character as a separate\n>  token, that is, it would be easier to read if we said \"( stayed intact,\n>  and another ( was added\", instead of saying \"( is changed to ((\".\n>\n>  So \"a run of punct chars\" rule only sometimes produces better output but\n>  otherwise worse output, and to make it produce better output consistently,\n>  we would need to know the syntax of the target language for tokenization,\n>  i.e. \">=\" and \">\" are comparison operators, while \"(\" is a token and \"((\"\n>  is better split into two open-paren tokens.\n>\n>  So as a very longer term subproject, we may want to teach the mechanism\n>  language specific tokenization rules, just like we can specify the hunk\n>  header pattern via gitattributes(5) to the diff output layer.\n>\n>  Of course, I do not expect you to do that during this round --- and if we\n>  choose to keep the rule simple, I think it is probably better to use\n>  one-char-one-token rule for now.\n>\n>\n>  > And when designing, i think it's better to take multi-byte characters\n>  > into account. For multi-byte characters (especially CJK), every\n>  > character should be considered as a token.\n>\n>  If we take an idealistic view for the longer term, we should be tokenizing\n>  even CJK sensibly, but unlike Occidental scripts, we cannot even use\n>  inter-word spacing for tokenizing hint, so unless we are willing to learn\n>  morphological analysis (which we are not for now), the best we can do is\n>  to use one-char-one-token rule.\n>\n>         Side Note.  For Japanese we could cheat and often do a slightly\n>         better job than simple one-char-one-token without having full\n>         morphological analysis by splicing between Kanji and Kana\n>         boundaries, but I'd prefer not to go there and keep the rules we\n>         would use to the minimum.\n>\n>  I should stress that I said \"character\" in the above \"punct\" and \"CJK\"\n>  discussions, not \"byte\".\n>\n\nThe one-char-one-token and multi-char-one-token rules may have\ndifferent implementation issues. I think multi-char-one-token rule may\nbe more representative. So for the current time, i prefer considering\nboth run of word characters and single non-word character as a token.\n\n\n\n-- \nPing Yin\n"},{"id":"76117","messageId":"alpine.DEB.1.00.0805051306160.30431@racer","threadId":"13350","inReplyTo":"46dff0320805040948g2956d724wb41f3eb8651443@mail.gmail.com","subject":"Re: [PATCH v3 3/6] --color-words: Fix showing trailing deleted words at another line","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-05T12:10:26Z","receivedAt":"2008-05-05T12:10:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 5 May 2008, Ping Yin wrote:\n\n> On Sun, May 4, 2008 at 5:52 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> >  On Sun, 4 May 2008, Ping Yin wrote:\n> >\n> >  > This is caused by the unsymmetrical handling of LF in the plus and \n> >  > minus buffer in fn_out_diff_words_aux.\n> >\n> >  Is it not rather caused by the need to replace non-word-characters \n> >  with LF?\n> \n> No, i think this has nothing to do with the replacing \n> non-word-characters with LF.\n\nI know that you _think_ that.  But you do a bad job convincing me (by \navoiding an explanation).\n\n> >  > Following is original unsymmetrical handling rules where LF \n> >  > represents a LF will be shown there.\n> >\n> >  I cannot parse this sentence.\n> \n> Following is the original unsymmetrical handling rules\n\nI am sorry, but this non-native English speaker still has trouble \nrecognizing the meaning in this.\n\n> >  > The second rule causes any word following the trailing plus word \n> >  > will be shown in a different line.\n> >\n> >  I cannot parse this sentence.\n> \n> The second rule causes any word following the trailing plus word to show \n> in a different line with the trailing plus word.\n\nAgain, I can only guess that you mean something like this:\n\n\t2nd rule: any word after an added word will be shown after a line \n\tbreak.\n\nAgain, I am not a native speaker, so IMO it is very important to be clear \nin crucial points such as this one.\n\nCiao,\nDscho\n"},{"id":"76118","messageId":"alpine.DEB.1.00.0805051310550.30431@racer","threadId":"13350","inReplyTo":"46dff0320805040953i4230a686j8e8d63eaa6728c2f@mail.gmail.com","subject":"Re: [PATCH v3 5/6] fn_out_diff_words_aux: Handle common diff line more carefully","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-05T12:11:44Z","receivedAt":"2008-05-05T12:11:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 5 May 2008, Ping Yin wrote:\n\n> On Sun, May 4, 2008 at 5:54 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >  On Sun, 4 May 2008, Ping Yin wrote:\n> >\n> >  > Before feeding minus and plus lines into xdi_diff, we replace non \n> >  > word characters with '\\n'. So we need recover the replaced \n> >  > character (always the last character) in the callback \n> >  > fn_out_diff_words_aux.\n> >  >\n> >  > Therefore, a common diff line beginning with ' ' is not always a \n> >  > real common line.\n> >\n> >  Umm, why?\n> \n> Because we need recover the replaced character.\n> \n> Say, for a common diff line \" foo\", after restoring the replaced \n> character, the corresponding line in minus and plus may be different. \n> For example, \"foo(\" and \"foo)\".\n\nWhy do I have to spend time trying to figure out what you meant, write an \nemail, and get the explanation only in a response (i.e. not the commit \nmessage, where it belongs)?\n\nCiao,\nDscho\n"},{"id":"76119","messageId":"alpine.DEB.1.00.0805051312560.30431@racer","threadId":"13350","inReplyTo":"m3ve1t6bli.fsf@localhost.localdomain","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-05T12:14:44Z","receivedAt":"2008-05-05T12:14:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 May 2008, Jakub Narebski wrote:\n\n> Junio C Hamano <junio@pobox.com> writes:\n> \n> > Let's step back a bit and try to clarify the problem with a bit of\n> > illustration.\n> > \n> > The motivation behind \"word diff\" is because line oriented diff is\n> > sometimes unwieldy.\n> > \n> >     -Hello world.\n> >     +Hi, world.\n> [...]\n> > We instead can do this word by word (note that I am doing this as a\n> > thought experiment, to illustrate what the problem is and what should\n> > conceptually happen, not suggesting this particular implementation):\n> > \n> >     preimage        postimage       word-diff\n> >     48656c6c6f                      -48656c6c6f Hello\n> >                     4869            +4869       Hi\n> >                     2c              +2c         ,\n> >     20              20               20         ' '\n> >     776f726c64      776f726c64       776f726c64 world      \n> >     2e              2e               2e         .\n> >     0a              0a               0a         '\\n'\n> > \n> > Which would give you \"/Hello/Hi,/ world.\\n\".\n> \n> Would it be possible instead of in-line word diff, use word coloring\n> to enhance traditional diff format?  Something like\n> \n>      -/Hello/ world.\n>      +/Hi,/ world.\n> \n> (We could use bold, or reverse for marking changed fragment, or use\n> color only for changed fragment).\n> \n> IMHO current output is nice, unless you have long lines and not very\n> wide screen...\n\n-S ;-)\n\nIIRC the code to display was not too complicated for the current mode, so \nit should be relatively simple for the mode you desire.\n\nBut first let's agree on the semantics of the \"tokens\", as Junio calls \nthem, okay?\n\nCiao,\nDscho\n"},{"id":"76128","messageId":"46dff0320805050718n1a57c691w8c33e26c59fe829c@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805051310550.30431@racer","subject":"Re: [PATCH v3 5/6] fn_out_diff_words_aux: Handle common diff line more carefully","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-05T14:18:09Z","receivedAt":"2008-05-05T14:18:09Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Mon, May 5, 2008 at 8:11 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>  >\n>  > Because we need recover the replaced character.\n>  >\n>  > Say, for a common diff line \" foo\", after restoring the replaced\n>  > character, the corresponding line in minus and plus may be different.\n>  > For example, \"foo(\" and \"foo)\".\n>\n>  Why do I have to spend time trying to figure out what you meant, write an\n>  email, and get the explanation only in a response (i.e. not the commit\n>  message, where it belongs)?\n>\n\nSince the new solution from junio has risen up, i think this patch\ndoesn't deserve us spending more time.\n\n\n\n-- \nPing Yin\n"},{"id":"76160","messageId":"46dff0320805051740o65eee07eqc7073e4fa7996277@mail.gmail.com","threadId":"13350","inReplyTo":"46dff0320805050510t3bc5fd0eq44e0d58d1bb57629@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-06T00:40:15Z","receivedAt":"2008-05-06T00:40:15Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Mon, May 5, 2008 at 8:10 PM, Ping Yin <pkufranky@gmail.com> wrote:\n\n>\n>  The one-char-one-token and multi-char-one-token rules may have\n>  different implementation issues. I think multi-char-one-token rule may\n>  be more representative. So for the current time, i prefer considering\n>  both run of word characters and single non-word character as a token.\n>\n\nIf we agree on this. I will come up with an implementation still using\ndiff.nonwordchars few days later.\n\n\n\n-- \nPing Yin\n"},{"id":"76189","messageId":"alpine.DEB.1.00.0805060954470.30431@racer","threadId":"13350","inReplyTo":"46dff0320805051740o65eee07eqc7073e4fa7996277@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-06T08:55:58Z","receivedAt":"2008-05-06T08:55:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 May 2008, Ping Yin wrote:\n\n> On Mon, May 5, 2008 at 8:10 PM, Ping Yin <pkufranky@gmail.com> wrote:\n> \n> I will come up with an implementation still using diff.nonwordchars few \n> days later.\n\nIf I did not like the unnecessary negative approach \"nonwordchars\" (as \nopposed to \"wordchars\"), it seems even less appropriate now, when you \nactually want to discern between \"spaceCharacters\", \n\"punctuationCharacters\" and \"wordCharacters\".\n\nHth,\nDscho\n"},{"id":"76240","messageId":"46dff0320805061815k6aca9020g285b09da2bcf29c3@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805060954470.30431@racer","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-07T01:15:35Z","receivedAt":"2008-05-07T01:15:35Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Tue, May 6, 2008 at 4:55 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> > I will come up with an implementation still using diff.nonwordchars few\n>  > days later.\n>\n>  If I did not like the unnecessary negative approach \"nonwordchars\" (as\n>  opposed to \"wordchars\"), it seems even less appropriate now, when you\n>  actually want to discern between \"spaceCharacters\",\n>  \"punctuationCharacters\" and \"wordCharacters\".\n>\n\nHmm, punctchars should be a better word than nonwordchars.\n\nSo how about this\n\n--color-words={char,punct,word}\n\n  - char: one char one token\n  - punct/word: a token can be either a run of word characters or a\nsingle punct character.  diff.punctchars is used for punct, and\ndiff.wordchars is used for word.\n\nWe leave the choice to the user.\n\n\n-- \nPing Yin\n"},{"id":"76270","messageId":"alpine.DEB.1.00.0805071223450.30431@racer","threadId":"13350","inReplyTo":"46dff0320805061815k6aca9020g285b09da2bcf29c3@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-07T11:24:19Z","receivedAt":"2008-05-07T11:24:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 May 2008, Ping Yin wrote:\n\n> On Tue, May 6, 2008 at 4:55 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > > I will come up with an implementation still using diff.nonwordchars few\n> >  > days later.\n> >\n> >  If I did not like the unnecessary negative approach \"nonwordchars\" (as\n> >  opposed to \"wordchars\"), it seems even less appropriate now, when you\n> >  actually want to discern between \"spaceCharacters\",\n> >  \"punctuationCharacters\" and \"wordCharacters\".\n> >\n> \n> Hmm, punctchars should be a better word than nonwordchars.\n> \n> So how about this\n> \n> --color-words={char,punct,word}\n> \n>   - char: one char one token\n>   - punct/word: a token can be either a run of word characters or a\n> single punct character.  diff.punctchars is used for punct, and\n> diff.wordchars is used for word.\n\nI am rather interested in the semantics, i.e. if you can punch holes into \nthis 3-class approach.\n\nBikeshedding comes later ;-)\n\nCiao,\nDscho\n"},{"id":"76273","messageId":"46dff0320805070519m569d9653ja276412fde135f45@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805071223450.30431@racer","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-07T12:19:52Z","receivedAt":"2008-05-07T12:19:52Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Wed, May 7, 2008 at 7:24 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>  > So how about this\n>  >\n>  > --color-words={char,punct,word}\n>  >\n>  >   - char: one char one token\n>  >   - punct/word: a token can be either a run of word characters or a\n>  > single punct character.  diff.punctchars is used for punct, and\n>  > diff.wordchars is used for word.\n>\n>  I am rather interested in the semantics, i.e. if you can punch holes into\n>  this 3-class approach.\n>\n>  Bikeshedding comes later ;-)\n\nSorry, but i can't parse both sentences, especially Bikeshedding and\n\"punch holes into\".\n\n\n\n-- \nPing Yin\n"},{"id":"76278","messageId":"alpine.DEB.1.00.0805071408360.30431@racer","threadId":"13350","inReplyTo":"46dff0320805070519m569d9653ja276412fde135f45@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-07T13:10:50Z","receivedAt":"2008-05-07T13:10:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 May 2008, Ping Yin wrote:\n\n> On Wed, May 7, 2008 at 7:24 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >  > So how about this\n> >  >\n> >  > --color-words={char,punct,word}\n> >  >\n> >  >   - char: one char one token\n> >  >   - punct/word: a token can be either a run of word characters or a\n> >  > single punct character.  diff.punctchars is used for punct, and\n> >  > diff.wordchars is used for word.\n> >\n> >  I am rather interested in the semantics, i.e. if you can punch holes into\n> >  this 3-class approach.\n> >\n> >  Bikeshedding comes later ;-)\n> \n> Sorry, but i can't parse both sentences, especially Bikeshedding and\n> \"punch holes into\".\n\n\"punch holes into\": find cases where Junio's proposed algorithm breaks \ndown.\n\n\"bikeshedding\": discussing minor implementation details that are not \nreally interesting at this stage.  See also \nhttp://en.wikipedia.org/wiki/Bikeshedding.\n\nCiao,\nDscho\n"},{"id":"76280","messageId":"46dff0320805070711vc4c9a63uba3eaa7743381f20@mail.gmail.com","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805071408360.30431@racer","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-07T14:11:49Z","receivedAt":"2008-05-07T14:11:49Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Wed, May 7, 2008 at 9:10 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n\n>  > Sorry, but i can't parse both sentences, especially Bikeshedding and\n>  > \"punch holes into\".\n>\n>  \"punch holes into\": find cases where Junio's proposed algorithm breaks\n>  down.\n\nI think the --color-words={char,punct,word} covers most of the cases\nthat junio mentioned, except the case that a token can be a run of\nword chars, a run of non-word chars or a run of whitespaces.\n\nIf anyone is interested in this case, he can extend --color-words with\na fourth option or tokenizer (i havn't yet found a good name for this\ntokenizer).\n\nOf course, if neccessary, one can implement other language-specific tokenizers.\n\nHowever, i think the 3 tokenizers i mentioned is enough for most cases.\n\n>\n>  \"bikeshedding\": discussing minor implementation details that are not\n>  really interesting at this stage.  See also\n>  http://en.wikipedia.org/wiki/Bikeshedding.\n>\n\nSee. THX.\n\n\n-- \nPing Yin\n"},{"id":"76315","messageId":"7viqxqc4gs.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"alpine.DEB.1.00.0805071223450.30431@racer","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-07T19:13:39Z","receivedAt":"2008-05-07T19:13:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> I am rather interested in the semantics, i.e. if you can punch holes into \n> this 3-class approach.\n\nThis is not the 3-class thing, but was done as a lunchtime hack.  It\nremoves more lines than it adds, with real comments ;-).\n\n * It removes the custom allocator used for minus/plus buffers and\n   replaces it with the bog-standard strbuf;\n\n * The tokenization is done when diff_words_append() is called, i.e. when\n   we read the original \"added or deleted _lines_\";\n\n * The tokenization function is separated out, and gets the emit_callback,\n   so anybody can enhance it with customization using gitattributes and\n   other heuristics.  More importantly, it is not byte oriented and would\n   be easier to extend it to UTF-8 contents;\n\n * It does not have to play \"suppressed_newline\" games anymore.  A LF is\n   just a token.\n\nI haven't tested this at all (this is a lunchtime hack) and have a mild\nsuspicion that it may have corner case miscounting (e.g. I blindly\nsubtracts 3 from len when dealing with a line that represents a single\ntoken from the internal diff output --- do I always have 3 there even when\nthe original file ends with an incomplete line?  I didn't check), but\nother than that I think this is a lot easier to read and follow.\n\n---\n\n diff.c |  216 +++++++++++++++++++++++++++++++--------------------------------\n 1 files changed, 106 insertions(+), 110 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex e35384b..344aaa6 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -351,87 +351,119 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)\n \treturn 0;\n }\n \n-struct diff_words_buffer {\n-\tmmfile_t text;\n-\tlong alloc;\n-\tlong current; /* output pointer */\n-\tint suppressed_newline;\n+typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);\n+\n+struct emit_callback {\n+\tstruct xdiff_emit_state xm;\n+\tint nparents, color_diff;\n+\tunsigned ws_rule;\n+\tsane_truncate_fn truncate;\n+\tconst char **label_path;\n+\tstruct diff_words_data *diff_words;\n+\tint *found_changesp;\n+\tFILE *file;\n };\n \n-static void diff_words_append(char *line, unsigned long len,\n-\t\tstruct diff_words_buffer *buffer)\n+static size_t diff_words_tokenize(struct emit_callback *ecbdata,\n+\t\t\t\t  char *line, unsigned long len)\n {\n-\tif (buffer->text.size + len > buffer->alloc) {\n-\t\tbuffer->alloc = (buffer->text.size + len) * 3 / 2;\n-\t\tbuffer->text.ptr = xrealloc(buffer->text.ptr, buffer->alloc);\n+\t/*\n+\t * This function currently is deliberately done very stupid,\n+\t * but passing ecbdata here means that you can potentially\n+\t * implement different tokenization rules depending on\n+\t * the content (e.g. \"gitattributes(5)\").\n+\t */\n+\tint is_space;\n+\tchar *line0 = line;\n+\n+\tif (!len)\n+\t\treturn 0;\n+\n+\tis_space = isspace(*line);\n+\twhile (len && (isspace(*line) == is_space)) {\n+\t\tline++;\n+\t\tlen--;\n \t}\n+\treturn line - line0;\n+}\n+\n+static void diff_words_append(struct emit_callback *ecbdata,\n+\t\t\t      char *line, unsigned long len,\n+\t\t\t      struct strbuf *text)\n+{\n+\t/* Skip leading +/- first. */\n \tline++;\n \tlen--;\n-\tmemcpy(buffer->text.ptr + buffer->text.size, line, len);\n-\tbuffer->text.size += len;\n+\n+\t/*\n+\t * Tokenize and stuff the words in.\n+\t */\n+\twhile (len) {\n+\t\tsize_t token_len = diff_words_tokenize(ecbdata, line, len);\n+\n+\t\tif (line[0] != '\\n') {\n+\t\t\t/*\n+\t\t\t * A nonempty token has ' ' stuffed in front,\n+\t\t\t * so that we can recover the original\n+\t\t\t * end-of-line easily.  Stupid, but works.\n+\t\t\t */\n+\t\t\tstrbuf_add(text, \" \", 1);\n+\t\t\tstrbuf_add(text, line, token_len);\n+\t\t\tstrbuf_add(text, \"\\n\", 1);\n+\t\t\tlen -= token_len;\n+\t\t\tline += token_len;\n+\t\t} else {\n+\t\t\t/* A real LF */\n+\t\t\tstrbuf_add(text, \"\\n\", 1);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n }\n \n struct diff_words_data {\n \tstruct xdiff_emit_state xm;\n-\tstruct diff_words_buffer minus, plus;\n+\tstruct strbuf minus;\n+\tstruct strbuf plus;\n \tFILE *file;\n };\n \n-static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,\n-\t\tint suppress_newline)\n+static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)\n {\n-\tconst char *ptr;\n-\tint eol = 0;\n-\n-\tif (len == 0)\n-\t\treturn;\n-\n-\tptr  = buffer->text.ptr + buffer->current;\n-\tbuffer->current += len;\n-\n-\tif (ptr[len - 1] == '\\n') {\n-\t\teol = 1;\n-\t\tlen--;\n-\t}\n-\n-\tfputs(diff_get_color(1, color), file);\n-\tfwrite(ptr, len, 1, file);\n-\tfputs(diff_get_color(1, DIFF_RESET), file);\n-\n-\tif (eol) {\n-\t\tif (suppress_newline)\n-\t\t\tbuffer->suppressed_newline = 1;\n-\t\telse\n-\t\t\tputc('\\n', file);\n-\t}\n+\tfputs(set, file);\n+\tfwrite(line, len, 1, file);\n+\tfputs(reset, file);\n }\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+\tconst char *set;\n+\tconst char *reset = diff_colors[DIFF_RESET];\n \n-\tif (diff_words->minus.suppressed_newline) {\n-\t\tif (line[0] != '+')\n-\t\t\tputc('\\n', diff_words->file);\n-\t\tdiff_words->minus.suppressed_newline = 0;\n+\tswitch (line[0]) {\n+\tcase '-':\n+\t\tset = diff_colors[DIFF_FILE_OLD];\n+\t\tbreak;\n+\tcase '+':\n+\t\tset = diff_colors[DIFF_FILE_NEW];\n+\t\tbreak;\n+\tcase ' ':\n+\t\tset = diff_colors[DIFF_PLAIN];\n+\t\tbreak;\n+\tdefault:\n+\t\treturn; /* omit @@ -j,k +l,m @@ header */\n \t}\n \n-\tlen--;\n-\tswitch (line[0]) {\n-\t\tcase '-':\n-\t\t\tprint_word(diff_words->file,\n-\t\t\t\t   &diff_words->minus, len, DIFF_FILE_OLD, 1);\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_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\tdiff_words->minus.current += len;\n-\t\t\tbreak;\n+\tif (line[1] == ' ') {\n+\t\t/* A token */\n+\t\tline += 2;\n+\t\tlen -= 3; /* drop the trailing LF */\n+\t} else {\n+\t\t/* A real LF */\n+\t\tline++;\n+\t\tlen--;\n \t}\n+\temit_line(diff_words->file, set, reset, line, len);\n }\n \n /* this executes the word diff on the accumulated buffers */\n@@ -441,27 +473,18 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \txdemitconf_t xecfg;\n \txdemitcb_t ecb;\n \tmmfile_t minus, plus;\n-\tint i;\n+\tunsigned long sz;\n \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-\tdiff_words->plus.current = 0;\n+\n+\tminus.ptr = strbuf_detach(&diff_words->minus, &sz);\n+\tminus.size = sz;\n+\tplus.ptr = strbuf_detach(&diff_words->plus, &sz);\n+\tplus.size = sz;\n \n \txpp.flags = XDF_NEED_MINIMAL;\n-\txecfg.ctxlen = diff_words->minus.alloc + diff_words->plus.alloc;\n+\t/* hack to make it a single hunk to show all */\n+\txecfg.ctxlen = minus.size + plus.size;\n \tecb.outf = xdiff_outf;\n \tecb.priv = diff_words;\n \tdiff_words->xm.consume = fn_out_diff_words_aux;\n@@ -469,37 +492,15 @@ static void diff_words_show(struct diff_words_data *diff_words)\n \n \tfree(minus.ptr);\n \tfree(plus.ptr);\n-\tdiff_words->minus.text.size = diff_words->plus.text.size = 0;\n-\n-\tif (diff_words->minus.suppressed_newline) {\n-\t\tputc('\\n', diff_words->file);\n-\t\tdiff_words->minus.suppressed_newline = 0;\n-\t}\n }\n \n-typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);\n-\n-struct emit_callback {\n-\tstruct xdiff_emit_state xm;\n-\tint nparents, color_diff;\n-\tunsigned ws_rule;\n-\tsane_truncate_fn truncate;\n-\tconst char **label_path;\n-\tstruct diff_words_data *diff_words;\n-\tint *found_changesp;\n-\tFILE *file;\n-};\n-\n static void free_diff_words_data(struct emit_callback *ecbdata)\n {\n \tif (ecbdata->diff_words) {\n \t\t/* flush buffers */\n-\t\tif (ecbdata->diff_words->minus.text.size ||\n-\t\t\t\tecbdata->diff_words->plus.text.size)\n+\t\tif (ecbdata->diff_words->minus.len ||\n+\t\t    ecbdata->diff_words->plus.len)\n \t\t\tdiff_words_show(ecbdata->diff_words);\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);\n \t\tecbdata->diff_words = NULL;\n \t}\n@@ -512,13 +513,6 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)\n \treturn \"\";\n }\n \n-static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)\n-{\n-\tfputs(set, file);\n-\tfwrite(line, len, 1, file);\n-\tfputs(reset, file);\n-}\n-\n static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)\n {\n \tconst char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);\n@@ -604,16 +598,16 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\tfree_diff_words_data(ecbdata);\n \tif (ecbdata->diff_words) {\n \t\tif (line[0] == '-') {\n-\t\t\tdiff_words_append(line, len,\n+\t\t\tdiff_words_append(ecbdata, line, len,\n \t\t\t\t\t  &ecbdata->diff_words->minus);\n \t\t\treturn;\n \t\t} else if (line[0] == '+') {\n-\t\t\tdiff_words_append(line, len,\n+\t\t\tdiff_words_append(ecbdata, line, len,\n \t\t\t\t\t  &ecbdata->diff_words->plus);\n \t\t\treturn;\n \t\t}\n-\t\tif (ecbdata->diff_words->minus.text.size ||\n-\t\t    ecbdata->diff_words->plus.text.size)\n+\t\tif (ecbdata->diff_words->minus.len ||\n+\t\t    ecbdata->diff_words->plus.len)\n \t\t\tdiff_words_show(ecbdata->diff_words);\n \t\tline++;\n \t\tlen--;\n@@ -1470,6 +1464,8 @@ static void builtin_diff(const char *name_a,\n \t\tif (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) {\n \t\t\tecbdata.diff_words =\n \t\t\t\txcalloc(1, sizeof(struct diff_words_data));\n+\t\t\tstrbuf_init(&ecbdata.diff_words->minus, 0);\n+\t\t\tstrbuf_init(&ecbdata.diff_words->plus, 0);\n \t\t\tecbdata.diff_words->file = o->file;\n \t\t}\n \t\txdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n"},{"id":"76318","messageId":"7vej8ddi4s.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"7viqxqc4gs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-07T19:33:07Z","receivedAt":"2008-05-07T19:33:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I haven't tested this at all (this is a lunchtime hack) and have a mild\n> suspicion that it may have corner case miscounting (e.g. I blindly\n> subtracts 3 from len when dealing with a line that represents a single\n> token from the internal diff output --- do I always have 3 there even when\n> the original file ends with an incomplete line?  I didn't check), but\n> other than that I think this is a lot easier to read and follow.\n\nAnd this adds \"--color-words -b\" support as an example.\n\nThe second hunk, however, is a bugfix to the previous one.  The code wants\nthe LF at the end of the line always returned as a single token.\n\n---\n diff.c |   30 +++++++++++++++++++++++++++---\n 1 files changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 344aaa6..bce0626 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -357,6 +357,7 @@ struct emit_callback {\n \tstruct xdiff_emit_state xm;\n \tint nparents, color_diff;\n \tunsigned ws_rule;\n+\tstruct diff_options *diffopt;\n \tsane_truncate_fn truncate;\n \tconst char **label_path;\n \tstruct diff_words_data *diff_words;\n@@ -378,19 +379,36 @@ static size_t diff_words_tokenize(struct emit_callback *ecbdata,\n \n \tif (!len)\n \t\treturn 0;\n+\t/*\n+\t * Always return LF at the end as a single separate token.\n+\t */\n+\tif ((len == 1) && *line == '\\n')\n+\t\treturn 1;\n \n \tis_space = isspace(*line);\n \twhile (len && (isspace(*line) == is_space)) {\n \t\tline++;\n \t\tlen--;\n \t}\n+\tif (is_space && !len)\n+\t\tline--;\n \treturn line - line0;\n }\n \n+static int token_is_ws_only(char *line, size_t len)\n+{\n+\twhile (len--)\n+\t\tif (!isspace(*line))\n+\t\t\treturn 0;\n+\treturn 1;\n+}\n+\n static void diff_words_append(struct emit_callback *ecbdata,\n \t\t\t      char *line, unsigned long len,\n \t\t\t      struct strbuf *text)\n {\n+\tstruct diff_options *diffopt = ecbdata->diffopt;\n+\n \t/* Skip leading +/- first. */\n \tline++;\n \tlen--;\n@@ -407,9 +425,14 @@ static void diff_words_append(struct emit_callback *ecbdata,\n \t\t\t * so that we can recover the original\n \t\t\t * end-of-line easily.  Stupid, but works.\n \t\t\t */\n-\t\t\tstrbuf_add(text, \" \", 1);\n-\t\t\tstrbuf_add(text, line, token_len);\n-\t\t\tstrbuf_add(text, \"\\n\", 1);\n+\t\t\tif ((diffopt->xdl_opts & XDF_IGNORE_WHITESPACE) &&\n+\t\t\t    token_is_ws_only(line, token_len)) {\n+\t\t\t\tstrbuf_add(text, \"  \\n\", 3);\n+\t\t\t} else {\n+\t\t\t\tstrbuf_add(text, \" \", 1);\n+\t\t\t\tstrbuf_add(text, line, token_len);\n+\t\t\t\tstrbuf_add(text, \"\\n\", 1);\n+\t\t\t}\n \t\t\tlen -= token_len;\n \t\t\tline += token_len;\n \t\t} else {\n@@ -1447,6 +1470,7 @@ static void builtin_diff(const char *name_a,\n \t\tecbdata.found_changesp = &o->found_changes;\n \t\tecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);\n \t\tecbdata.file = o->file;\n+\t\tecbdata.diffopt = o;\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n \t\txecfg.flags = XDL_EMIT_FUNCNAMES;\n"},{"id":"76319","messageId":"20080507194524.GA31500@sigill.intra.peff.net","threadId":"13350","inReplyTo":"7viqxqc4gs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-07T19:45:24Z","receivedAt":"2008-05-07T19:45:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 07, 2008 at 12:13:39PM -0700, Junio C Hamano wrote:\n\n>  /* this executes the word diff on the accumulated buffers */\n> @@ -441,27 +473,18 @@ static void diff_words_show(struct diff_words_data *diff_words)\n>  \txdemitconf_t xecfg;\n>  \txdemitcb_t ecb;\n>  \tmmfile_t minus, plus;\n> -\tint i;\n> +\tunsigned long sz;\n\nstrbuf uses size_t; since we pass sz in as a pointer to strbuf_detach,\nthere can be a pointer type mismatch.\n\nBut more big-picture, comparing the output of the old color words and\nthis implementation, there is one thing I don't like: the new one\ndoesn't bring together runs of additions and deletions, which can make\nparsing text much easier. For example:\n\n  $ echo This is a complete sentence. >one\n  $ echo Here is some totally different text. >two\n\n  # with old implementation; /-.../ is red, /+.../ is green\n  $ git diff --color-words one two\n  ...\n  /-This/ /+Here/ is /-a complete sentence./+some totally different text./\n\n  # with this patch\n  $ git diff --color-words one two\n  ...\n  /-This/+Here/ is /-a/+some/ /-complete/+totally/ /-sentence./+different text./\n\n-Peff\n"},{"id":"76320","messageId":"7vabj1dgr5.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"20080507194524.GA31500@sigill.intra.peff.net","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-07T20:02:54Z","receivedAt":"2008-05-07T20:02:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But more big-picture, comparing the output of the old color words and\n> this implementation, there is one thing I don't like: the new one\n> doesn't bring together runs of additions and deletions, which can make\n> parsing text much easier. For example:\n>\n>   $ echo This is a complete sentence. >one\n>   $ echo Here is some totally different text. >two\n>\n>   # with old implementation; /-.../ is red, /+.../ is green\n>   $ git diff --color-words one two\n>   ...\n>   /-This/ /+Here/ is /-a complete sentence./+some totally different text./\n>\n>   # with this patch\n>   $ git diff --color-words one two\n>   ...\n>   /-This/+Here/ is /-a/+some/ /-complete/+totally/ /-sentence./+different text./\n\nI suspect that heavily depends on the input text.  If you drop \"different\"\nin the example, the output becomes:\n\n    {-This|+Here} is {-a|+some} {-complete|+totally} {-sentence.|+text.}\n\nwhich is totally sensible.\n\nYou can get the output that is closer to the original by tweaking the\ndefinition of what a token is.  You can for example define a token as \"0 or\nmore non whitespace characters followed by 1 or more whitespace characters\"\nand then the internal diff would become ($ to show the end of line):\n\n    -This $\n    +Here $\n     is $\n    -a $\n    -complete $\n    -sentence.$\n    +some $\n    +totally $\n    +different $\n    +text.$\n\nwhich would yield on the output:\n\n    {-This |+Here }is {-a complete sentence.|+some totally different text.}\n\nIt's all in diff_words_tokenize(), which I kept deliberately stupid so\nthat people can tweak it to their liking.\n"},{"id":"76325","messageId":"20080507220434.GB5994@sigill.intra.peff.net","threadId":"13350","inReplyTo":"7vabj1dgr5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-07T22:04:34Z","receivedAt":"2008-05-07T22:04:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 07, 2008 at 01:02:54PM -0700, Junio C Hamano wrote:\n\n> I suspect that heavily depends on the input text.  If you drop \"different\"\n> in the example, the output becomes:\n> \n>     {-This|+Here} is {-a|+some} {-complete|+totally} {-sentence.|+text.}\n> \n> which is totally sensible.\n>\n> [...]\n> \n> which would yield on the output:\n> \n>     {-This |+Here }is {-a complete sentence.|+some totally different text.}\n\nSensible, perhaps, but I think the second one is much nicer for English\ntext (though the first is much nicer for code, I expect).\n\n> It's all in diff_words_tokenize(), which I kept deliberately stupid so\n> that people can tweak it to their liking.\n\nOK; I haven't been following the thread too closely, and I wanted to\nmake sure this was a question of how the tokenizing works, and not a\nfundamental problem with this approach. Thanks for the explanation.\n\n-Peff\n"},{"id":"76365","messageId":"20080508103436.GB3300@mithlond.arda.local","threadId":"13350","inReplyTo":"7viqxqc4gs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-08T10:34:36Z","receivedAt":"2008-05-08T10:34:36Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Junio C Hamano wrote (2008-05-07 12:13 -0700):\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > I am rather interested in the semantics, i.e. if you can punch holes\n> > into this 3-class approach.\n> \n> This is not the 3-class thing, but was done as a lunchtime hack.  It\n> removes more lines than it adds, with real comments ;-).\n\nI tested your lunchtime hack from the \"pu\" branch. I'm perfectly happy\nwith the colored output itself but I noticed some different line feed\nbehaviour that you might want to know. Look at the example below. The\nfirst is normal line diff. The second is the same text with the old\n--color-words behaviour and the last is with the lunchtime hack version.\nThere are only three words added to the text; additions are written as\n{+word} in the --color-words output.\n\n\nNormal line diff\n----------------\n\n-OpenOffice.org has user setting for defining the minimum length for\n+OpenOffice.org has a user setting for defining the minimum length for\n words to be hyphenated. By default the word length is counted from the\n-whole word - even for compound words. For example the word\n-'elokuvalippu' is 12 characters long. The word will be hyphenated like\n-'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n-12 or less. If the minimum length is set to 13 or more the word is not\n-hyphenated at all.\n+whole word - even for compound words. For example the compound word\n+'elokuvalippu' is considered 12 characters long. The word will be\n+hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word\n+length is set to 12 or less. If the minimum length is set to 13 or more\n+the word is not hyphenated at all.\n\nWith the old --color-words\n--------------------------\n\nOpenOffice.org has {+a }user setting for defining the minimum length for\nwords to be hyphenated. By default the word length is counted from the\nwhole word - even for compound words. For example the {+compound }word\n'elokuvalippu' is {+considered }12 characters long. The word will be\nhyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word\nlength is set to 12 or less. If the minimum length is set to 13 or more\nthe word is not hyphenated at all.\n\nWith the lunchtime hack --color-words\n-------------------------------------\n\nOpenOffice.org has {+a }user setting for defining the minimum length for\nwords to be hyphenated. By default the word length is counted from the\nwhole word - even for compound words. For example the {+compound }word\n'elokuvalippu' is {+considered }12 characters long. The word will be \nhyphenated like\n 'elo-ku-va-lip-pu' in all cases when the minimum word \nlength is set to\n 12 or less. If the minimum length is set to 13 or more \nthe word is not\n hyphenated at all.\n"},{"id":"76499","messageId":"46dff0320805100120xe967359v62d665fdbaca5e6a@mail.gmail.com","threadId":"13350","inReplyTo":"7viqxqc4gs.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-10T08:20:53Z","receivedAt":"2008-05-10T08:20:53Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Thu, May 8, 2008 at 3:13 AM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> I haven't tested this at all (this is a lunchtime hack) and have a mild\n> suspicion that it may have corner case miscounting (e.g. I blindly\n> subtracts 3 from len when dealing with a line that represents a single\n> token from the internal diff output --- do I always have 3 there even when\n> the original file ends with an incomplete line?  I didn't check), but\n> other than that I think this is a lot easier to read and follow.\n>\n> ---\n>\n>  diff.c |  216 +++++++++++++++++++++++++++++++--------------------------------\n>  1 files changed, 106 insertions(+), 110 deletions(-)\n>\n> diff --git a/diff.c b/diff.c\n> index e35384b..344aaa6 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -351,87 +351,119 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)\n>        return 0;\n>  }\n>\n> -struct diff_words_buffer {\n> -       mmfile_t text;\n> -       long alloc;\n> -       long current; /* output pointer */\n> -       int suppressed_newline;\n> +typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);\n> +\n> +struct emit_callback {\n> +       struct xdiff_emit_state xm;\n> +       int nparents, color_diff;\n> +       unsigned ws_rule;\n> +       sane_truncate_fn truncate;\n> +       const char **label_path;\n> +       struct diff_words_data *diff_words;\n> +       int *found_changesp;\n> +       FILE *file;\n>  };\n>\n> -static void diff_words_append(char *line, unsigned long len,\n> -               struct diff_words_buffer *buffer)\n> +static size_t diff_words_tokenize(struct emit_callback *ecbdata,\n> +                                 char *line, unsigned long len)\n>  {\n> -       if (buffer->text.size + len > buffer->alloc) {\n> -               buffer->alloc = (buffer->text.size + len) * 3 / 2;\n> -               buffer->text.ptr = xrealloc(buffer->text.ptr, buffer->alloc);\n> +       /*\n> +        * This function currently is deliberately done very stupid,\n> +        * but passing ecbdata here means that you can potentially\n> +        * implement different tokenization rules depending on\n> +        * the content (e.g. \"gitattributes(5)\").\n> +        */\n> +       int is_space;\n> +       char *line0 = line;\n> +\n> +       if (!len)\n> +               return 0;\n> +\n> +       is_space = isspace(*line);\n> +       while (len && (isspace(*line) == is_space)) {\n> +               line++;\n> +               len--;\n>        }\n> +       return line - line0;\n> +}\n> +\n> +static void diff_words_append(struct emit_callback *ecbdata,\n> +                             char *line, unsigned long len,\n> +                             struct strbuf *text)\n> +{\n> +       /* Skip leading +/- first. */\n>        line++;\n>        len--;\n> -       memcpy(buffer->text.ptr + buffer->text.size, line, len);\n> -       buffer->text.size += len;\n> +\n> +       /*\n> +        * Tokenize and stuff the words in.\n> +        */\n> +       while (len) {\n> +               size_t token_len = diff_words_tokenize(ecbdata, line, len);\n> +\n> +               if (line[0] != '\\n') {\n> +                       /*\n> +                        * A nonempty token has ' ' stuffed in front,\n> +                        * so that we can recover the original\n> +                        * end-of-line easily.  Stupid, but works.\n> +                        */\n> +                       strbuf_add(text, \" \", 1);\n> +                       strbuf_add(text, line, token_len);\n> +                       strbuf_add(text, \"\\n\", 1);\n> +                       len -= token_len;\n> +                       line += token_len;\n\nI still don't understand why a ' '  is prepended. See my comment for\nthe following part\n\n> +       if (line[1] == ' ') {\n> +               /* A token */\n> +               line += 2;\n> +               len -= 3; /* drop the trailing LF */\n> +       } else {\n> +               /* A real LF */\n> +               line++;\n> +               len--;\n>        }\n\nI think we can recognize a real LF by that the diff line should be a\nsingle '\\n', i.e. line[1] == '\\n'. So what's wrong by\ns/line[1] == ' '/line[1] != '\\n'/ ?\n\n-- \nPing Yin\n"},{"id":"76500","messageId":"46dff0320805100202j54b0922cy50a2c93c4eff1757@mail.gmail.com","threadId":"13350","inReplyTo":"20080508103436.GB3300@mithlond.arda.local","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-10T09:02:56Z","receivedAt":"2008-05-10T09:02:56Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Thu, May 8, 2008 at 6:34 PM, Teemu Likonen <tlikonen@iki.fi> wrote:\n> Junio C Hamano wrote (2008-05-07 12:13 -0700):\n>\n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>>\n>> > I am rather interested in the semantics, i.e. if you can punch holes\n>> > into this 3-class approach.\n>>\n>> This is not the 3-class thing, but was done as a lunchtime hack.  It\n>> removes more lines than it adds, with real comments ;-).\n>\n> I tested your lunchtime hack from the \"pu\" branch. I'm perfectly happy\n> with the colored output itself but I noticed some different line feed\n> behaviour that you might want to know. Look at the example below. The\n> first is normal line diff. The second is the same text with the old\n> --color-words behaviour and the last is with the lunchtime hack version.\n> There are only three words added to the text; additions are written as\n> {+word} in the --color-words output.\n\nYou not only added the three words, but also wrap line at different position.\n\n> Normal line diff\n> ----------------\n>\n> -OpenOffice.org has user setting for defining the minimum length for\n> +OpenOffice.org has a user setting for defining the minimum length for\n>  words to be hyphenated. By default the word length is counted from the\n> -whole word - even for compound words. For example the word\n> -'elokuvalippu' is 12 characters long. The word will be hyphenated like\n> -'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n> -12 or less. If the minimum length is set to 13 or more the word is not\n> -hyphenated at all.\n> +whole word - even for compound words. For example the compound word\n> +'elokuvalippu' is considered 12 characters long. The word will be\n> +hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word\n> +length is set to 12 or less. If the minimum length is set to 13 or more\n> +the word is not hyphenated at all.\n>\n> With the old --color-words\n> --------------------------\n>\n> OpenOffice.org has {+a }user setting for defining the minimum length for\n> words to be hyphenated. By default the word length is counted from the\n> whole word - even for compound words. For example the {+compound }word\n> 'elokuvalippu' is {+considered }12 characters long. The word will be\n> hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word\n> length is set to 12 or less. If the minimum length is set to 13 or more\n> the word is not hyphenated at all.\n>\n> With the lunchtime hack --color-words\n> -------------------------------------\n>\n> OpenOffice.org has {+a }user setting for defining the minimum length for\n> words to be hyphenated. By default the word length is counted from the\n> whole word - even for compound words. For example the {+compound }word\n> 'elokuvalippu' is {+considered }12 characters long. The word will be\n> hyphenated like\n>  'elo-ku-va-lip-pu' in all cases when the minimum word\n> length is set to\n>  12 or less. If the minimum length is set to 13 or more\n> the word is not\n>  hyphenated at all.\n>\n\nWith junio's following code, the number of LF can be more than any of\nthe original two input. Because it will output a LF whenever we\nencounter a real LF in the original input. So  some LFs are outputed\nas {-LF}  or {+LF} . We can't differentiate them because we can't see\nthe colored output for added or removed LF. The same case is that we\ncan't differentiate added and removed spaces.\n\nThat's why i proposed colored background for added/removed space\ncharacters in former reply of this thread.\n\n+       if (line[1] == ' ') {\n+               /* A token */\n+               line += 2;\n+               len -= 3; /* drop the trailing LF */\n+       } else {\n+               /* A real LF */\n+               line++;\n+               len--;\n       }\n+       emit_line(diff_words->file, set, reset, line, len);\n\n\n\n-- \nPing Yin\n"},{"id":"76502","messageId":"20080510091413.GB5542@mithlond.arda.local","threadId":"13350","inReplyTo":"46dff0320805100202j54b0922cy50a2c93c4eff1757@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Teemu Likonen","fromEmail":"tlikonen@iki.fi","sentAt":"2008-05-10T09:14:13Z","receivedAt":"2008-05-10T09:14:13Z","isPatch":true,"sender":{"key":"tlikonen@iki.fi","avatar":null},"body":"Ping Yin wrote (2008-05-10 17:02 +0800):\n\n> On Thu, May 8, 2008 at 6:34 PM, Teemu Likonen <tlikonen@iki.fi> wrote:\n> > There are only three words added to the text; additions are written\n> > as {+word} in the --color-words output.\n> \n> You not only added the three words, but also wrap line at different\n> position.\n\nAh, you're right of course. I'm too focused on just words and language.\nI don't mind these \"added\" LFs on Junio's version but I understand if\nsomeone finds them surprising when color-word-diffing natural languages.\n"},{"id":"76613","messageId":"46dff0320805110616s6df19657r1e4c80634267fd81@mail.gmail.com","threadId":"13350","inReplyTo":"46dff0320805100202j54b0922cy50a2c93c4eff1757@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-11T13:16:11Z","receivedAt":"2008-05-11T13:16:11Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Sat, May 10, 2008 at 5:02 PM, Ping Yin <pkufranky@gmail.com> wrote:\n> On Thu, May 8, 2008 at 6:34 PM, Teemu Likonen <tlikonen@iki.fi> wrote:\n>> Junio C Hamano wrote (2008-05-07 12:13 -0700):\n>>\n>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>>>\n>>> > I am rather interested in the semantics, i.e. if you can punch holes\n>>> > into this 3-class approach.\n>>>\n>>> This is not the 3-class thing, but was done as a lunchtime hack.  It\n>>> removes more lines than it adds, with real comments ;-).\n>>\n>> I tested your lunchtime hack from the \"pu\" branch. I'm perfectly happy\n>> with the colored output itself but I noticed some different line feed\n>> behaviour that you might want to know. Look at the example below. The\n>> first is normal line diff. The second is the same text with the old\n>> --color-words behaviour and the last is with the lunchtime hack version.\n>> There are only three words added to the text; additions are written as\n>> {+word} in the --color-words output.\n>\n> You not only added the three words, but also wrap line at different position.\n>\n>> Normal line diff\n>> ----------------\n>>\n>> -OpenOffice.org has user setting for defining the minimum length for\n>> +OpenOffice.org has a user setting for defining the minimum length for\n>>  words to be hyphenated. By default the word length is counted from the\n>> -whole word - even for compound words. For example the word\n>> -'elokuvalippu' is 12 characters long. The word will be hyphenated like\n>> -'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n>> -12 or less. If the minimum length is set to 13 or more the word is not\n>> -hyphenated at all.\n>> +whole word - even for compound words. For example the compound word\n>> +'elokuvalippu' is considered 12 characters long. The word will be\n>> +hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word\n>> +length is set to 12 or less. If the minimum length is set to 13 or more\n>> +the word is not hyphenated at all.\n>>\n>> With the old --color-words\n>> --------------------------\n>>\n>> OpenOffice.org has {+a }user setting for defining the minimum length for\n>> words to be hyphenated. By default the word length is counted from the\n>> whole word - even for compound words. For example the {+compound }word\n>> 'elokuvalippu' is {+considered }12 characters long. The word will be\n>> hyphenated like 'elo-ku-va-lip-pu' in all cases when the minimum word\n>> length is set to 12 or less. If the minimum length is set to 13 or more\n>> the word is not hyphenated at all.\n>>\n>> With the lunchtime hack --color-words\n>> -------------------------------------\n>>\n>> OpenOffice.org has {+a }user setting for defining the minimum length for\n>> words to be hyphenated. By default the word length is counted from the\n>> whole word - even for compound words. For example the {+compound }word\n>> 'elokuvalippu' is {+considered }12 characters long. The word will be\n>> hyphenated like\n>>  'elo-ku-va-lip-pu' in all cases when the minimum word\n>> length is set to\n>>  12 or less. If the minimum length is set to 13 or more\n>> the word is not\n>>  hyphenated at all.\n>>\n>\n> With junio's following code, the number of LF can be more than any of\n> the original two input. Because it will output a LF whenever we\n> encounter a real LF in the original input. So  some LFs are outputed\n> as {-LF}  or {+LF} . We can't differentiate them because we can't see\n> the colored output for added or removed LF. The same case is that we\n> can't differentiate added and removed spaces.\n>\n> That's why i proposed colored background for added/removed space\n> characters in former reply of this thread.\n>\n> +       if (line[1] == ' ') {\n> +               /* A token */\n> +               line += 2;\n> +               len -= 3; /* drop the trailing LF */\n> +       } else {\n> +               /* A real LF */\n> +               line++;\n> +               len--;\n>       }\n> +       emit_line(diff_words->file, set, reset, line, len);\n>\n\nWith following patch, the diff output becomes (i don't know which one is better)\n\nOpenOffice.org has {+a }user setting for defining the minimum length for\nwords to be hyphenated. By default the word length is counted from the\nwhole word - even for compound words. For example the {compound +}word\n'elokuvalippu' is {+considered }12 characters long. The word will be\nhyphenated like\n 'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n 12 or less. If the minimum length is set to 13 or more the word is not\n hyphenated at all.\n\n\ndiff --git a/diff.c b/diff.c\nindex 51048c6..06fbace 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -446,6 +446,7 @@ struct diff_words_data {\n \tstruct xdiff_emit_state xm;\n \tstruct strbuf minus;\n \tstruct strbuf plus;\n+\tint suppressed_newline;\n \tFILE *file;\n };\n\n@@ -480,12 +481,16 @@ static void fn_out_diff_words_aux(\n \t\t/* A token */\n \t\tline += 2;\n \t\tlen -= 3; /* drop the trailing LF */\n+\t\temit_line(diff_words->file, set, reset, line, len);\n \t} else {\n \t\t/* A real LF */\n-\t\tline++;\n-\t\tlen--;\n+\t\tif (diff_words->suppressed_newline || line[0] == ' ') {\n+\t\t\tdiff_words->suppressed_newline = 0;\n+\t\t\temit_line(diff_words->file, set, reset, \"\\n\", 1);\n+\t\t}\n+\t\telse\n+\t\t\tdiff_words->suppressed_newline = 1;\n \t}\n-\temit_line(diff_words->file, set, reset, line, len);\n }\n\n /* this executes the word diff on the accumulated buffers */\n@@ -510,8 +515,14 @@ static void diff_words_show(\n \tecb.outf = xdiff_outf;\n \tecb.priv = diff_words;\n \tdiff_words->xm.consume = fn_out_diff_words_aux;\n+\tdiff_words->suppressed_newline = 0;\n \txdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);\n\n+\tif (diff_words->suppressed_newline) {\n+\t\tputc('\\n', diff_words->file);\n+\t\tdiff_words->suppressed_newline = 0;\n+\t}\n+\n \tfree(minus.ptr);\n \tfree(plus.ptr);\n }\n\n\n-- \nPing Yin\n"},{"id":"76614","messageId":"20080511132746.GA26550@kooxoo235","threadId":"13350","inReplyTo":"46dff0320805110616s6df19657r1e4c80634267fd81@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-11T13:27:46Z","receivedAt":"2008-05-11T13:27:46Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"* Ping Yin <pkufranky@gmail.com> [2008-05-11 21:16:11 +0800]:\n\n> \n> With following patch, the diff output becomes (i don't know which one is better)\n> \n> OpenOffice.org has {+a }user setting for defining the minimum length for\n> words to be hyphenated. By default the word length is counted from the\n> whole word - even for compound words. For example the {compound +}word\n> 'elokuvalippu' is {+considered }12 characters long. The word will be\n> hyphenated like\n\nThe above two lines are auto wrapped by gmail client, should be\n\n'elokuvalippu' is {+considered }12 characters long. The word will be hyphenated like\n\nSorry for that.\n\n> \n> -- \n> Ping Yin\n"},{"id":"76633","messageId":"7vod7c6c24.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"46dff0320805110616s6df19657r1e4c80634267fd81@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-11T16:27:31Z","receivedAt":"2008-05-11T16:27:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n> With following patch, the diff output becomes (i don't know which one is better)\n>\n> OpenOffice.org has {+a }user setting for defining the minimum length for\n> words to be hyphenated. By default the word length is counted from the\n> whole word - even for compound words. For example the {compound +}word\n> 'elokuvalippu' is {+considered }12 characters long. The word will be hyphenated like\n>  'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n>  12 or less. If the minimum length is set to 13 or more the word is not\n>  hyphenated at all.\n\nYeah, after playing with it a bit, I realize that my original stated goal\nof not playing games with \"newline suppression\" goes very against what\ncolor-words, which is a word oriented diff, tries to achieve.  It appears\nthat it is necessary to reintroduce suppressed_newline.\n"},{"id":"76722","messageId":"46dff0320805120931u7609a5a2x5433d78e35a62c48@mail.gmail.com","threadId":"13350","inReplyTo":"7vod7c6c24.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-12T16:31:57Z","receivedAt":"2008-05-12T16:31:57Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Mon, May 12, 2008 at 12:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>\n>  > With following patch, the diff output becomes (i don't know which one is better)\n>  >\n>  > OpenOffice.org has {+a }user setting for defining the minimum length for\n>  > words to be hyphenated. By default the word length is counted from the\n>  > whole word - even for compound words. For example the {compound +}word\n>  > 'elokuvalippu' is {+considered }12 characters long. The word will be hyphenated like\n>  >  'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n>  >  12 or less. If the minimum length is set to 13 or more the word is not\n>  >  hyphenated at all.\n>\n>  Yeah, after playing with it a bit, I realize that my original stated goal\n>  of not playing games with \"newline suppression\" goes very against what\n>  color-words, which is a word oriented diff, tries to achieve.  It appears\n>  that it is necessary to reintroduce suppressed_newline.\n>\n\nNo matter how well we play with suppressed_newline, we still can't\nachieve the best result by doing word diff between multiple minus\nlines and multiple plus lines.\n\n ( i think the result of vimdiff can be considered as the best).\n\nTo achieve the best, we have to find the pairs of lines (one minus and\none plus for each pair) which most match each other, and then do the\nword diff for each pair.\n\n\n-- \nPing Yin\n"},{"id":"76750","messageId":"m34p934afu.fsf@localhost.localdomain","threadId":"13350","inReplyTo":"46dff0320805120931u7609a5a2x5433d78e35a62c48@mail.gmail.com","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-12T18:57:48Z","receivedAt":"2008-05-12T18:57:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"\"Ping Yin\" <pkufranky@gmail.com> writes:\n\n> On Mon, May 12, 2008 at 12:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>>\n>>> With following patch, the diff output becomes (i don't know which\n>>> one is better)\n>>>\n>>> OpenOffice.org has {+a }user setting for defining the minimum length for\n>>> words to be hyphenated. By default the word length is counted from the\n>>> whole word - even for compound words. For example the {compound +}word\n>>> 'elokuvalippu' is {+considered }12 characters long. The word will be hyphenated like\n>>>  'elo-ku-va-lip-pu' in all cases when the minimum word length is set to\n>>>  12 or less. If the minimum length is set to 13 or more the word is not\n>>>  hyphenated at all.\n>>\n>>  Yeah, after playing with it a bit, I realize that my original\n>>  stated goal of not playing games with \"newline suppression\" goes\n>>  very against what color-words, which is a word oriented diff,\n>>  tries to achieve.  It appears that it is necessary to reintroduce\n>>  suppressed_newline.\n>>\n> \n> No matter how well we play with suppressed_newline, we still can't\n> achieve the best result by doing word diff between multiple minus\n> lines and multiple plus lines.\n> \n>  ( i think the result of vimdiff can be considered as the best).\n\nIs the vimdiff algorithm described anywhere? What about wdiff output?\n \n> To achieve the best, we have to find the pairs of lines (one minus and\n> one plus for each pair) which most match each other, and then do the\n> word diff for each pair.\n\nWouldn't be enough to treat run of plus/minus lines as a single block,\ntokenize, do token-based (as opposed to line-based) diff, then show it\nusing linebreaks of the destination file (pluses line)?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"76755","messageId":"7vve1jxrg9.fsf@gitster.siamese.dyndns.org","threadId":"13350","inReplyTo":"m34p934afu.fsf@localhost.localdomain","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-12T19:17:26Z","receivedAt":"2008-05-12T19:17:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n>> To achieve the best, we have to find the pairs of lines (one minus and\n>> one plus for each pair) which most match each other, and then do the\n>> word diff for each pair.\n>\n> Wouldn't be enough to treat run of plus/minus lines as a single block,\n> tokenize, do token-based (as opposed to line-based) diff, then show it\n> using linebreaks of the destination file (pluses line)?\n\nI tried the \"using linebreaks\" but I discarded it because I did not think\nit would work.  If we rewrite the last three lines above with this single\nline:\n\n> Wouldn't be enough to use magic?\n\nand apply that algorithm between the two, then we would get a long single\nline that has words painted in red, two lines worth, followed by green \"to\nuse magic?\"  and finally an end-of-line.\n"},{"id":"76759","messageId":"200805122157.57366.jnareb@gmail.com","threadId":"13350","inReplyTo":"7vve1jxrg9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-05-12T19:57:55Z","receivedAt":"2008-05-12T19:57:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n>> \"Ping Yin\" <pkufranky@gmail.com> writes:\n>>>\n>>> To achieve the best, we have to find the pairs of lines (one minus and\n>>> one plus for each pair) which most match each other, and then do the\n>>> word diff for each pair.\n>>\n>> Wouldn't be enough to treat run of plus/minus lines as a single block,\n>> tokenize, do token-based (as opposed to line-based) diff, then show it\n>> using linebreaks of the destination file (pluses line)?\n> \n> I tried the \"using linebreaks\" but I discarded it because I did not think\n> it would work.  If we rewrite the last three lines above with this single\n> line:\n> \n>> Wouldn't be enough to use magic?\n> \n> and apply that algorithm between the two, then we would get a long single\n> line that has words painted in red, two lines worth, followed by green \"to\n> use magic?\"  and finally an end-of-line.\n\nIt looks then like inserting (retaining?) newlines in word/token based\n--color-words output isn't simple.  It would have to produce readable\noutput both for the case you stated/mentioned, and for the opposite case\n(replacing single line by multiple lines).  What's even more difficult,\nit should produce clear output for a simple case of rewrapping output,\ne.g. the following as replacement.\n\n>> Wouldn't be enough to treat run of plus/minus lines as a single\n>> block, tokenize, do token-based (as opposed to line-based) diff,\n>> then show it using linebreaks of the destination file (pluses\n>> line)? \n\nGaahh... I don't think this (word diff/token diff) is something computer\nscience has worked on?\n-- \nJakub Narebski\nPoland\n"},{"id":"76822","messageId":"20080513013753.GA17536@kooxoo235","threadId":"13350","inReplyTo":"7vve1jxrg9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-13T01:37:53Z","receivedAt":"2008-05-13T01:37:53Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"* Junio C Hamano <gitster@pobox.com> [2008-05-12 12:17:26 -0700]:\n\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> >> To achieve the best, we have to find the pairs of lines (one minus and\n> >> one plus for each pair) which most match each other, and then do the\n> >> word diff for each pair.\n> >\n> > Wouldn't be enough to treat run of plus/minus lines as a single block,\n> > tokenize, do token-based (as opposed to line-based) diff, then show it\n> > using linebreaks of the destination file (pluses line)?\n> \n> I tried the \"using linebreaks\" but I discarded it because I did not think\n> it would work.  If we rewrite the last three lines above with this single\n> line:\n> \n> > Wouldn't be enough to use magic?\n> \n> and apply that algorithm between the two, then we would get a long single\n> line that has words painted in red, two lines worth, followed by green \"to\n> use magic?\"  and finally an end-of-line.\n\nThat's why i said with current implementation we can't get the\nbest output which i think should be\n\nWouldn't be enough to {-treat run of plus/minus lines as a single block,}{+use magic?}\n{-tokenize, do token-based (as opposed to line-based) diff, then show it}\n{-using linebreaks of the destination file (pluses line)?}\n"},{"id":"76823","messageId":"20080513014255.GB17536@kooxoo235","threadId":"13350","inReplyTo":"m34p934afu.fsf@localhost.localdomain","subject":"Re: [PATCH v2 4/5] Make boundary characters for --color-words configurable","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-05-13T01:42:55Z","receivedAt":"2008-05-13T01:42:55Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"* Jakub Narebski <jnareb@gmail.com> [2008-05-12 11:57:48 -0700]:\n\n> >  ( i think the result of vimdiff can be considered as the best).\n> \n> Is the vimdiff algorithm described anywhere? What about wdiff output?\n\nI don't know about that. source code? I just show an example of best\n--color-words.\n\nI havn't yet used wdiff.\n"}]}