Hi Both, > -----Original Message----- > From: Jonathan Wakely <jwak...@redhat.com> > Sent: Monday, July 22, 2024 3:21 PM > To: Filip Kastl <fka...@suse.cz> > Cc: Tamar Christina <tamar.christ...@arm.com>; gcc-patches@gcc.gnu.org; nd > <n...@arm.com> > Subject: Re: [PATCH][contrib]: support json output from check_GNU_style_lib.py > > On Mon, 22 Jul 2024 at 14:54, Filip Kastl <fka...@suse.cz> wrote: > > > > Hi Tamar, > > > > I wanted to try reviewing a patch and this seemed simple enough so I gave > > it a > > shot. Hopefully this saves some time of the maintainer that eventually > > approves this :).
Thanks for the review! :)
> >
> > I don't see any bug in the code. I also tried running it on my own input
> > and
> > the output was correct. So functionally the patch is good. I have some
> > remarks about the style though:
> >
> > - You sometimes put a space between function name and parentheses. This
> > doesn't look python-ish and isn't consistent in the file.
>
> It's the GNU C convention, but I really wish we didn't use it for
> Python code too.
>
> > - There's one very long line (check_GNU_style_lib.py:335). I would shorten
> > it
> > so it is at most 79 characters long.
> > - On the same line, there is a space after { but not before }. For
> > consistency, I would erase the space after {
> > - On the same line there are spaces after :. I think a more python-ish way
> > would be not to have those spaces there. Here I'm maybe being too
> > pedantic
> > so feel free to ignore this. I think it will look nice either way.
> >
> > To summarize the last 3 points, I would replace this
> >
> > errlines.append({ "file" : locs[0], "row" : locs[1],
> > "column" : locs[2], "err"
> : e.console_error})
How about this formatting, I tend to find it a bit easier to read even.
I also updated the location numbering to be numerical so, removed the quotes.
Ok for master?
Thanks,
Tamar
contrib/ChangeLog:
* check_GNU_style.py: Add json format.
* check_GNU_style_lib.py: Likewise.
-- inline copy of patch --
diff --git a/contrib/check_GNU_style.py b/contrib/check_GNU_style.py
index
6b946a5bc3610b8ef70ba372ea800f892eeac85b..0890947f1f9b60c37ff62e23007c3a0735fd9c14
100755
--- a/contrib/check_GNU_style.py
+++ b/contrib/check_GNU_style.py
@@ -31,7 +31,7 @@ def main():
parser.add_argument('file', help = 'File with a patch')
parser.add_argument('-f', '--format', default = 'stdio',
help = 'Display format',
- choices = ['stdio', 'quickfix'])
+ choices = ['stdio', 'quickfix', 'json'])
args = parser.parse_args()
filename = args.file
format = args.format
diff --git a/contrib/check_GNU_style_lib.py b/contrib/check_GNU_style_lib.py
index
6dbe4b53559c63d2e0276d0ff88619cd2f7f8e06..ab21ed4607593668ab95f24715295a41ac7d8a21
100755
--- a/contrib/check_GNU_style_lib.py
+++ b/contrib/check_GNU_style_lib.py
@@ -29,6 +29,7 @@
import sys
import re
import unittest
+import json
def import_pip3(*args):
missing=[]
@@ -317,6 +318,33 @@ def check_GNU_style_file(file, format):
else:
print('%d error(s) written to %s file.' % (len(errors), f))
exit(1)
+ elif format == 'json':
+ fn = lambda x: x.error_message
+ i = 1
+ result = []
+ for (k, errors) in groupby(sorted(errors, key = fn), fn):
+ errors = list(errors)
+ entry = {}
+ entry['type'] = i
+ entry['msg'] = k
+ entry['count'] = len(errors)
+ i += 1
+ errlines = []
+ for e in errors:
+ locs = e.error_location ().split(':')
+ errlines.append({ "file": locs[0]
+ , "row": int(locs[1])
+ , "column": int(locs[2])
+ , "err": e.console_error })
+ entry['errors'] = errlines
+ result.append(entry)
+
+ if len(errors) == 0:
+ exit(0)
+ else:
+ json_string = json.dumps(result)
+ print(json_string)
+ exit(1)
else:
assert False
rb18652.patch
Description: rb18652.patch
