{"thread":{"id":"29344","subject":"[PATCH 1/2] t4034-diff-words: replace regex for diff driver","startedAt":"2012-01-11T17:25:01Z","lastAt":"2012-01-20T01:14:51Z","messageCount":8,"participants":["Tay Ray Chuan","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"182362","messageId":"1326302702-4536-1-git-send-email-rctay89@gmail.com","threadId":"29344","inReplyTo":null,"subject":"[PATCH 1/2] t4034-diff-words: replace regex for diff driver","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-01-11T17:25:01Z","receivedAt":"2012-01-11T17:25:01Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"The next patch uses a non-whitespace regex, similar to the regex\ncurrently used by the 'testdriver' diff driver; replace the regex with a\ndistinct one so that we can continue to conclude its effects.\n\nSigned-off-by: Tay Ray Chuan \n\n---\nKept separate to keep the next patch clean.\n---\n t/t4034-diff-words.sh | 20 +++++++++++++++++---\n 1 files changed, 17 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 6f1e5a2..9ae0e1a 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -46,6 +46,20 @@ cat >expect.non-whitespace-is-word <<-\\EOF\n \n \taeff = aeff * ( aaa )\n EOF\n+cat >expect.everything-is-word <<-\\EOF\n+\tdiff --git a/pre b/post\n+\tindex 330b04f..5ed8eff 100644\n+\t--- a/pre\n+\t+++ b/post\n+\t@@ -1,3 +1,7 @@\n+\th(4)h(4),hh[44]\n+\n+\ta = b + c\n+\n+\taa = a\n+\n+\taeff = aeff * ( aaa )\n+EOF\n \n word_diff () {\n \ttest_must_fail git diff --no-index \"$@\" pre post >output &&\n@@ -179,7 +193,7 @@ test_expect_success 'word diff with a regular expression' '\n '\n \n test_expect_success 'set up a diff driver' '\n-\tgit config diff.testdriver.wordRegex \"[^[:space:]]\" &&\n+\tgit config diff.testdriver.wordRegex \".+\" &&\n \tcat <<-\\EOF >.gitattributes\n \t\tpre diff=testdriver\n \t\tpost diff=testdriver\n@@ -192,7 +206,7 @@ test_expect_success 'option overrides .gitattributes' '\n '\n \n test_expect_success 'use regex supplied by driver' '\n-\tcp expect.non-whitespace-is-word expect &&\n+\tcp expect.everything-is-word expect &&\n \tword_diff --color-words\n '\n \n@@ -224,7 +238,7 @@ test_expect_success 'command-line overrides config: --word-diff-regex' '\n '\n \n test_expect_success '.gitattributes override config' '\n-\tcp expect.non-whitespace-is-word expect &&\n+\tcp expect.everything-is-word expect &&\n \tword_diff --color-words\n '\n \n-- \n1.7.7.584.g16d0ea\n"},{"id":"182363","messageId":"1326302702-4536-2-git-send-email-rctay89@gmail.com","threadId":"29344","inReplyTo":"1326302702-4536-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-01-11T17:25:02Z","receivedAt":"2012-01-11T17:25:02Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Factor out the comprehensive non-whitespace regex in use by PATTERNS and\nIPATTERN and use it as the word-diff regex for the default diff driver.\n\nAs the default regex is no longer non-empty, update the word-regex\nselection logic (non-default driver from pre-image, then post-image,\nthen the diff.wordRegex config) accordingly.\n\nSigned-off-by: Tay Ray Chuan \n---\n diff.c | 14 ++++++++------\n t/t4034-diff-words.sh | 31 +++++++++----------------------\n userdiff.c | 8 +++++---\n 3 files changed, 22 insertions(+), 31 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 374ecf3..5f71f9f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1987,9 +1987,10 @@ static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespe\n \treturn one->driver->funcname.pattern ? &one->driver->funcname : NULL;\n }\n \n-static const char *userdiff_word_regex(struct diff_filespec *one)\n+static const char *userdiff_word_regex(struct diff_filespec *one, int *is_default)\n {\n \tdiff_filespec_load_driver(one);\n+\t*is_default = !strcmp(one->driver->name, \"default\");\n \treturn one->driver->word_regex;\n }\n \n@@ -2180,17 +2181,18 @@ static void builtin_diff(const char *name_a,\n \t\telse if (!prefixcmp(diffopts, \"-u\"))\n \t\t\txecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);\n \t\tif (o->word_diff) {\n-\t\t\tint i;\n+\t\t\tint i, is_default;\n \n \t\t\tecbdata.diff_words =\n \t\t\t\txcalloc(1, sizeof(struct diff_words_data));\n \t\t\tecbdata.diff_words->type = o->word_diff;\n \t\t\tecbdata.diff_words->opt = o;\n+\t\t\tis_default = 0;\n \t\t\tif (!o->word_regex)\n-\t\t\t\to->word_regex = userdiff_word_regex(one);\n-\t\t\tif (!o->word_regex)\n-\t\t\t\to->word_regex = userdiff_word_regex(two);\n-\t\t\tif (!o->word_regex)\n+\t\t\t\to->word_regex = userdiff_word_regex(one, &is_default);\n+\t\t\tif (is_default)\n+\t\t\t\to->word_regex = userdiff_word_regex(two, &is_default);\n+\t\t\tif (is_default && diff_word_regex_cfg)\n \t\t\t\to->word_regex = diff_word_regex_cfg;\n \t\t\tif (o->word_regex) {\n \t\t\t\tecbdata.diff_words->word_regex = (regex_t *)\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 9ae0e1a..e588849 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -84,26 +84,13 @@ test_expect_success setup '\n \tgit config diff.color.func magenta\n '\n \n-test_expect_success 'set up pre and post with runs of whitespace' '\n+test_expect_success 'set up pre and post with runs of non-whitespace' '\n \tcp pre.simple pre &&\n-\tcp post.simple post\n+\tcp post.simple post &&\n+\tcp expect.non-whitespace-is-word expect\n '\n \n-test_expect_success 'word diff with runs of whitespace' '\n-\tcat >expect <<-\\EOF &&\n-\t\tdiff --git a/pre b/post\n-\t\tindex 330b04f..5ed8eff 100644\n-\t\t--- a/pre\n-\t\t+++ b/post\n-\t\t@@ -1,3 +1,7 @@\n-\t\th(4)h(4),hh[44]\n-\n-\t\ta = b + c\n-\n-\t\taa = a\n-\n-\t\taeff = aeff * ( aaa )\n-\tEOF\n+test_expect_success 'word diff defaults to runs of non-whitespace' '\n \tword_diff --color-words &&\n \tword_diff --word-diff=color &&\n \tword_diff --color --word-diff=color\n@@ -116,8 +103,8 @@ test_expect_success '--word-diff=porcelain' '\n \t\t--- a/pre\n \t\t+++ b/post\n \t\t@@ -1,3 +1,7 @@\n-\t\t-h(4)\n-\t\t+h(4),hh[44]\n+\t\t h(4)\n+\t\t+,hh[44]\n \t\t~\n \t\t # significant space\n \t\t~\n@@ -140,7 +127,7 @@ test_expect_success '--word-diff=plain' '\n \t\t--- a/pre\n \t\t+++ b/post\n \t\t@@ -1,3 +1,7 @@\n-\t\t[-h(4)-]{+h(4),hh[44]+}\n+\t\th(4){+,hh[44]+}\n \n \t\ta = b + c\n \n@@ -159,7 +146,7 @@ test_expect_success '--word-diff=plain --color' '\n \t\t--- a/pre\n \t\t+++ b/post\n \t\t@@ -1,3 +1,7 @@\n-\t\t[-h(4)-]{+h(4),hh[44]+}\n+\t\th(4){+,hh[44]+}\n \n \t\ta = b + c\n \n@@ -177,7 +164,7 @@ test_expect_success 'word diff without context' '\n \t\t--- a/pre\n \t\t+++ b/post\n \t\t@@ -1 +1 @@\n-\t\th(4)h(4),hh[44]\n+\t\th(4),hh[44]\n \t\t@@ -3,0 +4,4 @@ a = b + c\n \n \t\taa = a\ndiff --git a/userdiff.c b/userdiff.c\nindex 76109da..cf38566 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -7,12 +7,14 @@ static struct userdiff_driver *drivers;\n static int ndrivers;\n static int drivers_alloc;\n \n+#define NON_WHITESPACE\t\\\n+\t\"[^[:space:]]|[\\xc0-\\xff][\\x80-\\xbf]+\"\n #define PATTERNS(name, pattern, word_regex)\t\t\t\\\n \t{ name, NULL, -1, { pattern, REG_EXTENDED },\t\t\\\n-\t word_regex \"|[^[:space:]]|[\\xc0-\\xff][\\x80-\\xbf]+\" }\n+\t word_regex \"|\" NON_WHITESPACE }\n #define IPATTERN(name, pattern, word_regex)\t\t\t\\\n \t{ name, NULL, -1, { pattern, REG_EXTENDED | REG_ICASE }, \\\n-\t word_regex \"|[^[:space:]]|[\\xc0-\\xff][\\x80-\\xbf]+\" }\n+\t word_regex \"|\" NON_WHITESPACE }\n static struct userdiff_driver builtin_drivers[] = {\n IPATTERN(\"fortran\",\n \t \"!^([C*]|[ \\t]*!)\\n\"\n@@ -140,7 +142,7 @@ PATTERNS(\"csharp\",\n \t \"[a-zA-Z_][a-zA-Z0-9_]*\"\n \t \"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?\"\n \t \"|[-+*/<>%&^|=!]=|--|\\\\+\\\\+|<<=?|>>=?|&&|\\\\|\\\\||::|->\"),\n-{ \"default\", NULL, -1, { NULL, 0 } },\n+{ \"default\", NULL, -1, { NULL, 0 }, NON_WHITESPACE },\n };\n #undef PATTERNS\n #undef IPATTERN\n-- \n1.7.7.584.g16d0ea\n"},{"id":"182376","messageId":"87lipexawp.fsf@thomas.inf.ethz.ch","threadId":"29344","inReplyTo":"1326302702-4536-2-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-01-11T20:05:26Z","receivedAt":"2012-01-11T20:05:26Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Tay Ray Chuan writes:\n\n> Factor out the comprehensive non-whitespace regex in use by PATTERNS and\n> IPATTERN and use it as the word-diff regex for the default diff driver.\n\nWhy?\n\nI seem to recall that the motivation for keeping the original code as-is\ninstead of just emulating its behavior with a default regex was that it\nis faster. So disabling the default mode should at least have an\nadvantage?\n\n\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"182395","messageId":"CALUzUxo3DcKqC6sQFQ1Oi0vgASFSHCcmOgHAj2_4c3vEjy663w@mail.gmail.com","threadId":"29344","inReplyTo":"87lipexawp.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-01-12T00:52:49Z","receivedAt":"2012-01-12T00:52:49Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nThomas, first off, thanks for looking through this.\n\nOn Thu, Jan 12, 2012 at 4:05 AM, Thomas Rast wrote:\n> Tay Ray Chuan writes:\n>\n>> Factor out the comprehensive non-whitespace regex in use by PATTERNS and\n>> IPATTERN and use it as the word-diff regex for the default diff driver.\n>\n> Why?\n>\n> I seem to recall that the motivation for keeping the original code as-is\n> instead of just emulating its behavior with a default regex was that it\n> is faster.  So disabling the default mode should at least have an\n> advantage?\n>\n> \n\nIf you're talking about speed, yeah, that's probably true.\n\nBut I think it's worthwhile to trade-off performance for a sensible\ndefault. Something like\n\n matrix[a,b,c]\n matrix[d,b,c]\n\ngives\n\n matrix[[-a-]{+d+},b,c]\n\nand when we have\n\n ImagineALanguageLikeFoo\n ImagineALanguageLikeBar\n\nwe get\n\n ImagineALanguageLike[-Foo-]{+Bar+}\n\n(But I cheated. Foo and Bar have no common characters in common; if\nthey did, the word diff would be messy.)\n\nBoth of which seem sensible. From a usability/effectiveness\nstandpoint, I think it's more useful than what the current word-diff\ndefaults to - the whole line is taken as a \"word\", with the pre-image\nshown as deleted and the post-image as added; we don't even try to run\nLCS on it.\n\nExamples are lifted from:\n[1] http://article.gmane.org/gmane.comp.version-control.git/105896\n[2] http://article.gmane.org/gmane.comp.version-control.git/105237\n\n-- \nCheers,\nRay Chuan\n"},{"id":"182424","messageId":"87ipkhqnr8.fsf@thomas.inf.ethz.ch","threadId":"29344","inReplyTo":"CALUzUxo3DcKqC6sQFQ1Oi0vgASFSHCcmOgHAj2_4c3vEjy663w@mail.gmail.com","subject":"Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-01-12T09:22:03Z","receivedAt":"2012-01-12T09:22:03Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Tay Ray Chuan writes:\n\n> On Thu, Jan 12, 2012 at 4:05 AM, Thomas Rast wrote:\n>> Tay Ray Chuan writes:\n>>\n>>> Factor out the comprehensive non-whitespace regex in use by PATTERNS and\n>>> IPATTERN and use it as the word-diff regex for the default diff driver.\n>>\n>> Why?\n\nSorry for distracting you with the performance argument; it was mostly\nthe first thing that came to my mind that I could use to ask for the\nmotivation, and evaluation of tradeoffs, that both were missing from the\nproposed commit message.\n\n> But I think it's worthwhile to trade-off performance for a sensible\n> default. Something like\n>\n> matrix[a,b,c]\n> matrix[d,b,c]\n>\n> gives\n>\n> matrix[[-a-]{+d+},b,c]\n>\n> and when we have\n>\n> ImagineALanguageLikeFoo\n> ImagineALanguageLikeBar\n>\n> we get\n>\n> ImagineALanguageLike[-Foo-]{+Bar+}\n\nIn that case (and I should have read the original patch), I am\ndefinitely against this change. It turns the default word-diff into\ncharacter-diff, which is something entirely different, and frequently\nuseless precisely for the reason you state:\n\n> (But I cheated. Foo and Bar have no common characters in common; if\n> they did, the word diff would be messy.)\n\nCase in point, consider my patch sent out yesterday\n\n http://article.gmane.org/gmane.comp.version-control.git/188391\n\nIt consists of a one-hunk doc update. word-diff is not brilliant:\n\n -k::\n Usually the program [-'cleans up'-]{+removes email cruft from+} the Subject:\n header line to extract the title line for the commit log\n [-message,-]\n [- among which (1) remove 'Re:' or 're:', (2) leading-]\n [- whitespaces, (3) '[' up to ']', typically '[PATCH]', and-]\n [- then prepends \"[PATCH] \".-]{+message.+} This [-flag forbids-]{+option prevents+} this munging, and is most\n useful when used to read back 'git format-patch -k' output.\n[snip the rest as it's only {+}]\n\nBut character-diff tries too hard to find common subsequences:\n\n $ g show HEAD^^ --word-diff-regex='[^[:space:]]' | xsel\n -k::\n Usually the program [-'cl-]{+remov+}e[-an-]s {+email cr+}u[-p'-]{+ft from+} the Subject:\n header line to extract the title line for the commit log\n message[-,-]\n [- among which (1) remove 'Re:' or 're:', (2) leading-]\n [- w-]{+. T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[-ically '[PATCH]', and-]t[-he-]{+io+}n pre[-p-]{+v+}en[-ds \"[PATCH] \". This flag forbid-]{+t+}s this munging, and is most\n useful when used to read back 'git format-patch -k' output.\n[snip]\n\nWouldn't you agree that\n\n w-]{+. T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[\n\nis just line noise? The colors don't even help as most of it is removed\n(red).\n\nRegarding your examples\n\n> [1] http://article.gmane.org/gmane.comp.version-control.git/105896\n> [2] http://article.gmane.org/gmane.comp.version-control.git/105237\n\nfirst please notice that both of them were written before (and actually\ndiscussing) the introduction of the wordRegex feature. At this point,\nwe were trying to make up our minds w.r.t. how powerful the feature\nneeds to be. Nowadays (or in fact, starting a few days after those\nemails) the user can easily achieve everything discussed here by setting\nthe wordRegex to taste.\n\nThat being said, I can see some arguments for changing the default to\nsplit punctuation into a separate word. That is, whereas the current\ndefault is semantically equivalent to a wordRegex of\n\n [^[:space:]]*\n\n(but has a faster code path) and your proposal is equivalent to\n\n [^[:space:]]|UTF_8_GUARD\n\nI think there is a case to be made for a default of\n\n [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+\n\nor some such. There's a lot of bikeshedding lurking in the (non)extent\nof the [[:alnum:]] here, however.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"182718","messageId":"CALUzUxqXTXZv4RE=4rBa79T3_1y7UdqZ6okjC1y-Ve+=NDbQ2g@mail.gmail.com","threadId":"29344","inReplyTo":"87ipkhqnr8.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-01-18T07:32:29Z","receivedAt":"2012-01-18T07:32:29Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Thu, Jan 12, 2012 at 5:22 PM, Thomas Rast wrote:\n> [snip]\n> Case in point, consider my patch sent out yesterday\n>\n>  http://article.gmane.org/gmane.comp.version-control.git/188391\n>\n> It consists of a one-hunk doc update.  word-diff is not brilliant:\n>\n>  -k::\n>          Usually the program [-'cleans up'-]{+removes email cruft from+} the Subject:\n>          header line to extract the title line for the commit log\n>          [-message,-]\n>  [-      among which (1) remove 'Re:' or 're:', (2) leading-]\n>  [-      whitespaces, (3) '[' up to ']', typically '[PATCH]', and-]\n>  [-      then prepends \"[PATCH] \".-]{+message.+}  This [-flag forbids-]{+option prevents+} this munging, and is most\n>          useful when used to read back 'git format-patch -k' output.\n> [snip the rest as it's only {+}]\n>\n> But character-diff tries too hard to find common subsequences:\n>\n>  $ g show HEAD^^ --word-diff-regex='[^[:space:]]' | xsel\n>[snip]\n>  w-]{+.  T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[\n>\n> is just line noise?  The colors don't even help as most of it is removed\n> (red).\n\nYou missed the '+' quantifier, as in\n\n [^[:space:]]+\n\nUsing that regex, that abomination of a word-diff that you mentioned\ndisappears, like this:\n\n-k::\n\tUsually the program [-'cleans up'-]{+removes email cruft from+} the Subject:\n\theader line to extract the title line for the commit log\n\t[-message,-]\n[-\tamong which (1) remove 'Re:' or 're:', (2) leading-]\n[-\twhitespaces, (3) '[' up to ']', typically '[PATCH]', and-]\n[-\tthen prepends \"[PATCH] \".-]{+message.+} This [-flag\nforbids-]{+option prevents+} this munging, and is most\n\tuseful when used to read back 'git format-patch -k' output.\n\n> [snip]\n> That being said, I can see some arguments for changing the default to\n> split punctuation into a separate word.  That is, whereas the current\n> default is semantically equivalent to a wordRegex of\n>\n>  [^[:space:]]*\n>\n> (but has a faster code path)\n\nOh right, there *is* a sensible default implemented in. Somehow I was\nunder the impression that there wasn't.\n\nI wonder which is faster, using the non-whitespace regex, or the\nisspace() calls...\n\n> and your proposal is equivalent to\n>\n>  [^[:space:]]|UTF_8_GUARD\n>\n> I think there is a case to be made for a default of\n>\n>  [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+\n>\n> or some such.  There's a lot of bikeshedding lurking in the (non)extent\n> of the [[:alnum:]] here, however.\n\nCare to explain further? Not to sure what you mean here.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"182792","messageId":"87bopzofir.fsf@thomas.inf.ethz.ch","threadId":"29344","inReplyTo":"CALUzUxqXTXZv4RE=4rBa79T3_1y7UdqZ6okjC1y-Ve+=NDbQ2g@mail.gmail.com","subject":"Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-01-19T15:53:16Z","receivedAt":"2012-01-19T15:53:16Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Tay Ray Chuan writes:\n\n> On Thu, Jan 12, 2012 at 5:22 PM, Thomas Rast wrote:\n>> [snip]\n>> Case in point, consider my patch sent out yesterday\n>>\n>> http://article.gmane.org/gmane.comp.version-control.git/188391\n>>\n>> It consists of a one-hunk doc update. word-diff is not brilliant:\n>>\n>> -k::\n>> Usually the program [-'cleans up'-]{+removes email cruft from+} the Subject:\n>> header line to extract the title line for the commit log\n>> [-message,-]\n>> [- among which (1) remove 'Re:' or 're:', (2) leading-]\n>> [- whitespaces, (3) '[' up to ']', typically '[PATCH]', and-]\n>> [- then prepends \"[PATCH] \".-]{+message.+} This [-flag forbids-]{+option prevents+} this munging, and is most\n>> useful when used to read back 'git format-patch -k' output.\n>> [snip the rest as it's only {+}]\n>>\n>> But character-diff tries too hard to find common subsequences:\n>>\n>> $ g show HEAD^^ --word-diff-regex='[^[:space:]]' | xsel\n>>[snip]\n>> w-]{+. T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[\n>>\n>> is just line noise? The colors don't even help as most of it is removed\n>> (red).\n>\n> You missed the '+' quantifier, as in\n>\n> [^[:space:]]+\n\nDid I? I was working from the example you provided earlier\n\n} matrix[a,b,c]\n} matrix[d,b,c]\n} gives\n} matrix[[-a-]{+d+},b,c]\n} \n} and when we have\n} \n} ImagineALanguageLikeFoo\n} ImagineALanguageLikeBar\n} we get\n} ImagineALanguageLike[-Foo-]{+Bar+}\n\nUnder [^[:space:]]+ neither of the examples would work. Actually,\n[^[:space:]]+ is the same as today's default, the [^[:space:]]* I\nmentioned later is (strictly speaking) broken as it allows for a\n0-length match. (It doesn't really matter because IIRC the engine\nignores 0-length words.)\n\n>> That being said, I can see some arguments for changing the default to\n>> split punctuation into a separate word. That is, whereas the current\n>> default is semantically equivalent to a wordRegex of\n>>\n>> [^[:space:]]*\n>>\n>> (but has a faster code path)\n>\n> Oh right, there *is* a sensible default implemented in. Somehow I was\n> under the impression that there wasn't.\n>\n> I wonder which is faster, using the non-whitespace regex, or the\n> isspace() calls...\n\nI tried measuring it across a few commits, but it mostly gets drowned\nout by the diff effort. For a commit with stat\n\n exercises/cgal/cover/cover.cpp | 5 +-\n exercises/cgal/cover/cover.in1 |27014 +++++++++++++++-----\n exercises/cgal/cover/cover.in2 |48996 +++++++++++++++++++++++------------\n exercises/cgal/cover/cover.in3 |55041 +++++++++++++++++++++++++--------------\n exercises/cgal/cover/cover.in4 |47600 ++++++++++++++++++++--------------\n exercises/cgal/cover/cover.int |43491 ++++++++++++++++++++++---------\n exercises/cgal/cover/cover.out1 | 53 +-\n exercises/cgal/cover/cover.out2 | 24 +-\n exercises/cgal/cover/cover.out3 | 11 +-\n exercises/cgal/cover/cover.out4 | 2 +-\n exercises/cgal/cover/cover.outt | 23 +-\n exercises/cgal/cover/gen | 39 +-\n exercises/cgal/cover/gen-1.cpp | 4 +-\n exercises/cgal/cover/gen-2.cpp | 6 +-\n exercises/cgal/cover/gen-3.cpp | 6 +-\n\n(sorry, can't share as those testcases are secret) I get best-of-5\ntimings\n\n --word-diff-regex='[^[:space:]]+' 0:07.50real 7.40user 0.07system\n --word-diff 0:07.47real 7.41user 0.03system\n\nIn conclusion, \"meh\". I think ripping out the isspace() part would make\nfor a nice code reduction.\n\n>> and your proposal is equivalent to\n>>\n>> [^[:space:]]|UTF_8_GUARD\n>>\n>> I think there is a case to be made for a default of\n>>\n>> [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+\n>>\n>> or some such. There's a lot of bikeshedding lurking in the (non)extent\n>> of the [[:alnum:]] here, however.\n>\n> Care to explain further? Not to sure what you mean here.\n\nFor natural language, it may or may not make sense to match numbers as\npart of a word.\n\nFor typical use in e.g. emails, a lot of punctuation has a double role;\nbreaking words in\n\n http://article.gmane.org/gmane.comp.version-control.git/188391\n\nmay or may not make sense.\n\nFor some uses, especially source code, it would be better to match an\nunderscore _ as part of a complete word, too.\n\nFor some programming languages, say lisp, a dash - would also belong in\nthe same category.\n\nThere's no real reason other than ease of implementation why the pattern\nhandles ASCII non-alphanumerics separately, but non-ASCII UTF-8\nnon-alnums (like, say, unicode NO-BREAK SPACE which would show as \\xc2\n\\xa0) always goes into a word. But if you were to make UTF-8 sequences\na single word, text in (say) many European languages would become\nchunked at accented letters.\n\nI'm sure you can find more items for this list. It's a grey area.\n\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"182822","messageId":"CALUzUxq8dsc-rO6fVcOEvkaAtuJv7vHRYoUS++3D1nsJsyCrbw@mail.gmail.com","threadId":"29344","inReplyTo":"87bopzofir.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-01-20T01:14:51Z","receivedAt":"2012-01-20T01:14:51Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Thu, Jan 19, 2012 at 11:53 PM, Thomas Rast wrote:\n>[snip]\n> Under [^[:space:]]+ neither of the examples would work.  Actually,\n> [^[:space:]]+ is the same as today's default, the [^[:space:]]* I\n> mentioned later is (strictly speaking) broken as it allows for a\n> 0-length match.  (It doesn't really matter because IIRC the engine\n> ignores 0-length words.)\n\nMy bad.\n\n>[snip]\n> I tried measuring it across a few commits, but it mostly gets drowned\n> out by the diff effort.  For a commit with stat\n>\n>  exercises/cgal/cover/cover.cpp  |    5 +-\n>  exercises/cgal/cover/cover.in1  |27014 +++++++++++++++-----\n>  exercises/cgal/cover/cover.in2  |48996 +++++++++++++++++++++++------------\n>  exercises/cgal/cover/cover.in3  |55041 +++++++++++++++++++++++++--------------\n>  exercises/cgal/cover/cover.in4  |47600 ++++++++++++++++++++--------------\n>  exercises/cgal/cover/cover.int  |43491 ++++++++++++++++++++++---------\n>  exercises/cgal/cover/cover.out1 |   53 +-\n>  exercises/cgal/cover/cover.out2 |   24 +-\n>  exercises/cgal/cover/cover.out3 |   11 +-\n>  exercises/cgal/cover/cover.out4 |    2 +-\n>  exercises/cgal/cover/cover.outt |   23 +-\n>  exercises/cgal/cover/gen        |   39 +-\n>  exercises/cgal/cover/gen-1.cpp  |    4 +-\n>  exercises/cgal/cover/gen-2.cpp  |    6 +-\n>  exercises/cgal/cover/gen-3.cpp  |    6 +-\n>\n> (sorry, can't share as those testcases are secret) I get best-of-5\n> timings\n>\n>  --word-diff-regex='[^[:space:]]+'    0:07.50real 7.40user 0.07system\n>  --word-diff                          0:07.47real 7.41user 0.03system\n>\n> In conclusion, \"meh\".  I think ripping out the isspace() part would make\n> for a nice code reduction.\n\nThanks for the numbers. Well, that agrees with the intuition that\nregex is slower than isspace(), since you have run it through the\nregex engine.\n\n>>> and your proposal is equivalent to\n>>>\n>>>  [^[:space:]]|UTF_8_GUARD\n>>>\n>>> I think there is a case to be made for a default of\n>>>\n>>>  [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+\n>>>\n>>> or some such.  There's a lot of bikeshedding lurking in the (non)extent\n>>> of the [[:alnum:]] here, however.\n>>\n>> Care to explain further? Not to sure what you mean here.\n>\n> For natural language, it may or may not make sense to match numbers as\n> part of a word.\n>\n> For typical use in e.g. emails, a lot of punctuation has a double role;\n> breaking words in\n>\n>  http://article.gmane.org/gmane.comp.version-control.git/188391\n>\n> may or may not make sense.\n>\n> For some uses, especially source code, it would be better to match an\n> underscore _ as part of a complete word, too.\n>\n> For some programming languages, say lisp, a dash - would also belong in\n> the same category.\n>\n> There's no real reason other than ease of implementation why the pattern\n> handles ASCII non-alphanumerics separately, but non-ASCII UTF-8\n> non-alnums (like, say, unicode NO-BREAK SPACE which would show as \\xc2\n> \\xa0) always goes into a word.  But if you were to make UTF-8 sequences\n> a single word, text in (say) many European languages would become\n> chunked at accented letters.\n>\n> I'm sure you can find more items for this list.  It's a grey area.\n\nThanks.\n\n-- \nCheers,\nRay Chuan\n"}]}