{"thread":{"id":"25058","subject":"[PATCH 4/4] userdiff.c: add builtin fortran regex patterns","startedAt":"2010-09-09T19:02:45Z","lastAt":"2010-09-10T17:25:56Z","messageCount":16,"participants":["Brandon Casey","Jeff King","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"150390","messageId":"rRj7JpFIk_D_n7-wGkkucFJE0330IabsqMoSzalswHpjT-Z1HCuOFaIgMsaPgIuQSp0KUMW97Wo@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":null,"subject":"[PATCH 1/4] diff.c: call regfree to free memory allocated by regcomp when necessary","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:02:45Z","receivedAt":"2010-09-09T19:02:45Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n diff.c |    5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 144f2aa..9a5c77c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -919,7 +919,10 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n \t\tfree (ecbdata->diff_words->minus.orig);\n \t\tfree (ecbdata->diff_words->plus.text.ptr);\n \t\tfree (ecbdata->diff_words->plus.orig);\n-\t\tfree(ecbdata->diff_words->word_regex);\n+\t\tif (ecbdata->diff_words->word_regex) {\n+\t\t\tregfree(ecbdata->diff_words->word_regex);\n+\t\t\tfree(ecbdata->diff_words->word_regex);\n+\t\t}\n \t\tfree(ecbdata->diff_words);\n \t\tecbdata->diff_words = NULL;\n \t}\n-- \n1.7.2.1\n"},{"id":"150388","messageId":"rRj7JpFIk_D_n7-wGkkucCda_kClmekHTPEHOGwmU5nYdmNhqqgzF59v609YFx69vv526WtEpyU@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucFJE0330IabsqMoSzalswHpjT-Z1HCuOFaIgMsaPgIuQSp0KUMW97Wo@cipher.nrlssc.navy.mil","subject":"[PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:02:46Z","receivedAt":"2010-09-09T19:02:46Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nGenerally, trailing space is removed from the string matched by the\nxfuncname patterns.  The exception is when the matched string exceeds the\nlength of the fixed-size buffer that it will be copied in to.  But, a\nstring that exceeds the buffer can still contain trailing space in the\nportion of the string that will be copied into the buffer.  So, simplify\nthis code slightly, and just perform the trailing space removal always.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n xdiff-interface.c |    5 ++---\n 1 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex cd2285d..e1e054e 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -286,9 +286,8 @@ static long ff_regexp(const char *line, long len,\n \tresult = pmatch[i].rm_eo - pmatch[i].rm_so;\n \tif (result > buffer_size)\n \t\tresult = buffer_size;\n-\telse\n-\t\twhile (result > 0 && (isspace(line[result - 1])))\n-\t\t\tresult--;\n+\twhile (result > 0 && (isspace(line[result - 1])))\n+\t\tresult--;\n \tmemcpy(buffer, line, result);\n  fail:\n \tfree(line_buffer);\n-- \n1.7.2.1\n"},{"id":"150389","messageId":"rRj7JpFIk_D_n7-wGkkucC32o5VKAledjObzc1bFHX8SBUchsp6UDflVKjnjY9xKm9Q48ir5iu8@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucFJE0330IabsqMoSzalswHpjT-Z1HCuOFaIgMsaPgIuQSp0KUMW97Wo@cipher.nrlssc.navy.mil","subject":"[PATCH 3/4] t/t4018: test whether the word_regex patterns compile","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:02:47Z","receivedAt":"2010-09-09T19:02:47Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nPreviously (e3bf5e43), a test was added to test whether the builtin\nxfuncname regular expressions could be compiled without error by regcomp.\nLet's do the same for the word_regex patterns.  This should help catch any\ncross-platform incompatibilities that exist between the pattern creator's\nsystem and the various platforms that the test suite is commonly run on.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n t/t4018-diff-funcname.sh |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 61de8a2..620cd02 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -40,6 +40,11 @@ do\n \t\t! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |\n \t\t\tgrep \"fatal\" > /dev/null )\n \t'\n+\ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n+\t\t! ( git diff --no-index --word-diff \\\n+\t\t\tBeer.java Beer-correct.java 2>&1 |\n+\t\t\tgrep \"fatal\" > /dev/null )\n+\t'\n done\n \n test_expect_success 'default behaviour' '\n-- \n1.7.2.1\n"},{"id":"150387","messageId":"rRj7JpFIk_D_n7-wGkkucKFVckLzqXnIAA17N9A643YqyUkXvOqBHrthWFrRRIbouApw4_WLoBE@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucFJE0330IabsqMoSzalswHpjT-Z1HCuOFaIgMsaPgIuQSp0KUMW97Wo@cipher.nrlssc.navy.mil","subject":"[PATCH 4/4] userdiff.c: add builtin fortran regex patterns","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:02:48Z","receivedAt":"2010-09-09T19:02:48Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThis adds fortran xfuncname and wordRegex patterns to the list of builtin\npatterns.  The intention is for the patterns to be appropriate for all\nversions of fortran including 77, 90, 95.  The patterns can be enabled by\nadding the diff=fortran attribute to the .gitattributes file for the\ndesired file glob.\n\nThis also adds a new macro named iPATTERN which is just like the PATTERNS\nmacro except it sets the REG_ICASE flag so that case will be ignored.\n\nThe test code in t4018 and the docs were updated as appropriate.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n Documentation/gitattributes.txt |    2 ++\n t/t4018-diff-funcname.sh        |    2 +-\n userdiff.c                      |   17 +++++++++++++++++\n 3 files changed, 20 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex e5a27d8..fbf507a 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -477,6 +477,8 @@ patterns are available:\n \n - `csharp` suitable for source code in the C# language.\n \n+- `fortran` suitable for source code in the Fortran language.\n+\n - `html` suitable for HTML/XHTML documents.\n \n - `java` suitable for source code in the Java language.\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 620cd02..9a57898 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -32,7 +32,7 @@ EOF\n \n sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n \n-builtin_patterns=\"bibtex cpp csharp html java objc pascal php python ruby tex\"\n+builtin_patterns=\"bibtex cpp csharp fortran html java objc pascal php python ruby tex\"\n for p in $builtin_patterns\n do\n \ttest_expect_success \"builtin $p pattern compiles\" '\ndiff --git a/userdiff.c b/userdiff.c\nindex e552215..953bb5e 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -9,7 +9,23 @@ static int drivers_alloc;\n \n #define PATTERNS(name, pattern, word_regex)\t\t\t\\\n \t{ name, NULL, -1, { pattern, REG_EXTENDED }, word_regex }\n+#define iPATTERN(name, pattern, word_regex)\t\t\t\\\n+\t{ name, NULL, -1, { pattern, REG_EXTENDED | REG_ICASE }, word_regex }\n static struct userdiff_driver builtin_drivers[] = {\n+iPATTERN(\"fortran\",\n+\t \"!^([C*]|[ \\t]*!)\\n\"\n+\t \"!^[ \\t]*MODULE[ \\t]+PROCEDURE[ \\t]\\n\"\n+\t \"^[ \\t]*((END[ \\t]+)?(PROGRAM|MODULE|BLOCK[ \\t]+DATA\"\n+\t\t\"|([^'\\\" \\t]+[ \\t]+)*(SUBROUTINE|FUNCTION))[ \\t]+[A-Z].*)$\",\n+\t /* -- */\n+\t \"[a-zA-Z][a-zA-Z0-9_]*\"\n+\t \"|\\\\.([Ee][Qq]|[Nn][Ee]|[Gg][TtEe]|[Ll][TtEe]|[Tt][Rr][Uu][Ee]|[Ff][Aa][Ll][Ss][Ee]|[Aa][Nn][Dd]|[Oo][Rr]|[Nn]?[Ee][Qq][Vv]|[Nn][Oo][Tt])\\\\.\"\n+\t /* numbers and format statements like 2E14.4, or ES12.6, 9X.\n+\t  * Don't worry about format statements without leading digits since\n+\t  * they would have been matched above as a variable anyway. */\n+\t \"|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?\"\n+\t \"|//|\\\\*\\\\*|::|[/<>=]=\"\n+\t \"|[^[:space:]]|[\\x80-\\xff]+\"),\n PATTERNS(\"html\", \"^[ \\t]*(<[Hh][1-6][ \\t].*>.*)$\",\n \t \"[^<>= \\t]+|[^[:space:]]|[\\x80-\\xff]+\"),\n PATTERNS(\"java\",\n@@ -101,6 +117,7 @@ PATTERNS(\"csharp\",\n { \"default\", NULL, -1, { NULL, 0 } },\n };\n #undef PATTERNS\n+#undef iPATTERN\n \n static struct userdiff_driver driver_true = {\n \t\"diff=true\",\n-- \n1.7.2.1\n"},{"id":"150393","messageId":"20100909191557.GB32508@sigill.intra.peff.net","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucFJE0330IabsqMoSzalswHpjT-Z1HCuOFaIgMsaPgIuQSp0KUMW97Wo@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 1/4] diff.c: call regfree to free memory allocated by regcomp when necessary","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-09-09T19:15:57Z","receivedAt":"2010-09-09T19:15:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 09, 2010 at 02:02:45PM -0500, Brandon Casey wrote:\n\n> @@ -919,7 +919,10 @@ static void free_diff_words_data(struct emit_callback *ecbdata)\n>  \t\tfree (ecbdata->diff_words->minus.orig);\n>  \t\tfree (ecbdata->diff_words->plus.text.ptr);\n>  \t\tfree (ecbdata->diff_words->plus.orig);\n> -\t\tfree(ecbdata->diff_words->word_regex);\n> +\t\tif (ecbdata->diff_words->word_regex) {\n> +\t\t\tregfree(ecbdata->diff_words->word_regex);\n> +\t\t\tfree(ecbdata->diff_words->word_regex);\n> +\t\t}\n\nMakes sense, and closes a leak. But I wonder why word_regex is a pointer\nto malloc'd memory at all? Couldn't it just be an actual regex_t inside\nthe diff_words struct, and we regcomp and regfree it?\n\nI guess maybe having it be NULL is useful. You could check\ndiff_words->o->word_regex, but I suppose that would mean carrying an\nextra variable around through the callchain.\n\n-Peff\n"},{"id":"150396","messageId":"20100909192108.GC32508@sigill.intra.peff.net","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucCda_kClmekHTPEHOGwmU5nYdmNhqqgzF59v609YFx69vv526WtEpyU@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-09-09T19:21:08Z","receivedAt":"2010-09-09T19:21:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 09, 2010 at 02:02:46PM -0500, Brandon Casey wrote:\n\n> From: Brandon Casey <drafnel@gmail.com>\n> \n> Generally, trailing space is removed from the string matched by the\n> xfuncname patterns.  The exception is when the matched string exceeds the\n> length of the fixed-size buffer that it will be copied in to.  But, a\n> string that exceeds the buffer can still contain trailing space in the\n> portion of the string that will be copied into the buffer.  So, simplify\n> this code slightly, and just perform the trailing space removal always.\n\nHrm. So we are cutting off trailing space that might have been\nnon-trailing space in their original string? It is hard to argue that is\nmuch worse than truncating the original string in the first place. But I\nreally wonder whether we should be silently truncating anything, and not\njust dying or somehow handling this better?\n\nIf I understand what is going on (and I'm not sure that I do), are we\nsilently producing bogus word-diffs in the face of really long lines?\n\n-Peff\n"},{"id":"150397","messageId":"20100909192331.GD32508@sigill.intra.peff.net","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucC32o5VKAledjObzc1bFHX8SBUchsp6UDflVKjnjY9xKm9Q48ir5iu8@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 3/4] t/t4018: test whether the word_regex patterns compile","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-09-09T19:23:31Z","receivedAt":"2010-09-09T19:23:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 09, 2010 at 02:02:47PM -0500, Brandon Casey wrote:\n\n> From: Brandon Casey <drafnel@gmail.com>\n> \n> Previously (e3bf5e43), a test was added to test whether the builtin\n> xfuncname regular expressions could be compiled without error by regcomp.\n> Let's do the same for the word_regex patterns.  This should help catch any\n> cross-platform incompatibilities that exist between the pattern creator's\n> system and the various platforms that the test suite is commonly run on.\n\nDefinitely something we should be doing, but one nit:\n\n> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\n> index 61de8a2..620cd02 100755\n> --- a/t/t4018-diff-funcname.sh\n> +++ b/t/t4018-diff-funcname.sh\n> @@ -40,6 +40,11 @@ do\n>  \t\t! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |\n>  \t\t\tgrep \"fatal\" > /dev/null )\n>  \t'\n> +\ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n> +\t\t! ( git diff --no-index --word-diff \\\n> +\t\t\tBeer.java Beer-correct.java 2>&1 |\n> +\t\t\tgrep \"fatal\" > /dev/null )\n> +\t'\n\nWhy the subshell? Shouldn't just testing the pipeline outcome work?\n\n-Peff\n"},{"id":"150398","messageId":"20100909192556.GE32508@sigill.intra.peff.net","threadId":"25058","inReplyTo":"rRj7JpFIk_D_n7-wGkkucKFVckLzqXnIAA17N9A643YqyUkXvOqBHrthWFrRRIbouApw4_WLoBE@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 4/4] userdiff.c: add builtin fortran regex patterns","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-09-09T19:25:56Z","receivedAt":"2010-09-09T19:25:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 09, 2010 at 02:02:48PM -0500, Brandon Casey wrote:\n\n> This adds fortran xfuncname and wordRegex patterns to the list of builtin\n> patterns.  The intention is for the patterns to be appropriate for all\n> versions of fortran including 77, 90, 95.  The patterns can be enabled by\n> adding the diff=fortran attribute to the .gitattributes file for the\n> desired file glob.\n\nThe rest of your series looks reasonable, modulo the comments I posted\nto individual patches. But I won't torture my eyes by trying to actually\nsee how well this matches fortran code, and just assume this is a good\nchange. :)\n\n> This also adds a new macro named iPATTERN which is just like the PATTERNS\n> macro except it sets the REG_ICASE flag so that case will be ignored.\n\nStyle nit: please keep macros all uppercase.\n\n-Peff\n"},{"id":"150399","messageId":"GLdpD2WwjdTYfjYpj9FXho_K_YzeaiL1jdGACbdsXplUz5EPC0X3vA@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"20100909192108.GC32508@sigill.intra.peff.net","subject":"Re: [PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:33:15Z","receivedAt":"2010-09-09T19:33:15Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 09/09/2010 02:21 PM, Jeff King wrote:\n> On Thu, Sep 09, 2010 at 02:02:46PM -0500, Brandon Casey wrote:\n> \n>> From: Brandon Casey <drafnel@gmail.com>\n>>\n>> Generally, trailing space is removed from the string matched by the\n>> xfuncname patterns.  The exception is when the matched string exceeds the\n>> length of the fixed-size buffer that it will be copied in to.  But, a\n>> string that exceeds the buffer can still contain trailing space in the\n>> portion of the string that will be copied into the buffer.  So, simplify\n>> this code slightly, and just perform the trailing space removal always.\n> \n> Hrm. So we are cutting off trailing space that might have been\n> non-trailing space in their original string? It is hard to argue that is\n> much worse than truncating the original string in the first place. But I\n> really wonder whether we should be silently truncating anything, and not\n> just dying or somehow handling this better?\n> \n> If I understand what is going on (and I'm not sure that I do), are we\n> silently producing bogus word-diffs in the face of really long lines?\n\nI don't think this function is used to do the word-diffs.  Unless I'm\nmissing something, ff_regexp is only used to do the funcname matching to\nproduce the hunk header string.  An 80-byte buffer is used to hold\nthat string.  So, the trimming is performed on what is effectively a\ncomment.\n\n-Brandon\n"},{"id":"150400","messageId":"20100909193528.GA1092@sigill.intra.peff.net","threadId":"25058","inReplyTo":"GLdpD2WwjdTYfjYpj9FXho_K_YzeaiL1jdGACbdsXplUz5EPC0X3vA@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-09-09T19:35:28Z","receivedAt":"2010-09-09T19:35:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 09, 2010 at 02:33:15PM -0500, Brandon Casey wrote:\n\n> On 09/09/2010 02:21 PM, Jeff King wrote:\n> > On Thu, Sep 09, 2010 at 02:02:46PM -0500, Brandon Casey wrote:\n> > \n> >> From: Brandon Casey <drafnel@gmail.com>\n> >>\n> >> Generally, trailing space is removed from the string matched by the\n> >> xfuncname patterns.  The exception is when the matched string exceeds the\n> >> length of the fixed-size buffer that it will be copied in to.  But, a\n> >> string that exceeds the buffer can still contain trailing space in the\n> >> portion of the string that will be copied into the buffer.  So, simplify\n> >> this code slightly, and just perform the trailing space removal always.\n> > \n> > Hrm. So we are cutting off trailing space that might have been\n> > non-trailing space in their original string? It is hard to argue that is\n> > much worse than truncating the original string in the first place. But I\n> > really wonder whether we should be silently truncating anything, and not\n> > just dying or somehow handling this better?\n> > \n> > If I understand what is going on (and I'm not sure that I do), are we\n> > silently producing bogus word-diffs in the face of really long lines?\n> \n> I don't think this function is used to do the word-diffs.  Unless I'm\n> missing something, ff_regexp is only used to do the funcname matching to\n> produce the hunk header string.  An 80-byte buffer is used to hold\n> that string.  So, the trimming is performed on what is effectively a\n> comment.\n\nAh, OK, that makes a lot more sense. Then yeah, we should always be\ntrimming trailing whitespace from the result, so your patch is good.\n\n-Peff\n"},{"id":"150402","messageId":"j45XQiddacaKt4L280HjYoaYzAnnDCuR2Yspxw_iRDyU3Y9YkKMxAQ@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"20100909192331.GD32508@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] t/t4018: test whether the word_regex patterns compile","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:39:32Z","receivedAt":"2010-09-09T19:39:32Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 09/09/2010 02:23 PM, Jeff King wrote:\n> On Thu, Sep 09, 2010 at 02:02:47PM -0500, Brandon Casey wrote:\n> \n>> From: Brandon Casey <drafnel@gmail.com>\n>>\n>> Previously (e3bf5e43), a test was added to test whether the builtin\n>> xfuncname regular expressions could be compiled without error by regcomp.\n>> Let's do the same for the word_regex patterns.  This should help catch any\n>> cross-platform incompatibilities that exist between the pattern creator's\n>> system and the various platforms that the test suite is commonly run on.\n> \n> Definitely something we should be doing, but one nit:\n> \n>> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\n>> index 61de8a2..620cd02 100755\n>> --- a/t/t4018-diff-funcname.sh\n>> +++ b/t/t4018-diff-funcname.sh\n>> @@ -40,6 +40,11 @@ do\n>>  \t\t! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |\n>>  \t\t\tgrep \"fatal\" > /dev/null )\n>>  \t'\n>> +\ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n>> +\t\t! ( git diff --no-index --word-diff \\\n>> +\t\t\tBeer.java Beer-correct.java 2>&1 |\n>> +\t\t\tgrep \"fatal\" > /dev/null )\n>> +\t'\n> \n> Why the subshell? Shouldn't just testing the pipeline outcome work?\n\nNotice the similarity between the added lines and the lines just\nabove them?  Where were you when I submitted those? :)\n\nThanks,\n-Brandon\n"},{"id":"150403","messageId":"6F7MXe3ksgGGsKBKvAOQG_pNROYqzpzjQTM_YARyO5d1RK6OVY8oLg@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"20100909192556.GE32508@sigill.intra.peff.net","subject":"Re: [PATCH 4/4] userdiff.c: add builtin fortran regex patterns","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-09T19:41:47Z","receivedAt":"2010-09-09T19:41:47Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On 09/09/2010 02:25 PM, Jeff King wrote:\n> On Thu, Sep 09, 2010 at 02:02:48PM -0500, Brandon Casey wrote:\n> \n>> This adds fortran xfuncname and wordRegex patterns to the list of builtin\n>> patterns.  The intention is for the patterns to be appropriate for all\n>> versions of fortran including 77, 90, 95.  The patterns can be enabled by\n>> adding the diff=fortran attribute to the .gitattributes file for the\n>> desired file glob.\n> \n> The rest of your series looks reasonable, modulo the comments I posted\n> to individual patches. But I won't torture my eyes by trying to actually\n> see how well this matches fortran code, and just assume this is a good\n> change. :)\n\nI can send you some. :)\n\n>> This also adds a new macro named iPATTERN which is just like the PATTERNS\n>> macro except it sets the REG_ICASE flag so that case will be ignored.\n> \n> Style nit: please keep macros all uppercase.\n\nAh, yeah, I guess you're right.\n\n  IPATTERN\n\nThanks,\n-Brandon\n"},{"id":"150408","messageId":"20100909195904.GE1146@sigill.intra.peff.net","threadId":"25058","inReplyTo":"j45XQiddacaKt4L280HjYoaYzAnnDCuR2Yspxw_iRDyU3Y9YkKMxAQ@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 3/4] t/t4018: test whether the word_regex patterns compile","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-09-09T19:59:04Z","receivedAt":"2010-09-09T19:59:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 09, 2010 at 02:39:32PM -0500, Brandon Casey wrote:\n\n> > Definitely something we should be doing, but one nit:\n> > \n> >> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\n> >> index 61de8a2..620cd02 100755\n> >> --- a/t/t4018-diff-funcname.sh\n> >> +++ b/t/t4018-diff-funcname.sh\n> >> @@ -40,6 +40,11 @@ do\n> >>  \t\t! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |\n> >>  \t\t\tgrep \"fatal\" > /dev/null )\n> >>  \t'\n> >> +\ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n> >> +\t\t! ( git diff --no-index --word-diff \\\n> >> +\t\t\tBeer.java Beer-correct.java 2>&1 |\n> >> +\t\t\tgrep \"fatal\" > /dev/null )\n> >> +\t'\n> > \n> > Why the subshell? Shouldn't just testing the pipeline outcome work?\n> \n> Notice the similarity between the added lines and the lines just\n> above them?  Where were you when I submitted those? :)\n\nI did. I almost said \"obviously you copied from above, but that is no\nexcuse\". So let me say it now. :)\n\n-Peff\n"},{"id":"150445","messageId":"rl_Fe3GvwRHMfzdMCqwnBhdCJhaaE21fdWY64ux0ATvXGt5hFA80h-4iMktgWtjAQ_54uD09KJc@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"20100909195904.GE1146@sigill.intra.peff.net","subject":"[PATCH] t/t4018: avoid two unnecessary sub-shell invocations","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-10T16:13:38Z","receivedAt":"2010-09-10T16:13:38Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nThanks again Jeff for your, as always, very valuable review.\n\n\n t/t4018-diff-funcname.sh |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex 620cd02..c8e1937 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -37,13 +37,13 @@ for p in $builtin_patterns\n do\n \ttest_expect_success \"builtin $p pattern compiles\" '\n \t\techo \"*.java diff=$p\" > .gitattributes &&\n-\t\t! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |\n-\t\t\tgrep \"fatal\" > /dev/null )\n+\t\t! { git diff --no-index Beer.java Beer-correct.java 2>&1 |\n+\t\t\tgrep \"fatal\" > /dev/null; }\n \t'\n \ttest_expect_success \"builtin $p wordRegex pattern compiles\" '\n-\t\t! ( git diff --no-index --word-diff \\\n+\t\t! { git diff --no-index --word-diff \\\n \t\t\tBeer.java Beer-correct.java 2>&1 |\n-\t\t\tgrep \"fatal\" > /dev/null )\n+\t\t\tgrep \"fatal\" > /dev/null; }\n \t'\n done\n \n-- \n1.7.2.1\n"},{"id":"150449","messageId":"KtBylmXQLVPyA9SToc1FN6znL4kFcYiYuvLgq017oHm66wTCLQA6Tsi_ja4x5cfwDmaQJs2cGi4@cipher.nrlssc.navy.mil","threadId":"25058","inReplyTo":"6F7MXe3ksgGGsKBKvAOQG_pNROYqzpzjQTM_YARyO5d1RK6OVY8oLg@cipher.nrlssc.navy.mil","subject":"[PATCH 4/4 v2] userdiff.c: add builtin fortran regex patterns","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2010-09-10T16:18:14Z","receivedAt":"2010-09-10T16:18:14Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThis adds fortran xfuncname and wordRegex patterns to the list of builtin\npatterns.  The intention is for the patterns to be appropriate for all\nversions of fortran including 77, 90, 95.  The patterns can be enabled by\nadding the diff=fortran attribute to the .gitattributes file for the\ndesired file glob.\n\nThis also adds a new macro named IPATTERN which is just like the PATTERNS\nmacro except it sets the REG_ICASE flag so that case will be ignored.\n\nThe test code in t4018 and the docs were updated as appropriate.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n\n\nThis version renames the iPATTERN macro to the all uppercase IPATTERN, and\nis a replacement for the version in pu.\n\n\n Documentation/gitattributes.txt |    2 ++\n t/t4018-diff-funcname.sh        |    2 +-\n userdiff.c                      |   17 +++++++++++++++++\n 3 files changed, 20 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex e5a27d8..fbf507a 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -477,6 +477,8 @@ patterns are available:\n \n - `csharp` suitable for source code in the C# language.\n \n+- `fortran` suitable for source code in the Fortran language.\n+\n - `html` suitable for HTML/XHTML documents.\n \n - `java` suitable for source code in the Java language.\ndiff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\nindex c8e1937..0a61b57 100755\n--- a/t/t4018-diff-funcname.sh\n+++ b/t/t4018-diff-funcname.sh\n@@ -32,7 +32,7 @@ EOF\n \n sed 's/beer\\\\/beer,\\\\/' < Beer.java > Beer-correct.java\n \n-builtin_patterns=\"bibtex cpp csharp html java objc pascal php python ruby tex\"\n+builtin_patterns=\"bibtex cpp csharp fortran html java objc pascal php python ruby tex\"\n for p in $builtin_patterns\n do\n \ttest_expect_success \"builtin $p pattern compiles\" '\ndiff --git a/userdiff.c b/userdiff.c\nindex e552215..f9e05b5 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -9,7 +9,23 @@ static int drivers_alloc;\n \n #define PATTERNS(name, pattern, word_regex)\t\t\t\\\n \t{ name, NULL, -1, { pattern, REG_EXTENDED }, word_regex }\n+#define IPATTERN(name, pattern, word_regex)\t\t\t\\\n+\t{ name, NULL, -1, { pattern, REG_EXTENDED | REG_ICASE }, word_regex }\n static struct userdiff_driver builtin_drivers[] = {\n+IPATTERN(\"fortran\",\n+\t \"!^([C*]|[ \\t]*!)\\n\"\n+\t \"!^[ \\t]*MODULE[ \\t]+PROCEDURE[ \\t]\\n\"\n+\t \"^[ \\t]*((END[ \\t]+)?(PROGRAM|MODULE|BLOCK[ \\t]+DATA\"\n+\t\t\"|([^'\\\" \\t]+[ \\t]+)*(SUBROUTINE|FUNCTION))[ \\t]+[A-Z].*)$\",\n+\t /* -- */\n+\t \"[a-zA-Z][a-zA-Z0-9_]*\"\n+\t \"|\\\\.([Ee][Qq]|[Nn][Ee]|[Gg][TtEe]|[Ll][TtEe]|[Tt][Rr][Uu][Ee]|[Ff][Aa][Ll][Ss][Ee]|[Aa][Nn][Dd]|[Oo][Rr]|[Nn]?[Ee][Qq][Vv]|[Nn][Oo][Tt])\\\\.\"\n+\t /* numbers and format statements like 2E14.4, or ES12.6, 9X.\n+\t  * Don't worry about format statements without leading digits since\n+\t  * they would have been matched above as a variable anyway. */\n+\t \"|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?\"\n+\t \"|//|\\\\*\\\\*|::|[/<>=]=\"\n+\t \"|[^[:space:]]|[\\x80-\\xff]+\"),\n PATTERNS(\"html\", \"^[ \\t]*(<[Hh][1-6][ \\t].*>.*)$\",\n \t \"[^<>= \\t]+|[^[:space:]]|[\\x80-\\xff]+\"),\n PATTERNS(\"java\",\n@@ -101,6 +117,7 @@ PATTERNS(\"csharp\",\n { \"default\", NULL, -1, { NULL, 0 } },\n };\n #undef PATTERNS\n+#undef IPATTERN\n \n static struct userdiff_driver driver_true = {\n \t\"diff=true\",\n-- \n1.7.2.1\n"},{"id":"150456","messageId":"AANLkTindP1KiyFjF+7EVhKDciBr-g_cg3jfHCRB0Qfxo@mail.gmail.com","threadId":"25058","inReplyTo":"rl_Fe3GvwRHMfzdMCqwnBhdCJhaaE21fdWY64ux0ATvXGt5hFA80h-4iMktgWtjAQ_54uD09KJc@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] t/t4018: avoid two unnecessary sub-shell invocations","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-10T17:25:56Z","receivedAt":"2010-09-10T17:25:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Sep 10, 2010 at 16:13, Brandon Casey <casey@nrlssc.navy.mil> wrote:\n> From: Brandon Casey <drafnel@gmail.com>\n>\n>\n> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n> ---\n> Thanks again Jeff for your, as always, very valuable review.\n>\n>\n>  t/t4018-diff-funcname.sh |    8 ++++----\n>  1 files changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh\n> index 620cd02..c8e1937 100755\n> --- a/t/t4018-diff-funcname.sh\n> +++ b/t/t4018-diff-funcname.sh\n> @@ -37,13 +37,13 @@ for p in $builtin_patterns\n>  do\n>        test_expect_success \"builtin $p pattern compiles\" '\n>                echo \"*.java diff=$p\" > .gitattributes &&\n> -               ! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |\n> -                       grep \"fatal\" > /dev/null )\n> +               ! { git diff --no-index Beer.java Beer-correct.java 2>&1 |\n> +                       grep \"fatal\" > /dev/null; }\n>        '\n>        test_expect_success \"builtin $p wordRegex pattern compiles\" '\n> -               ! ( git diff --no-index --word-diff \\\n> +               ! { git diff --no-index --word-diff \\\n>                        Beer.java Beer-correct.java 2>&1 |\n> -                       grep \"fatal\" > /dev/null )\n> +                       grep \"fatal\" > /dev/null; }\n>        '\n>  done\n\nFWIW (but don't think you need to change 'em) I'd do these as\n(untested):\n\ntest_expect_success \"builtin $p wordRegex pattern compiles\" '\n    git diff --no-index --word-diff >out 2>err &&\n    # Or whatever out should be..\n    ! test -s out &&\n    grep fatal err\n'\n\nIt's much easier to debug tests that use intermediate files with\n--immediate --debug when they break, because you can just check out\nwhat out and err contain. You can't do that if the output was\nforgotten in some pipe.\n\nMaybe I should add a bit into t/README about this ...\n"}]}