[{"id":3507,"web_url":"https://patchwork.libcamera.org/comment/3507/","msgid":"<20200118175923.GC5095@pendragon.ideasonboard.com>","date":"2020-01-18T17:59:23","subject":"Re: [libcamera-devel] [PATCH v3 4/6] checkstyle: Add support for\n\tchecking style on staged 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 10:54:46PM -0500, Nicolas Dufresne wrote:\n> From: Nicolas Dufresne \n> \n> This introduce a new command line \"--staged\" and a new special type of\n\ns/introduce/introduces/\n\n> commit \"StagedChanges\". It will check the style of changes that are in\n> the 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\ns/alias/aliases/\n\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 \n> ---\n> utils/checkstyle.py | 36 ++++++++++++++++++++++++++++++++++--\n> 1 file changed, 34 insertions(+), 2 deletions(-)\n> \n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index 828605a..1cd5476 100755\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -481,6 +481,25 @@ class Commit:\n> stdout=subprocess.PIPE).stdout.decode('utf-8')\n> \n> \n> +class StagedChanges(Commit):\n> + def __init__(self):\n> + Commit.__init__(self, None)\n\nIf you passed '' instead of None to the Commit class you could drop the\ncustom implementation of get_file.\n\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> @@ -611,7 +630,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> @@ -646,7 +667,18 @@ def main(argv):\n> if top_level is None:\n> return 1\n> \n> - revlist = extract_commits(args.revision_range)\n> + revlist = []\n> + if args.staged:\n> + revlist.append(StagedChanges())\n> +\n> + # If not --staged\n> + if len(revlist) == 0:\n> + # And no revisions was passed, then default to HEAD\n\nwas/were/\n\nThank you for the patch.\n\n> + if not args.revision_range:\n> + args.revision_range = 'HEAD'\n> +\n> + if args.revision_range:\n> + revlist += extract_commits(args.revision_range)\n> \n> issues = 0\n> for commit in revlist:","headers":{"Return-Path":"","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 1C5DC60455\n\tfor ;\n\tSat, 18 Jan 2020 18:59:38 +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 92C9297A;\n\tSat, 18 Jan 2020 18:59:37 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579370377;\n\tbh=0kYavWndHuMqhZrUUGiHe1zasW8s3LogRnGVn+g3pYw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Pw1QspIiVGbKCP2vdKuDY+OgSlR3AmLECebj0ky7GD5aiAm5SmD/gcc7PTnAacIBM\n\t7wcEmt0/E/gY/p8b2JVEfjFN5tNE+goBTiqGOsVNm7fSwEh1DURaAjQRS0F/XvoSEl\n\tQZK1evem3EtwBve0ht1JsJuxBB5hczTLEi0DIBVs=","Date":"Sat, 18 Jan 2020 19:59:23 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne ","Message-ID":"<20200118175923.GC5095@pendragon.ideasonboard.com>","References":"<20200118035448.230530-1-nicolas@ndufresne.ca>\n\t<20200118035448.230530-5-nicolas@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200118035448.230530-5-nicolas@ndufresne.ca>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] checkstyle: Add support for\n\tchecking style on staged changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 18 Jan 2020 17:59:38 -0000"}},{"id":3511,"web_url":"https://patchwork.libcamera.org/comment/3511/","msgid":"<20200118182124.GG5095@pendragon.ideasonboard.com>","date":"2020-01-18T18:21:24","subject":"Re: [libcamera-devel] [PATCH v3 4/6] checkstyle: Add support for\n\tchecking style on staged changes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sat, Jan 18, 2020 at 07:59:23PM +0200, Laurent Pinchart wrote:\n> On Fri, Jan 17, 2020 at 10:54:46PM -0500, Nicolas Dufresne wrote:\n> > From: Nicolas Dufresne \n> > \n> > This introduce a new command line \"--staged\" and a new special type of\n> \n> s/introduce/introduces/\n> \n> > commit \"StagedChanges\". It will check the style of changes that are in\n> > the 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> \n> s/alias/aliases/\n> \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 \n> > ---\n> > utils/checkstyle.py | 36 ++++++++++++++++++++++++++++++++++--\n> > 1 file changed, 34 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> > index 828605a..1cd5476 100755\n> > --- a/utils/checkstyle.py\n> > +++ b/utils/checkstyle.py\n> > @@ -481,6 +481,25 @@ class Commit:\n> > stdout=subprocess.PIPE).stdout.decode('utf-8')\n> > \n> > \n> > +class StagedChanges(Commit):\n> > + def __init__(self):\n> > + Commit.__init__(self, None)\n> \n> If you passed '' instead of None to the Commit class you could drop the\n> custom implementation of get_file.\n> \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> > @@ -611,7 +630,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> > @@ -646,7 +667,18 @@ def main(argv):\n> > if top_level is None:\n> > return 1\n> > \n> > - revlist = extract_commits(args.revision_range)\n> > + revlist = []\n> > + if args.staged:\n> > + revlist.append(StagedChanges())\n> > +\n> > + # If not --staged\n> > + if len(revlist) == 0:\n> > + # And no revisions was passed, then default to HEAD\n> \n> was/were/\n> \n> Thank you for the patch.\n\nI must be very tired. I meant\n\nReviewed-by: Laurent Pinchart \n\n> > + if not args.revision_range:\n> > + args.revision_range = 'HEAD'\n> > +\n> > + if args.revision_range:\n> > + revlist += extract_commits(args.revision_range)\n> > \n> > issues = 0\n> > for commit in revlist:","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 683DF60455\n\tfor ;\n\tSat, 18 Jan 2020 19:21:39 +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 DC7EB97A;\n\tSat, 18 Jan 2020 19:21:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1579371699;\n\tbh=pUha6izuPSP3f38guUxxXgqsGhhX6Vo8WA1bYb0HDg0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H1upu3Skuz3K2uAqBHWToxk+n06w7lmo3xnEGih0pzY5pe7oJ0z7O+2Ht/DXlDdap\n\tF1u0UwWORwUjSZZAyELA1rqTJf1OH26U5duulGXPlenQZcvj64GQwiVtwPXvkX8AW2\n\t9FKY6JcjGVYO375IniQVuvLyOX40owVszLfKHEz8=","Date":"Sat, 18 Jan 2020 20:21:24 +0200","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Cc":"libcamera-devel@lists.libcamera.org,\n\tNicolas Dufresne ","Message-ID":"<20200118182124.GG5095@pendragon.ideasonboard.com>","References":"<20200118035448.230530-1-nicolas@ndufresne.ca>\n\t<20200118035448.230530-5-nicolas@ndufresne.ca>\n\t<20200118175923.GC5095@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200118175923.GC5095@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/6] checkstyle: Add support for\n\tchecking style on staged changes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 18 Jan 2020 18:21:39 -0000"}}]