Re: [PATCH v2] userdiff: funcname and word patterns for sh
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Mar 14, 2015, 17:19 UTC
- Message-ID
- <vpqoanvjy3k.fsf@anie.imag.fr>
- In-Reply-To
- <1425944432-23642-1-git-send-email-adrien+dev@schischi.me>
On overall, the patch looks good. Some suggestions to improve the tests and a minor nitpick below.
Adrien Schildknecht <adrien+dev@schischi.me> 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?
Show 15 quoted lines
> +$((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).
Show 16 quoted lines
> +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/