2009-11-21 Eric Seidel <eric@webkit.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Nov 2009 21:34:49 +0000 (21:34 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Nov 2009 21:34:49 +0000 (21:34 +0000)
        Reviewed by Adam Barth.

        bugzilla-tool --help spews way too much text
        https://bugs.webkit.org/show_bug.cgi?id=31771

        * Scripts/bugzilla-tool:
         - Remove self.cached_scm initialization hack.
        * Scripts/modules/buildbot.py:
         - Make default_host accessible to callers.
        * Scripts/modules/commands/download.py:
         - Phrase help for all commands consistently and remove spurious help text punctuation.
        * Scripts/modules/commands/queries.py: Ditto.
        * Scripts/modules/commands/queues.py: Ditto.
        * Scripts/modules/commands/upload.py: Ditto.
        * Scripts/modules/multicommandtool.py:
         - Add HelpPrintingOptionParser.format_epilog to replace
           NonWrappingEpilogIndentedHelpFormatter and allow us to lazily initialize
           per-command help (thus removing the need for the cached_scm hack in BugzillaTool).
         - Make --help only show a list of commands like "svn help" and "git help" do --
           previously --help was listing all commands and options.
         - Sort list of commands alphabetically.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@51287 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebKitTools/ChangeLog
WebKitTools/Scripts/bugzilla-tool
WebKitTools/Scripts/modules/buildbot.py
WebKitTools/Scripts/modules/commands/download.py
WebKitTools/Scripts/modules/commands/queries.py
WebKitTools/Scripts/modules/commands/queues.py
WebKitTools/Scripts/modules/commands/upload.py
WebKitTools/Scripts/modules/multicommandtool.py

index c793a5edbfeef0cb7c610d0f615ab4dd9e6428a7..18e9fbdce1fc4d99561aa6eafd8118ca2dc3b1c4 100644 (file)
@@ -1,3 +1,27 @@
+2009-11-21  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        bugzilla-tool --help spews way too much text
+        https://bugs.webkit.org/show_bug.cgi?id=31771
+
+        * Scripts/bugzilla-tool:
+         - Remove self.cached_scm initialization hack.
+        * Scripts/modules/buildbot.py:
+         - Make default_host accessible to callers.
+        * Scripts/modules/commands/download.py:
+         - Phrase help for all commands consistently and remove spurious help text punctuation.
+        * Scripts/modules/commands/queries.py: Ditto.
+        * Scripts/modules/commands/queues.py: Ditto.
+        * Scripts/modules/commands/upload.py: Ditto.
+        * Scripts/modules/multicommandtool.py:
+         - Add HelpPrintingOptionParser.format_epilog to replace
+           NonWrappingEpilogIndentedHelpFormatter and allow us to lazily initialize
+           per-command help (thus removing the need for the cached_scm hack in BugzillaTool).
+         - Make --help only show a list of commands like "svn help" and "git help" do --
+           previously --help was listing all commands and options.
+         - Sort list of commands alphabetically.
+
 2009-11-21  Eric Seidel  <eric@webkit.org>
 
         No review.  Fixing a typo from the previous patch for bug 31767.
index 3a48c5b69e433b003b063a254f8e0c7e39131a6c..fda3548bc426a326b94759cf8028442d51458585 100755 (executable)
@@ -40,16 +40,12 @@ from modules.logging import log
 
 class BugzillaTool(MultiCommandTool):
     def __init__(self):
-        # HACK: Set self.cached_scm before calling MultiCommandTool.__init__ because
-        # MultiCommandTool._commands_usage() will call self.should_show_command_help which uses scm().
-        # This hack can be removed by overriding usage() printing in HelpPrintingOptionParser
-        # so that we don't need to create 'epilog' before constructing HelpPrintingOptionParser.
-        self.cached_scm = None
         MultiCommandTool.__init__(self)
         self.global_option_parser.add_option("--dry-run", action="callback", help="do not touch remote servers", callback=self.dry_run_callback)
 
         self.bugs = Bugzilla()
         self.buildbot = BuildBot()
+        self.cached_scm = None
 
     def dry_run_callback(self, option, opt, value, parser):
         self.scm().dryrun = True
index e948d8cf1232a0b47bbe67ffa36145ccaf138636..548cad89fdf571389a9b5f40d7d457f8040c3d07 100644 (file)
@@ -39,7 +39,8 @@ from modules.logging import log
 from .BeautifulSoup import BeautifulSoup
 
 class BuildBot:
-    def __init__(self, host="build.webkit.org"):
+    default_host = "build.webkit.org"
+    def __init__(self, host=default_host):
         self.buildbot_host = host
         self.buildbot_server_url = "http://%s/" % self.buildbot_host
         
index 1e194593649efcd7bc90be970a9196c794c139ad..bd5edc6cb8769721bcc2a7ef092068db559fac2e 100644 (file)
@@ -71,7 +71,7 @@ class Build(Command):
         options = WebKitLandingScripts.cleaning_options()
         options += WebKitLandingScripts.build_options()
         options += WebKitLandingScripts.land_options()
-        Command.__init__(self, "Updates working copy and does a build.", "", options)
+        Command.__init__(self, "Update working copy and build", "", options)
 
     def execute(self, options, args, tool):
         sequence = BuildSequence(options, tool)
@@ -82,7 +82,7 @@ class ApplyAttachment(Command):
     name = "apply-attachment"
     def __init__(self):
         options = WebKitApplyingScripts.apply_options() + WebKitLandingScripts.cleaning_options()
-        Command.__init__(self, "Applies an attachment to the local working directory.", "ATTACHMENT_ID", options=options)
+        Command.__init__(self, "Apply an attachment to the local working directory", "ATTACHMENT_ID", options=options)
 
     def execute(self, options, args, tool):
         WebKitApplyingScripts.setup_for_patch_apply(tool.scm(), options)
@@ -95,7 +95,7 @@ class ApplyPatches(Command):
     name = "apply-patches"
     def __init__(self):
         options = WebKitApplyingScripts.apply_options() + WebKitLandingScripts.cleaning_options()
-        Command.__init__(self, "Applies all patches on a bug to the local working directory.", "BUGID", options=options)
+        Command.__init__(self, "Apply reviewed patches from provided bugs to the local working directory", "BUGID", options=options)
 
     def execute(self, options, args, tool):
         WebKitApplyingScripts.setup_for_patch_apply(tool.scm(), options)
@@ -165,7 +165,7 @@ class LandDiff(Command):
         ]
         options += WebKitLandingScripts.build_options()
         options += WebKitLandingScripts.land_options()
-        Command.__init__(self, "Lands the current working directory diff and updates the bug if provided.", "[BUGID]", options=options)
+        Command.__init__(self, "Land the current working directory diff and updates the associated bug if any", "[BUGID]", options=options)
 
     def guess_reviewer_from_bug(self, bugs, bug_id):
         patches = bugs.fetch_reviewed_patches_from_bug(bug_id)
@@ -261,7 +261,7 @@ class CheckStyle(AbstractPatchProcessingCommand):
     def __init__(self):
         options = WebKitLandingScripts.cleaning_options()
         options += WebKitLandingScripts.build_options()
-        AbstractPatchProcessingCommand.__init__(self, "Runs check-webkit-style on the specified attachments.", "ATTACHMENT_ID [ATTACHMENT_IDS]", options)
+        AbstractPatchProcessingCommand.__init__(self, "Run check-webkit-style on the specified attachments", "ATTACHMENT_ID [ATTACHMENT_IDS]", options)
 
     def _fetch_list_of_patches_to_process(self, options, args, tool):
         return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
@@ -290,7 +290,7 @@ class BuildAttachment(AbstractPatchProcessingCommand):
     def __init__(self):
         options = WebKitLandingScripts.cleaning_options()
         options += WebKitLandingScripts.build_options()
-        AbstractPatchProcessingCommand.__init__(self, "Builds patches from bugzilla", "ATTACHMENT_ID [ATTACHMENT_IDS]", options)
+        AbstractPatchProcessingCommand.__init__(self, "Apply and build patches from bugzilla", "ATTACHMENT_ID [ATTACHMENT_IDS]", options)
 
     def _fetch_list_of_patches_to_process(self, options, args, tool):
         return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
@@ -323,7 +323,7 @@ class AbstractPatchLandingCommand(AbstractPatchProcessingCommand):
 class LandAttachment(AbstractPatchLandingCommand):
     name = "land-attachment"
     def __init__(self):
-        AbstractPatchLandingCommand.__init__(self, "Lands patches from bugzilla, optionally building and testing them first", "ATTACHMENT_ID [ATTACHMENT_IDS]")
+        AbstractPatchLandingCommand.__init__(self, "Land patches from bugzilla, optionally building and testing them first", "ATTACHMENT_ID [ATTACHMENT_IDS]")
 
     def _fetch_list_of_patches_to_process(self, options, args, tool):
         return map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args)
@@ -332,7 +332,7 @@ class LandAttachment(AbstractPatchLandingCommand):
 class LandPatches(AbstractPatchLandingCommand):
     name = "land-patches"
     def __init__(self):
-        AbstractPatchLandingCommand.__init__(self, "Lands all patches on the given bugs, optionally building and testing them first", "BUGID [BUGIDS]")
+        AbstractPatchLandingCommand.__init__(self, "Land all patches on the given bugs, optionally building and testing them first", "BUGID [BUGIDS]")
 
     def _fetch_list_of_patches_to_process(self, options, args, tool):
         all_patches = []
@@ -349,8 +349,8 @@ class Rollout(Command):
         options = WebKitLandingScripts.cleaning_options()
         options += WebKitLandingScripts.build_options()
         options += WebKitLandingScripts.land_options()
-        options.append(make_option("--complete-rollout", action="store_true", dest="complete_rollout", help="Experimental support for complete unsupervised rollouts, including re-opening the bug.  Not recommended."))
-        Command.__init__(self, "Reverts the given revision and commits the revert and re-opens the original bug.", "REVISION [BUGID]", options=options)
+        options.append(make_option("--complete-rollout", action="store_true", dest="complete_rollout", help="Commit the revert and re-open the original bug."))
+        Command.__init__(self, "Revert the given revision in the working copy and optionally commit the revert and re-open the original bug", "REVISION [BUGID]", options=options)
 
     @staticmethod
     def _create_changelogs_for_revert(scm, revision):
index 78cac6d5749ce15ffe1035ccc6da595430028352..b30481154e023ba1af5714a9e9fa97b6bed72d54 100644 (file)
@@ -57,7 +57,7 @@ from modules.workqueue import WorkQueue, WorkQueueDelegate
 class BugsToCommit(Command):
     name = "bugs-to-commit"
     def __init__(self):
-        Command.__init__(self, "Bugs in the commit queue")
+        Command.__init__(self, "List bugs in the commit-queue")
 
     def execute(self, options, args, tool):
         bug_ids = tool.bugs.fetch_bug_ids_from_commit_queue()
@@ -68,7 +68,7 @@ class BugsToCommit(Command):
 class PatchesToCommit(Command):
     name = "patches-to-commit"
     def __init__(self):
-        Command.__init__(self, "Patches in the commit queue")
+        Command.__init__(self, "List patches in the commit-queue")
 
     def execute(self, options, args, tool):
         patches = tool.bugs.fetch_patches_from_commit_queue()
@@ -80,7 +80,7 @@ class PatchesToCommit(Command):
 class ReviewedPatches(Command):
     name = "reviewed-patches"
     def __init__(self):
-        Command.__init__(self, "r+'d patches on a bug", "BUGID")
+        Command.__init__(self, "List r+'d patches on a bug", "BUGID")
 
     def execute(self, options, args, tool):
         bug_id = args[0]
@@ -92,7 +92,7 @@ class ReviewedPatches(Command):
 class TreeStatus(Command):
     name = "tree-status"
     def __init__(self):
-        Command.__init__(self, "Print out the status of the webkit builders.")
+        Command.__init__(self, "Print the status of the %s buildbots" % BuildBot.default_host)
 
     def execute(self, options, args, tool):
         for builder in tool.buildbot.builder_statuses():
index 742affba06e7b9d32342fa3f5ac5e8603710c4ab..1a5fa3361e67e5d3741e65364f220dbc01890946 100644 (file)
@@ -60,7 +60,7 @@ class AbstractQueue(Command, WorkQueueDelegate):
             make_option("--no-confirm", action="store_false", dest="confirm", default=True, help="Do not ask the user for confirmation before running the queue.  Dangerous!"),
             make_option("--status-host", action="store", type="string", dest="status_host", default=StatusBot.default_host, help="Hostname (e.g. localhost or commit.webkit.org) where status updates should be posted."),
         ]
-        Command.__init__(self, "Run the %s." % self.name, options=options)
+        Command.__init__(self, "Run the %s" % self.name, options=options)
 
     def queue_log_path(self):
         return "%s.log" % self.name
index 42e23d3ab9c94e081c59e4d15af28e5092cda864..7ec42385106919a65ff8f92f56c9923049ac57f0 100644 (file)
@@ -57,7 +57,7 @@ from modules.workqueue import WorkQueue, WorkQueueDelegate
 class CommitMessageForCurrentDiff(Command):
     name = "commit-message"
     def __init__(self):
-        Command.__init__(self, "Prints a commit message suitable for the uncommitted changes.")
+        Command.__init__(self, "Print a commit message suitable for the uncommitted changes")
 
     def execute(self, options, args, tool):
         os.chdir(tool.scm().checkout_root)
@@ -67,7 +67,7 @@ class CommitMessageForCurrentDiff(Command):
 class ObsoleteAttachments(Command):
     name = "obsolete-attachments"
     def __init__(self):
-        Command.__init__(self, "Marks all attachments on a bug as obsolete.", "BUGID")
+        Command.__init__(self, "Mark all attachments on a bug as obsolete", "BUGID")
 
     def execute(self, options, args, tool):
         bug_id = args[0]
@@ -84,7 +84,7 @@ class PostDiff(Command):
             make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: \"patch\")"),
         ]
         options += self.posting_options()
-        Command.__init__(self, "Attaches the current working directory diff to a bug as a patch file.", "[BUGID]", options=options)
+        Command.__init__(self, "Attach the current working directory diff to a bug as a patch file", "[BUGID]", options=options)
 
     @staticmethod
     def posting_options():
@@ -127,7 +127,7 @@ class PostCommits(Command):
             make_option("-m", "--description", action="store", type="string", dest="description", help="Description string for the attachment (default: description from commit message)"),
         ]
         options += PostDiff.posting_options()
-        Command.__init__(self, "Attaches a range of local commits to bugs as patch files.", "COMMITISH", options=options, requires_local_commits=True)
+        Command.__init__(self, "Attach a range of local commits to bugs as patch files", "COMMITISH", options=options, requires_local_commits=True)
 
     def _comment_text_for_commit(self, options, commit_message, tool, commit_id):
         comment_text = None
@@ -179,7 +179,7 @@ class CreateBug(Command):
             make_option("--no-review", action="store_false", dest="review", default=True, help="Do not mark the patch for review."),
             make_option("--request-commit", action="store_true", dest="request_commit", default=False, help="Mark the patch as needing auto-commit after review."),
         ]
-        Command.__init__(self, "Create a bug from local changes or local commits.", "[COMMITISH]", options=options)
+        Command.__init__(self, "Create a bug from local changes or local commits", "[COMMITISH]", options=options)
 
     def create_bug_from_commit(self, options, args, tool):
         commit_ids = tool.scm().commit_ids_from_commitish_arguments(args)
index 90148b28cb469a5a6876b993e59aeac2cacd378e..fbebb7341c7b34011346086f5a9e92ea4983680c 100644 (file)
@@ -60,30 +60,31 @@ class Command(object):
     def execute(self, options, args, tool):
         raise NotImplementedError, "subclasses must implement"
 
-class NonWrappingEpilogIndentedHelpFormatter(IndentedHelpFormatter):
-    # The standard IndentedHelpFormatter paragraph-wraps the epilog, killing our custom formatting.
-    def format_epilog(self, epilog):
-        if epilog:
-            return "\n" + epilog + "\n"
-        return ""
-
 
 class HelpPrintingOptionParser(OptionParser):
+    def __init__(self, epilog_method=None, *args, **kwargs):
+        self.epilog_method = epilog_method
+        OptionParser.__init__(self, *args, **kwargs)
+
     def error(self, msg):
         self.print_usage(sys.stderr)
         error_message = "%s: error: %s\n" % (self.get_prog_name(), msg)
         error_message += "\nType \"" + self.get_prog_name() + " --help\" to see usage.\n"
         self.exit(1, error_message)
 
+    # We override format_epilog to avoid the default formatting which would paragraph-wrap the epilog
+    # and also to allow us to compute the epilog lazily instead of in the constructor (allowing it to be context sensitive).
+    def format_epilog(self, epilog):
+        if self.epilog_method:
+            return "\n%s\n" % self.epilog_method()
+        return ""
+
 
 class MultiCommandTool(object):
     def __init__(self, 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]
-        # FIXME: Calling self._commands_usage() in the constructor is bad because
-        # it calls self.should_show_command_help which is subclass-defined.
-        # The subclass will not be fully initialized at this point.
-        self.global_option_parser = HelpPrintingOptionParser(usage=self._usage_line(), formatter=NonWrappingEpilogIndentedHelpFormatter(), epilog=self._commands_usage())
+        self.global_option_parser = HelpPrintingOptionParser(epilog_method=self._help_epilog, usage=self._usage_line())
 
     @classmethod
     def _add_all_subclasses(cls, class_to_crawl, seen_classes):
@@ -100,31 +101,24 @@ class MultiCommandTool(object):
 
     @staticmethod
     def _usage_line():
-        return "Usage: %prog [options] command [command-options] [command-arguments]"
-
-    def _command_help_formatter(self):
-        # Use our own help formatter so as to indent enough.
-        formatter = IndentedHelpFormatter()
-        formatter.indent()
-        formatter.indent()
-        return formatter
-
-    @classmethod
-    def _help_for_command(cls, command, formatter, longest_name_length):
-        help_text = "  " + command.name_with_arguments().ljust(longest_name_length + 3) + command.help_text + "\n"
-        help_text += command.option_parser.format_option_help(formatter)
-        return help_text
+        return "Usage: %prog [options] COMMAND [ARGS]"
 
     @classmethod
     def _standalone_help_for_command(cls, command):
-        return cls._help_for_command(command, IndentedHelpFormatter(), len(command.name_with_arguments()))
+        help_text = command.name_with_arguments().ljust(len(command.name_with_arguments()) + 3) + command.help_text + "\n"
+        help_text += command.option_parser.format_option_help(IndentedHelpFormatter())
+        return help_text
 
-    def _commands_usage(self):
-        # Only show commands which are relevant to this checkout.  This might be confusing to some users?
+    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)
-        longest_name_length = max(map(lambda command: len(command.name_with_arguments()), relevant_commands))
-        command_help_texts = map(lambda command: self._help_for_command(command, self._command_help_formatter(), longest_name_length), relevant_commands)
-        return "Commands:\n" + "".join(command_help_texts)
+        longest_name_length = max(map(lambda command: len(command.name), relevant_commands))
+        relevant_commands.sort(lambda a, b: cmp(a.name, b.name))
+        command_help_texts = map(lambda command: "   %s   %s\n" % (command.name.ljust(longest_name_length), command.help_text), relevant_commands)
+        epilog = "%prog supports the following commands:\n"
+        epilog += "%s\n" % "".join(command_help_texts)
+        epilog += "See '%prog help COMMAND' for more information on a specific command.\n"
+        return self.global_option_parser.expand_prog_name(epilog)
 
     def handle_global_args(self, args):
         (options, args) = self.global_option_parser.parse_args(args)