[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
eric at webkit.org
eric at webkit.org
Wed Apr 7 23:56:58 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 7495c1c02fe8438efd1552e2a0d78e8923af7868
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Nov 25 16:05:31 2009 +0000
2009-11-25 Eric Seidel <eric at webkit.org>
Reviewed by Adam Barth.
Centralize required argument parsing in Command
https://bugs.webkit.org/show_bug.cgi?id=31872
* Scripts/modules/commands/download.py: remove custom required arg message.
* Scripts/modules/commands/upload.py: ditto.
* Scripts/modules/multicommandtool.py:
- Add _parse_required_arguments.
- Pass program name off to OptionParser.
- Add name() for access to tool name.
- Add check_arguments_and_execute and make it return a return code.
- Replace a couple uses of + with %.
* Scripts/modules/multicommandtool_unittest.py: test _parse_required_arguments
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51383 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 31c4fbc..e140f20 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -2,6 +2,23 @@
Reviewed by Adam Barth.
+ Centralize required argument parsing in Command
+ https://bugs.webkit.org/show_bug.cgi?id=31872
+
+ * Scripts/modules/commands/download.py: remove custom required arg message.
+ * Scripts/modules/commands/upload.py: ditto.
+ * Scripts/modules/multicommandtool.py:
+ - Add _parse_required_arguments.
+ - Pass program name off to OptionParser.
+ - Add name() for access to tool name.
+ - Add check_arguments_and_execute and make it return a return code.
+ - Replace a couple uses of + with %.
+ * Scripts/modules/multicommandtool_unittest.py: test _parse_required_arguments
+
+2009-11-25 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Adam Barth.
+
Abstract out capturing stdout/stderr into a new OutputCapture class for re-use among the various unit tests.
https://bugs.webkit.org/show_bug.cgi?id=31870
diff --git a/WebKitTools/Scripts/modules/commands/download.py b/WebKitTools/Scripts/modules/commands/download.py
index bd5edc6..307bcdf 100644
--- a/WebKitTools/Scripts/modules/commands/download.py
+++ b/WebKitTools/Scripts/modules/commands/download.py
@@ -227,9 +227,6 @@ class AbstractPatchProcessingCommand(Command):
return bugs_to_patches
def execute(self, options, args, tool):
- if not args:
- error("%s required" % self.argument_names)
-
self._prepare_to_process(options, args, tool)
patches = self._fetch_list_of_patches_to_process(options, args, tool)
@@ -378,8 +375,6 @@ class Rollout(Command):
log("No bugs were updated or re-opened to reflect this rollout.")
def execute(self, options, args, tool):
- if not args:
- error("REVISION is required, see --help.")
revision = args[0]
bug_id = self._parse_bug_id_from_revision_diff(tool, revision)
if options.complete_rollout:
diff --git a/WebKitTools/Scripts/modules/commands/upload.py b/WebKitTools/Scripts/modules/commands/upload.py
index 7ec4238..1561a55 100644
--- a/WebKitTools/Scripts/modules/commands/upload.py
+++ b/WebKitTools/Scripts/modules/commands/upload.py
@@ -142,9 +142,6 @@ class PostCommits(Command):
return StringIO.StringIO(diff) # add_patch_to_bug expects a file-like object
def execute(self, options, args, tool):
- if not args:
- error("%s argument is required" % self.argument_names)
-
commit_ids = tool.scm().commit_ids_from_commitish_arguments(args)
if len(commit_ids) > 10: # We could lower this limit, 10 is too many for one bug as-is.
error("bugzilla-tool does not support attaching %s at once. Are you sure you passed the right commit range?" % (pluralize("patch", len(commit_ids))))
diff --git a/WebKitTools/Scripts/modules/multicommandtool.py b/WebKitTools/Scripts/modules/multicommandtool.py
index fbebb73..893cba2 100644
--- a/WebKitTools/Scripts/modules/multicommandtool.py
+++ b/WebKitTools/Scripts/modules/multicommandtool.py
@@ -35,6 +35,7 @@ import sys
from optparse import OptionParser, IndentedHelpFormatter, SUPPRESS_USAGE, make_option
+from modules.grammar import pluralize
from modules.logging import log
class Command(object):
@@ -42,10 +43,27 @@ class Command(object):
def __init__(self, help_text, argument_names=None, options=None, requires_local_commits=False):
self.help_text = help_text
self.argument_names = argument_names
+ self.required_arguments = self._parse_required_arguments(argument_names)
self.options = options
self.option_parser = HelpPrintingOptionParser(usage=SUPPRESS_USAGE, add_help_option=False, option_list=self.options)
self.requires_local_commits = requires_local_commits
+ @staticmethod
+ def _parse_required_arguments(argument_names):
+ required_args = []
+ if not argument_names:
+ return required_args
+ split_args = argument_names.split(" ")
+ for argument in split_args:
+ if argument[0] == '[':
+ # For now our parser is rather dumb. Do some minimal validation that
+ # we haven't confused it.
+ if argument[-1] != ']':
+ raise Exception("Failure to parse argument string %s. Argument %s is missing ending ]" % (argument_names, argument))
+ else:
+ required_args.append(argument)
+ return required_args
+
def name_with_arguments(self):
usage_string = self.name
if self.options:
@@ -57,6 +75,20 @@ class Command(object):
def parse_args(self, args):
return self.option_parser.parse_args(args)
+ def check_arguments_and_execute(self, args_after_command_name, tool):
+ (command_options, command_args) = self.parse_args(args_after_command_name)
+
+ if len(command_args) < len(self.required_arguments):
+ log("%s required, %s provided. Provided: %s Required: %s\nSee '%s help %s' for usage." % (
+ pluralize("argument", len(self.required_arguments)),
+ pluralize("argument", len(command_args)),
+ "'%s'" % " ".join(command_args),
+ " ".join(self.required_arguments),
+ tool.name(),
+ self.name))
+ return 1
+ return self.execute(command_options, command_args, tool) or 0
+
def execute(self, options, args, tool):
raise NotImplementedError, "subclasses must implement"
@@ -81,10 +113,10 @@ class HelpPrintingOptionParser(OptionParser):
class MultiCommandTool(object):
- def __init__(self, commands=None):
+ def __init__(self, name=None, commands=None):
# Allow the unit tests to disable command auto-discovery.
self.commands = commands or [cls() for cls in self._find_all_commands() if cls.name]
- self.global_option_parser = HelpPrintingOptionParser(epilog_method=self._help_epilog, usage=self._usage_line())
+ self.global_option_parser = HelpPrintingOptionParser(epilog_method=self._help_epilog, prog=name, usage=self._usage_line())
@classmethod
def _add_all_subclasses(cls, class_to_crawl, seen_classes):
@@ -109,6 +141,9 @@ class MultiCommandTool(object):
help_text += command.option_parser.format_option_help(IndentedHelpFormatter())
return help_text
+ def name(self):
+ return self.global_option_parser.get_prog_name()
+
def _help_epilog(self):
# Only show commands which are relevant to this checkout's SCM system. Might this be confusing to some users?
relevant_commands = filter(self.should_show_command_help, self.commands)
@@ -124,7 +159,7 @@ class MultiCommandTool(object):
(options, args) = self.global_option_parser.parse_args(args)
# We should never hit this because _split_args splits at the first arg without a leading "-".
if args:
- self.global_option_parser.error("Extra arguments before command: " + args)
+ self.global_option_parser.error("Extra arguments before command: %s" % args)
@staticmethod
def _split_args(args):
@@ -176,12 +211,11 @@ class MultiCommandTool(object):
command = self.command_by_name(command_name)
if not command:
- self.global_option_parser.error(command_name + " is not a recognized command")
+ self.global_option_parser.error("%s is not a recognized command" % command_name)
(should_execute, failure_reason) = self.should_execute_command(command)
if not should_execute:
log(failure_reason)
return 0
- (command_options, command_args) = command.parse_args(args_after_command_name)
- return command.execute(command_options, command_args, self)
+ return command.check_arguments_and_execute(args_after_command_name, self)
diff --git a/WebKitTools/Scripts/modules/multicommandtool_unittest.py b/WebKitTools/Scripts/modules/multicommandtool_unittest.py
index a7d59ec..4a62f54 100644
--- a/WebKitTools/Scripts/modules/multicommandtool_unittest.py
+++ b/WebKitTools/Scripts/modules/multicommandtool_unittest.py
@@ -50,10 +50,28 @@ class CommandTest(unittest.TestCase):
command_with_args = TrivialCommand(options=[make_option("--my_option")])
self.assertEqual(command_with_args.name_with_arguments(), "trivial [options]")
+ def test_parse_required_arguments(self):
+ self.assertEqual(Command._parse_required_arguments("ARG1 ARG2"), ["ARG1", "ARG2"])
+ self.assertEqual(Command._parse_required_arguments("[ARG1] [ARG2]"), [])
+ self.assertEqual(Command._parse_required_arguments("[ARG1] ARG2"), ["ARG2"])
+ # Note: We might make our arg parsing smarter in the future and allow this type of arguments string.
+ self.assertRaises(Exception, Command._parse_required_arguments, "[ARG1 ARG2]")
+
+ def test_required_arguments(self):
+ two_required_arguments = TrivialCommand(argument_names="ARG1 ARG2 [ARG3]")
+ capture = OutputCapture()
+ capture.capture_output()
+ exit_code = two_required_arguments.check_arguments_and_execute(["foo"], TrivialTool())
+ (stdout_string, stderr_string) = capture.restore_output()
+ expected_missing_args_error = "2 arguments required, 1 argument provided. Provided: 'foo' Required: ARG1 ARG2\nSee 'trivial-tool help trivial' for usage.\n"
+ self.assertEqual(exit_code, 1)
+ self.assertEqual(stdout_string, "")
+ self.assertEqual(stderr_string, expected_missing_args_error)
+
class TrivialTool(MultiCommandTool):
def __init__(self, commands=None):
- MultiCommandTool.__init__(self, commands)
+ MultiCommandTool.__init__(self, name="trivial-tool", commands=commands)
def path():
return __file__
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list