{"thread":{"id":"29951","subject":"[PATCH 2/3] diff: refactor the word-diff setup from builtin_diff_cmd","startedAt":"2012-03-14T18:24:07Z","lastAt":"2012-03-14T21:53:29Z","messageCount":10,"participants":["Thomas Rast","Junio C Hamano","Johannes Sixt","Stefano Lattarini"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"186975","messageId":"3bb99e938624ae674674b304c24c48b9dc05871b.1331749299.git.trast@student.ethz.ch","threadId":"29951","inReplyTo":null,"subject":"[PATCH v2 1/3] Demonstrate a bug in --word-diff where diff.*.wordregex is \"sticky\"","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-03-14T18:24:07Z","receivedAt":"2012-03-14T18:24:07Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"From: Johannes Sixt \n\nThe test case applies a custom wordRegex to one file in a diff, and expects\nthat the default word splitting applies to the second file in the diff.\nBut the custom wordRegex is also incorrectly used for the second file.\n\n[tr: unset the diff.wordRegex variable to make the test meaningful]\n\nSigned-off-by: Johannes Sixt \nSigned-off-by: Thomas Rast \n---\n\nCompared to your version, I added the first hunk. Otherwise the\ndiff.wordRegex=[[:alnum:]]+ setting carries over and makes the test\nfail even with the bug fixed.\n\nI deliberately put it as early as possible, rather than into the setup\nfor your test, to avoid confusion next time someone patches that file.\n\n t/t4034-diff-words.sh | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 5c20121..69e81f3 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -293,6 +293,10 @@ test_expect_success '--word-diff=none' '\n \tword_diff --word-diff=plain --word-diff=none\n '\n \n+test_expect_success 'unset default driver' '\n+\tgit config --unset diff.wordregex\n+'\n+\n test_language_driver bibtex\n test_language_driver cpp\n test_language_driver csharp\n@@ -348,4 +352,37 @@ test_expect_success 'word-diff with no newline at EOF' '\n \tword_diff --word-diff=plain\n '\n \n+test_expect_success 'setup history with two files' '\n+\techo \"a b; c\" >a &&\n+\techo \"a b; c\" >z &&\n+\tgit add a z &&\n+\tgit commit -minitial &&\n+\n+\t# modify both\n+\techo \"a bx; c\" >a &&\n+\techo \"a bx; c\" >z &&\n+\tgit commit -mmodified -a\n+'\n+\n+test_expect_failure 'wordRegex for the first file does not apply to the second' '\n+\techo \"a diff=tex\" >.gitattributes &&\n+\tgit config diff.tex.wordRegex \"[a-z]+|.\" &&\n+\tcat >expect <<-\\EOF &&\n+\t\tdiff --git a/a b/a\n+\t\tindex 9823d38..b09f967 100644\n+\t\t--- a/a\n+\t\t+++ b/a\n+\t\t@@ -1 +1 @@\n+\t\ta [-b-]{+bx+}; c\n+\t\tdiff --git a/z b/z\n+\t\tindex 9823d38..b09f967 100644\n+\t\t--- a/z\n+\t\t+++ b/z\n+\t\t@@ -1 +1 @@\n+\t\ta [-b;-]{+bx;+} c\n+\tEOF\n+\tgit diff --word-diff HEAD~ >actual\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.10.rc0.286.gd2cb29\n"},{"id":"186974","messageId":"9f2f3a870a8d88850444090318647f3c7b8c0743.1331749299.git.trast@student.ethz.ch","threadId":"29951","inReplyTo":"3bb99e938624ae674674b304c24c48b9dc05871b.1331749299.git.trast@student.ethz.ch","subject":"[PATCH 2/3] diff: refactor the word-diff setup from builtin_diff_cmd","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-03-14T18:24:08Z","receivedAt":"2012-03-14T18:24:08Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Quite a chunk of builtin_diff_cmd deals with word-diff setup, defaults\nand such. This makes the function a bit hard to read, but is also\nasymmetric because the corresponding teardown lives in free_diff_words_data\nalready.\n\nRefactor into a new function init_diff_words_data. For simplicity,\nalso shuffle around some functions it depends on.\n\nSigned-off-by: Thomas Rast \n---\n diff.c | 119 ++++++++++++++++++++++++++++++++++------------------------------\n 1 file changed, 63 insertions(+), 56 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a698a3f..aac86d3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -989,6 +989,67 @@ static void diff_words_flush(struct emit_callback *ecbdata)\n \t\tdiff_words_show(ecbdata->diff_words);\n }\n \n+static void diff_filespec_load_driver(struct diff_filespec *one)\n+{\n+\t/* Use already-loaded driver */\n+\tif (one->driver)\n+\t\treturn;\n+\n+\tif (S_ISREG(one->mode))\n+\t\tone->driver = userdiff_find_by_path(one->path);\n+\n+\t/* Fallback to default settings */\n+\tif (!one->driver)\n+\t\tone->driver = userdiff_find_by_name(\"default\");\n+}\n+\n+static const char *userdiff_word_regex(struct diff_filespec *one)\n+{\n+\tdiff_filespec_load_driver(one);\n+\treturn one->driver->word_regex;\n+}\n+\n+static void init_diff_words_data(struct emit_callback *ecbdata,\n+\t\t\t\t struct diff_options *o,\n+\t\t\t\t struct diff_filespec *one,\n+\t\t\t\t struct diff_filespec *two)\n+{\n+\tint i;\n+\n+\tecbdata->diff_words =\n+\t\txcalloc(1, sizeof(struct diff_words_data));\n+\tecbdata->diff_words->type = o->word_diff;\n+\tecbdata->diff_words->opt = o;\n+\tif (!o->word_regex)\n+\t\to->word_regex = userdiff_word_regex(one);\n+\tif (!o->word_regex)\n+\t\to->word_regex = userdiff_word_regex(two);\n+\tif (!o->word_regex)\n+\t\to->word_regex = diff_word_regex_cfg;\n+\tif (o->word_regex) {\n+\t\tecbdata->diff_words->word_regex = (regex_t *)\n+\t\t\txmalloc(sizeof(regex_t));\n+\t\tif (regcomp(ecbdata->diff_words->word_regex,\n+\t\t\t o->word_regex,\n+\t\t\t REG_EXTENDED | REG_NEWLINE))\n+\t\t\tdie (\"Invalid regular expression: %s\",\n+\t\t\t o->word_regex);\n+\t}\n+\tfor (i = 0; i < ARRAY_SIZE(diff_words_styles); i++) {\n+\t\tif (o->word_diff == diff_words_styles[i].type) {\n+\t\t\tecbdata->diff_words->style =\n+\t\t\t\t&diff_words_styles[i];\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tif (want_color(o->use_color)) {\n+\t\tstruct diff_words_style *st = ecbdata->diff_words->style;\n+\t\tst->old.color = diff_get_color_opt(o, DIFF_FILE_OLD);\n+\t\tst->new.color = diff_get_color_opt(o, DIFF_FILE_NEW);\n+\t\tst->ctx.color = diff_get_color_opt(o, DIFF_PLAIN);\n+\t}\n+}\n+\n static void free_diff_words_data(struct emit_callback *ecbdata)\n {\n \tif (ecbdata->diff_words) {\n@@ -2061,20 +2122,6 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *pre\n \temit_binary_diff_body(file, two, one, prefix);\n }\n \n-static void diff_filespec_load_driver(struct diff_filespec *one)\n-{\n-\t/* Use already-loaded driver */\n-\tif (one->driver)\n-\t\treturn;\n-\n-\tif (S_ISREG(one->mode))\n-\t\tone->driver = userdiff_find_by_path(one->path);\n-\n-\t/* Fallback to default settings */\n-\tif (!one->driver)\n-\t\tone->driver = userdiff_find_by_name(\"default\");\n-}\n-\n int diff_filespec_is_binary(struct diff_filespec *one)\n {\n \tif (one->is_binary == -1) {\n@@ -2100,12 +2147,6 @@ static const struct userdiff_funcname *diff_funcname_pattern(struct diff_filespe\n \treturn one->driver->funcname.pattern ? &one->driver->funcname : NULL;\n }\n \n-static const char *userdiff_word_regex(struct diff_filespec *one)\n-{\n-\tdiff_filespec_load_driver(one);\n-\treturn one->driver->word_regex;\n-}\n-\n void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b)\n {\n \tif (!options->a_prefix)\n@@ -2292,42 +2333,8 @@ static void builtin_diff(const char *name_a,\n \t\t\txecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);\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-\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\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 = diff_word_regex_cfg;\n-\t\t\tif (o->word_regex) {\n-\t\t\t\tecbdata.diff_words->word_regex = (regex_t *)\n-\t\t\t\t\txmalloc(sizeof(regex_t));\n-\t\t\t\tif (regcomp(ecbdata.diff_words->word_regex,\n-\t\t\t\t\t\to->word_regex,\n-\t\t\t\t\t\tREG_EXTENDED | REG_NEWLINE))\n-\t\t\t\t\tdie (\"Invalid regular expression: %s\",\n-\t\t\t\t\t\t\to->word_regex);\n-\t\t\t}\n-\t\t\tfor (i = 0; i < ARRAY_SIZE(diff_words_styles); i++) {\n-\t\t\t\tif (o->word_diff == diff_words_styles[i].type) {\n-\t\t\t\t\tecbdata.diff_words->style =\n-\t\t\t\t\t\t&diff_words_styles[i];\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\tif (want_color(o->use_color)) {\n-\t\t\t\tstruct diff_words_style *st = ecbdata.diff_words->style;\n-\t\t\t\tst->old.color = diff_get_color_opt(o, DIFF_FILE_OLD);\n-\t\t\t\tst->new.color = diff_get_color_opt(o, DIFF_FILE_NEW);\n-\t\t\t\tst->ctx.color = diff_get_color_opt(o, DIFF_PLAIN);\n-\t\t\t}\n-\t\t}\n+\t\tif (o->word_diff)\n+\t\t\tinit_diff_words_data(&ecbdata, o, one, two);\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n \t\t\t &xpp, &xecfg);\n \t\tif (o->word_diff)\n-- \n1.7.10.rc0.286.gd2cb29\n"},{"id":"186976","messageId":"b7d30afe65910ffe614a894db65386477778c4da.1331749299.git.trast@student.ethz.ch","threadId":"29951","inReplyTo":"3bb99e938624ae674674b304c24c48b9dc05871b.1331749299.git.trast@student.ethz.ch","subject":"[PATCH 3/3] diff: tweak a _copy_ of diff_options with word-diff","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2012-03-14T18:24:09Z","receivedAt":"2012-03-14T18:24:09Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"When using word diff, the code sets the word_regex from various\ndefaults if it was not set already. The problem is that it does this\non the original diff_options, which will also be used in subsequent\ndiffs.\n\nThis means that when the word_regex is not given on the command line,\nonly the first diff for which a setting for word_regex (either from\nattributes or diff.wordRegex) ever takes effect. This value then\npropagates to the rest of the diff runs and in particular prevents\nfurther attribute lookups.\n\nFix the problem of changing diff state once and for all, by working\nwith a _copy_ of the diff_options.\n\nNoticed-by: Johannes Sixt \nSigned-off-by: Thomas Rast \n---\n diff.c | 5 ++++-\n t/t4034-diff-words.sh | 2 +-\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex aac86d3..c6104b9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1010,11 +1010,13 @@ static const char *userdiff_word_regex(struct diff_filespec *one)\n }\n \n static void init_diff_words_data(struct emit_callback *ecbdata,\n-\t\t\t\t struct diff_options *o,\n+\t\t\t\t struct diff_options *orig_opts,\n \t\t\t\t struct diff_filespec *one,\n \t\t\t\t struct diff_filespec *two)\n {\n \tint i;\n+\tstruct diff_options *o = xmalloc(sizeof(struct diff_options));\n+\tmemcpy(o, orig_opts, sizeof(struct diff_options));\n \n \tecbdata->diff_words =\n \t\txcalloc(1, sizeof(struct diff_words_data));\n@@ -1054,6 +1056,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n {\n \tif (ecbdata->diff_words) {\n \t\tdiff_words_flush(ecbdata);\n+\t\tfree (ecbdata->diff_words->opt);\n \t\tfree (ecbdata->diff_words->minus.text.ptr);\n \t\tfree (ecbdata->diff_words->minus.orig);\n \t\tfree (ecbdata->diff_words->plus.text.ptr);\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 69e81f3..c04756e 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -364,7 +364,7 @@ test_expect_success 'setup history with two files' '\n \tgit commit -mmodified -a\n '\n \n-test_expect_failure 'wordRegex for the first file does not apply to the second' '\n+test_expect_success 'wordRegex for the first file does not apply to the second' '\n \techo \"a diff=tex\" >.gitattributes &&\n \tgit config diff.tex.wordRegex \"[a-z]+|.\" &&\n \tcat >expect <<-\\EOF &&\n-- \n1.7.10.rc0.286.gd2cb29\n"},{"id":"186983","messageId":"7vsjhboujp.fsf@alter.siamese.dyndns.org","threadId":"29951","inReplyTo":"3bb99e938624ae674674b304c24c48b9dc05871b.1331749299.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 1/3] Demonstrate a bug in --word-diff where diff.*.wordregex is \"sticky\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T19:24:10Z","receivedAt":"2012-03-14T19:24:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> From: Johannes Sixt \n>\n> The test case applies a custom wordRegex to one file in a diff, and expects\n> that the default word splitting applies to the second file in the diff.\n> But the custom wordRegex is also incorrectly used for the second file.\n>\n> [tr: unset the diff.wordRegex variable to make the test meaningful]\n>\n> Signed-off-by: Johannes Sixt \n> Signed-off-by: Thomas Rast \n> ---\n>\n> Compared to your version, I added the first hunk. Otherwise the\n> diff.wordRegex=[[:alnum:]]+ setting carries over and makes the test\n> fail even with the bug fixed.\n>\n> I deliberately put it as early as possible, rather than into the setup\n> for your test, to avoid confusion next time someone patches that file.\n>\n> t/t4034-diff-words.sh | 37 +++++++++++++++++++++++++++++++++++++\n> 1 file changed, 37 insertions(+)\n>\n> diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\n> index 5c20121..69e81f3 100755\n> --- a/t/t4034-diff-words.sh\n> +++ b/t/t4034-diff-words.sh\n> @@ -293,6 +293,10 @@ test_expect_success '--word-diff=none' '\n> \tword_diff --word-diff=plain --word-diff=none\n> '\n> \n> +test_expect_success 'unset default driver' '\n> +\tgit config --unset diff.wordregex\n> +'\n\nIsn't this unsafe if some of the tests before this one failed?\n\n\ttest_unconfig diff.wordregex\n\nBy the way, I really loathe the change that gutted major parts out of\nt/test-lib.sh and moved them to another file; now I have to eyeball two\nfiles to write a response like this instead of one.\n\n> +test_expect_failure 'wordRegex for the first file does not apply to the second' '\n> +\techo \"a diff=tex\" >.gitattributes &&\n> +\tgit config diff.tex.wordRegex \"[a-z]+|.\" &&\n\nThe use of files \"a\" and \"z\" as an example of pair of a tex and non tex\ninput makes the test look overly artificial (why not a.tex vs z.txt or\nsomething?), but other than that, looks cleanly done.\n\nThanks.\n\n> +\tcat >expect <<-\\EOF &&\n> +\t\tdiff --git a/a b/a\n> +\t\tindex 9823d38..b09f967 100644\n> +\t\t--- a/a\n> +\t\t+++ b/a\n> +\t\t@@ -1 +1 @@\n> +\t\ta [-b-]{+bx+}; c\n> +\t\tdiff --git a/z b/z\n> +\t\tindex 9823d38..b09f967 100644\n> +\t\t--- a/z\n> +\t\t+++ b/z\n> +\t\t@@ -1 +1 @@\n> +\t\ta [-b;-]{+bx;+} c\n> +\tEOF\n> +\tgit diff --word-diff HEAD~ >actual\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n"},{"id":"186984","messageId":"7vobrzoug4.fsf@alter.siamese.dyndns.org","threadId":"29951","inReplyTo":"9f2f3a870a8d88850444090318647f3c7b8c0743.1331749299.git.trast@student.ethz.ch","subject":"Re: [PATCH 2/3] diff: refactor the word-diff setup from builtin_diff_cmd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T19:26:19Z","receivedAt":"2012-03-14T19:26:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Quite a chunk of builtin_diff_cmd deals with word-diff setup, defaults\n> and such. This makes the function a bit hard to read, but is also\n> asymmetric because the corresponding teardown lives in free_diff_words_data\n> already.\n\nYay! \"A bit hard to read\" is a gross understatement ;-)\n\nWhether the series fixes a bug of not, this alone makes the patch\nworthwhile to look.\n\nThanks.\n"},{"id":"186985","messageId":"7vhaxrotwh.fsf@alter.siamese.dyndns.org","threadId":"29951","inReplyTo":"b7d30afe65910ffe614a894db65386477778c4da.1331749299.git.trast@student.ethz.ch","subject":"Re: [PATCH 3/3] diff: tweak a _copy_ of diff_options with word-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T19:38:06Z","receivedAt":"2012-03-14T19:38:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Makes sense. Thanks.\n"},{"id":"186988","messageId":"4F60F67D.9050003@kdbg.org","threadId":"29951","inReplyTo":"7vsjhboujp.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3 1/3] Demonstrate a bug in --word-diff where diff.*.wordregex is \"sticky\"","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-03-14T19:50:21Z","receivedAt":"2012-03-14T19:50:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The test case applies a custom wordRegex to one file in a diff, and expects\nthat the default word splitting applies to the second file in the diff.\nBut the custom wordRegex is also incorrectly used for the second file.\n\nHelped-by: Thomas Rast \nSigned-off-by: Johannes Sixt \n---\nChanges in this round, where I do not resend 2/3 and 3/3:\n\n- Use test_unconfig.\n- Use compare_diff_patch to check the result.\n- Fix a broken && chain at the end of the last test.\n- Name the files a.tex and z.tex as per your suggestion.\n\n t/t4034-diff-words.sh | 36 ++++++++++++++++++++++++++++++++++++\n 1 files changed, 36 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 5c20121..9e9643e 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -3,6 +3,7 @@\n test_description='word diff colors'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/diff-lib.sh\n \n cat >pre.simple <<-\\EOF\n \th(4)\n@@ -293,6 +294,10 @@ test_expect_success '--word-diff=none' '\n \tword_diff --word-diff=plain --word-diff=none\n '\n \n+test_expect_success 'unset default driver' '\n+\ttest_unconfig diff.wordregex\n+'\n+\n test_language_driver bibtex\n test_language_driver cpp\n test_language_driver csharp\n@@ -348,4 +353,35 @@ test_expect_success 'word-diff with no newline at EOF' '\n \tword_diff --word-diff=plain\n '\n \n+test_expect_success 'setup history with two files' '\n+\techo \"a b; c\" >a.tex &&\n+\techo \"a b; c\" >z.tex &&\n+\tgit add a.tex z.tex &&\n+\tgit commit -minitial &&\n+\n+\t# modify both\n+\techo \"a bx; c\" >a.tex &&\n+\techo \"a bx; c\" >z.tex &&\n+\tgit commit -mmodified -a\n+'\n+\n+test_expect_failure 'wordRegex for the first file does not apply to the second' '\n+\techo \"a.tex diff=tex\" >.gitattributes &&\n+\tgit config diff.tex.wordRegex \"[a-z]+|.\" &&\n+\tcat >expect <<-\\EOF &&\n+\t\tdiff --git a/a.tex b/a.tex\n+\t\t--- a/a.tex\n+\t\t+++ b/a.tex\n+\t\t@@ -1 +1 @@\n+\t\ta [-b-]{+bx+}; c\n+\t\tdiff --git a/z.tex b/z.tex\n+\t\t--- a/z.tex\n+\t\t+++ b/z.tex\n+\t\t@@ -1 +1 @@\n+\t\ta [-b;-]{+bx;+} c\n+\tEOF\n+\tgit diff --word-diff HEAD~ >actual &&\n+\tcompare_diff_patch expect actual\n+'\n+\n test_done\n-- \n1.7.8.216.g2e426\n"},{"id":"186994","messageId":"4F610788.3090604@gmail.com","threadId":"29951","inReplyTo":"4F60F67D.9050003@kdbg.org","subject":"Re: [PATCH v3 1/3] Demonstrate a bug in --word-diff where diff.*.wordregex is \"sticky\"","fromName":"Stefano Lattarini","fromEmail":"stefano.lattarini@gmail.com","sentAt":"2012-03-14T21:03:04Z","receivedAt":"2012-03-14T21:03:04Z","isPatch":true,"sender":{"key":"stefano.lattarini@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1429199?v=4"},"body":"On 03/14/2012 08:50 PM, Johannes Sixt wrote:\n> The test case applies a custom wordRegex to one file in a diff, and expects\n> that the default word splitting applies to the second file in the diff.\n> But the custom wordRegex is also incorrectly used for the second file.\n> \n> Helped-by: Thomas Rast \n> Signed-off-by: Johannes Sixt \n> ---\n> Changes in this round, where I do not resend 2/3 and 3/3:\n> \n> - Use test_unconfig.\n> - Use compare_diff_patch to check the result.\n> - Fix a broken && chain at the end of the last test.\n> - Name the files a.tex and z.tex as per your suggestion.\n>\nBut Junio suggested using 'z.txt' (not 'z.tex'); quoting his last message:\n\n The use of files \"a\" and \"z\" as an example of pair of a tex and non tex\n input makes the test look overly artificial (why not a.tex vs z.txt or\n something?), but other than that, looks cleanly done.\n\nRegards,\n Stefano\n"},{"id":"186995","messageId":"7vhaxqoobe.fsf@alter.siamese.dyndns.org","threadId":"29951","inReplyTo":"4F60F67D.9050003@kdbg.org","subject":"Re: [PATCH v3 1/3] Demonstrate a bug in --word-diff where diff.*.wordregex is \"sticky\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-14T21:38:45Z","receivedAt":"2012-03-14T21:38:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> The test case applies a custom wordRegex to one file in a diff, and expects\n> that the default word splitting applies to the second file in the diff.\n> But the custom wordRegex is also incorrectly used for the second file.\n>\n> Helped-by: Thomas Rast \n> Signed-off-by: Johannes Sixt \n> ---\n> Changes in this round, where I do not resend 2/3 and 3/3:\n>\n> - Use test_unconfig.\n> - Use compare_diff_patch to check the result.\n> - Fix a broken && chain at the end of the last test.\n> - Name the files a.tex and z.tex as per your suggestion.\n\nThanks, but using a.tex and z.tex and marking only the former as tex\ndoes not change anything in the puzzlement I mentioned in my response.\n\nPerhaps this on top of your patch?\n\n t/t4034-diff-words.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh\nindex 9e9643e..310ace1 100755\n--- a/t/t4034-diff-words.sh\n+++ b/t/t4034-diff-words.sh\n@@ -355,18 +355,18 @@ test_expect_success 'word-diff with no newline at EOF' '\n \n test_expect_success 'setup history with two files' '\n \techo \"a b; c\" >a.tex &&\n-\techo \"a b; c\" >z.tex &&\n-\tgit add a.tex z.tex &&\n+\techo \"a b; c\" >z.txt &&\n+\tgit add a.tex z.txt &&\n \tgit commit -minitial &&\n \n \t# modify both\n \techo \"a bx; c\" >a.tex &&\n-\techo \"a bx; c\" >z.tex &&\n+\techo \"a bx; c\" >z.txt &&\n \tgit commit -mmodified -a\n '\n \n test_expect_failure 'wordRegex for the first file does not apply to the second' '\n-\techo \"a.tex diff=tex\" >.gitattributes &&\n+\techo \"*.tex diff=tex\" >.gitattributes &&\n \tgit config diff.tex.wordRegex \"[a-z]+|.\" &&\n \tcat >expect <<-\\EOF &&\n \t\tdiff --git a/a.tex b/a.tex\n@@ -374,9 +374,9 @@ test_expect_failure 'wordRegex for the first file does not apply to the second'\n \t\t+++ b/a.tex\n \t\t@@ -1 +1 @@\n \t\ta [-b-]{+bx+}; c\n-\t\tdiff --git a/z.tex b/z.tex\n-\t\t--- a/z.tex\n-\t\t+++ b/z.tex\n+\t\tdiff --git a/z.txt b/z.txt\n+\t\t--- a/z.txt\n+\t\t+++ b/z.txt\n \t\t@@ -1 +1 @@\n \t\ta [-b;-]{+bx;+} c\n \tEOF\n-- \n1.7.10.rc0.65.g3445e\n"},{"id":"186997","messageId":"4F611359.70109@kdbg.org","threadId":"29951","inReplyTo":"7vhaxqoobe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/3] Demonstrate a bug in --word-diff where diff.*.wordregex is \"sticky\"","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-03-14T21:53:29Z","receivedAt":"2012-03-14T21:53:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.03.2012 22:38, schrieb Junio C Hamano:\n> Thanks, but using a.tex and z.tex and marking only the former as tex\n> does not change anything in the puzzlement I mentioned in my response.\n\nOh, I see. I didn't notice this detail. Please squash in your proposed\nfixup that uses z.txt.\n\nThanks,\n-- Hannes\n"}]}