[libcamera-devel,v3,4/6] checkstyle: Add support for checking style on staged changes
diff mbox series

Message ID 20200118035448.230530-5-nicolas@ndufresne.ca
State Superseded
Headers show
Series
  • Add the ability to do pre-commit style check
Related show

Commit Message

Nicolas Dufresne Jan. 18, 2020, 3:54 a.m. UTC 
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This introduce a new command line "--staged" and a new special type of
commit "StagedChanges". 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@collabora.com>
---
 utils/checkstyle.py | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Jan. 18, 2020, 5:59 p.m. UTC | #1 
Hi Nicolas,

Thank you for the patch.

On Fri, Jan 17, 2020 at 10:54:46PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> This introduce a new command line "--staged" and a new special type of

s/introduce/introduces/

> commit "StagedChanges". 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

s/alias/aliases/

> 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@collabora.com>
> ---
>  utils/checkstyle.py | 36 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index 828605a..1cd5476 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -481,6 +481,25 @@ class Commit:
>                                stdout=subprocess.PIPE).stdout.decode('utf-8')
>  
>  
> +class StagedChanges(Commit):
> +    def __init__(self):
> +        Commit.__init__(self, None)

If you passed '' instead of None to the Commit class you could drop the
custom implementation of get_file.

> +
> +    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)
> @@ -611,7 +630,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:])
>  
> @@ -646,7 +667,18 @@ def main(argv):
>      if top_level is None:
>              return 1
>  
> -    revlist = extract_commits(args.revision_range)
> +    revlist = []
> +    if args.staged:
> +        revlist.append(StagedChanges())
> +
> +    # If not --staged
> +    if len(revlist) == 0:
> +        # And no revisions was passed, then default to HEAD

was/were/

Thank you for the patch.

> +        if not args.revision_range:
> +            args.revision_range = 'HEAD'
> +
> +    if args.revision_range:
> +        revlist += extract_commits(args.revision_range)
>  
>      issues = 0
>      for commit in revlist:
Laurent Pinchart Jan. 18, 2020, 6:21 p.m. UTC | #2 
On Sat, Jan 18, 2020 at 07:59:23PM +0200, Laurent Pinchart wrote:
> On Fri, Jan 17, 2020 at 10:54:46PM -0500, Nicolas Dufresne wrote:
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > This introduce a new command line "--staged" and a new special type of
> 
> s/introduce/introduces/
> 
> > commit "StagedChanges". 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
> 
> s/alias/aliases/
> 
> > 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@collabora.com>
> > ---
> >  utils/checkstyle.py | 36 ++++++++++++++++++++++++++++++++++--
> >  1 file changed, 34 insertions(+), 2 deletions(-)
> > 
> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> > index 828605a..1cd5476 100755
> > --- a/utils/checkstyle.py
> > +++ b/utils/checkstyle.py
> > @@ -481,6 +481,25 @@ class Commit:
> >                                stdout=subprocess.PIPE).stdout.decode('utf-8')
> >  
> >  
> > +class StagedChanges(Commit):
> > +    def __init__(self):
> > +        Commit.__init__(self, None)
> 
> If you passed '' instead of None to the Commit class you could drop the
> custom implementation of get_file.
> 
> > +
> > +    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)
> > @@ -611,7 +630,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:])
> >  
> > @@ -646,7 +667,18 @@ def main(argv):
> >      if top_level is None:
> >              return 1
> >  
> > -    revlist = extract_commits(args.revision_range)
> > +    revlist = []
> > +    if args.staged:
> > +        revlist.append(StagedChanges())
> > +
> > +    # If not --staged
> > +    if len(revlist) == 0:
> > +        # And no revisions was passed, then default to HEAD
> 
> was/were/
> 
> Thank you for the patch.

I must be very tired. I meant

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > +        if not args.revision_range:
> > +            args.revision_range = 'HEAD'
> > +
> > +    if args.revision_range:
> > +        revlist += extract_commits(args.revision_range)
> >  
> >      issues = 0
> >      for commit in revlist:

Patch
diff mbox series 

diff --git a/utils/checkstyle.py b/utils/checkstyle.py
index 828605a..1cd5476 100755
--- a/utils/checkstyle.py
+++ b/utils/checkstyle.py
@@ -481,6 +481,25 @@  class Commit:
                               stdout=subprocess.PIPE).stdout.decode('utf-8')
 
 
+class StagedChanges(Commit):
+    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)
@@ -611,7 +630,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:])
 
@@ -646,7 +667,18 @@  def main(argv):
     if top_level is None:
             return 1
 
-    revlist = extract_commits(args.revision_range)
+    revlist = []
+    if args.staged:
+        revlist.append(StagedChanges())
+
+    # If not --staged
+    if len(revlist) == 0:
+        # And no revisions was passed, then default to HEAD
+        if not args.revision_range:
+            args.revision_range = 'HEAD'
+
+    if args.revision_range:
+        revlist += extract_commits(args.revision_range)
 
     issues = 0
     for commit in revlist: