Re: [PATCH rtems-tools 2/2] misc: Add rtems-style command

2021-08-25 Thread Gedare Bloom
On Mon, Aug 16, 2021 at 4:14 PM Ida Delphine  wrote:
>
> Signed-off-by: Ida Delphine 
>
> 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 000..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 000..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
> +
> +  

[PATCH rtems-tools 2/2] misc: Add rtems-style command

2021-08-16 Thread Ida Delphine
Signed-off-by: Ida Delphine 

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 000..5a3e0e8
--- /dev/null
+++ b/misc/rtems-style
@@ -0,0 +1,16 @@
+#! /usr/bin/env python
+
+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 000..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()]
+
+
+
+def get_diff(path, ignore_file=None):
+diff = ""
+ex = execute.capture_execution()
+
+
+def clang_to_git_diff(clang_output, path):
+import os
+import tempfile
+
+fd, tmp_path = tempfile.mkstemp()
+try:
+with os.fdopen(fd, 'w') as tmp:
+
+tmp.write(clang_output)
+repo = git.repo(".")
+return repo.diff(['--no-index', path, tmp_path])
+
+finally:
+os.remove(tmp_path)
+
+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:
+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"
+
+
+def handle_errors(path, output, verbose=False,):
+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:
+