[libcamera-devel] [PATCH v2] utils: checkstyle.py: Add pep8 checker

[Kieran Bingham]

Hi Laurent,

On 04/07/2019 21:24, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thu, Jul 04, 2019 at 04:06:58PM +0100, Kieran Bingham wrote:
>> Process python additions with pep8 and report any errors that are added.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>> --
> 
> This should be --- to be stripped by git am. Don't forget to remove it
> before pushing in any case :-)
> 
>> v2:
>>  - Now catches if pep8 is not available
>>  - line_no_regex renamed to results_regex and contained in Pep8Checker
>>    class
>>  - Single quotes used instead of double quotes
>>  - 'data' moved to local variable instance of class instance
>> ---
>>  utils/checkstyle.py | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
>> index fab4b116d2ff..c5ecdc12112b 100755
>> --- a/utils/checkstyle.py
>> +++ b/utils/checkstyle.py
>> @@ -276,6 +276,39 @@ class MesonChecker(StyleChecker):
>>          return issues
>>  
>>  
>> +class Pep8Checker(StyleChecker):
>> +    patterns = ('*.py',)
>> +    results_regex = re.compile('stdin:([0-9]+):([0-9]+)(.*)')
>> +
>> +    def __init__(self, content):
>> +        super().__init__()
>> +        self.__content = content
>> +
>> +    def check(self, line_numbers):
>> +        issues = []
>> +        data = ''.join(self.__content).encode('utf-8')
>> +
>> +        try:
>> +            ret = subprocess.run(['pep8', '--ignore=E501', '-'],
>> +                                 input=data, stdout=subprocess.PIPE)
>> +        except FileNotFoundError:
>> +            issues.append(StyleIssue(0, '', "Please install pep8 to validate python additions"))
>> +            return issues
>> +
>> +        results = ret.stdout.decode('utf-8').splitlines()
>> +        for item in results:
>> +            search = re.search(Pep8Checker.results_regex, item)
>> +            line_number = int(search.group(1))
>> +            position = int(search.group(2))
>> +            msg = search.group(3)
>> +
>> +            if line_number in line_numbers:
>> +                line = self.__content[line_number - 1]
>> +                issues.append(StyleIssue(line_number, line, msg))
>> +
>> +        return issues
>> +
>> +
> 
> This looks good to me. It however prints an empty line below the
> message when pep8 isn't installed. How about squashing the following
> change in ?
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index c5ecdc12112b..42a96f6d6102 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -292,7 +292,7 @@ class Pep8Checker(StyleChecker):
>              ret = subprocess.run(['pep8', '--ignore=E501', '-'],
>                                   input=data, stdout=subprocess.PIPE)
>          except FileNotFoundError:
> -            issues.append(StyleIssue(0, '', "Please install pep8 to validate python additions"))
> +            issues.append(StyleIssue(0, None, "Please install pep8 to validate python additions"))
>              return issues
> 
>          results = ret.stdout.decode('utf-8').splitlines()
> @@ -483,7 +483,8 @@ def check_file(top_level, commit, filename):
>          issues = sorted(issues, key=lambda i: i.line_number)
>          for issue in issues:
>              print('%s#%u: %s' % (Colours.fg(Colours.Yellow), issue.line_number, issue.msg))
> -            print('+%s%s' % (issue.line.rstrip(), Colours.reset()))
> +            if issue.line is not None:
> +                print('+%s%s' % (issue.line.rstrip(), Colours.reset()))
> 
>      return len(formatted_diff) + len(issues)
>  
> 
> With that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I've tested your change, and it works fine.

Pushed with your changes
--
Kieran


> 
>>  # ------------------------------------------------------------------------------
>>  # Formatters
>>  #
> 

-- 
Regards
--
Kieran