[{"id":3487,"web_url":"https://patchwork.libcamera.org/comment/3487/","msgid":"<20200117220838.GH1074550@oden.dyn.berto.se>","date":"2020-01-17T22:08:38","subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Ni Nicolas,\n\nThanks for your work.\n\nOn 2020-01-17 14:17:31 -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> \n> This introduce a new command line \"--staged\" and a new special type of\n> commit \"Index\". It will check the style of changes that are in the\n> index, so the changes that would be committed by \"git commit\".\n> \n> \"--staged\" was chosen to match with \"git diff --staged\" command line.\n> Other valid name could have been \"--index\" or \"--cached\". This was\n> my personal preference, alias can be added later. Note that we must\n> not confuse this with working tree changes, as these changes are not\n> picked by \"git commit\".\n> \n> This feature is needed to implement pre-commit hook.\n> \n> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> ---\n>  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--\n>  1 file changed, 32 insertions(+), 2 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index fb865c8..8e456cd 100644\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -482,6 +482,25 @@ class Commit:\n>                                stdout=subprocess.PIPE).stdout.decode('utf-8')\n>  \n>  \n> +class Index(Commit):\n\nNit and only for discussion but would it make sens to rename the class \nStaged ?\n\n> +    def __init__(self):\n> +        Commit.__init__(self, None)\n> +\n> +    def get_info(self, top_level):\n> +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],\n> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +        return \"Staged changes\", ret.splitlines()\n> +\n> +    def get_diff(self, top_level, filename):\n> +        return subprocess.run(['git', 'diff', '--staged', '--',\n> +                               '%s/%s' % (top_level, filename)],\n> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +    def get_file(self, filename):\n> +        return subprocess.run(['git', 'show', ':%s' % (filename)],\n> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +\n>  def check_file(top_level, commit, filename):\n>      # Extract the line numbers touched by the commit.\n>      diff = commit.get_diff(top_level, filename)\n> @@ -612,7 +631,9 @@ def main(argv):\n>      parser = argparse.ArgumentParser()\n>      parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'],\n>                          help='Code formatter. Default to clang-format if not specified.')\n> -    parser.add_argument('revision_range', type=str, default='HEAD', nargs='?',\n> +    parser.add_argument('--staged', '-s', action='store_true',\n> +                        help='Include the changes in the index. Defaults to False')\n> +    parser.add_argument('revision_range', type=str, default=None, nargs='?',\n>                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')\n>      args = parser.parse_args(argv[1:])\n>  \n> @@ -647,7 +668,16 @@ def main(argv):\n>      if top_level is None:\n>              return 1\n>  \n> -    revlist = extract_revlist(args.revision_range)\n> +    revlist = []\n> +    if args.staged:\n> +        revlist.append(Index())\n> +\n> +    # If nothing of --staged or --amend was passed, defaults to HEAD\n> +    if len(revlist) == 0 and not args.revision_range:\n\nThe 'not' is quiet hard to spot and I had to read it twice to find out \nwhat as going on, how about\n\n    if args.staged:\n        revlist.append(Index())\n\n    if args.revision_range:\n        revlist += extract_revlist(args.revision_range)\n\n    if len(revlist) == 0:\n        args.revision_range = 'HEAD'\n\n> +        args.revision_range = 'HEAD'\n> +\n> +    if args.revision_range:\n> +        revlist += extract_revlist(args.revision_range)\n>  \n>      issues = 0\n>      for commit in revlist:\n> -- \n> 2.24.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 950E560782\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:08:39 +0100 (CET)","by mail-lj1-x244.google.com with SMTP id l2so27915680lja.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 14:08:39 -0800 (PST)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tt10sm12835429lji.61.2020.01.17.14.08.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 17 Jan 2020 14:08:38 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=oDG4GJiXakZuoYDjJ4x21giyjAnqExxK4gr4XWlqGHk=;\n\tb=b0mqT4ItMFLkyvMBH6YT4gtKszVG+kx1qTVtiazNypKZ5HblBcCNUFkVfOHn6WQIGm\n\tS3Rtm1Y86sB+4U2EmH3B/x4nNqaEcVP0xmugZnHkKmJUY8SOhRswcNVPpZkiMlq/ITQ9\n\tABeavmIc7lMxbVwI2NAy6NFW0HywsnQHgY6eh2ysMNQsEyER4PsPspQ0b9XDWlueW4wG\n\tWA0yVg6XLTH/otzLpgdTTU4MiR2no6X6eXOo1HbSS1wErRkDIRD+pYwLUy8c5c1SeOO+\n\t34nG9a+OE34I12imf9niHVZyeE/fpY5IEUI+qu4Vj3GaOJMb6loUOkAd5JvAmz6lvbC9\n\t9xWw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=oDG4GJiXakZuoYDjJ4x21giyjAnqExxK4gr4XWlqGHk=;\n\tb=k+4JHo4Uol5M9qI7GGIWYbvxBR0XG7ZK9tHXAKdraB42en2ag0sEBi/tKKIr4KdCNg\n\t+ItqMclWgclbPSU0wSLt2gavcntdIE5ZDcRbYm5YvaQW4EpHQr+AFrs4B5oi64JoL8Dt\n\tab3Vf7ktkKrjh859nbFjYeVsK3fyy1NYAdF/jYRJT4XG3On3ietY447NgxxJN6VaUDWa\n\tpXVzT/lAnuwHHBl3rvP1ZW4bRQ6SuV5NCowDtLG/BCoFZkltR9/qt8CrSfBBY81T4yqd\n\t1Pl4G5D4FlNSX/eH0HslNlorZ2UqA2+c6EwDNk7uFQ+kXmCSVlrtM5cJav2azesGnByj\n\t9nbg==","X-Gm-Message-State":"APjAAAU2m9LUN9Scx9ZCFxHTGOGN8OTHSIhv5lHbkCUwBIro91qCgLLR\n\tRb0w/pPRgaFyDghIkQFm20CgpQ==","X-Google-Smtp-Source":"APXvYqyRLj0CHQqj4YF90QfXt4G7zn6UZ1BBRQyZX12rxhiBua+oYiMW8MLeR/ypG6Lj+rf+E5xW5g==","X-Received":"by 2002:a2e:9596:: with SMTP id w22mr6014146ljh.21.1579298919031;\n\tFri, 17 Jan 2020 14:08:39 -0800 (PST)","Date":"Fri, 17 Jan 2020 23:08:38 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne <nicolas.dufresne@collabora.com>","Message-ID":"<20200117220838.GH1074550@oden.dyn.berto.se>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-5-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200117191733.198897-5-nicolas@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:08:39 -0000"}},{"id":3494,"web_url":"https://patchwork.libcamera.org/comment/3494/","msgid":"<20200117223353.GR5711@pendragon.ideasonboard.com>","date":"2020-01-17T22:33:53","subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Fri, Jan 17, 2020 at 02:17:31PM -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> \n> This introduce a new command line \"--staged\" and a new special type of\n> commit \"Index\". It will check the style of changes that are in the\n> index, so the changes that would be committed by \"git commit\".\n> \n> \"--staged\" was chosen to match with \"git diff --staged\" command line.\n> Other valid name could have been \"--index\" or \"--cached\". This was\n> my personal preference, alias can be added later. Note that we must\n> not confuse this with working tree changes, as these changes are not\n> picked by \"git commit\".\n> \n> This feature is needed to implement pre-commit hook.\n> \n> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> ---\n>  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--\n>  1 file changed, 32 insertions(+), 2 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index fb865c8..8e456cd 100644\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -482,6 +482,25 @@ class Commit:\n>                                stdout=subprocess.PIPE).stdout.decode('utf-8')\n>  \n>  \n> +class Index(Commit):\n\nIndex is a bit generic as a class name to represent a commit that takes\nthe index into account. Maybe StagedChanges ? StagedCommit ?\n\n> +    def __init__(self):\n> +        Commit.__init__(self, None)\n> +\n> +    def get_info(self, top_level):\n> +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],\n> +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +        return \"Staged changes\", ret.splitlines()\n> +\n> +    def get_diff(self, top_level, filename):\n> +        return subprocess.run(['git', 'diff', '--staged', '--',\n> +                               '%s/%s' % (top_level, filename)],\n> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +    def get_file(self, filename):\n> +        return subprocess.run(['git', 'show', ':%s' % (filename)],\n> +                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n> +\n> +\n>  def check_file(top_level, commit, filename):\n>      # Extract the line numbers touched by the commit.\n>      diff = commit.get_diff(top_level, filename)\n> @@ -612,7 +631,9 @@ def main(argv):\n>      parser = argparse.ArgumentParser()\n>      parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'],\n>                          help='Code formatter. Default to clang-format if not specified.')\n> -    parser.add_argument('revision_range', type=str, default='HEAD', nargs='?',\n> +    parser.add_argument('--staged', '-s', action='store_true',\n> +                        help='Include the changes in the index. Defaults to False')\n> +    parser.add_argument('revision_range', type=str, default=None, nargs='?',\n>                          help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.')\n>      args = parser.parse_args(argv[1:])\n>  \n> @@ -647,7 +668,16 @@ def main(argv):\n>      if top_level is None:\n>              return 1\n>  \n> -    revlist = extract_revlist(args.revision_range)\n> +    revlist = []\n> +    if args.staged:\n> +        revlist.append(Index())\n> +\n> +    # If nothing of --staged or --amend was passed, defaults to HEAD\n> +    if len(revlist) == 0 and not args.revision_range:\n> +        args.revision_range = 'HEAD'\n> +\n> +    if args.revision_range:\n> +        revlist += extract_revlist(args.revision_range)\n\nAs mentioned in the review of another patch, I'm wondering if we should\nignore the revision range when --staged is set, it doesn't make much\nsense to check both a list of commits and staged changes. It makes it\nvery tempting to replaced --staged and --amend with @STAGED@ and @AMEND@\n:-) I know I said it may generate conflicts with future version of git,\nbut I think the risk is very small with this syntax, and we can also fix\nthe tool if issues arise.\n\n>  \n>      issues = 0\n>      for commit in revlist:","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 112B360792\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:34:09 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 997B29A1;\n\tFri, 17 Jan 2020 23:34:08 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579300448;\n\tbh=3q4G5QUC785x8ctJWXuynRYQ/w76v+zctmoGkkP/d0I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=k9SFjb9u5IAR/31W0ppqEMFoeiAeWConlB2rhoxndVSXn5HNtGjhntt1dJqTwve15\n\tcoNYvJkHGqZ2ekBEGaU2v9akGCFtkaJDHDoLWu5ZxbNdoMn7cImrvOD78oSWqEfN4r\n\t19TCnmtt/5Yt0Qsvv+oAH2veudhfx4T3ILolaaGE=","Date":"Sat, 18 Jan 2020 00:33:53 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne <nicolas.dufresne@collabora.com>","Message-ID":"<20200117223353.GR5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-5-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200117191733.198897-5-nicolas@ndufresne.ca>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:34:09 -0000"}},{"id":3496,"web_url":"https://patchwork.libcamera.org/comment/3496/","msgid":"<10649ff9065a185d263ed10275e00e40913a5c8a.camel@collabora.com>","date":"2020-01-17T22:39:49","subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le vendredi 17 janvier 2020 à 23:08 +0100, Niklas Söderlund a écrit :\n> Ni Nicolas,\n> \n> Thanks for your work.\n> \n> On 2020-01-17 14:17:31 -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > \n> > This introduce a new command line \"--staged\" and a new special type of\n> > commit \"Index\". It will check the style of changes that are in the\n> > index, so the changes that would be committed by \"git commit\".\n> > \n> > \"--staged\" was chosen to match with \"git diff --staged\" command line.\n> > Other valid name could have been \"--index\" or \"--cached\". This was\n> > my personal preference, alias can be added later. Note that we must\n> > not confuse this with working tree changes, as these changes are not\n> > picked by \"git commit\".\n> > \n> > This feature is needed to implement pre-commit hook.\n> > \n> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > ---\n> >  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--\n> >  1 file changed, 32 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > index fb865c8..8e456cd 100644\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -482,6 +482,25 @@ class Commit:\n> >                                stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> >  \n> >  \n> > +class Index(Commit):\n> \n> Nit and only for discussion but would it make sens to rename the class \n> Staged ?\n\nStage, Index or Cache is three synonyme in git. If you look at the documentation\ntext, it's all about the Index, while in the command line it's all about staged\nor cached (no trace of indexed). So the answer will depend on what you read\nfirst, I wrote this after reading \"git add\" doc as refered by Laurent. I don't\nthink it matters really.\n\n> \n> > +    def __init__(self):\n> > +        Commit.__init__(self, None)\n> > +\n> > +    def get_info(self, top_level):\n> > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],\n> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > +        return \"Staged changes\", ret.splitlines()\n> > +\n> > +    def get_diff(self, top_level, filename):\n> > +        return subprocess.run(['git', 'diff', '--staged', '--',\n> > +                               '%s/%s' % (top_level, filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +    def get_file(self, filename):\n> > +        return subprocess.run(['git', 'show', ':%s' % (filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +\n> >  def check_file(top_level, commit, filename):\n> >      # Extract the line numbers touched by the commit.\n> >      diff = commit.get_diff(top_level, filename)\n> > @@ -612,7 +631,9 @@ def main(argv):\n> >      parser = argparse.ArgumentParser()\n> >      parser.add_argument('--formatter', '-f', type=str, choices=['astyle',\n> > 'clang-format'],\n> >                          help='Code formatter. Default to clang-format if\n> > not specified.')\n> > -    parser.add_argument('revision_range', type=str, default='HEAD',\n> > nargs='?',\n> > +    parser.add_argument('--staged', '-s', action='store_true',\n> > +                        help='Include the changes in the index. Defaults to\n> > False')\n> > +    parser.add_argument('revision_range', type=str, default=None,\n> > nargs='?',\n> >                          help='Revision range (as defined by git rev-parse). \n> > Defaults to HEAD if not specified.')\n> >      args = parser.parse_args(argv[1:])\n> >  \n> > @@ -647,7 +668,16 @@ def main(argv):\n> >      if top_level is None:\n> >              return 1\n> >  \n> > -    revlist = extract_revlist(args.revision_range)\n> > +    revlist = []\n> > +    if args.staged:\n> > +        revlist.append(Index())\n> > +\n> > +    # If nothing of --staged or --amend was passed, defaults to HEAD\n> > +    if len(revlist) == 0 and not args.revision_range:\n> \n> The 'not' is quiet hard to spot and I had to read it twice to find out \n> what as going on, how about\n> \n>     if args.staged:\n>         revlist.append(Index())\n> \n>     if args.revision_range:\n>         revlist += extract_revlist(args.revision_range)\n> \n>     if len(revlist) == 0:\n>         args.revision_range = 'HEAD'\n\nMake sense.\n\n> \n> > +        args.revision_range = 'HEAD'\n> > +\n> > +    if args.revision_range:\n> > +        revlist += extract_revlist(args.revision_range)\n> >  \n> >      issues = 0\n> >      for commit in revlist:\n> > -- \n> > 2.24.1\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09BB160792\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:39:58 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::127])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id 6BF6726B37F;\n\tFri, 17 Jan 2020 22:39:57 +0000 (GMT)"],"Message-ID":"<10649ff9065a185d263ed10275e00e40913a5c8a.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 17:39:49 -0500","In-Reply-To":"<20200117220838.GH1074550@oden.dyn.berto.se>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-5-nicolas@ndufresne.ca>\n\t<20200117220838.GH1074550@oden.dyn.berto.se>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:39:58 -0000"}},{"id":3498,"web_url":"https://patchwork.libcamera.org/comment/3498/","msgid":"<19835e008674353c1e7bab241f492e6998e8d6b3.camel@collabora.com>","date":"2020-01-17T22:47:05","subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le samedi 18 janvier 2020 à 00:33 +0200, Laurent Pinchart a écrit :\n> Hi Nicolas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jan 17, 2020 at 02:17:31PM -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > \n> > This introduce a new command line \"--staged\" and a new special type of\n> > commit \"Index\". It will check the style of changes that are in the\n> > index, so the changes that would be committed by \"git commit\".\n> > \n> > \"--staged\" was chosen to match with \"git diff --staged\" command line.\n> > Other valid name could have been \"--index\" or \"--cached\". This was\n> > my personal preference, alias can be added later. Note that we must\n> > not confuse this with working tree changes, as these changes are not\n> > picked by \"git commit\".\n> > \n> > This feature is needed to implement pre-commit hook.\n> > \n> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > ---\n> >  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--\n> >  1 file changed, 32 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > index fb865c8..8e456cd 100644\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -482,6 +482,25 @@ class Commit:\n> >                                stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> >  \n> >  \n> > +class Index(Commit):\n> \n> Index is a bit generic as a class name to represent a commit that takes\n> the index into account. Maybe StagedChanges ? StagedCommit ?\n\nI can suffix with the base class name if you prefer, but again, Index, Cache or\nStage is all the same, git fault here. So we can spend a year flipping over, it\nwill always be right. Remember that the Index word came from your review.\n\n> \n> > +    def __init__(self):\n> > +        Commit.__init__(self, None)\n> > +\n> > +    def get_info(self, top_level):\n> > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],\n> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > +        return \"Staged changes\", ret.splitlines()\n> > +\n> > +    def get_diff(self, top_level, filename):\n> > +        return subprocess.run(['git', 'diff', '--staged', '--',\n> > +                               '%s/%s' % (top_level, filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +    def get_file(self, filename):\n> > +        return subprocess.run(['git', 'show', ':%s' % (filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +\n> >  def check_file(top_level, commit, filename):\n> >      # Extract the line numbers touched by the commit.\n> >      diff = commit.get_diff(top_level, filename)\n> > @@ -612,7 +631,9 @@ def main(argv):\n> >      parser = argparse.ArgumentParser()\n> >      parser.add_argument('--formatter', '-f', type=str, choices=['astyle',\n> > 'clang-format'],\n> >                          help='Code formatter. Default to clang-format if\n> > not specified.')\n> > -    parser.add_argument('revision_range', type=str, default='HEAD',\n> > nargs='?',\n> > +    parser.add_argument('--staged', '-s', action='store_true',\n> > +                        help='Include the changes in the index. Defaults to\n> > False')\n> > +    parser.add_argument('revision_range', type=str, default=None,\n> > nargs='?',\n> >                          help='Revision range (as defined by git rev-parse). \n> > Defaults to HEAD if not specified.')\n> >      args = parser.parse_args(argv[1:])\n> >  \n> > @@ -647,7 +668,16 @@ def main(argv):\n> >      if top_level is None:\n> >              return 1\n> >  \n> > -    revlist = extract_revlist(args.revision_range)\n> > +    revlist = []\n> > +    if args.staged:\n> > +        revlist.append(Index())\n> > +\n> > +    # If nothing of --staged or --amend was passed, defaults to HEAD\n> > +    if len(revlist) == 0 and not args.revision_range:\n> > +        args.revision_range = 'HEAD'\n> > +\n> > +    if args.revision_range:\n> > +        revlist += extract_revlist(args.revision_range)\n> \n> As mentioned in the review of another patch, I'm wondering if we should\n> ignore the revision range when --staged is set, it doesn't make much\n> sense to check both a list of commits and staged changes. It makes it\n> very tempting to replaced --staged and --amend with @STAGED@ and @AMEND@\n> :-) I know I said it may generate conflicts with future version of git,\n> but I think the risk is very small with this syntax, and we can also fix\n> the tool if issues arise.\n\nWell, doing:\n\n  ./checkstyle.py --stage HEAD~ HEAD~~\n\nWorks perfectly and is unambiguous. It will check three set of changes as\nexpected. I'm don't want to dictate what useful or not, so I'm tempted to just\nkeep it like this (with the suggested fix for readability of course).\n\n> \n> >  \n> >      issues = 0\n> >      for commit in revlist:","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A0B3460792\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 23:47:14 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::127])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id CCC7C2947A0;\n\tFri, 17 Jan 2020 22:47:13 +0000 (GMT)"],"Message-ID":"<19835e008674353c1e7bab241f492e6998e8d6b3.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 17:47:05 -0500","In-Reply-To":"<20200117223353.GR5711@pendragon.ideasonboard.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-5-nicolas@ndufresne.ca>\n\t<20200117223353.GR5711@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 22:47:14 -0000"}},{"id":3502,"web_url":"https://patchwork.libcamera.org/comment/3502/","msgid":"<10f019473f75802fa0d5180514d2936c391ac398.camel@ndufresne.ca>","date":"2020-01-17T23:25:10","subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le vendredi 17 janvier 2020 à 23:08 +0100, Niklas Söderlund a écrit :\n> Ni Nicolas,\n> \n> Thanks for your work.\n> \n> On 2020-01-17 14:17:31 -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > \n> > This introduce a new command line \"--staged\" and a new special type of\n> > commit \"Index\". It will check the style of changes that are in the\n> > index, so the changes that would be committed by \"git commit\".\n> > \n> > \"--staged\" was chosen to match with \"git diff --staged\" command line.\n> > Other valid name could have been \"--index\" or \"--cached\". This was\n> > my personal preference, alias can be added later. Note that we must\n> > not confuse this with working tree changes, as these changes are not\n> > picked by \"git commit\".\n> > \n> > This feature is needed to implement pre-commit hook.\n> > \n> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > ---\n> >  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--\n> >  1 file changed, 32 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > index fb865c8..8e456cd 100644\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -482,6 +482,25 @@ class Commit:\n> >                                stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> >  \n> >  \n> > +class Index(Commit):\n> \n> Nit and only for discussion but would it make sens to rename the class \n> Staged ?\n> \n> > +    def __init__(self):\n> > +        Commit.__init__(self, None)\n> > +\n> > +    def get_info(self, top_level):\n> > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],\n> > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > +        return \"Staged changes\", ret.splitlines()\n> > +\n> > +    def get_diff(self, top_level, filename):\n> > +        return subprocess.run(['git', 'diff', '--staged', '--',\n> > +                               '%s/%s' % (top_level, filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +    def get_file(self, filename):\n> > +        return subprocess.run(['git', 'show', ':%s' % (filename)],\n> > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > 8')\n> > +\n> > +\n> >  def check_file(top_level, commit, filename):\n> >      # Extract the line numbers touched by the commit.\n> >      diff = commit.get_diff(top_level, filename)\n> > @@ -612,7 +631,9 @@ def main(argv):\n> >      parser = argparse.ArgumentParser()\n> >      parser.add_argument('--formatter', '-f', type=str, choices=['astyle',\n> > 'clang-format'],\n> >                          help='Code formatter. Default to clang-format if\n> > not specified.')\n> > -    parser.add_argument('revision_range', type=str, default='HEAD',\n> > nargs='?',\n> > +    parser.add_argument('--staged', '-s', action='store_true',\n> > +                        help='Include the changes in the index. Defaults to\n> > False')\n> > +    parser.add_argument('revision_range', type=str, default=None,\n> > nargs='?',\n> >                          help='Revision range (as defined by git rev-parse). \n> > Defaults to HEAD if not specified.')\n> >      args = parser.parse_args(argv[1:])\n> >  \n> > @@ -647,7 +668,16 @@ def main(argv):\n> >      if top_level is None:\n> >              return 1\n> >  \n> > -    revlist = extract_revlist(args.revision_range)\n> > +    revlist = []\n> > +    if args.staged:\n> > +        revlist.append(Index())\n> > +\n> > +    # If nothing of --staged or --amend was passed, defaults to HEAD\n> > +    if len(revlist) == 0 and not args.revision_range:\n> \n> The 'not' is quiet hard to spot and I had to read it twice to find out \n> what as going on, how about\n> \n>     if args.staged:\n>         revlist.append(Index())\n> \n>     if args.revision_range:\n>         revlist += extract_revlist(args.revision_range)\n> \n>     if len(revlist) == 0:\n>         args.revision_range = 'HEAD'\n\nSorry, that was not logically equivalent. I've done that instead to clarify,\nhopefully it covers your concern.\n\ndiff --git a/utils/checkstyle.py b/utils/checkstyle.py\nindex 17d7c82..9e1f16a 100755\n--- a/utils/checkstyle.py\n+++ b/utils/checkstyle.py\n@@ -695,9 +695,11 @@ def main(argv):\n     if args.amend:\n         revlist.append(Amendment())\n \n-    # If nothing of --staged or --amend was passed, defaults to HEAD\n-    if len(revlist) == 0 and not args.revision_range:\n-        args.revision_range = 'HEAD'\n+    # If none of --staged or --amend was passed\n+    if len(revlist) == 0:\n+        # And no revisions was passed, then default to HEAD\n+        if not args.revision_range:\n+            args.revision_range = 'HEAD'\n \n     if args.revision_range:\n         revlist += extract_revlist(args.revision_range)\n\n> \n> > +        args.revision_range = 'HEAD'\n> > +\n> > +    if args.revision_range:\n> > +        revlist += extract_revlist(args.revision_range)\n> >  \n> >      issues = 0\n> >      for commit in revlist:\n> > -- \n> > 2.24.1\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<nicolas@ndufresne.ca>","Received":["from mail-qk1-x744.google.com (mail-qk1-x744.google.com\n\t[IPv6:2607:f8b0:4864:20::744])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1ECFE60705\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jan 2020 00:25:14 +0100 (CET)","by mail-qk1-x744.google.com with SMTP id x1so24373367qkl.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 17 Jan 2020 15:25:14 -0800 (PST)","from nicolas-tpx395.localdomain ([2610:98:8005::127])\n\tby smtp.gmail.com with ESMTPSA id\n\ts26sm12788739qkj.24.2020.01.17.15.25.12\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 17 Jan 2020 15:25:12 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:user-agent:mime-version:content-transfer-encoding;\n\tbh=qjAVDIl3EXLBctV2KjQGVk9mPYbDwGemz15bLWqXNNw=;\n\tb=oJuwKo/uxyMq+WLRpWspkgpf5yhDf/GFdt35rkI15db2qc0BrUZcv0h9pjPz9UMQCm\n\tGGT1+jEbAa+4GliOsWNhCxaNlKICPM8Z36NP8T42RouTQSP468LROII1vOlhuPbZ+pg+\n\tuD9UXbaYUXdjsEaXIlPrNIPevbbzrAloAEyUsb31uEWoY93xYLeSdh5IfIdjj7mOCvPL\n\t0PNsVMruAF3liYnEI8R8qR2/BKcNw1t6zOCMyfHQytEMDlgzicaxLDnsD1QBab01zvso\n\tYG/LdbGvM9n7EGVqQ2S6ElQSTaFL8VNvRwrNEiipKBTIKjykzLITX3/pBRD3D3gFJGzi\n\t/FVg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=qjAVDIl3EXLBctV2KjQGVk9mPYbDwGemz15bLWqXNNw=;\n\tb=f1FUCoEUG/4T1OE2JPoZq7mktABoLXvomjfHZRZ6+Q3J5XpoG3jdIlp2qtdrN1gocM\n\tBoC+pp9zg0KXjbtze1o2RGJfVCpEFGcs6bPfgyREptNBkNAoHTRJweHZiTL24a4H73Dr\n\tDXamXKVX6llAcwbSwsgULLI07/gjEy5btTMSx40o35z1XEXD0s4O56orP/t7hxDIkJ5x\n\t416T1G5yjLxfwId0rQsVTG8qy7aGDCEwXUdUxHBmbiR5hX7fEV0ihTjM9snzxcl5QWPR\n\tThLvW5V/rGbq3uauGkoBhduLEI8diGaf+36TYXoZMV1gN4nS2eDNjmW4lxfv1bj8cAVg\n\tVxvA==","X-Gm-Message-State":"APjAAAXVjIY5491iQSe/SU2x7w/NyzaLnLyQewVMIqNKHbvdZtiLUv9c\n\tM4tXgww2AQxQtLL7fO42HdDYQ6Pq6CM=","X-Google-Smtp-Source":"APXvYqxSuABF/l8G1ntMOKFLxQaLtuiIBSAuUD8toPswvUoq2YxbP0FcWyD9gEEDCvI9VJrpcPpQgw==","X-Received":"by 2002:a37:5c83:: with SMTP id\n\tq125mr40300642qkb.446.1579303513243; \n\tFri, 17 Jan 2020 15:25:13 -0800 (PST)","Message-ID":"<10f019473f75802fa0d5180514d2936c391ac398.camel@ndufresne.ca>","From":"Nicolas Dufresne <nicolas@ndufresne.ca>","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 18:25:10 -0500","In-Reply-To":"<20200117220838.GH1074550@oden.dyn.berto.se>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-5-nicolas@ndufresne.ca>\n\t<20200117220838.GH1074550@oden.dyn.berto.se>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 23:25:15 -0000"}},{"id":3503,"web_url":"https://patchwork.libcamera.org/comment/3503/","msgid":"<e7b413823b23e1cfafb10cdc490e44efa47a7d91.camel@collabora.com>","date":"2020-01-17T23:27:14","subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le vendredi 17 janvier 2020 à 17:47 -0500, Nicolas Dufresne a écrit :\n> Le samedi 18 janvier 2020 à 00:33 +0200, Laurent Pinchart a écrit :\n> > Hi Nicolas,\n> > \n> > Thank you for the patch.\n> > \n> > On Fri, Jan 17, 2020 at 02:17:31PM -0500, Nicolas Dufresne wrote:\n> > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > > \n> > > This introduce a new command line \"--staged\" and a new special type of\n> > > commit \"Index\". It will check the style of changes that are in the\n> > > index, so the changes that would be committed by \"git commit\".\n> > > \n> > > \"--staged\" was chosen to match with \"git diff --staged\" command line.\n> > > Other valid name could have been \"--index\" or \"--cached\". This was\n> > > my personal preference, alias can be added later. Note that we must\n> > > not confuse this with working tree changes, as these changes are not\n> > > picked by \"git commit\".\n> > > \n> > > This feature is needed to implement pre-commit hook.\n> > > \n> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > > ---\n> > >  utils/checkstyle.py | 34 ++++++++++++++++++++++++++++++++--\n> > >  1 file changed, 32 insertions(+), 2 deletions(-)\n> > > \n> > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > > index fb865c8..8e456cd 100644\n> > > --- a/utils/checkstyle.py\n> > > +++ b/utils/checkstyle.py\n> > > @@ -482,6 +482,25 @@ class Commit:\n> > >                                stdout=subprocess.PIPE).stdout.decode('utf-\n> > > 8')\n> > >  \n> > >  \n> > > +class Index(Commit):\n> > \n> > Index is a bit generic as a class name to represent a commit that takes\n> > the index into account. Maybe StagedChanges ? StagedCommit ?\n> \n> I can suffix with the base class name if you prefer, but again, Index, Cache or\n> Stage is all the same, git fault here. So we can spend a year flipping over, it\n> will always be right. Remember that the Index word came from your review.\n\nChanged to StagedChanges() as per our IRC discussion.\n\n> \n> > > +    def __init__(self):\n> > > +        Commit.__init__(self, None)\n> > > +\n> > > +    def get_info(self, top_level):\n> > > +        ret = subprocess.run(['git', 'diff', '--staged', '--name-only'],\n> > > +                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > > +        return \"Staged changes\", ret.splitlines()\n> > > +\n> > > +    def get_diff(self, top_level, filename):\n> > > +        return subprocess.run(['git', 'diff', '--staged', '--',\n> > > +                               '%s/%s' % (top_level, filename)],\n> > > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > > 8')\n> > > +\n> > > +    def get_file(self, filename):\n> > > +        return subprocess.run(['git', 'show', ':%s' % (filename)],\n> > > +                              stdout=subprocess.PIPE).stdout.decode('utf-\n> > > 8')\n> > > +\n> > > +\n> > >  def check_file(top_level, commit, filename):\n> > >      # Extract the line numbers touched by the commit.\n> > >      diff = commit.get_diff(top_level, filename)\n> > > @@ -612,7 +631,9 @@ def main(argv):\n> > >      parser = argparse.ArgumentParser()\n> > >      parser.add_argument('--formatter', '-f', type=str, choices=['astyle',\n> > > 'clang-format'],\n> > >                          help='Code formatter. Default to clang-format if\n> > > not specified.')\n> > > -    parser.add_argument('revision_range', type=str, default='HEAD',\n> > > nargs='?',\n> > > +    parser.add_argument('--staged', '-s', action='store_true',\n> > > +                        help='Include the changes in the index. Defaults to\n> > > False')\n> > > +    parser.add_argument('revision_range', type=str, default=None,\n> > > nargs='?',\n> > >                          help='Revision range (as defined by git rev-parse). \n> > > Defaults to HEAD if not specified.')\n> > >      args = parser.parse_args(argv[1:])\n> > >  \n> > > @@ -647,7 +668,16 @@ def main(argv):\n> > >      if top_level is None:\n> > >              return 1\n> > >  \n> > > -    revlist = extract_revlist(args.revision_range)\n> > > +    revlist = []\n> > > +    if args.staged:\n> > > +        revlist.append(Index())\n> > > +\n> > > +    # If nothing of --staged or --amend was passed, defaults to HEAD\n> > > +    if len(revlist) == 0 and not args.revision_range:\n> > > +        args.revision_range = 'HEAD'\n> > > +\n> > > +    if args.revision_range:\n> > > +        revlist += extract_revlist(args.revision_range)\n> > \n> > As mentioned in the review of another patch, I'm wondering if we should\n> > ignore the revision range when --staged is set, it doesn't make much\n> > sense to check both a list of commits and staged changes. It makes it\n> > very tempting to replaced --staged and --amend with @STAGED@ and @AMEND@\n> > :-) I know I said it may generate conflicts with future version of git,\n> > but I think the risk is very small with this syntax, and we can also fix\n> > the tool if issues arise.\n> \n> Well, doing:\n> \n>   ./checkstyle.py --stage HEAD~ HEAD~~\n> \n> Works perfectly and is unambiguous. It will check three set of changes as\n> expected. I'm don't want to dictate what useful or not, so I'm tempted to just\n> keep it like this (with the suggested fix for readability of course).\n> \n> > >  \n> > >      issues = 0\n> > >      for commit in revlist:\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6B18C60705\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jan 2020 00:27:24 +0100 (CET)","from nicolas-tpx395.localdomain (unknown [IPv6:2610:98:8005::127])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256\n\tbits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bhuna.collabora.co.uk (Postfix) with ESMTPSA id CF9D52947A8;\n\tFri, 17 Jan 2020 23:27:22 +0000 (GMT)"],"Message-ID":"<e7b413823b23e1cfafb10cdc490e44efa47a7d91.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Fri, 17 Jan 2020 18:27:14 -0500","In-Reply-To":"<19835e008674353c1e7bab241f492e6998e8d6b3.camel@collabora.com>","References":"<20200117191733.198897-1-nicolas@ndufresne.ca>\n\t<20200117191733.198897-5-nicolas@ndufresne.ca>\n\t<20200117223353.GR5711@pendragon.ideasonboard.com>\n\t<19835e008674353c1e7bab241f492e6998e8d6b3.camel@collabora.com>","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.34.3 (3.34.3-1.fc31) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 4/6] checkstyle: Add support for\n\tchecking style on indexed changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 17 Jan 2020 23:27:24 -0000"}}]