diff --git a/kobo/cli.py b/kobo/cli.py index f1767e3..e7ce605 100644 --- a/kobo/cli.py +++ b/kobo/cli.py @@ -22,9 +22,9 @@ def __init__(self, *args, **kwargs): -------------------------- # It usually makes sense to inherit directly from Command class. # All common methods and attributes should be in the container. -# Specify any OptionParser options in options() method. -# OptionParser.parse_args() result is automatically passed to run(*args, **kwargs) method. -# A OptionParser instance os available in self.parser attribute. +# Specify command arguments in the options() method using add_argument(). +# ArgumentParser.parse_args() result is automatically passed to run(*args, **kwargs) method. +# An ArgumentParser instance is available in self.parser attribute. class Make_Dirs(Command): '''create directories''' @@ -32,8 +32,8 @@ class Make_Dirs(Command): admin = False def options(self): - self.parser.usage = "%%prog %s [options] " % self.normalized_name - self.parser.add_option("-m", "--mode", help="set directory perms (0xxx)") + self.parser.usage = "%(prog)s %s [options] " % self.normalized_name + self.parser.add_argument("-m", "--mode", help="set directory perms (0xxx)") def run(self, *args, **kwargs): if len(args) < 1: @@ -68,9 +68,8 @@ def run(self, *args, **kwargs): from __future__ import print_function import sys -import optparse +import argparse import datetime -from optparse import Option from six.moves.xmlrpc_client import Fault from kobo.plugins import Plugin, PluginContainer @@ -183,17 +182,15 @@ def normalize_name(cls, name): return name.lower().replace('_', '-').replace(' ', '-') -class CommandOptionParser(optparse.OptionParser): +class CommandOptionParser(argparse.ArgumentParser): """Enhanced OptionParser with plugin support.""" def __init__(self, usage=None, - option_list=None, - option_class=Option, version=None, conflict_handler="error", description=None, - formatter=None, - add_help_option=True, + formatter_class=None, + add_help=True, prog=None, command_container=None, default_command="help", @@ -201,14 +198,25 @@ def __init__(self, add_hub_option=False, default_profile="", configuration_directory="/etc"): - - usage = usage or "%prog [args] [--help]" + usage = usage or "%(prog)s [args] [--help]" self.container = command_container self.default_command = default_command self.command = None - formatter = formatter or optparse.IndentedHelpFormatter(max_help_position=33) - - optparse.OptionParser.__init__(self, usage, option_list, option_class, version, conflict_handler, description, formatter, add_help_option, prog) + formatter_class = formatter_class or argparse.RawTextHelpFormatter + + # Initialize the argument parser + super(CommandOptionParser, self).__init__( + prog=prog, + usage=usage, + description=description, + conflict_handler=conflict_handler, + add_help=add_help, + formatter_class=formatter_class, + ) + + # Add version argument if provided + if version: + self.add_argument('--version', action='version', version=version) if add_username_password_options: self._add_opts( @@ -263,7 +271,7 @@ def format_help_commands(self, admin=False): def parse_args(self, args=None, values=None): """return (command_instance, opts, args)""" - args = self._get_args(args) + args = args if args is not None else sys.argv[1:] command = None if len(args) > 0 and not args[0].startswith("-"): @@ -281,25 +289,37 @@ def parse_args(self, args=None, values=None): if self.command != cmd.normalized_name: self.command = cmd.normalized_name cmd.options() - cmd_opts, cmd_args = optparse.OptionParser.parse_args(self, args, values) - return (cmd, cmd_opts, cmd_args) - def run(self, args=None, values=None): + # Parse arguments using argparse + parsed_args = super(CommandOptionParser, self).parse_args(args) + + # Get remaining positional arguments (if any) + remaining_args = getattr(parsed_args, 'args', []) + + return (cmd, parsed_args, remaining_args) + + def run(self, args=None): """parse arguments and run a command""" - cmd, cmd_opts, cmd_args = self.parse_args(args, values) - cmd_kwargs = cmd_opts.__dict__ + # Get command instance and parsed arguments + cmd, parsed_args, remaining_args = self.parse_args(args) + + # Convert Namespace to dictionary for kwargs + cmd_kwargs = vars(parsed_args) - # this block should only be evaluated if default_profile has been set at instantiation + # Handle profile if specified if self.default_profile and 'profile' in cmd_kwargs: - self._load_profile(cmd_kwargs['profile']) + self._load_profile(cmd_kwargs['profile']) - cmd.run(*cmd_args, **cmd_kwargs) + # Run command with positional args and keyword args + cmd.run(*remaining_args, **cmd_kwargs) def _add_opts(self, *args): """populates one or more options with their respective help texts""" - option_list = [optparse.Option(option, help=help_text) for option, help_text in args] - self._populate_option_list(option_list=option_list, add_help=False) + for option, help_text in args: + # Strip leading dashes and use as destination + dest = option.lstrip('-').replace('-', '_') + self.add_argument(option, dest=dest, help=help_text) def _load_profile(self, profile): """load configuration file under location /.conf""" @@ -326,9 +346,12 @@ class Help_Admin(Command): def options(self): # override default --help option - opt = self.parser.get_option("--help") - opt.action = "store_true" - opt.dest = "help" + self.parser.add_argument( + "--help", + action="store_true", + dest="help", + help="show help message and exit" + ) def run(self, *args, **kwargs): self.parser.print_help(admin=True) @@ -366,12 +389,14 @@ def run(self, *args, **kwargs): print("--------") for command_name, CommandClass in sorted(self.parser.container.plugins.items()): - parser = optparse.OptionParser(usage=self.parser.usage) + parser = argparse.ArgumentParser( + usage=self.parser.usage, + formatter_class=argparse.RawTextHelpFormatter + ) cmd = CommandClass(parser) cmd.normalized_name = command_name cmd.options() cmd.container = self.parser.container - cmd_opts, cmd_args = parser.parse_args() print(command_name) print("-" * len(command_name)) @@ -380,24 +405,39 @@ def run(self, *args, **kwargs): print("[ADMIN ONLY]", end=' ') print(cmd.__doc__.strip(), end="\n\n") - usage = parser.get_usage().strip().replace("Usage: ", "**Usage:** ", 1) + + # Get formatted usage + usage = parser.format_usage() if usage: - print(usage, end="\n\n") + print(usage.replace("usage: ", "**Usage:** "), end="\n\n") - for opt in sorted(parser.option_list, key=str): - if "-h/--help" in str(opt): + # Process and display arguments + for action in parser._actions: + # Skip help action + if action.dest == 'help': continue - if opt.nargs: - metavar = opt.metavar or opt.dest.upper() - opt_list = [] - for opt_str in opt._short_opts + opt._long_opts: - if opt.nargs is not None: - opt_list.append("%s=%s" % (opt_str, metavar)) + + # Format option strings + opts = [] + for opt_str in action.option_strings: + if action.metavar: + opts.append(f"{opt_str}={action.metavar}") + elif action.nargs: + opts.append(f"{opt_str}={action.dest.upper()}") else: - opt_list.append(opt_str) - print("/".join(opt_list)) - print(" %s" % opt.help) - if opt.action == "append": + opts.append(opt_str) + + if opts: # Optional arguments + print("/".join(opts)) + elif action.dest != 'help': # Positional arguments + print(action.dest) + + # Print help text + if action.help: + print(f" {action.help}") + + # Show if argument can be specified multiple times + if action.nargs in ('+', '*', argparse.REMAINDER): print("\n This option can be specified multiple times") print() print() diff --git a/kobo/notification.py b/kobo/notification.py index c17588c..204c9bc 100755 --- a/kobo/notification.py +++ b/kobo/notification.py @@ -9,7 +9,7 @@ import smtplib import sys -import optparse +import argparse import kobo.shortcuts import six @@ -54,28 +54,28 @@ def send(self, from_addr, recipients, subject, body, reply_to=None, xheaders=Non def main(argv): """Main function for command line usage""" - parser = optparse.OptionParser("usage: %prog [to_addr]...") - parser.add_option( + parser = argparse.ArgumentParser(description="Send email notifications") + parser.add_argument( "--server", help="specify SMTP server address" ) - parser.add_option( + parser.add_argument( "-f", "--from", dest="from_addr", help="set the From address" ) - parser.add_option( + parser.add_argument( "-s", "--subject", help="set email Subject" ) - parser.add_option( + parser.add_argument( "-r", "--reply-to", help="set the Reply-To address" ) - parser.add_option( + parser.add_argument( "-x", "--xheader", nargs=2, @@ -83,15 +83,20 @@ def main(argv): action="append", help="set X-Headers; takes two arguments (-x X-Spam eggs)" ) + parser.add_argument( + "recipients", + nargs="+", + help="recipient email addresses" + ) - (opts, args) = parser.parse_args(argv) + args = parser.parse_args(argv) - server = opts.server - from_addr = opts.from_addr - subject = opts.subject - reply_to = opts.reply_to - xheaders = opts.xheaders and dict(opts.xheaders) or {} - recipients = args + server = args.server + from_addr = args.from_addr + subject = args.subject + reply_to = args.reply_to + xheaders = args.xheaders and dict(args.xheaders) or {} + recipients = args.recipients if not server: parser.error("SMTP server address required") diff --git a/kobo/worker/main.py b/kobo/worker/main.py index eca27f7..0e9ac3b 100644 --- a/kobo/worker/main.py +++ b/kobo/worker/main.py @@ -4,7 +4,7 @@ import os import sys import signal -import optparse +import argparse import logging # IMPORTANT: import taskd first to set os.environ["PROJECT_DEFAULT_CONFIG_FILE"] @@ -100,38 +100,36 @@ def sighup_handler(*_): def main(conf, argv=None, task_manager_class=None): - parser = optparse.OptionParser() - parser.add_option( + parser = argparse.ArgumentParser() + parser.add_argument( "-f", "--foreground", default=False, action="store_true", help="run in foreground (do not spawn a daemon)", ) - parser.add_option( + parser.add_argument( "-k", "--kill", default=False, action="store_true", help="kill the daemon", ) - parser.add_option( + parser.add_argument( "-p", "--pid-file", help="specify a pid file", ) - opts, args = parser.parse_args(argv) + args = parser.parse_args(argv) - pid_file = opts.pid_file - if pid_file is None: - pid_file = conf.get("PID_FILE") + pid_file = args.pid_file or conf.get("PID_FILE") if pid_file is None: parser.error("No pid file specified.") - if opts.kill: + if args.kill: pid = open(pid_file, "r").read() os.kill(int(pid), 15) sys.exit(0) - if opts.foreground: + if args.foreground: main_loop(conf, foreground=True, task_manager_class=task_manager_class) else: kobo.process.daemonize( diff --git a/tests/test_parser.py b/tests/test_parser.py new file mode 100644 index 0000000..5f285c8 --- /dev/null +++ b/tests/test_parser.py @@ -0,0 +1,50 @@ +import argparse +import pytest +from kobo.cli import CommandOptionParser, CommandContainer, Command + + +class DummyCommand(Command): + """dummy command""" + + enabled = True + + def options(self): + self.parser.add_argument("--foo", help="foo option") + + def run(self, *args, **kwargs): + self.called = True + self.kwargs = kwargs + + +def test_parse_args(): + container = CommandContainer() + container.register_plugin(DummyCommand) + + parser = CommandOptionParser(command_container=container) + + cmd, parsed_args, remaining = parser.parse_args(["dummycommand", "--foo", "x"]) + + assert isinstance(cmd, DummyCommand) + assert parsed_args.foo == "x" + assert remaining == [] + + +def test_run_executes_command(monkeypatch): + container = CommandContainer() + container.register_plugin(DummyCommand) + parser = CommandOptionParser(command_container=container) + + called = {} + + def fake_parse_args(args=None): + cmd = DummyCommand(parser) + return cmd, argparse.Namespace(foo="X"), [] + + monkeypatch.setattr(parser, "parse_args", fake_parse_args) + + DummyCommand.run = lambda self, *a, **kw: called.update( + {"ran": True, "foo": kw["foo"]} + ) + parser.run(["dummycommand", "--foo", "X"]) + + assert called == {"ran": True, "foo": "X"} diff --git a/tests/test_profile.py b/tests/test_profile.py index 97d5859..433535e 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -23,17 +23,24 @@ def setUp(self): def test_profile_option_unset(self): parser = CommandOptionParser(command_container=self.command_container) - option = parser.get_option("--profile") self.assertEqual(parser.default_profile, "") - self.assertEqual(option, None) + + option_strings = [opt for a in parser._actions for opt in a.option_strings] + self.assertNotIn("--profile", option_strings) def test_profile_option_set(self): parser = CommandOptionParser(command_container=self.command_container, default_profile="default-profile") - option = parser.get_option("--profile") self.assertEqual(parser.default_profile, "default-profile") - self.assertEqual(option.get_opt_string(), "--profile") + + option = next( + (a for a in parser._actions if "--profile" in a.option_strings), + None + ) + + self.assertIsNotNone(option) + self.assertIn("--profile", option.option_strings) self.assertEqual(option.help, "specify profile (default: default-profile)") def test_configuration_directory_option_unset(self):