Commit 3d1a19a2 authored by qyearsley's avatar qyearsley Committed by Commit bot

Fix webkit-patch --help.

I believe that after http://crrev.com/2276713002, invoking webkit-patch
with --help rather than help was broken.

When `webkit-patch help` is invoked, the codepath that's used goes through
the execute method of HelpCommand, and the _tool attribute is set, so it
worked.

But when --help is passed, a different codepath is used, that goes through
optparse which directly uses HelpCommand.help_epilog and the _tool attribute
was never set.

This CL fixes this case by setting the _tool attribute in the constructor
of HelpCommand.

Review-Url: https://codereview.chromium.org/2385373003
Cr-Commit-Position: refs/heads/master@{#423017}
parent 1bc946cf
...@@ -36,7 +36,7 @@ class HelpCommand(Command): ...@@ -36,7 +36,7 @@ class HelpCommand(Command):
help_text = "Display information about this program or its subcommands" help_text = "Display information about this program or its subcommands"
argument_names = "[COMMAND]" argument_names = "[COMMAND]"
def __init__(self): def __init__(self, tool=None):
options = [ options = [
optparse.make_option( optparse.make_option(
"-a", "-a",
...@@ -46,11 +46,12 @@ class HelpCommand(Command): ...@@ -46,11 +46,12 @@ class HelpCommand(Command):
help="Print all available commands"), help="Print all available commands"),
] ]
super(HelpCommand, self).__init__(options) super(HelpCommand, self).__init__(options)
# A hack used to pass --all-commands to _help_epilog even though it's called by the OptionParser. # A hack used to pass --all-commands to help_epilog even though it's called by the OptionParser.
self.show_all_commands = False self.show_all_commands = False
self._tool = None # self._tool is used in help_epilog, so it's passed in the initializer rather than set in the execute method.
self._tool = tool
def _help_epilog(self): def help_epilog(self):
# Only show commands which are relevant to this checkout's SCM system. Might this be confusing to some users? # Only show commands which are relevant to this checkout's SCM system. Might this be confusing to some users?
if self.show_all_commands: if self.show_all_commands:
epilog = "All %prog commands:\n" epilog = "All %prog commands:\n"
...@@ -73,7 +74,6 @@ class HelpCommand(Command): ...@@ -73,7 +74,6 @@ class HelpCommand(Command):
self.option_parser.remove_option(option.get_opt_string()) self.option_parser.remove_option(option.get_opt_string())
def execute(self, options, args, tool): def execute(self, options, args, tool):
self._tool = tool
if args: if args:
command = self._tool.command_by_name(args[0]) command = self._tool.command_by_name(args[0])
if command: if command:
......
...@@ -96,7 +96,7 @@ class WebKitPatch(Host): ...@@ -96,7 +96,7 @@ class WebKitPatch(Host):
RebaselineServer(), RebaselineServer(),
RebaselineTest(), RebaselineTest(),
] ]
self.help_command = HelpCommand() self.help_command = HelpCommand(tool=self)
self.commands.append(self.help_command) self.commands.append(self.help_command)
def main(self, argv=None): def main(self, argv=None):
...@@ -142,7 +142,7 @@ class WebKitPatch(Host): ...@@ -142,7 +142,7 @@ class WebKitPatch(Host):
def _create_option_parser(self): def _create_option_parser(self):
usage = "Usage: %prog [options] COMMAND [ARGS]" usage = "Usage: %prog [options] COMMAND [ARGS]"
name = optparse.OptionParser().get_prog_name() name = optparse.OptionParser().get_prog_name()
return HelpPrintingOptionParser(epilog_method=self.help_command._help_epilog, prog=name, usage=usage) return HelpPrintingOptionParser(epilog_method=self.help_command.help_epilog, prog=name, usage=usage)
def _add_global_options(self, option_parser): def _add_global_options(self, option_parser):
global_options = self.global_options or [] global_options = self.global_options or []
......
...@@ -30,7 +30,7 @@ class WebKitPatchTest(unittest.TestCase): ...@@ -30,7 +30,7 @@ class WebKitPatchTest(unittest.TestCase):
self.assertEqual(tool.command_by_name('help').name, 'help') self.assertEqual(tool.command_by_name('help').name, 'help')
self.assertIsNone(tool.command_by_name('non-existent')) self.assertIsNone(tool.command_by_name('non-existent'))
def test_help(self): def test_help_command(self):
oc = OutputCapture() oc = OutputCapture()
oc.capture_output() oc.capture_output()
tool = WebKitPatch('path') tool = WebKitPatch('path')
...@@ -39,3 +39,17 @@ class WebKitPatchTest(unittest.TestCase): ...@@ -39,3 +39,17 @@ class WebKitPatchTest(unittest.TestCase):
self.assertTrue(out.startswith('Usage: ')) self.assertTrue(out.startswith('Usage: '))
self.assertEqual('', err) self.assertEqual('', err)
self.assertEqual('', logs) self.assertEqual('', logs)
def test_help_argument(self):
oc = OutputCapture()
oc.capture_output()
tool = WebKitPatch('path')
try:
tool.main(['tool', '--help'])
except SystemExit:
pass # optparse calls sys.exit after showing help.
finally:
out, err, logs = oc.restore_output()
self.assertTrue(out.startswith('Usage: '))
self.assertEqual('', err)
self.assertEqual('', logs)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment