[libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for checking style on indexed changes

[Niklas Söderlund]

Ni Nicolas,

Thanks for your work.

On 2020-01-17 14:17:31 -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> This introduce a new command line "--staged" and a new special type of
> commit "Index". It will check the style of changes that are in the
> index, so the changes that would be committed by "git commit".
> 
> "--staged" was chosen to match with "git diff --staged" command line.
> Other valid name could have been "--index" or "--cached". This was
> my personal preference, alias can be added later. Note that we must
> not confuse this with working tree changes, as these changes are not
> picked by "git commit".
> 
> This feature is needed to implement pre-commit hook.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> ---
>  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index fb865c8..8e456cd 100644
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -482,6 +482,25 @@ class Commit:
>                                stdout=subprocess.PIPE).stdout.decode('utf-8')
>  
>  
> +class Index(Commit):

Nit and only for discussion but would it make sens to rename the class 
Staged ?

> +    def __init__(self):
> +        Commit.__init__(self, None)
> +
> +    def get_info(self, top_level):
> +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],
> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')
> +        return "Staged changes", ret.splitlines()
> +
> +    def get_diff(self, top_level, filename):
> +        return subprocess.run(['git', 'diff', '--staged', '--',
> +                               '%s/%s' % (top_level, filename)],
> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')
> +
> +    def get_file(self, filename):
> +        return subprocess.run(['git', 'show', ':%s' % (filename)],
> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')
> +
> +
>  def check_file(top_level, commit, filename):
>      # Extract the line numbers touched by the commit.
>      diff = commit.get_diff(top_level, filename)
> @@ -612,7 +631,9 @@ def main(argv):
>      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('revision_range', type=str, default='HEAD', nargs='?',
> +    parser.add_argument('--staged', '-s', action='store_true',
> +                        help='Include the changes in the index. Defaults to False')
> +    parser.add_argument('revision_range', type=str, default=None, nargs='?',
>                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')
>      args = parser.parse_args(argv[1:])
>  
> @@ -647,7 +668,16 @@ def main(argv):
>      if top_level is None:
>              return 1
>  
> -    revlist = extract_revlist(args.revision_range)
> +    revlist = []
> +    if args.staged:
> +        revlist.append(Index())
> +
> +    # If nothing of --staged or --amend was passed, defaults to HEAD
> +    if len(revlist) == 0 and not args.revision_range:

The 'not' is quiet hard to spot and I had to read it twice to find out 
what as going on, how about

    if args.staged:
        revlist.append(Index())

    if args.revision_range:
        revlist += extract_revlist(args.revision_range)

    if len(revlist) == 0:
        args.revision_range = 'HEAD'

> +        args.revision_range = 'HEAD'
> +
> +    if args.revision_range:
> +        revlist += extract_revlist(args.revision_range)
>  
>      issues = 0
>      for commit in revlist:
> -- 
> 2.24.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund