| Message ID | 20190704150658.18809-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@ideasonboard.com> > # ------------------------------------------------------------------------------ > # Formatters > #
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@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@ideasonboard.com> I've tested your change, and it works fine. Pushed with your changes -- Kieran > >> # ------------------------------------------------------------------------------ >> # Formatters >> # >
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 + + # ------------------------------------------------------------------------------ # Formatters #
Process python additions with pep8 and report any errors that are added. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> -- 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(+)