Improve description layout. Improve coloring and add legend in help.

Convert monkey patching to class inheritance for OptionParser usage in:
commit_queue.py, gclient.py and trychange.py.

Monkey patching confuses the hell out of pylint.

R=jochen@chromium.org
BUG=

Review URL: https://chromiumcodereview.appspot.com/19552004

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@213423 0039d316-1c4b-4281-b951-d872f2087c98

commit_queue.py

@@ -34,8 +34,8 @@ def need_issue(fn):
   def hook(parser, args, *extra_args, **kwargs):
     old_parse_args = parser.parse_args
 
-    def new_parse_args(args):
-      options, args = old_parse_args(args)
+    def new_parse_args(args=None, values=None):
+      options, args = old_parse_args(args, values)
       if not options.issue:
         parser.error('Require --issue')
       obj = rietveld.Rietveld(options.server, options.user, None)
@@ -103,32 +103,28 @@ def CMDclear(parser, args):
 ## Boilerplate code
 
 
-def gen_parser():
-  """Returns an OptionParser instance with default options.
+class OptionParser(optparse.OptionParser):
+  """An OptionParser instance with default options.
 
   It should be then processed with gen_usage() before being used.
   """
-  parser = optparse.OptionParser(version=__version__)
-  # Remove description formatting
-  parser.format_description = (
-      lambda _: parser.description)  # pylint: disable=E1101
-  # Add common parsing.
-  old_parser_args = parser.parse_args
-
-  def Parse(*args, **kwargs):
-    options, args = old_parser_args(*args, **kwargs)
+  def __init__(self, *args, **kwargs):
+    optparse.OptionParser.__init__(self, *args, **kwargs)
+    self.add_option(
+        '-v', '--verbose', action='count', default=0,
+        help='Use multiple times to increase logging level')
+
+  def parse_args(self, args=None, values=None):
+    options, args = optparse.OptionParser.parse_args(self, args, values)
+    levels = [logging.WARNING, logging.INFO, logging.DEBUG]
     logging.basicConfig(
-        level=[logging.WARNING, logging.INFO, logging.DEBUG][
-            min(2, options.verbose)],
+        level=levels[min(len(levels) - 1, options.verbose)],
         format='%(levelname)s %(filename)s(%(lineno)d): %(message)s')
     return options, args
 
-  parser.parse_args = Parse
-
-  parser.add_option(
-      '-v', '--verbose', action='count', default=0,
-      help='Use multiple times to increase logging level')
-  return parser
+  def format_description(self, _):
+    """Removes description formatting."""
+    return self.description.rstrip() + '\n'
 
 
 def Command(name):
@@ -168,7 +164,7 @@ def gen_usage(parser, command):
 def main(args=None):
   # Do it late so all commands are listed.
   # pylint: disable=E1101
-  parser = gen_parser()
+  parser = OptionParser(version=__version__)
   if args is None:
     args = sys.argv[1:]
   if args:

gclient.py

@@ -1379,7 +1379,7 @@ def CMDfetch(parser, args):
 Completely git-specific. Simply runs 'git fetch [args ...]' for each module.
 """
   (options, args) = parser.parse_args(args)
-  return CMDrecurse(Parser(), [
+  return CMDrecurse(OptionParser(), [
       '--jobs=%d' % options.jobs, '--scm=git', 'git', 'fetch'] + args)
 
 
@@ -1735,52 +1735,57 @@ def GenUsage(parser, command):
   parser.epilog = getattr(obj, 'epilog', None)
 
 
-def Parser():
-  """Returns the default parser."""
-  parser = optparse.OptionParser(version='%prog ' + __version__)
-  # some arm boards have issues with parallel sync.
-  if platform.machine().startswith('arm'):
-    jobs = 1
-  else:
-    jobs = max(8, gclient_utils.NumLocalCpus())
-  # cmp: 2013/06/19
-  # Temporary workaround to lower bot-load on SVN server.
-  if os.environ.get('CHROME_HEADLESS') == '1':
-    jobs = 4
+class OptionParser(optparse.OptionParser):
   gclientfile_default = os.environ.get('GCLIENT_FILE', '.gclient')
-  parser.add_option('-j', '--jobs', default=jobs, type='int',
-                    help='Specify how many SCM commands can run in parallel; '
-                          'defaults to number of cpu cores (%default)')
-  parser.add_option('-v', '--verbose', action='count', default=0,
-                    help='Produces additional output for diagnostics. Can be '
-                          'used up to three times for more logging info.')
-  parser.add_option('--gclientfile', dest='config_filename',
-                    default=None,
-                    help='Specify an alternate %s file' % gclientfile_default)
-  parser.add_option('--spec',
-                    default=None,
-                    help='create a gclient file containing the provided '
-                         'string. Due to Cygwin/Python brokenness, it '
-                         'probably can\'t contain any newlines.')
-  parser.add_option('--no-nag-max', default=False, action='store_true',
-                    help='If a subprocess runs for too long without generating'
-                         ' terminal output, generate warnings, but do not kill'
-                         ' the process.')
-  # Integrate standard options processing.
-  old_parser = parser.parse_args
-  def Parse(args):
-    (options, args) = old_parser(args)
-    level = [logging.ERROR, logging.WARNING, logging.INFO, logging.DEBUG][
-        min(options.verbose, 3)]
-    logging.basicConfig(level=level,
+
+  def __init__(self, **kwargs):
+    optparse.OptionParser.__init__(
+        self, version='%prog ' + __version__, **kwargs)
+
+    # Some arm boards have issues with parallel sync.
+    if platform.machine().startswith('arm'):
+      jobs = 1
+    else:
+      jobs = max(8, gclient_utils.NumLocalCpus())
+    # cmp: 2013/06/19
+    # Temporary workaround to lower bot-load on SVN server.
+    if os.environ.get('CHROME_HEADLESS') == '1':
+      jobs = 4
+
+    self.add_option(
+        '-j', '--jobs', default=jobs, type='int',
+        help='Specify how many SCM commands can run in parallel; defaults to '
+             'number of cpu cores (%default)')
+    self.add_option(
+        '-v', '--verbose', action='count', default=0,
+        help='Produces additional output for diagnostics. Can be used up to '
+             'three times for more logging info.')
+    self.add_option(
+        '--gclientfile', dest='config_filename',
+        help='Specify an alternate %s file' % self.gclientfile_default)
+    self.add_option(
+        '--spec',
+        help='create a gclient file containing the provided string. Due to '
+            'Cygwin/Python brokenness, it can\'t contain any newlines.')
+    self.add_option(
+        '--no-nag-max', default=False, action='store_true',
+        help='If a subprocess runs for too long without generating terminal '
+             'output, generate warnings, but do not kill the process.')
+
+  def parse_args(self, args=None, values=None):
+    """Integrates standard options processing."""
+    options, args = optparse.OptionParser.parse_args(self, args, values)
+    levels = [logging.ERROR, logging.WARNING, logging.INFO, logging.DEBUG]
+    logging.basicConfig(
+        level=levels[min(options.verbose, len(levels) - 1)],
         format='%(module)s(%(lineno)d) %(funcName)s:%(message)s')
     if options.config_filename and options.spec:
-      parser.error('Cannot specifiy both --gclientfile and --spec')
+      self.error('Cannot specifiy both --gclientfile and --spec')
     if not options.config_filename:
-      options.config_filename = gclientfile_default
+      options.config_filename = self.gclientfile_default
     options.entries_filename = options.config_filename + '_entries'
     if options.jobs < 1:
-      parser.error('--jobs must be 1 or higher')
+      self.error('--jobs must be 1 or higher')
 
     # These hacks need to die.
     if not hasattr(options, 'revisions'):
@@ -1799,10 +1804,10 @@ def Parser():
     if options.no_nag_max:
       gclient_scm.SCMWrapper.nag_max = None
     return (options, args)
-  parser.parse_args = Parse
-  # We don't want wordwrapping in epilog (usually examples)
-  parser.format_epilog = lambda _: parser.epilog or ''
-  return parser
+
+  def format_epilog(self, _):
+    """Disables wordwrapping in epilog (usually examples)."""
+    return self.epilog or ''
 
 
 def Main(argv):
@@ -1835,7 +1840,7 @@ def Main(argv):
         to_str(fn) for fn in dir(sys.modules[__name__]) if fn.startswith('CMD')
     )
     CMDhelp.usage = '\n\nCommands are:\n' + '\n'.join(cmds)
-    parser = Parser()
+    parser = OptionParser()
     if argv:
       command = Command(argv[0])
       if command:

tests/gclient_test.py

@@ -86,7 +86,7 @@ class GclientTest(trial_dir.TestCase):
     Args:
       |jobs| is the number of parallel jobs simulated.
     """
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, args = parser.parse_args(['--jobs', jobs])
     write(
         '.gclient',
@@ -213,7 +213,7 @@ class GclientTest(trial_dir.TestCase):
     self.assertEquals('proto://host/path@revision', d.url)
 
   def testStr(self):
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args([])
     obj = gclient.GClient('foo', options)
     obj.add_dependencies_and_close(
@@ -265,7 +265,7 @@ class GclientTest(trial_dir.TestCase):
 
     os.chdir(topdir)
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args([])
     options.force = True
     client = gclient.GClient.LoadCurrentConfig(options)
@@ -314,7 +314,7 @@ class GclientTest(trial_dir.TestCase):
 
     os.chdir(topdir)
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args([])
     options.force = True
     client = gclient.GClient.LoadCurrentConfig(options)
@@ -350,7 +350,7 @@ class GclientTest(trial_dir.TestCase):
         '  "baz": { "foo/dir3": "/dir3", },\n'
         '}')
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args(['--jobs', '1'])
     options.deps_os = "unix"
 
@@ -385,7 +385,7 @@ class GclientTest(trial_dir.TestCase):
         '  "baz": { "foo/dir3": "/dir3", },\n'
         '}')
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args(['--jobs', '1'])
     options.deps_os = "unix"
 
@@ -413,7 +413,7 @@ class GclientTest(trial_dir.TestCase):
         '  "unix": { "foo/dir2": "/dir2", },\n'
         '}')
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args(['--jobs', '1'])
     options.deps_os = "unix"
 
@@ -458,7 +458,7 @@ class GclientTest(trial_dir.TestCase):
         '  "jaz": { "bar/jaz": "/jaz", },\n'
         '}')
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args(['--jobs', '1'])
     options.deps_os = 'unix'
 
@@ -499,7 +499,7 @@ class GclientTest(trial_dir.TestCase):
         '  "jaz": { "foo/jaz": "/jaz", },\n'
         '}')
 
-    parser = gclient.Parser()
+    parser = gclient.OptionParser()
     options, _ = parser.parse_args(['--jobs', '1'])
     options.deps_os = 'unix'
 
@@ -552,7 +552,7 @@ class GclientTest(trial_dir.TestCase):
         '  "fuzz": "/fuzz",\n'
         '}')
 
-    options, _ = gclient.Parser().parse_args([])
+    options, _ = gclient.OptionParser().parse_args([])
     obj = gclient.GClient.LoadCurrentConfig(options)
     obj.RunOnDeps('None', [])
     self.assertEquals(

tests/trychange_unittest.py

@@ -47,7 +47,8 @@ class TryChangeUnittest(TryChangeTestsBase):
   def testMembersChanged(self):
     members = [
       'DieWithError', 'EPILOG', 'Escape', 'GIT', 'GetMungedDiff', 'GuessVCS',
-      'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'PrintSuccess',
+      'HELP_STRING', 'InvalidScript', 'NoTryServerAccess', 'OptionParser',
+      'PrintSuccess',
       'RunCommand', 'RunGit', 'SCM', 'SVN', 'TryChange', 'USAGE', 'breakpad',
       'datetime', 'errno', 'fix_encoding', 'gcl', 'gclient_utils', 'gen_parser',
       'getpass', 'json', 'logging', 'optparse', 'os', 'posixpath', 're', 'scm',

trychange.py

@@ -524,11 +524,15 @@ def GetMungedDiff(path_diff, diff):
   return (diff, changed_files)
 
 
+class OptionParser(optparse.OptionParser):
+  def format_epilog(self, _):
+    """Removes epilog formatting."""
+    return self.epilog or ''
+
+
 def gen_parser(prog):
   # Parse argv
-  parser = optparse.OptionParser(usage=USAGE,
-                                 version=__version__,
-                                 prog=prog)
+  parser = OptionParser(usage=USAGE, version=__version__, prog=prog)
   parser.add_option("-v", "--verbose", action="count", default=0,
                     help="Prints debugging infos")
   group = optparse.OptionGroup(parser, "Result and status")
@@ -675,8 +679,6 @@ def TryChange(argv,
   if extra_epilog:
     epilog += extra_epilog
   parser.epilog = epilog
-  # Remove epilog formatting
-  parser.format_epilog = lambda x: parser.epilog
 
   options, args = parser.parse_args(argv)