On overall, the patch looks good. Some suggestions to improve the tests and a minor nitpick below.
Adrien Schildknecht <[email protected]> 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
