git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] userdiff: support Bash
@ 2020-10-20  7:10 Victor Engmark
  2020-10-20 23:39 ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Victor Engmark @ 2020-10-20  7:10 UTC (permalink / raw)
  To: git

Supports POSIX, bashism and mixed function declarations, all four compound
command types, trailing comments and mixed whitespace.

Uses the POSIX.1-2017 definition of allowed characters in names
<https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_235>
since actual allowed characters in Bash function names are locale
dependent <https://unix.stackexchange.com/a/245336/3645>.

Uses the default `IFS` characters to define words.

Signed-off-by: Victor Engmark <victor@engmark.name>
---
 Documentation/gitattributes.txt     | 2 ++
 t/t4018-diff-funcname.sh            | 1 +
 t/t4018/bash-arithmetic-function    | 4 ++++
 t/t4018/bash-bashism-style-function | 4 ++++
 t/t4018/bash-conditional-function   | 4 ++++
 t/t4018/bash-mixed-style-function   | 4 ++++
 t/t4018/bash-other-characters       | 4 ++++
 t/t4018/bash-posix-style-function   | 4 ++++
 t/t4018/bash-subshell-function      | 4 ++++
 t/t4018/bash-trailing-comment       | 4 ++++
 t/t4018/bash-whitespace             | 4 ++++
 userdiff.c                          | 6 ++++++
 12 files changed, 45 insertions(+)
 create mode 100644 t/t4018/bash-arithmetic-function
 create mode 100644 t/t4018/bash-bashism-style-function
 create mode 100644 t/t4018/bash-conditional-function
 create mode 100644 t/t4018/bash-mixed-style-function
 create mode 100644 t/t4018/bash-other-characters
 create mode 100644 t/t4018/bash-posix-style-function
 create mode 100644 t/t4018/bash-subshell-function
 create mode 100644 t/t4018/bash-trailing-comment
 create mode 100644 t/t4018/bash-whitespace

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2d0a03715b..8a15ff6bdf 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -802,6 +802,8 @@ patterns are available:
 
 - `ada` suitable for source code in the Ada language.
 
+- `bash` suitable for source code in the Bourne-Again SHell language.
+
 - `bibtex` suitable for files with BibTeX coded references.
 
 - `cpp` suitable for source code in the C and C++ languages.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 9d07797579..9675bc17db 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -27,6 +27,7 @@ test_expect_success 'setup' '
 
 diffpatterns="
 	ada
+	bash
 	bibtex
 	cpp
 	csharp
diff --git a/t/t4018/bash-arithmetic-function b/t/t4018/bash-arithmetic-function
new file mode 100644
index 0000000000..c0b276cb50
--- /dev/null
+++ b/t/t4018/bash-arithmetic-function
@@ -0,0 +1,4 @@
+RIGHT() ((
+
+    ChangeMe = "$x" + "$y"
+))
diff --git a/t/t4018/bash-bashism-style-function b/t/t4018/bash-bashism-style-function
new file mode 100644
index 0000000000..f1de4fa831
--- /dev/null
+++ b/t/t4018/bash-bashism-style-function
@@ -0,0 +1,4 @@
+function RIGHT {
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-conditional-function b/t/t4018/bash-conditional-function
new file mode 100644
index 0000000000..c5949e829b
--- /dev/null
+++ b/t/t4018/bash-conditional-function
@@ -0,0 +1,4 @@
+RIGHT() [[ \
+
+    "$a" > "$ChangeMe"
+]]
diff --git a/t/t4018/bash-mixed-style-function b/t/t4018/bash-mixed-style-function
new file mode 100644
index 0000000000..555f9b2466
--- /dev/null
+++ b/t/t4018/bash-mixed-style-function
@@ -0,0 +1,4 @@
+function RIGHT() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-other-characters b/t/t4018/bash-other-characters
new file mode 100644
index 0000000000..a3f390d525
--- /dev/null
+++ b/t/t4018/bash-other-characters
@@ -0,0 +1,4 @@
+_RIGHT_0n() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-function b/t/t4018/bash-posix-style-function
new file mode 100644
index 0000000000..a4d144856e
--- /dev/null
+++ b/t/t4018/bash-posix-style-function
@@ -0,0 +1,4 @@
+RIGHT() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-subshell-function b/t/t4018/bash-subshell-function
new file mode 100644
index 0000000000..80baa09484
--- /dev/null
+++ b/t/t4018/bash-subshell-function
@@ -0,0 +1,4 @@
+RIGHT() (
+
+    ChangeMe=2
+)
diff --git a/t/t4018/bash-trailing-comment b/t/t4018/bash-trailing-comment
new file mode 100644
index 0000000000..f1edbeda31
--- /dev/null
+++ b/t/t4018/bash-trailing-comment
@@ -0,0 +1,4 @@
+RIGHT() { # Comment
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-whitespace b/t/t4018/bash-whitespace
new file mode 100644
index 0000000000..62e7d1f25c
--- /dev/null
+++ b/t/t4018/bash-whitespace
@@ -0,0 +1,4 @@
+	 function 	RIGHT() 	{
+
+	    ChangeMe
+	 }
diff --git a/userdiff.c b/userdiff.c
index fde02f225b..9de0497007 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -23,6 +23,12 @@ IPATTERN("ada",
 	 "[a-zA-Z][a-zA-Z0-9_]*"
 	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
 	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
+PATTERNS("bash",
+	 /* POSIX & bashism form; all four compound command types */
+	 "^[ \t]*((function[ \t]*)?[a-zA-Z_][a-zA-Z0-9_]*(\\(\\))?[ \t]*(\\{|\\(\\(?|\\[\\[))",
+	 /* -- */
+	 /* Characters not in the default $IFS value */
+	 "[^ \t]+"),
 PATTERNS("dts",
 	 "!;\n"
 	 "!=\n"


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH] userdiff: support Bash
  2020-10-20  7:10 [PATCH] userdiff: support Bash Victor Engmark
@ 2020-10-20 23:39 ` Junio C Hamano
  2020-10-21  3:00   ` [PATCH v2] " Victor Engmark
  0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2020-10-20 23:39 UTC (permalink / raw)
  To: Victor Engmark; +Cc: git

Victor Engmark <victor@engmark.name> writes:

It's a bit troubling not to see the most basic form, i.e.

	RIGHT () {
		: ChangeMe
	}

in the set of tests.

> diff --git a/t/t4018/bash-trailing-comment b/t/t4018/bash-trailing-comment
> new file mode 100644
> index 0000000000..f1edbeda31
> --- /dev/null
> +++ b/t/t4018/bash-trailing-comment
> @@ -0,0 +1,4 @@
> +RIGHT() { # Comment
> +
> +    ChangeMe
> +}

This is the closest, but it tests "# with comment" at the same time.

> diff --git a/userdiff.c b/userdiff.c
> index fde02f225b..9de0497007 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -23,6 +23,12 @@ IPATTERN("ada",
>  	 "[a-zA-Z][a-zA-Z0-9_]*"
>  	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
>  	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
> +PATTERNS("bash",
> +	 /* POSIX & bashism form; all four compound command types */
> +	 "^[ \t]*((function[ \t]*)?[a-zA-Z_][a-zA-Z0-9_]*(\\(\\))?[ \t]*(\\{|\\(\\(?|\\[\\[))",

We allow an optional leading indent, an optional noiseword
"function" plus a run of whitespaces, and then an identifier
followed by "line noise".

The pattern makes the run of whitespaces after "function" entirely
optional, which makes "functionabc" be taken as a single identifier
without noiseword "function", but it's not like we are parsing and
painting only the identifer in a color different from the keyword,
so it is OK.  Using [ \t]+ instead of [ \t]* after "function" would
probably make the result more clear, even though it does not make a
practical difference.

Then the "line noise" part.  I am not sure if I follow this part
correctly:

	"(\\(\\))?[ \t]*(\\{|\\(\\(?|\\[\\[)"

is what follows the identifier that is the function name.  We may
have 0 or 1 "()", followed by an optional run of whitespaces.  And
then one of '{', '(', '((', '[[' would come.

Did I read it correctly?

Even though it may be unusual to write open and close parentheses
not directly next to each other, or with a space after the name of
the function, i.e.

	RIGHT ( ) {

would also be a valid header, no?  The way I read the pattern in the
above, it should not hit, as the pattern does not allow anything but
"()" as the "the previous identifier is the name of the function we
are defining" sign, and it does not allow any whitespace between the
identifier and "()".

But it does, and the reason why this most basic form happens to work
is because it relies on the support for "bash-ism" forms.  Even if
the "()" after identifier is missing, you'll match as long as the
identifier is followed by an optional run of whitespace and then an
open paren, brace or bracket:

	"[ \t]*(\\{|\\(\\(?|\\[\\[)"

And I do not like code or pattern that happens to appear to work by
accident.  Can we tighten it a bit?

	"^[ \t]*"		     /* (op) leading indent */
	"("			     /* here comes the whole thing */
	"(function[ \t]+)?"	     /* (op) noiseword "function" */
	"[a-zA-Z_][a-zA-Z0-9_]*"     /* identifier - function name */
	"[ \t]*(\\([ \t]*\\))?"      /* (op) start of func () */
	"[ \t]*(\\{|\\(\\(?|\\[\\[)" /* various "opening" of body */
	")",

is my attempt to break it down to make it readable and more correct.

> +	 /* -- */
> +	 /* Characters not in the default $IFS value */
> +	 "[^ \t]+"),
>  PATTERNS("dts",
>  	 "!;\n"
>  	 "!=\n"

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v2] userdiff: support Bash
  2020-10-20 23:39 ` Junio C Hamano
@ 2020-10-21  3:00   ` Victor Engmark
  2020-10-21  7:07     ` Johannes Sixt
  0 siblings, 1 reply; 9+ messages in thread
From: Victor Engmark @ 2020-10-21  3:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Supports POSIX, bashism and mixed function declarations, all four compound
command types, trailing comments and mixed whitespace.

Uses the POSIX.1-2017 definition of allowed characters in names
<https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_235>
since actual allowed characters in Bash function names are locale
dependent <https://unix.stackexchange.com/a/245336/3645>.

Uses the default `IFS` characters to define words.

Adds testing functionality to verify non-matches by including the
literal string "non-match" somewhere in the test file. To verify that
only the matching files are syntactically valid:

for file in t/t4018/bash*
do
    echo "$file"
    if grep non-match "$file"
    then
        . "$file" && echo FAILED
    else
        . "$file" || echo FAILED
    fi
done 2>/dev/null | grep FAILED

Signed-off-by: Victor Engmark <victor@engmark.name>
---
Fixes issues found by Junio C Hamano.

Supports more valid function declaration formats.

Verifies non-matching code which looks similar to function
declarations.

Does *not* include the following suggested test:

> 	RIGHT () {
> 		: ChangeMe
> 	}

Quoting t/t4018/README:

    Insert the word "ChangeMe" (exactly this form) at a distance of
    at least two lines from the line that must appear in the hunk
    header.

t/t4018-diff-funcname.sh verifies the hunk header as part of `git diff
-U1`, so there's one line of context. In your example "RIGHT" is on the
line directly above "ChangeMe", so it's part of the context. I assume
it is intentional that no part of the context is duplicated in the hunk
header. The other tests, for example t/t4018/python-def, are
implemented in the same way, with at least one line of content between
"RIGHT" and "ChangeMe".

> > diff --git a/t/t4018/bash-trailing-comment b/t/t4018/bash-trailing-
> > comment
> > new file mode 100644
> > index 0000000000..f1edbeda31
> > --- /dev/null
> > +++ b/t/t4018/bash-trailing-comment
> > @@ -0,0 +1,4 @@
> > +RIGHT() { # Comment
> > +
> > +    ChangeMe
> > +}
> 
> This is the closest, but it tests "# with comment" at the same time.

It's meant to test only the comment part, so that is intentional.
t/t4018/bash-trailing-comment is identical to t/t4018/bash-posix-style-
function except for the comment.
---
 Documentation/gitattributes.txt       |  2 ++
 t/t4018-diff-funcname.sh              | 18 ++++++++++++++----
 t/t4018/README                        |  3 +++
 t/t4018/bash-arithmetic-function      |  4 ++++
 t/t4018/bash-bashism-style-compact    |  4 ++++
 t/t4018/bash-bashism-style-function   |  4 ++++
 t/t4018/bash-bashism-style-whitespace |  4 ++++
 t/t4018/bash-conditional-function     |  4 ++++
 t/t4018/bash-missing-parentheses      |  4 ++++
 t/t4018/bash-mixed-style-compact      |  4 ++++
 t/t4018/bash-mixed-style-function     |  4 ++++
 t/t4018/bash-other-characters         |  4 ++++
 t/t4018/bash-posix-style-compact      |  4 ++++
 t/t4018/bash-posix-style-function     |  4 ++++
 t/t4018/bash-posix-style-whitespace   |  4 ++++
 t/t4018/bash-subshell-function        |  4 ++++
 t/t4018/bash-trailing-comment         |  4 ++++
 userdiff.c                            | 22 ++++++++++++++++++++++
 18 files changed, 97 insertions(+), 4 deletions(-)
 create mode 100644 t/t4018/bash-arithmetic-function
 create mode 100644 t/t4018/bash-bashism-style-compact
 create mode 100644 t/t4018/bash-bashism-style-function
 create mode 100644 t/t4018/bash-bashism-style-whitespace
 create mode 100644 t/t4018/bash-conditional-function
 create mode 100644 t/t4018/bash-missing-parentheses
 create mode 100644 t/t4018/bash-mixed-style-compact
 create mode 100644 t/t4018/bash-mixed-style-function
 create mode 100644 t/t4018/bash-other-characters
 create mode 100644 t/t4018/bash-posix-style-compact
 create mode 100644 t/t4018/bash-posix-style-function
 create mode 100644 t/t4018/bash-posix-style-whitespace
 create mode 100644 t/t4018/bash-subshell-function
 create mode 100644 t/t4018/bash-trailing-comment

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2d0a03715b..8a15ff6bdf 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -802,6 +802,8 @@ patterns are available:
 
 - `ada` suitable for source code in the Ada language.
 
+- `bash` suitable for source code in the Bourne-Again SHell language.
+
 - `bibtex` suitable for files with BibTeX coded references.
 
 - `cpp` suitable for source code in the C and C++ languages.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 9d07797579..d18c4669d2 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -27,6 +27,7 @@ test_expect_success 'setup' '
 
 diffpatterns="
 	ada
+	bash
 	bibtex
 	cpp
 	csharp
@@ -106,10 +107,19 @@ do
 	else
 		result=success
 	fi
-	test_expect_$result "hunk header: $i" "
-		git diff -U1 $i >actual &&
-		grep '@@ .* @@.*RIGHT' actual
-	"
+
+	if grep non-match "$i" >/dev/null 2>&1
+	then
+		test_expect_$result "hunk header non-match: $i" "
+			git diff -U1 $i >actual &&
+			! grep '@@ .* @@.*RIGHT' actual
+		"
+	else
+		test_expect_$result "hunk header: $i" "
+			git diff -U1 $i >actual &&
+			grep '@@ .* @@.*RIGHT' actual
+		"
+	fi
 done
 
 test_done
diff --git a/t/t4018/README b/t/t4018/README
index 283e01cca1..cfe2b86ee7 100644
--- a/t/t4018/README
+++ b/t/t4018/README
@@ -10,6 +10,9 @@ The text that must appear in the hunk header must contain the word
 To mark a test case that highlights a malfunction, insert the word
 BROKEN in all lower-case somewhere in the file.
 
+To mark a test case that verifies some text is not matched, insert the
+word NON-MATCH in all lower-case somewhere in the file.
+
 This text is a bit twisted and out of order, but it is itself a
 test case for the default hunk header pattern. Know what you are doing
 if you change it.
diff --git a/t/t4018/bash-arithmetic-function b/t/t4018/bash-arithmetic-function
new file mode 100644
index 0000000000..c0b276cb50
--- /dev/null
+++ b/t/t4018/bash-arithmetic-function
@@ -0,0 +1,4 @@
+RIGHT() ((
+
+    ChangeMe = "$x" + "$y"
+))
diff --git a/t/t4018/bash-bashism-style-compact b/t/t4018/bash-bashism-style-compact
new file mode 100644
index 0000000000..9a06bb08fd
--- /dev/null
+++ b/t/t4018/bash-bashism-style-compact
@@ -0,0 +1,4 @@
+function RIGHT{ # non-match
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-bashism-style-function b/t/t4018/bash-bashism-style-function
new file mode 100644
index 0000000000..f1de4fa831
--- /dev/null
+++ b/t/t4018/bash-bashism-style-function
@@ -0,0 +1,4 @@
+function RIGHT {
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-bashism-style-whitespace b/t/t4018/bash-bashism-style-whitespace
new file mode 100644
index 0000000000..ade85dd3a5
--- /dev/null
+++ b/t/t4018/bash-bashism-style-whitespace
@@ -0,0 +1,4 @@
+	 function 	RIGHT 	( 	) 	{
+
+	    ChangeMe
+	 }
diff --git a/t/t4018/bash-conditional-function b/t/t4018/bash-conditional-function
new file mode 100644
index 0000000000..c5949e829b
--- /dev/null
+++ b/t/t4018/bash-conditional-function
@@ -0,0 +1,4 @@
+RIGHT() [[ \
+
+    "$a" > "$ChangeMe"
+]]
diff --git a/t/t4018/bash-missing-parentheses b/t/t4018/bash-missing-parentheses
new file mode 100644
index 0000000000..d112761300
--- /dev/null
+++ b/t/t4018/bash-missing-parentheses
@@ -0,0 +1,4 @@
+functionRIGHT { # non-match
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-mixed-style-compact b/t/t4018/bash-mixed-style-compact
new file mode 100644
index 0000000000..d9364cba67
--- /dev/null
+++ b/t/t4018/bash-mixed-style-compact
@@ -0,0 +1,4 @@
+function RIGHT(){
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-mixed-style-function b/t/t4018/bash-mixed-style-function
new file mode 100644
index 0000000000..555f9b2466
--- /dev/null
+++ b/t/t4018/bash-mixed-style-function
@@ -0,0 +1,4 @@
+function RIGHT() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-other-characters b/t/t4018/bash-other-characters
new file mode 100644
index 0000000000..a3f390d525
--- /dev/null
+++ b/t/t4018/bash-other-characters
@@ -0,0 +1,4 @@
+_RIGHT_0n() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-compact b/t/t4018/bash-posix-style-compact
new file mode 100644
index 0000000000..045bd2029b
--- /dev/null
+++ b/t/t4018/bash-posix-style-compact
@@ -0,0 +1,4 @@
+RIGHT(){
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-function b/t/t4018/bash-posix-style-function
new file mode 100644
index 0000000000..a4d144856e
--- /dev/null
+++ b/t/t4018/bash-posix-style-function
@@ -0,0 +1,4 @@
+RIGHT() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-whitespace b/t/t4018/bash-posix-style-whitespace
new file mode 100644
index 0000000000..4d984f0aa4
--- /dev/null
+++ b/t/t4018/bash-posix-style-whitespace
@@ -0,0 +1,4 @@
+	 RIGHT 	( 	) 	{
+
+	    ChangeMe
+	 }
diff --git a/t/t4018/bash-subshell-function b/t/t4018/bash-subshell-function
new file mode 100644
index 0000000000..80baa09484
--- /dev/null
+++ b/t/t4018/bash-subshell-function
@@ -0,0 +1,4 @@
+RIGHT() (
+
+    ChangeMe=2
+)
diff --git a/t/t4018/bash-trailing-comment b/t/t4018/bash-trailing-comment
new file mode 100644
index 0000000000..f1edbeda31
--- /dev/null
+++ b/t/t4018/bash-trailing-comment
@@ -0,0 +1,4 @@
+RIGHT() { # Comment
+
+    ChangeMe
+}
diff --git a/userdiff.c b/userdiff.c
index fde02f225b..8830019f05 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -23,6 +23,28 @@ IPATTERN("ada",
 	 "[a-zA-Z][a-zA-Z0-9_]*"
 	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
 	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
+PATTERNS("bash",
+	 /* Optional leading indentation */
+	 "^[ \t]*"
+	 /* Start of function definition */
+	 "("
+	 /* Start of POSIX/Bashism grouping */
+	 "("
+	 /* POSIX identifier with mandatory parentheses */
+	 "[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\))"
+	 /* Bashism identifier with optional parentheses */
+	 "|(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
+	 /* End of POSIX/Bashism grouping */
+	 ")"
+	 /* Optional whitespace */
+	 "[ \t]*"
+	 /* Compound command starting with `{`, `(`, `((` or `[[` */
+	 "(\\{|\\(\\(?|\\[\\[)"
+	 /* End of function definition */
+	 ")",
+	 /* -- */
+	 /* Characters not in the default $IFS value */
+	 "[^ \t]+"),
 PATTERNS("dts",
 	 "!;\n"
 	 "!=\n"


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] userdiff: support Bash
  2020-10-21  3:00   ` [PATCH v2] " Victor Engmark
@ 2020-10-21  7:07     ` Johannes Sixt
  2020-10-21 18:39       ` Junio C Hamano
                         ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Johannes Sixt @ 2020-10-21  7:07 UTC (permalink / raw)
  To: Victor Engmark; +Cc: Junio C Hamano, git

Am 21.10.20 um 05:00 schrieb Victor Engmark:
> Supports POSIX, bashism and mixed function declarations, all four compound
> command types, trailing comments and mixed whitespace.
> 
> Uses the POSIX.1-2017 definition of allowed characters in names
> <https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_235>
> since actual allowed characters in Bash function names are locale
> dependent <https://unix.stackexchange.com/a/245336/3645>.

So, this is more like

   Even though bash allows locale-dependet characters in function names,
   only the allowed characters per the POSIX.1-2017 definition are
   implemented for simplicity. We can expect that this catches the vast
   majority of use-cases.

> 
> Uses the default `IFS` characters to define words.

We could do better than this, I think. At a minimum, the equal sign,
single quote, double quote, parentheses, and braces should also
delineate words. $(, ${, $((, ((, )), [[, ]], should be words. I would
exclude single brackets because they could only occur in globs, IIRC,
and they need not be broken into words at brackets. $var should be a
single word, IMO.

That said, this can be presented as a patch on top of this one.

> 
> Adds testing functionality to verify non-matches by including the
> literal string "non-match" somewhere in the test file. To verify that
> only the matching files are syntactically valid:
> 
> for file in t/t4018/bash*
> do
>     echo "$file"
>     if grep non-match "$file"
>     then
>         . "$file" && echo FAILED
>     else
>         . "$file" || echo FAILED
>     fi
> done 2>/dev/null | grep FAILED

This complication is not necessary. See below for an example how to do
negative tests.

While speaking of that: it is very refreshing to see negative tests!

> 
> Signed-off-by: Victor Engmark <victor@engmark.name>
> ---

When you write a commit message, please always answer the question WHY
the change should be made. For example, the notice "use IFS characters"
alone does not add value; that much can be seen in the patch text. How
about:

   Since a word pattern has to be specified, but there is no easy way
   to request the default word pattern, use the standard IFS characters
   for a starter. A later patch can improve this.

In general, a justification of why something should be added, should
also be answered. But in the case of "bash pattern for userdiff" the
answer is too obvious and trivial, that an exception is warranted.

Please write the commit message in imperative mood. "Support" instead of
"Supports", etc.

> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 2d0a03715b..8a15ff6bdf 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -802,6 +802,8 @@ patterns are available:
>  
>  - `ada` suitable for source code in the Ada language.
>  
> +- `bash` suitable for source code in the Bourne-Again SHell language.

Can we mention POSIX shell language as well?

> diff --git a/t/t4018/bash-missing-parentheses b/t/t4018/bash-missing-parentheses
> new file mode 100644
> index 0000000000..d112761300
> --- /dev/null
> +++ b/t/t4018/bash-missing-parentheses
> @@ -0,0 +1,4 @@
> +functionRIGHT { # non-match
> +    :
> +    echo 'ChangeMe'
> +}

To check for a non-match, you write the test like this:

	function RIGHT () {
	}
	# the following must not be picked up:
	functionWrong {
		:
		ChangeMe
	}

That is, you present a positive match, and then expect that the
subsequent negative match is not picked up.

> diff --git a/t/t4018/bash-posix-style-compact b/t/t4018/bash-posix-style-compact
> new file mode 100644
> index 0000000000..045bd2029b
> --- /dev/null
> +++ b/t/t4018/bash-posix-style-compact
> @@ -0,0 +1,4 @@
> +RIGHT(){
> +
> +    ChangeMe
> +}
> diff --git a/t/t4018/bash-posix-style-function b/t/t4018/bash-posix-style-function
> new file mode 100644
> index 0000000000..a4d144856e
> --- /dev/null
> +++ b/t/t4018/bash-posix-style-function
> @@ -0,0 +1,4 @@
> +RIGHT() {
> +
> +    ChangeMe
> +}
> diff --git a/t/t4018/bash-posix-style-whitespace b/t/t4018/bash-posix-style-whitespace
> new file mode 100644
> index 0000000000..4d984f0aa4
> --- /dev/null
> +++ b/t/t4018/bash-posix-style-whitespace
> @@ -0,0 +1,4 @@
> +	 RIGHT 	( 	) 	{
> +
> +	    ChangeMe
> +	 }

Good to see POSIX-style function tests.

> diff --git a/userdiff.c b/userdiff.c
> index fde02f225b..8830019f05 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -23,6 +23,28 @@ IPATTERN("ada",
>  	 "[a-zA-Z][a-zA-Z0-9_]*"
>  	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
>  	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
> +PATTERNS("bash",
> +	 /* Optional leading indentation */
> +	 "^[ \t]*"
> +	 /* Start of function definition */
> +	 "("

The purpose of this outer-most pair of parentheses is actually to mark
the captured text, not so much "the function definition".

> +	 /* Start of POSIX/Bashism grouping */
> +	 "("

You could omit the comment if you indent the parts that are inside the
parentheses:

	"("
		"..."
	"|"
		"..."
	")"

(But perhaps don't indent between the outer-most parentheses; it would
get us too far to the right. But judge yourself.)

> +	 /* POSIX identifier with mandatory parentheses */
> +	 "[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\))"
> +	 /* Bashism identifier with optional parentheses */
> +	 "|(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
> +	 /* End of POSIX/Bashism grouping */
> +	 ")"
> +	 /* Optional whitespace */
> +	 "[ \t]*"
> +	 /* Compound command starting with `{`, `(`, `((` or `[[` */
> +	 "(\\{|\\(\\(?|\\[\\[)"
> +	 /* End of function definition */
> +	 ")",
> +	 /* -- */
> +	 /* Characters not in the default $IFS value */
> +	 "[^ \t]+"),

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] userdiff: support Bash
  2020-10-21  7:07     ` Johannes Sixt
@ 2020-10-21 18:39       ` Junio C Hamano
  2020-10-21 22:48       ` Victor Engmark
  2020-10-21 23:45       ` [PATCH v3] " Victor Engmark
  2 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2020-10-21 18:39 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Victor Engmark, git

Johannes Sixt <j6t@kdbg.org> writes:

>> diff --git a/t/t4018/bash-missing-parentheses b/t/t4018/bash-missing-parentheses
>> new file mode 100644
>> index 0000000000..d112761300
>> --- /dev/null
>> +++ b/t/t4018/bash-missing-parentheses
>> @@ -0,0 +1,4 @@
>> +functionRIGHT { # non-match
>> +    :
>> +    echo 'ChangeMe'
>> +}
>
> To check for a non-match, you write the test like this:
>
> 	function RIGHT () {
> 	}
> 	# the following must not be picked up:
> 	functionWrong {
> 		:
> 		ChangeMe
> 	}
>
> That is, you present a positive match, and then expect that the
> subsequent negative match is not picked up.

All good suggestions, but I especially appreciate this one ;-).

>> diff --git a/userdiff.c b/userdiff.c
>> index fde02f225b..8830019f05 100644
>> --- a/userdiff.c
>> +++ b/userdiff.c
>> @@ -23,6 +23,28 @@ IPATTERN("ada",
>>  	 "[a-zA-Z][a-zA-Z0-9_]*"
>>  	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
>>  	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
>> +PATTERNS("bash",
>> +	 /* Optional leading indentation */
>> +	 "^[ \t]*"
>> +	 /* Start of function definition */
>> +	 "("
>
> The purpose of this outer-most pair of parentheses is actually to mark
> the captured text, not so much "the function definition".

This, too (I called it "here comes the whole thing" in my suggested
version ).

>> +	 /* Start of POSIX/Bashism grouping */
>> +	 "("
>
> You could omit the comment if you indent the parts that are inside the
> parentheses:
>
> 	"("
> 		"..."
> 	"|"
> 		"..."
> 	")"
>

An excellent readability suggestion.

Thanks for a review.  Especially the parts that mine didn't touch
(i.e. the proposed log message).

> (But perhaps don't indent between the outer-most parentheses; it would
> get us too far to the right. But judge yourself.)
>
>> +	 /* POSIX identifier with mandatory parentheses */
>> +	 "[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\))"
>> +	 /* Bashism identifier with optional parentheses */
>> +	 "|(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
>> +	 /* End of POSIX/Bashism grouping */
>> +	 ")"
>> +	 /* Optional whitespace */
>> +	 "[ \t]*"
>> +	 /* Compound command starting with `{`, `(`, `((` or `[[` */
>> +	 "(\\{|\\(\\(?|\\[\\[)"
>> +	 /* End of function definition */
>> +	 ")",
>> +	 /* -- */
>> +	 /* Characters not in the default $IFS value */
>> +	 "[^ \t]+"),

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v2] userdiff: support Bash
  2020-10-21  7:07     ` Johannes Sixt
  2020-10-21 18:39       ` Junio C Hamano
@ 2020-10-21 22:48       ` Victor Engmark
  2020-10-21 23:45       ` [PATCH v3] " Victor Engmark
  2 siblings, 0 replies; 9+ messages in thread
From: Victor Engmark @ 2020-10-21 22:48 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git

On Wed, 2020-10-21 at 09:07 +0200, Johannes Sixt wrote:
> Am 21.10.20 um 05:00 schrieb Victor Engmark:
> > Uses the default `IFS` characters to define words.
> 
> We could do better than this, I think. At a minimum, the equal sign,
> single quote, double quote, parentheses, and braces should also
> delineate words. $(, ${, $((, ((, )), [[, ]], should be words. I
> would
> exclude single brackets because they could only occur in globs, IIRC,
> and they need not be broken into words at brackets. $var should be a
> single word, IMO.
> 
> That said, this can be presented as a patch on top of this one.

I can't tell where this word definition is used, and I can't seem to
find any tests for the word regexes for the other languages. Is it used
for word diffs somehow? I'll leave it for now, but if you have some
pointers I could look into that later.

Thank you very much for the comments!


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH v3] userdiff: support Bash
  2020-10-21  7:07     ` Johannes Sixt
  2020-10-21 18:39       ` Junio C Hamano
  2020-10-21 22:48       ` Victor Engmark
@ 2020-10-21 23:45       ` Victor Engmark
  2020-10-22  6:08         ` Johannes Sixt
  2 siblings, 1 reply; 9+ messages in thread
From: Victor Engmark @ 2020-10-21 23:45 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git

Support POSIX, bashism and mixed function declarations, all four
compound command types, trailing comments and mixed whitespace.

Even though Bash allows locale-dependent characters in function names
<https://unix.stackexchange.com/a/245336/3645>, only detect function
names with characters allowed by POSIX.1-2017
<https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_235>
for simplicity. This should cover the vast majority of use cases, and
produces system-agnostic results.

Since a word pattern has to be specified, but there is no easy way to
know the default word pattern, use the default `IFS` characters for a
starter. A later patch can improve this.

Signed-off-by: Victor Engmark <victor@engmark.name>
---
Includes suggestions by Johannes Sixt <j6t@kdbg.org>.
---
 Documentation/gitattributes.txt       |  3 +++
 t/t4018-diff-funcname.sh              |  1 +
 t/t4018/bash-arithmetic-function      |  4 ++++
 t/t4018/bash-bashism-style-compact    |  6 ++++++
 t/t4018/bash-bashism-style-function   |  4 ++++
 t/t4018/bash-bashism-style-whitespace |  4 ++++
 t/t4018/bash-conditional-function     |  4 ++++
 t/t4018/bash-missing-parentheses      |  6 ++++++
 t/t4018/bash-mixed-style-compact      |  4 ++++
 t/t4018/bash-mixed-style-function     |  4 ++++
 t/t4018/bash-nested-functions         |  6 ++++++
 t/t4018/bash-other-characters         |  4 ++++
 t/t4018/bash-posix-style-compact      |  4 ++++
 t/t4018/bash-posix-style-function     |  4 ++++
 t/t4018/bash-posix-style-whitespace   |  4 ++++
 t/t4018/bash-subshell-function        |  4 ++++
 t/t4018/bash-trailing-comment         |  4 ++++
 userdiff.c                            | 21 +++++++++++++++++++++
 18 files changed, 91 insertions(+)
 create mode 100644 t/t4018/bash-arithmetic-function
 create mode 100644 t/t4018/bash-bashism-style-compact
 create mode 100644 t/t4018/bash-bashism-style-function
 create mode 100644 t/t4018/bash-bashism-style-whitespace
 create mode 100644 t/t4018/bash-conditional-function
 create mode 100644 t/t4018/bash-missing-parentheses
 create mode 100644 t/t4018/bash-mixed-style-compact
 create mode 100644 t/t4018/bash-mixed-style-function
 create mode 100644 t/t4018/bash-nested-functions
 create mode 100644 t/t4018/bash-other-characters
 create mode 100644 t/t4018/bash-posix-style-compact
 create mode 100644 t/t4018/bash-posix-style-function
 create mode 100644 t/t4018/bash-posix-style-whitespace
 create mode 100644 t/t4018/bash-subshell-function
 create mode 100644 t/t4018/bash-trailing-comment

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2d0a03715b..5e8a973449 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -802,6 +802,9 @@ patterns are available:
 
 - `ada` suitable for source code in the Ada language.
 
+- `bash` suitable for source code in the Bourne-Again SHell language.
+  Covers a superset of POSIX function definitions.
+
 - `bibtex` suitable for files with BibTeX coded references.
 
 - `cpp` suitable for source code in the C and C++ languages.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 9d07797579..9675bc17db 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -27,6 +27,7 @@ test_expect_success 'setup' '
 
 diffpatterns="
 	ada
+	bash
 	bibtex
 	cpp
 	csharp
diff --git a/t/t4018/bash-arithmetic-function b/t/t4018/bash-arithmetic-function
new file mode 100644
index 0000000000..c0b276cb50
--- /dev/null
+++ b/t/t4018/bash-arithmetic-function
@@ -0,0 +1,4 @@
+RIGHT() ((
+
+    ChangeMe = "$x" + "$y"
+))
diff --git a/t/t4018/bash-bashism-style-compact b/t/t4018/bash-bashism-style-compact
new file mode 100644
index 0000000000..1ca3126f61
--- /dev/null
+++ b/t/t4018/bash-bashism-style-compact
@@ -0,0 +1,6 @@
+function RIGHT {
+    function InvalidSyntax{
+        :
+        echo 'ChangeMe'
+    }
+}
diff --git a/t/t4018/bash-bashism-style-function b/t/t4018/bash-bashism-style-function
new file mode 100644
index 0000000000..f1de4fa831
--- /dev/null
+++ b/t/t4018/bash-bashism-style-function
@@ -0,0 +1,4 @@
+function RIGHT {
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-bashism-style-whitespace b/t/t4018/bash-bashism-style-whitespace
new file mode 100644
index 0000000000..ade85dd3a5
--- /dev/null
+++ b/t/t4018/bash-bashism-style-whitespace
@@ -0,0 +1,4 @@
+	 function 	RIGHT 	( 	) 	{
+
+	    ChangeMe
+	 }
diff --git a/t/t4018/bash-conditional-function b/t/t4018/bash-conditional-function
new file mode 100644
index 0000000000..c5949e829b
--- /dev/null
+++ b/t/t4018/bash-conditional-function
@@ -0,0 +1,4 @@
+RIGHT() [[ \
+
+    "$a" > "$ChangeMe"
+]]
diff --git a/t/t4018/bash-missing-parentheses b/t/t4018/bash-missing-parentheses
new file mode 100644
index 0000000000..8c8a05dd7a
--- /dev/null
+++ b/t/t4018/bash-missing-parentheses
@@ -0,0 +1,6 @@
+function RIGHT {
+    functionInvalidSyntax {
+        :
+        echo 'ChangeMe'
+    }
+}
diff --git a/t/t4018/bash-mixed-style-compact b/t/t4018/bash-mixed-style-compact
new file mode 100644
index 0000000000..d9364cba67
--- /dev/null
+++ b/t/t4018/bash-mixed-style-compact
@@ -0,0 +1,4 @@
+function RIGHT(){
+    :
+    echo 'ChangeMe'
+}
diff --git a/t/t4018/bash-mixed-style-function b/t/t4018/bash-mixed-style-function
new file mode 100644
index 0000000000..555f9b2466
--- /dev/null
+++ b/t/t4018/bash-mixed-style-function
@@ -0,0 +1,4 @@
+function RIGHT() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-nested-functions b/t/t4018/bash-nested-functions
new file mode 100644
index 0000000000..2c9237ead4
--- /dev/null
+++ b/t/t4018/bash-nested-functions
@@ -0,0 +1,6 @@
+outer() {
+    RIGHT() {
+        :
+        echo 'ChangeMe'
+    }
+}
diff --git a/t/t4018/bash-other-characters b/t/t4018/bash-other-characters
new file mode 100644
index 0000000000..a3f390d525
--- /dev/null
+++ b/t/t4018/bash-other-characters
@@ -0,0 +1,4 @@
+_RIGHT_0n() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-compact b/t/t4018/bash-posix-style-compact
new file mode 100644
index 0000000000..045bd2029b
--- /dev/null
+++ b/t/t4018/bash-posix-style-compact
@@ -0,0 +1,4 @@
+RIGHT(){
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-function b/t/t4018/bash-posix-style-function
new file mode 100644
index 0000000000..a4d144856e
--- /dev/null
+++ b/t/t4018/bash-posix-style-function
@@ -0,0 +1,4 @@
+RIGHT() {
+
+    ChangeMe
+}
diff --git a/t/t4018/bash-posix-style-whitespace b/t/t4018/bash-posix-style-whitespace
new file mode 100644
index 0000000000..4d984f0aa4
--- /dev/null
+++ b/t/t4018/bash-posix-style-whitespace
@@ -0,0 +1,4 @@
+	 RIGHT 	( 	) 	{
+
+	    ChangeMe
+	 }
diff --git a/t/t4018/bash-subshell-function b/t/t4018/bash-subshell-function
new file mode 100644
index 0000000000..80baa09484
--- /dev/null
+++ b/t/t4018/bash-subshell-function
@@ -0,0 +1,4 @@
+RIGHT() (
+
+    ChangeMe=2
+)
diff --git a/t/t4018/bash-trailing-comment b/t/t4018/bash-trailing-comment
new file mode 100644
index 0000000000..f1edbeda31
--- /dev/null
+++ b/t/t4018/bash-trailing-comment
@@ -0,0 +1,4 @@
+RIGHT() { # Comment
+
+    ChangeMe
+}
diff --git a/userdiff.c b/userdiff.c
index fde02f225b..eb698eaca7 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -23,6 +23,27 @@ IPATTERN("ada",
 	 "[a-zA-Z][a-zA-Z0-9_]*"
 	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
 	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
+PATTERNS("bash",
+	 /* Optional leading indentation */
+	 "^[ \t]*"
+	 /* Start of captured text */
+	 "("
+	 "("
+	     /* POSIX identifier with mandatory parentheses */
+	     "[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\))"
+	 "|"
+	     /* Bashism identifier with optional parentheses */
+	     "(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
+	 ")"
+	 /* Optional whitespace */
+	 "[ \t]*"
+	 /* Compound command starting with `{`, `(`, `((` or `[[` */
+	 "(\\{|\\(\\(?|\\[\\[)"
+	 /* End of captured text */
+	 ")",
+	 /* -- */
+	 /* Characters not in the default $IFS value */
+	 "[^ \t]+"),
 PATTERNS("dts",
 	 "!;\n"
 	 "!=\n"


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH v3] userdiff: support Bash
  2020-10-21 23:45       ` [PATCH v3] " Victor Engmark
@ 2020-10-22  6:08         ` Johannes Sixt
  2020-10-22 17:30           ` Junio C Hamano
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Sixt @ 2020-10-22  6:08 UTC (permalink / raw)
  To: Victor Engmark; +Cc: Junio C Hamano, git

Am 22.10.20 um 01:45 schrieb Victor Engmark:
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 2d0a03715b..5e8a973449 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -802,6 +802,9 @@ patterns are available:
>  
>  - `ada` suitable for source code in the Ada language.
>  
> +- `bash` suitable for source code in the Bourne-Again SHell language.
> +  Covers a superset of POSIX function definitions.

OK. POSIX *shell* function definitions would have been even better, but
I think I can live with this version.

> diff --git a/t/t4018/bash-bashism-style-compact b/t/t4018/bash-bashism-style-compact
> new file mode 100644
> index 0000000000..1ca3126f61
> --- /dev/null
> +++ b/t/t4018/bash-bashism-style-compact
> @@ -0,0 +1,6 @@
> +function RIGHT {
> +    function InvalidSyntax{

Nicely done!

> +        :
> +        echo 'ChangeMe'
> +    }
> +}

> diff --git a/t/t4018/bash-nested-functions b/t/t4018/bash-nested-functions
> new file mode 100644
> index 0000000000..2c9237ead4
> --- /dev/null
> +++ b/t/t4018/bash-nested-functions
> @@ -0,0 +1,6 @@
> +outer() {
> +    RIGHT() {
> +        :
> +        echo 'ChangeMe'
> +    }
> +}

That's another very good addition!

> diff --git a/userdiff.c b/userdiff.c
> index fde02f225b..eb698eaca7 100644
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -23,6 +23,27 @@ IPATTERN("ada",
>  	 "[a-zA-Z][a-zA-Z0-9_]*"
>  	 "|[-+]?[0-9][0-9#_.aAbBcCdDeEfF]*([eE][+-]?[0-9_]+)?"
>  	 "|=>|\\.\\.|\\*\\*|:=|/=|>=|<=|<<|>>|<>"),
> +PATTERNS("bash",
> +	 /* Optional leading indentation */
> +	 "^[ \t]*"
> +	 /* Start of captured text */
> +	 "("
> +	 "("
> +	     /* POSIX identifier with mandatory parentheses */
> +	     "[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\))"
> +	 "|"
> +	     /* Bashism identifier with optional parentheses */
> +	     "(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
> +	 ")"
> +	 /* Optional whitespace */
> +	 "[ \t]*"
> +	 /* Compound command starting with `{`, `(`, `((` or `[[` */
> +	 "(\\{|\\(\\(?|\\[\\[)"
> +	 /* End of captured text */
> +	 ")",
> +	 /* -- */
> +	 /* Characters not in the default $IFS value */
> +	 "[^ \t]+"),
>  PATTERNS("dts",
>  	 "!;\n"
>  	 "!=\n"
> 

This is very well done. Thank you!

Acked-by: Johannes Sixt <j6t@kdbg.org>

-- Hannes

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH v3] userdiff: support Bash
  2020-10-22  6:08         ` Johannes Sixt
@ 2020-10-22 17:30           ` Junio C Hamano
  0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2020-10-22 17:30 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Victor Engmark, git

Johannes Sixt <j6t@kdbg.org> writes:

> Am 22.10.20 um 01:45 schrieb Victor Engmark:
>> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
>> index 2d0a03715b..5e8a973449 100644
>> --- a/Documentation/gitattributes.txt
>> +++ b/Documentation/gitattributes.txt
>> @@ -802,6 +802,9 @@ patterns are available:
>>  
>>  - `ada` suitable for source code in the Ada language.
>>  
>> +- `bash` suitable for source code in the Bourne-Again SHell language.
>> +  Covers a superset of POSIX function definitions.
>
> OK. POSIX *shell* function definitions would have been even better, but
> I think I can live with this version.

I can't, so I'll locally amend ...

> This is very well done. Thank you!
>
> Acked-by: Johannes Sixt <j6t@kdbg.org>

... and with this in the trailer block.

Thanks, both.  Queued.

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-22 17:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20  7:10 [PATCH] userdiff: support Bash Victor Engmark
2020-10-20 23:39 ` Junio C Hamano
2020-10-21  3:00   ` [PATCH v2] " Victor Engmark
2020-10-21  7:07     ` Johannes Sixt
2020-10-21 18:39       ` Junio C Hamano
2020-10-21 22:48       ` Victor Engmark
2020-10-21 23:45       ` [PATCH v3] " Victor Engmark
2020-10-22  6:08         ` Johannes Sixt
2020-10-22 17:30           ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).