From d801a6a6810858dd0ad0897bf07789a49df87c56 Mon Sep 17 00:00:00 2001
From: Andrew Halberstadt <ahalberstadt@mozilla.com>
Date: Fri, 27 Oct 2017 11:06:24 -0400
Subject: [PATCH] Bug 1400503 - [tryselect] Separate 'common_arguments' into
 groups sub-parsers can opt-in to, r=armenzg

This allows subparsers more control over which sets of arguments they need to
implement.  For example, it doesn't make sense for the 'empty' selector to
accept the preset arguments.  Now it can opt-out of those and only implement
the 'push' arguments.

MozReview-Commit-ID: GOfjcFtlfDD

--HG--
extra : rebase_source : 4542adc49e38ff8c450a16363706aba669ad6594
---
 tools/tryselect/cli.py             | 50 ++++++++++++++++++------------
 tools/tryselect/mach_commands.py   | 11 ++++---
 tools/tryselect/selectors/empty.py |  6 ++++
 3 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/tools/tryselect/cli.py b/tools/tryselect/cli.py
index 6671b8968d5a..8449bfee4e96 100644
--- a/tools/tryselect/cli.py
+++ b/tools/tryselect/cli.py
@@ -12,9 +12,8 @@ from argparse import ArgumentParser
 from .templates import all_templates
 
 
-class BaseTryParser(ArgumentParser):
-    name = 'try'
-    common_arguments = [
+COMMON_ARGUMENT_GROUPS = {
+    'push': [
         [['-m', '--message'],
          {'const': 'editor',
           'default': '{msg}',
@@ -29,6 +28,13 @@ class BaseTryParser(ArgumentParser):
                   'specified this command will only print calculated try '
                   'syntax and selection info).',
           }],
+        [['--closed-tree'],
+         {'action': 'store_true',
+          'default': False,
+          'help': 'Push despite a closed try tree',
+          }],
+    ],
+    'preset': [
         [['--save'],
          {'default': None,
           'help': 'Save selection for future use with --preset.',
@@ -42,12 +48,13 @@ class BaseTryParser(ArgumentParser):
           'default': False,
           'help': 'List available preset selections.',
           }],
-        [['--closed-tree'],
-         {'action': 'store_true',
-          'default': False,
-          'help': 'Push despite a closed try tree',
-          }],
-    ]
+    ],
+}
+
+
+class BaseTryParser(ArgumentParser):
+    name = 'try'
+    common_groups = ['push', 'preset']
     arguments = []
     templates = []
 
@@ -58,9 +65,11 @@ class BaseTryParser(ArgumentParser):
         for cli, kwargs in self.arguments:
             group.add_argument(*cli, **kwargs)
 
-        group = self.add_argument_group("common arguments")
-        for cli, kwargs in self.common_arguments:
-            group.add_argument(*cli, **kwargs)
+        for name in self.common_groups:
+            group = self.add_argument_group("{} arguments".format(name))
+            arguments = COMMON_ARGUMENT_GROUPS[name]
+            for cli, kwargs in arguments:
+                group.add_argument(*cli, **kwargs)
 
         group = self.add_argument_group("template arguments")
         self.templates = {t: all_templates[t]() for t in self.templates}
@@ -68,16 +77,17 @@ class BaseTryParser(ArgumentParser):
             template.add_arguments(group)
 
     def validate(self, args):
-        if args.message == 'editor':
-            if 'EDITOR' not in os.environ:
-                self.error("must set the $EDITOR environment variable to use blank --message")
+        if hasattr(args, 'message'):
+            if args.message == 'editor':
+                if 'EDITOR' not in os.environ:
+                    self.error("must set the $EDITOR environment variable to use blank --message")
 
-            with tempfile.NamedTemporaryFile(mode='r') as fh:
-                subprocess.call([os.environ['EDITOR'], fh.name])
-                args.message = fh.read().strip()
+                with tempfile.NamedTemporaryFile(mode='r') as fh:
+                    subprocess.call([os.environ['EDITOR'], fh.name])
+                    args.message = fh.read().strip()
 
-        if '{msg}' not in args.message:
-            args.message = '{}\n\n{}'.format(args.message, '{msg}')
+            if '{msg}' not in args.message:
+                args.message = '{}\n\n{}'.format(args.message, '{msg}')
 
     def parse_known_args(self, *args, **kwargs):
         args, remainder = ArgumentParser.parse_known_args(self, *args, **kwargs)
diff --git a/tools/tryselect/mach_commands.py b/tools/tryselect/mach_commands.py
index 2c5c23dd7019..0390c02250b8 100644
--- a/tools/tryselect/mach_commands.py
+++ b/tools/tryselect/mach_commands.py
@@ -38,13 +38,14 @@ def fuzzy_parser():
     return FuzzyParser()
 
 
-def base_parser():
-    from tryselect.cli import BaseTryParser
-    return BaseTryParser()
+def empty_parser():
+    from tryselect.selectors.empty import EmptyParser
+    return EmptyParser()
 
 
 def generic_parser():
-    parser = base_parser()
+    from tryselect.cli import BaseTryParser
+    parser = BaseTryParser()
     parser.add_argument('argv', nargs=argparse.REMAINDER)
     return parser
 
@@ -149,7 +150,7 @@ class TrySelect(MachCommandBase):
     @SubCommand('try',
                 'empty',
                 description='Push to try without scheduling any tasks.',
-                parser=base_parser)
+                parser=empty_parser)
     def try_empty(self, **kwargs):
         """Push to try, running no builds or tests
 
diff --git a/tools/tryselect/selectors/empty.py b/tools/tryselect/selectors/empty.py
index 0d04539d3411..84c5078ee5b4 100644
--- a/tools/tryselect/selectors/empty.py
+++ b/tools/tryselect/selectors/empty.py
@@ -4,9 +4,15 @@
 
 from __future__ import absolute_import, print_function, unicode_literals
 
+from ..cli import BaseTryParser
 from ..vcs import VCSHelper
 
 
+class SyntaxParser(BaseTryParser):
+    name = 'empty'
+    common_groups = ['push']
+
+
 def run_empty_try(message='{msg}', push=True, **kwargs):
     vcs = VCSHelper.create()
     msg = 'No try selector specified, use "Add New Jobs" to select tasks.'