On overall, the patch looks good. Some suggestions to improve the tests
and a minor nitpick below.
Adrien Schildknecht <adrien+dev@xxxxxxxxxxx> writes:
> +++ b/t/t4034/sh/post
> @@ -0,0 +1,36 @@
> +foo() {ls&echo}
This part is unchanged here and in the pre file. What does it test?
> +$((x++))
> +$((x--))
> +$((--x))
> +$((++x))
> +$((x*y))
> +$((x&y))
> +$((x**y))
> +$((x/y))
> +$((x%y))
> +$((x+y))
> +$((x-y))
> +[ x<=y ]
> +[ x>=y ]
> +[ x==y ]
> +[ x!=y ]
Not sure what the last ones are testing. If it's "[" as "the test
command, spelled as [", then spaces are mandatory around the operators
(and equality should be written =, not == in POSIX).
> +x<<y x>>y x<<-y x<y x>y x>|y x<&y x>&y x<>y
> +x&y
> +x&&y
> +x|y
> +x||y
> +x=y
> +$((x+=y))
> +$((x-=y))
> +$((x*=y))
> +$((x/=y))
> +$((x%=y))
> +$((x<<=y))
> +$((x>>=y))
> +$((x&=y))
> +$((x^=y))
> +$((x|=y))
I think you should test the case of multiple-letters identifiers. One of
the benefit of having a proper word-diff pattern is that e.g.
- pre=foo
+ post=bar
will consider the change "pre" -> "post", and not an unmodified "p" with
the change "re" -> "ost" (otherwise, --color-words=. just works).
> +PATTERNS("sh",
> + "^([ \t]*(function[ \t]+)?[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\).*)$",
> + /* -- */
> + "[a-zA-Z0-9_]+"
Nitpick: the indentation is not homogeneous. You should add a space
after the tab on the first two lines to get a correct alignment.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html