[Piglit] [PATCH 18/26] parse_glspec.py: PEP8 compliance

[Chad Versace]

On 07/10/2013 03:19 PM, Dylan Baker wrote:
> There are a 3 lines that are just over the recommended 79 character
> limit, however, breaking them made them harder to read so they were
> left.
> ---
>   glapi/parse_glspec.py | 76 ++++++++++++++++++++++++---------------------------
>   1 file changed, 36 insertions(+), 40 deletions(-)



>
>       m = GLES_VERSION_REGEXP.match(category_name)
>       if m:
>           ones = int(m.group(1))
>           tenths = int(m.group(2))
> -        return 'GLES{0}.{1}'.format(ones, tenths), {
> -            'kind': 'GLES',
> -            'gl_10x_version': 10 * ones + tenths
> -            }
> +        return 'GLES{0}.{1}'.format(ones, tenths), {'kind': 'GLES',
> +                                                    'gl_10x_version':
> +                                                    10 * ones + tenths}
>


The original was significantly more readable. Maybe you could achieve PEP-8
correctness while maintaining readability by using explicit parens around the
tuple. Maybe like this:

     return ('GLES{0}.{1}'.format(ones, tenths),
             {'kind': 'GLES',
              'gl_10x_version': 10 * ones + tenths})

or this

     return (
         'GLES{0}.{1}'.format(ones, tenths),
         {'kind': 'GLES',
          'gl_10x_version': 10 * ones + tenths}
     )

 From my interpretation of the PEP8 doc, both are PEP8 correct.



>       extension_name = 'GL_' + category_name
> -    return extension_name, {
> -        'kind': 'extension',
> -        'extension_name': extension_name
> -        }
> +    return extension_name, {'kind': 'extension',
> +                            'extension_name': extension_name}


While your changing the formatting of tuples, could you add explicit
parens to aid readability? There are several other locations in this
patch where parens could be added. Or do you think that belongs to
a separate patch?



>           if name not in self.functions:
> -            self.functions[name] = {
> -                'return_type': return_type,
> -                'param_names': param_names,
> -                'param_types': param_types,
> -                'categories': [category],
> -                }
> +            self.functions[name] = {'return_type': return_type,
> +                                    'param_names': param_names,
> +                                    'param_types': param_types,
> +                                    'categories': [category]}

Did the PEP8 tool complain about this one?