On Mon, Aug 16, 2021 at 4:14 PM Ida Delphine <[email protected]> wrote: > > Signed-off-by: Ida Delphine <[email protected]> > > This command helps to check for style issues or reformat code given a file > or directory. There are 4 flags: > * -c, --check : Checks for style issues > * -r, --reformat : Reformats the code > * -p, --path : Path to file or dir to be checked or reformatted > * -i, --ignore : Files to be ignored when checking or reformatting > * -v, --verbose : Produces a more detailed output. > --- > misc/rtems-style | 16 +++++ > misc/tools/style.py | 144 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 160 insertions(+) > create mode 100644 misc/rtems-style > create mode 100644 misc/tools/style.py > > diff --git a/misc/rtems-style b/misc/rtems-style > new file mode 100644 > index 0000000..5a3e0e8 > --- /dev/null > +++ b/misc/rtems-style > @@ -0,0 +1,16 @@ > +#! /usr/bin/env python > +
See https://docs.rtems.org/branches/master/eng/coding-file-hdr.html#python-file-template I think this tool should be maybe rtems-c-style? It only focuses on C and C Header files. Maybe, later, we need to include rtems-asm-style and rtems-py-style? Just some thoughts. > +from __future__ import print_function > + > +import sys, os > + > +base = os.path.dirname(os.path.dirname(os.path.abspath(sys.argv[0]))) > +rtems = os.path.join(base, 'share', 'rtems') > +sys.path = sys.path[0:1] + [rtems, base] + sys.path[1:] > + > +try: > + import misc.tools.style > + misc.tools.style.run() > +except ImportError: > + print("Incorrect RTEMS Tools installation", file = sys.stderr) > + sys.exit(1) > \ No newline at end of file > diff --git a/misc/tools/style.py b/misc/tools/style.py > new file mode 100644 > index 0000000..a101146 > --- /dev/null > +++ b/misc/tools/style.py > @@ -0,0 +1,144 @@ > +import argparse > +import os > +import sys > +import re > + > +from rtemstoolkit import execute > +from rtemstoolkit import git > + > + > +def get_command(): > + from rtemstoolkit import check > + > + for version in ['', '8', '9', '10', '11']: > + if check.check_exe(None, 'clang-format' + version): > + command = 'clang-format' + version > + return command > + print("Clang-format not found in your system") > + sys.exit(1) > + > + > +def arguments(): > + parser = argparse.ArgumentParser(description="Tool for code formatting > and style checking \ > + for RTEMS") > + parser.add_argument("-c", "--check", dest="check", help="Check for style > differences and \ > + report the number of issues if found", action="store_true") > + parser.add_argument("-r", "--reformat", dest="reformat", help="Reformat > the file/directory \ > + with any style differences found", action="store_true") > + parser.add_argument("-p", "--path", dest="path", help="The path to be > checked for style issues \ > + or reformatted") > + parser.add_argument("--ignore", dest="ignore", help="Ignore files to be > checked or reformatted") > + parser.add_argument("-v", "--verbose", dest="verbose", help="A more > detailed outline of the \ > + style issues", action='store_true') > + return [parser.parse_args(), parser.print_usage()] > + > + > + Check the style of your Python code. Make sure it conforms https://docs.rtems.org/branches/master/eng/python-devel.html I think we only want two blank lines between function bodies. > +def get_diff(path, ignore_file=None): > + diff = "" > + ex = execute.capture_execution() > + > + > + def clang_to_git_diff(clang_output, path): Why is this indented to here? I think this changes the visibility of it to make it a nested function. Is this necessary/desired here? > + import os > + import tempfile > + > + fd, tmp_path = tempfile.mkstemp() > + try: > + with os.fdopen(fd, 'w') as tmp: I think this is trying to open fd again? Seems wrong, since the fd file descriptor is already open? I'm not sure. > + > + tmp.write(clang_output) > + repo = git.repo(".") I wonder if we need to provide an argument to the location of the git repo instead of assume the script runs from within it? > + return repo.diff(['--no-index', path, tmp_path]) > + > + finally: > + os.remove(tmp_path) Is this all the cleanup needed? what about closing the open fd? > + > + if os.path.isfile(path) == True: > + cmd = get_command() + " --style=file " + path > + output_clang = ex.command(command=cmd, shell=True) > + output_clang = output_clang[2] > + diff = clang_to_git_diff(output_clang, path) > + else: > + onlyfiles = [f for f in os.listdir(path)] > + for file in onlyfiles: > + file = os.path.join(path, file) > + > + if ignore_file is not None and file == ignore_file: is there only one ignore_file? Shouldn't there be a set of them? I'm not sure how this works. > + continue > + > + if file.endswith('.c') or file.endswith('.h'): > + cmd = get_command() + " --style=file " + file > + output_clang = ex.command(command=cmd, shell=True) > + output_clang = output_clang[2] > + diff += clang_to_git_diff(output_clang, os.path.join(path, > file)) > + return diff > + > + > +def color_text(string, col, style=1): > + return f"\033[{style};{col};{col}m{string}\033[0;0m" > + I think we need to make sure colored text works well cross-platform. It might be safer to avoid color in here. > + > +def handle_errors(path, output, verbose=False,): extra comma here? > + if len(output) < 1: > + print("Everything is clean - No style issues") > + else: > + print(color_text("Checking for style differences...", 34, style=3)) > + > + out = output.split('\n') > + files = [] > + num_diff = 0 > + for line in out: > + > + if line.startswith("---"): > + file = str(re.sub('^---\s[ab]', '', line)) > + files.append(file) > + > + elif line.startswith('+'): > + num_diff += 1 > + if verbose == True: > + print(color_text(line, 34)) > + continue > + if verbose == True: > + print(line) > + > + print(color_text("\nFiles affected:", 33)) > + > + for file in files: > + print(file) > + > + message = "\nStyleWarning: You have about a total of " + > str(num_diff) + \ > + " style issue(s) in the " + str(len(files)) + " file(s) > listed above" > + print(color_text(message, 31)) > + > + > +def reformat(path, output): > + if len(output) > 0: > + onlyfiles = [f for f in os.listdir(path)] > + for file in onlyfiles: > + if file.endswith('.c') or file.endswith('.h'): > + cmd = get_command() + " -i -style=file " + > os.path.join(path, file) > + ex = execute.capture_execution() > + ex.command(command=cmd, shell=True) > + else: > + print("Everything is clean. No style issues") > + return 0 > + > + > +def run(args = sys.argv): > + args = arguments() > + if os.path.exists(args[0].path) == False: > + print("Please enter a correct path!!") It can be helpful to show what was the incorrect path input to help someone know what they did wrong. > + sys.exit(1) Why exiting instead of returning? > + output = get_diff(args[0].path) > + if args[0].check == True: > + handle_errors(args[0].path, output, verbose=args[0].verbose) > + elif args[0].reformat == True: > + reformat(args[0].path, output) > + else: > + args[1] ? > + sys.exit(1) > + > + > +if __name__ == "__main__": > + run() > -- > 2.25.1 > > _______________________________________________ > devel mailing list > [email protected] > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list [email protected] http://lists.rtems.org/mailman/listinfo/devel
