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

[Dylan Baker]

I'll look through for the parens, It might be in a seperate patch, but I
think it deserves to be here, since PEP8 is all about readability.

The final one it complained about, but after reading PEP8 again I
understand why, and can fix it. In this case it wants something like this:
variable = {
     key: value,
     key: value
}

With the important note that the closing brace (or parentheses or bracket)
should either be on the last line of text, or should be on the next line
even with the indent of the first line.


On Mon, Jul 22, 2013 at 2:26 PM, Chad Versace
<chad.versace at linux.intel.com>wrote:

> 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?
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20130723/53a92ac3/attachment.html>