[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 2/2] Document bracket whitespace rules & add syntax-check rule



On 11/01/2012 11:53 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
[...]
> ---
>  build-aux/bracket-spacing.pl | 116 +++++++++++++++++++++++++++++++++++++++++++
>  cfg.mk                       |   7 ++-
>  docs/hacking.html.in         |  49 ++++++++++++++++++
>  3 files changed, 171 insertions(+), 1 deletion(-)
>  create mode 100755 build-aux/bracket-spacing.pl
> 

Even though it is pushed already, as nobody else had a look, I'm going
for it just to have a clean mind.

> diff --git a/build-aux/bracket-spacing.pl b/build-aux/bracket-spacing.pl
> new file mode 100755
> index 0000000..d3a916f
> --- /dev/null
> +++ b/build-aux/bracket-spacing.pl
> @@ -0,0 +1,116 @@
> +#!/usr/bin/perl
> +#
> +# bracket-spacing.pl: Report any usage of 'function (..args..)'
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library.  If not, see
> +# <http://www.gnu.org/licenses/>.
> +#
> +# Authors:
> +#     Daniel P. Berrange <berrange redhat com>
> +
> +use strict;
> +use warnings;
> +
> +my $ret = 0;
> +my $incomment = 0;
> +
> +foreach my $file (@ARGV) {
> +    open FILE, $file;
> +
> +    while (defined (my $line = <FILE>)) {
> +        my $data = $line;
> +
> +        # Kill any quoted strongs
> +        $data =~ s,".*?","XXX",g;

This doesn't match everything, for example:

printf(_("This \" %s error"), func ("is"));

However, as this is so unlikely to appear, it should be ok as-is.  The
only places we have in the code don't usually have a function after
themselves.  Checked with:

git grep -E '(^|[^"\\])"[^"\\]*(\\\\)*\\"[^"\\]*".*[[:alnum:]_]*\s*\('

[...]
> diff --git a/docs/hacking.html.in b/docs/hacking.html.in
> index d41b39c..37ed00b 100644
> --- a/docs/hacking.html.in
> +++ b/docs/hacking.html.in
> @@ -212,6 +212,55 @@
>      </p>
>  
>  
> +    <h2><a name="bracket_spacing">Bracket spacing</a></h2>
> +
> +    <p>
> +      The keywords <code>if</code>, <code>for</code>, <code>while</code>,
> +      and <code>switch</code> must have a single space following them
> +      before the opening bracket. eg

This would look better with just "Example:", IMHO.

[...]

Other than that, everything's ok, I'll add the aesthetic change to my
series of super small cleanups, so post-ACK from me, thanks for the cleanup.

Martin


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]