Re: [gem5-dev] changeset in gem5: style: Fixup strange semantics in hg m5style

2014-08-28 Thread Andreas Sandberg via gem5-dev

Currently, the only documentation is the doc string, which Mercurial
formats into a usage string, in do_check_style. Is there anything
specific you feel is missing from that piece of documentation?

I just did a quick search on the wiki and it seems like the style script
isn't really documented anywhere. Should I add a section about it to the
Coding Style page?

//Andreas

On 26/08/14 17:57, Steve Reinhardt via gem5-dev wrote:

Thanks for this, Andreas!  I've tried to use it in the past and it never
did what I wanted... I assumed it was broken, not that it was working
correctly according to obscure semantics.

Is there any documentation for this beyond the brief 'usage'  lines in the
script?  It would be great if there were (1) an extended help text like
other mercurial commands and (2) something on the wiki to let people know
it exists and how to get it to work with hg.

Thanks,

Steve



On Tue, Aug 26, 2014 at 8:14 AM, Andreas Sandberg via gem5-dev 
gem5-dev@gem5.org wrote:


changeset 62c95c428a3d in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=62c95c428a3d
description:
 style: Fixup strange semantics in hg m5style

 The 'hg m5style' command had some rather strange semantics. When
 called without arguments, it applied the style checker to all added
 files and modified regions of modified files. However, when
providing
 a list of files, it used that list as an ignore list instead of
 specifically checking those files.

 This patch makes the m5style command behave more like other
Mercurial
 commands where the arguments are used to specify which files to
work
 on instead of which files to ignore.

diffstat:

  util/style.py |  114
-
  1 files changed, 64 insertions(+), 50 deletions(-)

diffs (159 lines):

diff -r 933dfb9d8279 -r 62c95c428a3d util/style.py
--- a/util/style.py Tue Aug 26 10:13:45 2014 -0400
+++ b/util/style.py Tue Aug 26 10:14:07 2014 -0400
@@ -1,4 +1,16 @@
  #! /usr/bin/env python
+# Copyright (c) 2014 ARM Limited
+# All rights reserved
+#
+# The license below extends only to copyright in the software and shall
+# not be construed as granting a license to any other intellectual
+# property including but not limited to intellectual property relating
+# to a hardware implementation of the functionality of the software
+# licensed hereunder.  You may use the software subject to the license
+# terms below provided that you ensure that this notice is replicated
+# unmodified and in its entirety in all distributions of the software,
+# modified or unmodified, in source code or in binary form.
+#
  # Copyright (c) 2006 The Regents of The University of Michigan
  # Copyright (c) 2007,2011 The Hewlett-Packard Development Company
  # All rights reserved.
@@ -35,7 +47,7 @@

  from os.path import dirname, join as joinpath
  from itertools import count
-from mercurial import bdiff, mdiff
+from mercurial import bdiff, mdiff, commands

  current_dir = dirname(__file__)
  sys.path.insert(0, current_dir)
@@ -378,27 +390,25 @@
  msg(i, line, 'improper spacing after %s' %
match.group(1))
  bad()

-def do_check_style(hgui, repo, *files, **args):
-check files for proper m5 style guidelines
+
+def do_check_style(hgui, repo, *pats, **opts):
+check files for proper m5 style guidelines
+
+Without an argument, checks all modified and added files for gem5
+coding style violations. A list of files can be specified to limit
+the checker to a subset of the repository. The style rules are
+normally applied on a diff of the repository state (i.e., added
+files are checked in their entirety while only modifications of
+modified files are checked).
+
+The --all option can be specified to include clean files and check
+modified files in their entirety.
+
  from mercurial import mdiff, util

-auto = args.get('auto', False)
-if auto:
-auto = 'f'
-ui = MercurialUI(hgui, hgui.verbose, auto)
-
-if files:
-files = frozenset(files)
-
-def skip(name):
-# We never want to handle symlinks, so always skip them: If the
location
-# pointed to is a directory, skip it. If the location is a file
inside
-# the gem5 directory, it will be checked as a file, so symlink
can be
-# skipped. If the location is a file outside gem5, we don't want
to
-# check it anyway.
-if os.path.islink(name):
-return True
-return files and name in files
+opt_fix_white = opts.get('fix_white', False)
+opt_all = opts.get('all', False)
+ui = MercurialUI(hgui, hgui.verbose, opt_fix_white)

  def prompt(name, func, regions=all_regions):
  result = ui.prompt((a)bort, (i)gnore, or (f)ix?, 'aif', 'a')
@@ -409,39 +419,40 @@

  return False

-modified, added, removed, deleted, unknown, ignore, 

Re: [gem5-dev] changeset in gem5: style: Fixup strange semantics in hg m5style

2014-08-26 Thread Steve Reinhardt via gem5-dev
Thanks for this, Andreas!  I've tried to use it in the past and it never
did what I wanted... I assumed it was broken, not that it was working
correctly according to obscure semantics.

Is there any documentation for this beyond the brief 'usage'  lines in the
script?  It would be great if there were (1) an extended help text like
other mercurial commands and (2) something on the wiki to let people know
it exists and how to get it to work with hg.

Thanks,

Steve



On Tue, Aug 26, 2014 at 8:14 AM, Andreas Sandberg via gem5-dev 
gem5-dev@gem5.org wrote:

 changeset 62c95c428a3d in /z/repo/gem5
 details: http://repo.gem5.org/gem5?cmd=changeset;node=62c95c428a3d
 description:
 style: Fixup strange semantics in hg m5style

 The 'hg m5style' command had some rather strange semantics. When
 called without arguments, it applied the style checker to all added
 files and modified regions of modified files. However, when
 providing
 a list of files, it used that list as an ignore list instead of
 specifically checking those files.

 This patch makes the m5style command behave more like other
 Mercurial
 commands where the arguments are used to specify which files to
 work
 on instead of which files to ignore.

 diffstat:

  util/style.py |  114
 -
  1 files changed, 64 insertions(+), 50 deletions(-)

 diffs (159 lines):

 diff -r 933dfb9d8279 -r 62c95c428a3d util/style.py
 --- a/util/style.py Tue Aug 26 10:13:45 2014 -0400
 +++ b/util/style.py Tue Aug 26 10:14:07 2014 -0400
 @@ -1,4 +1,16 @@
  #! /usr/bin/env python
 +# Copyright (c) 2014 ARM Limited
 +# All rights reserved
 +#
 +# The license below extends only to copyright in the software and shall
 +# not be construed as granting a license to any other intellectual
 +# property including but not limited to intellectual property relating
 +# to a hardware implementation of the functionality of the software
 +# licensed hereunder.  You may use the software subject to the license
 +# terms below provided that you ensure that this notice is replicated
 +# unmodified and in its entirety in all distributions of the software,
 +# modified or unmodified, in source code or in binary form.
 +#
  # Copyright (c) 2006 The Regents of The University of Michigan
  # Copyright (c) 2007,2011 The Hewlett-Packard Development Company
  # All rights reserved.
 @@ -35,7 +47,7 @@

  from os.path import dirname, join as joinpath
  from itertools import count
 -from mercurial import bdiff, mdiff
 +from mercurial import bdiff, mdiff, commands

  current_dir = dirname(__file__)
  sys.path.insert(0, current_dir)
 @@ -378,27 +390,25 @@
  msg(i, line, 'improper spacing after %s' %
 match.group(1))
  bad()

 -def do_check_style(hgui, repo, *files, **args):
 -check files for proper m5 style guidelines
 +
 +def do_check_style(hgui, repo, *pats, **opts):
 +check files for proper m5 style guidelines
 +
 +Without an argument, checks all modified and added files for gem5
 +coding style violations. A list of files can be specified to limit
 +the checker to a subset of the repository. The style rules are
 +normally applied on a diff of the repository state (i.e., added
 +files are checked in their entirety while only modifications of
 +modified files are checked).
 +
 +The --all option can be specified to include clean files and check
 +modified files in their entirety.
 +
  from mercurial import mdiff, util

 -auto = args.get('auto', False)
 -if auto:
 -auto = 'f'
 -ui = MercurialUI(hgui, hgui.verbose, auto)
 -
 -if files:
 -files = frozenset(files)
 -
 -def skip(name):
 -# We never want to handle symlinks, so always skip them: If the
 location
 -# pointed to is a directory, skip it. If the location is a file
 inside
 -# the gem5 directory, it will be checked as a file, so symlink
 can be
 -# skipped. If the location is a file outside gem5, we don't want
 to
 -# check it anyway.
 -if os.path.islink(name):
 -return True
 -return files and name in files
 +opt_fix_white = opts.get('fix_white', False)
 +opt_all = opts.get('all', False)
 +ui = MercurialUI(hgui, hgui.verbose, opt_fix_white)

  def prompt(name, func, regions=all_regions):
  result = ui.prompt((a)bort, (i)gnore, or (f)ix?, 'aif', 'a')
 @@ -409,39 +419,40 @@

  return False

 -modified, added, removed, deleted, unknown, ignore, clean =
 repo.status()
 +
 +# Import the match (repository file name matching helper)
 +# function. Different versions of Mercurial keep it in different
 +# modules and implement them differently.
 +try:
 +from mercurial import scmutil
 +m = scmutil.match(repo[None], pats, opts)
 +except ImportError:
 +from mercurial