threads / patch / 25058

patch, 4 partsuserdiff.c: add builtin fortran regex patterns

Subject: [PATCH 4/4] userdiff.c: add builtin fortran regex patterns

## tl;dr

16 messages between Sep 9, 2010 and Sep 10, 2010. Diffs are folded; open one to read it.

replies: 15people: 3as markdown or json

Brandon Casey· Sep 9, 2010, 19:02 UTC · lore

[PATCH 1/4] diff.c: call regfree to free memory allocated by regcomp when necessary

From: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 diff.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Show changes to diff.c +4 −1
diff --git a/diff.c b/diff.c
index 144f2aa..9a5c77c 100644
--- a/diff.c
+++ b/diff.c
@@ -919,7 +919,10 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
 		free (ecbdata->diff_words->minus.orig);
 		free (ecbdata->diff_words->plus.text.ptr);
 		free (ecbdata->diff_words->plus.orig);
-		free(ecbdata->diff_words->word_regex);
+		if (ecbdata->diff_words->word_regex) {
+			regfree(ecbdata->diff_words->word_regex);
+			free(ecbdata->diff_words->word_regex);
+		}
 		free(ecbdata->diff_words);
 		ecbdata->diff_words = NULL;
 	}

-- 
1.7.2.1
Brandon Casey· Sep 9, 2010, 19:02 UTC · re: Brandon Casey · lore

[PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches

From: Brandon Casey <drafnel@gmail.com>

Generally, trailing space is removed from the string matched by the xfuncname patterns. The exception is when the matched string exceeds the length of the fixed-size buffer that it will be copied in to. But, a string that exceeds the buffer can still contain trailing space in the portion of the string that will be copied into the buffer. So, simplify this code slightly, and just perform the trailing space removal always.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 xdiff-interface.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)
Show changes to xdiff-interface.c +2 −3
diff --git a/xdiff-interface.c b/xdiff-interface.c
index cd2285d..e1e054e 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -286,9 +286,8 @@ static long ff_regexp(const char *line, long len,
 	result = pmatch[i].rm_eo - pmatch[i].rm_so;
 	if (result > buffer_size)
 		result = buffer_size;
-	else
-		while (result > 0 && (isspace(line[result - 1])))
-			result--;
+	while (result > 0 && (isspace(line[result - 1])))
+		result--;
 	memcpy(buffer, line, result);
  fail:
 	free(line_buffer);

-- 
1.7.2.1
Jeff King· Sep 9, 2010, 19:21 UTC · re: Brandon Casey · lore

Re: [PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches

On Thu, Sep 09, 2010 at 02:02:46PM -0500, Brandon Casey wrote:
Show 8 quoted lines
> From: Brandon Casey <drafnel@gmail.com>
> 
> Generally, trailing space is removed from the string matched by the
> xfuncname patterns.  The exception is when the matched string exceeds the
> length of the fixed-size buffer that it will be copied in to.  But, a
> string that exceeds the buffer can still contain trailing space in the
> portion of the string that will be copied into the buffer.  So, simplify
> this code slightly, and just perform the trailing space removal always.

Hrm. So we are cutting off trailing space that might have been non-trailing space in their original string? It is hard to argue that is much worse than truncating the original string in the first place. But I really wonder whether we should be silently truncating anything, and not just dying or somehow handling this better?

If I understand what is going on (and I'm not sure that I do), are we silently producing bogus word-diffs in the face of really long lines?

-Peff
Brandon Casey· Sep 9, 2010, 19:33 UTC · re: Jeff King · lore

Re: [PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches

On 09/09/2010 02:21 PM, Jeff King wrote:
Show 19 quoted lines
> On Thu, Sep 09, 2010 at 02:02:46PM -0500, Brandon Casey wrote:
> 
>> From: Brandon Casey <drafnel@gmail.com>
>>
>> Generally, trailing space is removed from the string matched by the
>> xfuncname patterns.  The exception is when the matched string exceeds the
>> length of the fixed-size buffer that it will be copied in to.  But, a
>> string that exceeds the buffer can still contain trailing space in the
>> portion of the string that will be copied into the buffer.  So, simplify
>> this code slightly, and just perform the trailing space removal always.
> 
> Hrm. So we are cutting off trailing space that might have been
> non-trailing space in their original string? It is hard to argue that is
> much worse than truncating the original string in the first place. But I
> really wonder whether we should be silently truncating anything, and not
> just dying or somehow handling this better?
> 
> If I understand what is going on (and I'm not sure that I do), are we
> silently producing bogus word-diffs in the face of really long lines?

I don't think this function is used to do the word-diffs. Unless I'm missing something, ff_regexp is only used to do the funcname matching to produce the hunk header string. An 80-byte buffer is used to hold that string. So, the trimming is performed on what is effectively a comment.

-Brandon
Jeff King· Sep 9, 2010, 19:35 UTC · re: Brandon Casey · lore

Re: [PATCH 2/4] xdiff-interface.c: always trim trailing space from xfuncname matches

On Thu, Sep 09, 2010 at 02:33:15PM -0500, Brandon Casey wrote:
Show 26 quoted lines
> On 09/09/2010 02:21 PM, Jeff King wrote:
> > On Thu, Sep 09, 2010 at 02:02:46PM -0500, Brandon Casey wrote:
> > 
> >> From: Brandon Casey <drafnel@gmail.com>
> >>
> >> Generally, trailing space is removed from the string matched by the
> >> xfuncname patterns.  The exception is when the matched string exceeds the
> >> length of the fixed-size buffer that it will be copied in to.  But, a
> >> string that exceeds the buffer can still contain trailing space in the
> >> portion of the string that will be copied into the buffer.  So, simplify
> >> this code slightly, and just perform the trailing space removal always.
> > 
> > Hrm. So we are cutting off trailing space that might have been
> > non-trailing space in their original string? It is hard to argue that is
> > much worse than truncating the original string in the first place. But I
> > really wonder whether we should be silently truncating anything, and not
> > just dying or somehow handling this better?
> > 
> > If I understand what is going on (and I'm not sure that I do), are we
> > silently producing bogus word-diffs in the face of really long lines?
> 
> I don't think this function is used to do the word-diffs.  Unless I'm
> missing something, ff_regexp is only used to do the funcname matching to
> produce the hunk header string.  An 80-byte buffer is used to hold
> that string.  So, the trimming is performed on what is effectively a
> comment.

Ah, OK, that makes a lot more sense. Then yeah, we should always be trimming trailing whitespace from the result, so your patch is good.

-Peff
Brandon Casey· Sep 9, 2010, 19:02 UTC · re: Brandon Casey · lore

[PATCH 3/4] t/t4018: test whether the word_regex patterns compile

From: Brandon Casey <drafnel@gmail.com>

Previously (e3bf5e43), a test was added to test whether the builtin xfuncname regular expressions could be compiled without error by regcomp. Let's do the same for the word_regex patterns. This should help catch any cross-platform incompatibilities that exist between the pattern creator's system and the various platforms that the test suite is commonly run on.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t4018-diff-funcname.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Show changes to t/t4018-diff-funcname.sh +5 −0
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 61de8a2..620cd02 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -40,6 +40,11 @@ do
 		! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
 			grep "fatal" > /dev/null )
 	'
+	test_expect_success "builtin $p wordRegex pattern compiles" '
+		! ( git diff --no-index --word-diff \
+			Beer.java Beer-correct.java 2>&1 |
+			grep "fatal" > /dev/null )
+	'
 done
 
 test_expect_success 'default behaviour' '

-- 
1.7.2.1
Jeff King· Sep 9, 2010, 19:23 UTC · re: Brandon Casey · lore

Re: [PATCH 3/4] t/t4018: test whether the word_regex patterns compile

On Thu, Sep 09, 2010 at 02:02:47PM -0500, Brandon Casey wrote:
Show 7 quoted lines
> From: Brandon Casey <drafnel@gmail.com>
> 
> Previously (e3bf5e43), a test was added to test whether the builtin
> xfuncname regular expressions could be compiled without error by regcomp.
> Let's do the same for the word_regex patterns.  This should help catch any
> cross-platform incompatibilities that exist between the pattern creator's
> system and the various platforms that the test suite is commonly run on.
Definitely something we should be doing, but one nit:
Show 13 quoted lines
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index 61de8a2..620cd02 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -40,6 +40,11 @@ do
>  		! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
>  			grep "fatal" > /dev/null )
>  	'
> +	test_expect_success "builtin $p wordRegex pattern compiles" '
> +		! ( git diff --no-index --word-diff \
> +			Beer.java Beer-correct.java 2>&1 |
> +			grep "fatal" > /dev/null )
> +	'
Why the subshell? Shouldn't just testing the pipeline outcome work?
-Peff
Brandon Casey· Sep 9, 2010, 19:39 UTC · re: Jeff King · lore

Re: [PATCH 3/4] t/t4018: test whether the word_regex patterns compile

On 09/09/2010 02:23 PM, Jeff King wrote:
Show 27 quoted lines
> On Thu, Sep 09, 2010 at 02:02:47PM -0500, Brandon Casey wrote:
> 
>> From: Brandon Casey <drafnel@gmail.com>
>>
>> Previously (e3bf5e43), a test was added to test whether the builtin
>> xfuncname regular expressions could be compiled without error by regcomp.
>> Let's do the same for the word_regex patterns.  This should help catch any
>> cross-platform incompatibilities that exist between the pattern creator's
>> system and the various platforms that the test suite is commonly run on.
> 
> Definitely something we should be doing, but one nit:
> 
>> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
>> index 61de8a2..620cd02 100755
>> --- a/t/t4018-diff-funcname.sh
>> +++ b/t/t4018-diff-funcname.sh
>> @@ -40,6 +40,11 @@ do
>>  		! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
>>  			grep "fatal" > /dev/null )
>>  	'
>> +	test_expect_success "builtin $p wordRegex pattern compiles" '
>> +		! ( git diff --no-index --word-diff \
>> +			Beer.java Beer-correct.java 2>&1 |
>> +			grep "fatal" > /dev/null )
>> +	'
> 
> Why the subshell? Shouldn't just testing the pipeline outcome work?

Notice the similarity between the added lines and the lines just above them? Where were you when I submitted those? :)

Thanks, -Brandon

Jeff King· Sep 9, 2010, 19:59 UTC · re: Brandon Casey · lore

Re: [PATCH 3/4] t/t4018: test whether the word_regex patterns compile

On Thu, Sep 09, 2010 at 02:39:32PM -0500, Brandon Casey wrote:
Show 20 quoted lines
> > Definitely something we should be doing, but one nit:
> > 
> >> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> >> index 61de8a2..620cd02 100755
> >> --- a/t/t4018-diff-funcname.sh
> >> +++ b/t/t4018-diff-funcname.sh
> >> @@ -40,6 +40,11 @@ do
> >>  		! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
> >>  			grep "fatal" > /dev/null )
> >>  	'
> >> +	test_expect_success "builtin $p wordRegex pattern compiles" '
> >> +		! ( git diff --no-index --word-diff \
> >> +			Beer.java Beer-correct.java 2>&1 |
> >> +			grep "fatal" > /dev/null )
> >> +	'
> > 
> > Why the subshell? Shouldn't just testing the pipeline outcome work?
> 
> Notice the similarity between the added lines and the lines just
> above them?  Where were you when I submitted those? :)

I did. I almost said "obviously you copied from above, but that is no excuse". So let me say it now. :)

-Peff
Brandon Casey· Sep 10, 2010, 16:13 UTC · re: Jeff King · lore

[PATCH] t/t4018: avoid two unnecessary sub-shell invocations

From: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
Thanks again Jeff for your, as always, very valuable review.
 t/t4018-diff-funcname.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to t/t4018-diff-funcname.sh +4 −4
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 620cd02..c8e1937 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -37,13 +37,13 @@ for p in $builtin_patterns
 do
 	test_expect_success "builtin $p pattern compiles" '
 		echo "*.java diff=$p" > .gitattributes &&
-		! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
-			grep "fatal" > /dev/null )
+		! { git diff --no-index Beer.java Beer-correct.java 2>&1 |
+			grep "fatal" > /dev/null; }
 	'
 	test_expect_success "builtin $p wordRegex pattern compiles" '
-		! ( git diff --no-index --word-diff \
+		! { git diff --no-index --word-diff \
 			Beer.java Beer-correct.java 2>&1 |
-			grep "fatal" > /dev/null )
+			grep "fatal" > /dev/null; }
 	'
 done
 

-- 
1.7.2.1
Ævar Arnfjörð Bjarmason· Sep 10, 2010, 17:25 UTC · re: Brandon Casey · lore

Re: [PATCH] t/t4018: avoid two unnecessary sub-shell invocations

On Fri, Sep 10, 2010 at 16:13, Brandon Casey <casey@nrlssc.navy.mil> wrote:
Show 32 quoted lines
> From: Brandon Casey <drafnel@gmail.com>
>
>
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
> Thanks again Jeff for your, as always, very valuable review.
>
>
>  t/t4018-diff-funcname.sh |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index 620cd02..c8e1937 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -37,13 +37,13 @@ for p in $builtin_patterns
>  do
>        test_expect_success "builtin $p pattern compiles" '
>                echo "*.java diff=$p" > .gitattributes &&
> -               ! ( git diff --no-index Beer.java Beer-correct.java 2>&1 |
> -                       grep "fatal" > /dev/null )
> +               ! { git diff --no-index Beer.java Beer-correct.java 2>&1 |
> +                       grep "fatal" > /dev/null; }
>        '
>        test_expect_success "builtin $p wordRegex pattern compiles" '
> -               ! ( git diff --no-index --word-diff \
> +               ! { git diff --no-index --word-diff \
>                        Beer.java Beer-correct.java 2>&1 |
> -                       grep "fatal" > /dev/null )
> +                       grep "fatal" > /dev/null; }
>        '
>  done

FWIW (but don't think you need to change 'em) I'd do these as (untested):

test_expect_success "builtin $p wordRegex pattern compiles" '
    git diff --no-index --word-diff >out 2>err &&
    # Or whatever out should be..
    ! test -s out &&
    grep fatal err
'

It's much easier to debug tests that use intermediate files with --immediate --debug when they break, because you can just check out what out and err contain. You can't do that if the output was forgotten in some pipe.

Maybe I should add a bit into t/README about this ...
Brandon Casey· Sep 9, 2010, 19:02 UTC · re: Brandon Casey · lore
From: Brandon Casey <drafnel@gmail.com>

This adds fortran xfuncname and wordRegex patterns to the list of builtin patterns. The intention is for the patterns to be appropriate for all versions of fortran including 77, 90, 95. The patterns can be enabled by adding the diff=fortran attribute to the .gitattributes file for the desired file glob.

This also adds a new macro named iPATTERN which is just like the PATTERNS macro except it sets the REG_ICASE flag so that case will be ignored.

The test code in t4018 and the docs were updated as appropriate.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 Documentation/gitattributes.txt |    2 ++
 t/t4018-diff-funcname.sh        |    2 +-
 userdiff.c                      |   17 +++++++++++++++++
 3 files changed, 20 insertions(+), 1 deletions(-)
Show changes to 3 files +20 −1

Documentation/gitattributes.txt, t/t4018-diff-funcname.sh, userdiff.c

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e5a27d8..fbf507a 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -477,6 +477,8 @@ patterns are available:
 
 - `csharp` suitable for source code in the C# language.
 
+- `fortran` suitable for source code in the Fortran language.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 620cd02..9a57898 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -32,7 +32,7 @@ EOF
 
 sed 's/beer\\/beer,\\/' < Beer.java > Beer-correct.java
 
-builtin_patterns="bibtex cpp csharp html java objc pascal php python ruby tex"
+builtin_patterns="bibtex cpp csharp fortran html java objc pascal php python ruby tex"
 for p in $builtin_patterns
 do
 	test_expect_success "builtin $p pattern compiles" '
diff --git a/userdiff.c b/userdiff.c
index e552215..953bb5e 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -9,7 +9,23 @@ static int drivers_alloc;
 
 #define PATTERNS(name, pattern, word_regex)			\
 	{ name, NULL, -1, { pattern, REG_EXTENDED }, word_regex }
+#define iPATTERN(name, pattern, word_regex)			\
+	{ name, NULL, -1, { pattern, REG_EXTENDED | REG_ICASE }, word_regex }
 static struct userdiff_driver builtin_drivers[] = {
+iPATTERN("fortran",
+	 "!^([C*]|[ \t]*!)\n"
+	 "!^[ \t]*MODULE[ \t]+PROCEDURE[ \t]\n"
+	 "^[ \t]*((END[ \t]+)?(PROGRAM|MODULE|BLOCK[ \t]+DATA"
+		"|([^'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
+	 /* -- */
+	 "[a-zA-Z][a-zA-Z0-9_]*"
+	 "|\\.([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])\\."
+	 /* numbers and format statements like 2E14.4, or ES12.6, 9X.
+	  * Don't worry about format statements without leading digits since
+	  * they would have been matched above as a variable anyway. */
+	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
+	 "|//|\\*\\*|::|[/<>=]="
+	 "|[^[:space:]]|[\x80-\xff]+"),
 PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
 	 "[^<>= \t]+|[^[:space:]]|[\x80-\xff]+"),
 PATTERNS("java",
@@ -101,6 +117,7 @@ PATTERNS("csharp",
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
+#undef iPATTERN
 
 static struct userdiff_driver driver_true = {
 	"diff=true",

-- 
1.7.2.1
Jeff King· Sep 9, 2010, 19:25 UTC · re: Brandon Casey · lore

Re: [PATCH 4/4] userdiff.c: add builtin fortran regex patterns

On Thu, Sep 09, 2010 at 02:02:48PM -0500, Brandon Casey wrote:
Show 5 quoted lines
> This adds fortran xfuncname and wordRegex patterns to the list of builtin
> patterns.  The intention is for the patterns to be appropriate for all
> versions of fortran including 77, 90, 95.  The patterns can be enabled by
> adding the diff=fortran attribute to the .gitattributes file for the
> desired file glob.

The rest of your series looks reasonable, modulo the comments I posted to individual patches. But I won't torture my eyes by trying to actually see how well this matches fortran code, and just assume this is a good change. :)

> This also adds a new macro named iPATTERN which is just like the PATTERNS
> macro except it sets the REG_ICASE flag so that case will be ignored.
Style nit: please keep macros all uppercase.
-Peff
Brandon Casey· Sep 9, 2010, 19:41 UTC · re: Jeff King · lore

Re: [PATCH 4/4] userdiff.c: add builtin fortran regex patterns

On 09/09/2010 02:25 PM, Jeff King wrote:
Show 12 quoted lines
> On Thu, Sep 09, 2010 at 02:02:48PM -0500, Brandon Casey wrote:
> 
>> This adds fortran xfuncname and wordRegex patterns to the list of builtin
>> patterns.  The intention is for the patterns to be appropriate for all
>> versions of fortran including 77, 90, 95.  The patterns can be enabled by
>> adding the diff=fortran attribute to the .gitattributes file for the
>> desired file glob.
> 
> The rest of your series looks reasonable, modulo the comments I posted
> to individual patches. But I won't torture my eyes by trying to actually
> see how well this matches fortran code, and just assume this is a good
> change. :)
I can send you some. :)
>> This also adds a new macro named iPATTERN which is just like the PATTERNS
>> macro except it sets the REG_ICASE flag so that case will be ignored.
> 
> Style nit: please keep macros all uppercase.
Ah, yeah, I guess you're right.
  IPATTERN

Thanks, -Brandon

Brandon Casey· Sep 10, 2010, 16:18 UTC · re: Brandon Casey · lore

[PATCH 4/4 v2] userdiff.c: add builtin fortran regex patterns

From: Brandon Casey <drafnel@gmail.com>

This adds fortran xfuncname and wordRegex patterns to the list of builtin patterns. The intention is for the patterns to be appropriate for all versions of fortran including 77, 90, 95. The patterns can be enabled by adding the diff=fortran attribute to the .gitattributes file for the desired file glob.

This also adds a new macro named IPATTERN which is just like the PATTERNS macro except it sets the REG_ICASE flag so that case will be ignored.

The test code in t4018 and the docs were updated as appropriate.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---

This version renames the iPATTERN macro to the all uppercase IPATTERN, and is a replacement for the version in pu.

 Documentation/gitattributes.txt |    2 ++
 t/t4018-diff-funcname.sh        |    2 +-
 userdiff.c                      |   17 +++++++++++++++++
 3 files changed, 20 insertions(+), 1 deletions(-)
Show changes to 3 files +20 −1

Documentation/gitattributes.txt, t/t4018-diff-funcname.sh, userdiff.c

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e5a27d8..fbf507a 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -477,6 +477,8 @@ patterns are available:
 
 - `csharp` suitable for source code in the C# language.
 
+- `fortran` suitable for source code in the Fortran language.
+
 - `html` suitable for HTML/XHTML documents.
 
 - `java` suitable for source code in the Java language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index c8e1937..0a61b57 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -32,7 +32,7 @@ EOF
 
 sed 's/beer\\/beer,\\/' < Beer.java > Beer-correct.java
 
-builtin_patterns="bibtex cpp csharp html java objc pascal php python ruby tex"
+builtin_patterns="bibtex cpp csharp fortran html java objc pascal php python ruby tex"
 for p in $builtin_patterns
 do
 	test_expect_success "builtin $p pattern compiles" '
diff --git a/userdiff.c b/userdiff.c
index e552215..f9e05b5 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -9,7 +9,23 @@ static int drivers_alloc;
 
 #define PATTERNS(name, pattern, word_regex)			\
 	{ name, NULL, -1, { pattern, REG_EXTENDED }, word_regex }
+#define IPATTERN(name, pattern, word_regex)			\
+	{ name, NULL, -1, { pattern, REG_EXTENDED | REG_ICASE }, word_regex }
 static struct userdiff_driver builtin_drivers[] = {
+IPATTERN("fortran",
+	 "!^([C*]|[ \t]*!)\n"
+	 "!^[ \t]*MODULE[ \t]+PROCEDURE[ \t]\n"
+	 "^[ \t]*((END[ \t]+)?(PROGRAM|MODULE|BLOCK[ \t]+DATA"
+		"|([^'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
+	 /* -- */
+	 "[a-zA-Z][a-zA-Z0-9_]*"
+	 "|\\.([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])\\."
+	 /* numbers and format statements like 2E14.4, or ES12.6, 9X.
+	  * Don't worry about format statements without leading digits since
+	  * they would have been matched above as a variable anyway. */
+	 "|[-+]?[0-9.]+([AaIiDdEeFfLlTtXx][Ss]?[-+]?[0-9.]*)?(_[a-zA-Z0-9][a-zA-Z0-9_]*)?"
+	 "|//|\\*\\*|::|[/<>=]="
+	 "|[^[:space:]]|[\x80-\xff]+"),
 PATTERNS("html", "^[ \t]*(<[Hh][1-6][ \t].*>.*)$",
 	 "[^<>= \t]+|[^[:space:]]|[\x80-\xff]+"),
 PATTERNS("java",
@@ -101,6 +117,7 @@ PATTERNS("csharp",
 { "default", NULL, -1, { NULL, 0 } },
 };
 #undef PATTERNS
+#undef IPATTERN
 
 static struct userdiff_driver driver_true = {
 	"diff=true",

-- 
1.7.2.1
Jeff King· Sep 9, 2010, 19:15 UTC · re: Brandon Casey · lore

Re: [PATCH 1/4] diff.c: call regfree to free memory allocated by regcomp when necessary

On Thu, Sep 09, 2010 at 02:02:45PM -0500, Brandon Casey wrote:
Show 9 quoted lines
> @@ -919,7 +919,10 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
>  		free (ecbdata->diff_words->minus.orig);
>  		free (ecbdata->diff_words->plus.text.ptr);
>  		free (ecbdata->diff_words->plus.orig);
> -		free(ecbdata->diff_words->word_regex);
> +		if (ecbdata->diff_words->word_regex) {
> +			regfree(ecbdata->diff_words->word_regex);
> +			free(ecbdata->diff_words->word_regex);
> +		}

Makes sense, and closes a leak. But I wonder why word_regex is a pointer to malloc'd memory at all? Couldn't it just be an actual regex_t inside the diff_words struct, and we regcomp and regfree it?

I guess maybe having it be NULL is useful. You could check diff_words->o->word_regex, but I suppose that would mean carrying an extra variable around through the callchain.

-Peff

← back to recent threads