[libcamera-devel] [PATCH v2] utils: checkstyle.py: Drop astyle support
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun Dec 27 11:11:16 CET 2020
Hi Laurent,
Thanks for your cleanup work.
On 2020-12-24 15:52:30 +0200, Laurent Pinchart wrote:
> Formatting code using astyle doesn't lead to results as good as with
> clang-format, and doesn't receive much test coverage as most developers
> use clang-format. The code is thus bitrotting. Drop it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I like it, if it's not used drop it :-)
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> Documentation/coding-style.rst | 32 ++++----------------
> utils/checkstyle.py | 54 ++--------------------------------
> 2 files changed, 8 insertions(+), 78 deletions(-)
>
> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> index 71d5c0b2e842..30c1778ed8bf 100644
> --- a/Documentation/coding-style.rst
> +++ b/Documentation/coding-style.rst
> @@ -283,28 +283,12 @@ The 'clang-format' code formatting tool can be used to reformat source files
> with the libcamera coding style, defined in the .clang-format file at the root
> of the source tree.
>
> -Alternatively the 'astyle' tool can also be used, with the following arguments.
> -
> -::
> -
> - --style=linux
> - --indent=force-tab=8
> - --attach-namespaces
> - --attach-extern-c
> - --pad-oper
> - --align-pointer=name
> - --align-reference=name
> - --max-code-length=120
> -
> -Use of astyle is discouraged as clang-format better matches the libcamera coding
> -style.
> -
> -As both astyle and clang-format are code formatters, they operate on full files
> -and output reformatted source code. While they can be used to reformat code
> -before sending patches, it may generate unrelated changes. To avoid this,
> -libcamera provides a 'checkstyle.py' script wrapping the formatting tools to
> -only retain related changes. This should be used to validate modifications
> -before submitting them for review.
> +As clang-format is a code formatter, it operates on full files and outputs
> +reformatted source code. While it can be used to reformat code before sending
> +patches, it may generate unrelated changes. To avoid this, libcamera provides a
> +'checkstyle.py' script wrapping the formatting tools to only retain related
> +changes. This should be used to validate modifications before submitting them
> +for review.
>
> The script operates on one or multiple git commits specified on the command
> line. It does not modify the git tree, the index or the working directory and
> @@ -393,8 +377,4 @@ diff that fixes the issues, on top of the corresponding commit. As the script is
> in early development false positive are expected. The flagged issues should be
> reviewed, but the diff doesn't need to be applied blindly.
>
> -The checkstyle.py script uses clang-format by default if found, and otherwise
> -falls back to astyle. The formatter can be manually selected with the
> -'--formatter' argument.
> -
> Happy hacking, libcamera awaits your patches!
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index e618db937c2b..0e9659e98518 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -4,7 +4,7 @@
> #
> # Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> #
> -# checkstyle.py - A patch style checker script based on astyle or clang-format
> +# checkstyle.py - A patch style checker script based on clang-format
> #
> # TODO:
> #
> @@ -22,22 +22,8 @@ import shutil
> import subprocess
> import sys
>
> -astyle_options = (
> - '-n',
> - '--style=linux',
> - '--indent=force-tab=8',
> - '--attach-namespaces',
> - '--attach-extern-c',
> - '--pad-oper',
> - '--align-pointer=name',
> - '--align-reference=name',
> - '--keep-one-line-blocks',
> - '--max-code-length=120'
> -)
> -
> dependencies = {
> - 'astyle': False,
> - 'clang-format': False,
> + 'clang-format': True,
> 'git': True,
> }
>
> @@ -550,7 +536,6 @@ class ShellChecker(StyleChecker):
> #
>
> class Formatter(metaclass=ClassRegistry):
> - enabled = True
> subclasses = []
>
> def __init__(self):
> @@ -562,15 +547,11 @@ class Formatter(metaclass=ClassRegistry):
> @classmethod
> def formatters(cls, filename):
> for formatter in cls.subclasses:
> - if not cls.enabled:
> - continue
> if formatter.supports(filename):
> yield formatter
>
> @classmethod
> def supports(cls, filename):
> - if not cls.enabled:
> - return False
> for pattern in cls.patterns:
> if fnmatch.fnmatch(os.path.basename(filename), pattern):
> return True
> @@ -580,26 +561,12 @@ class Formatter(metaclass=ClassRegistry):
> def all_patterns(cls):
> patterns = set()
> for formatter in cls.subclasses:
> - if not cls.enabled:
> - continue
> patterns.update(formatter.patterns)
>
> return patterns
>
>
> -class AStyleFormatter(Formatter):
> - enabled = False
> - patterns = ('*.c', '*.cpp', '*.h')
> -
> - @classmethod
> - def format(cls, filename, data):
> - ret = subprocess.run(['astyle', *astyle_options],
> - input=data.encode('utf-8'), stdout=subprocess.PIPE)
> - return ret.stdout.decode('utf-8')
> -
> -
> class CLangFormatter(Formatter):
> - enabled = False
> patterns = ('*.c', '*.cpp', '*.h')
>
> @classmethod
> @@ -854,8 +821,6 @@ def main(argv):
>
> # Parse command line arguments
> parser = argparse.ArgumentParser()
> - parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'],
> - help='Code formatter. Default to clang-format if not specified.')
> parser.add_argument('--staged', '-s', action='store_true',
> help='Include the changes in the index. Defaults to False')
> parser.add_argument('--amend', '-a', action='store_true',
> @@ -873,21 +838,6 @@ def main(argv):
>
> dependencies[command] = found
>
> - if args.formatter:
> - if not args.formatter in dependencies or \
> - not dependencies[args.formatter]:
> - print("Formatter %s not available" % args.formatter)
> - return 1
> - formatter = args.formatter
> - else:
> - if dependencies['clang-format']:
> - CLangFormatter.enabled = True
> - elif dependencies['astyle']:
> - AStyleFormatter.enabled = True
> - else:
> - print("No formatter found, please install clang-format or astyle")
> - return 1
> -
> # Get the top level directory to pass absolute file names to git diff
> # commands, in order to support execution from subdirectories of the git
> # tree.
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list