From a97aab75313021681bd7b6d5a81eb53a226b0f7d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Apr 2013 21:23:13 +0200 Subject: [PATCH 1/8] argparse: Convert mopidy command --- mopidy/__main__.py | 66 ++++++++++++++++++++++---------------------- mopidy/utils/deps.py | 9 ++---- 2 files changed, 35 insertions(+), 40 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 59063b84..26a02094 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals +import argparse import logging -import optparse import os import signal import sys @@ -37,11 +37,16 @@ def main(): signal.signal(signal.SIGTERM, process.exit_handler) signal.signal(signal.SIGUSR1, pykka.debug.log_thread_tracebacks) - loop = gobject.MainLoop() - options = parse_options() - config_files = options.config.split(b':') - config_overrides = options.overrides + args = parse_args() + if args.show_config: + show_config_task(args) + if args.show_deps: + deps.show_deps_task() + config_files = args.config.split(b':') + config_overrides = args.overrides + + loop = gobject.MainLoop() enabled_extensions = [] # Make sure it is defined before the finally block logging_initialized = False @@ -54,7 +59,7 @@ def main(): # TODO: setup_logging needs defaults in-case config values are None log.setup_logging( - logging_config, options.verbosity_level, options.save_debug_log) + logging_config, args.verbosity_level, args.save_debug_log) logging_initialized = True installed_extensions = ext.load_extensions() @@ -123,59 +128,54 @@ def check_config_errors(errors): sys.exit(1) -def check_config_override(option, opt, override): +def config_override_type(value): try: - return config_lib.parse_override(override) + return config_lib.parse_override(value) except ValueError: - raise optparse.OptionValueError( - 'option %s: must have the format section/key=value' % opt) + raise argparse.ArgumentTypeError( + '%s must have the format section/key=value' % value) -def parse_options(): - parser = optparse.OptionParser( +def parse_args(): + parser = argparse.ArgumentParser() + parser.add_argument( + '--version', action='version', version='Mopidy %s' % versioning.get_version()) - - # Ugly extension of optparse type checking magic :/ - optparse.Option.TYPES += ('config_override',) - optparse.Option.TYPE_CHECKER['config_override'] = check_config_override - - parser.add_option( + parser.add_argument( '-q', '--quiet', action='store_const', const=0, dest='verbosity_level', help='less output (warning level)') - parser.add_option( + parser.add_argument( '-v', '--verbose', action='count', default=1, dest='verbosity_level', help='more output (debug level)') - parser.add_option( + parser.add_argument( '--save-debug-log', action='store_true', dest='save_debug_log', help='save debug log to "./mopidy.log"') - parser.add_option( + parser.add_argument( '--show-config', - action='callback', callback=show_config_callback, + action='store_true', dest='show_config', help='show current config') - parser.add_option( + parser.add_argument( '--show-deps', - action='callback', callback=deps.show_deps_optparse_callback, + action='store_true', dest='show_deps', help='show dependencies and their versions') - parser.add_option( + parser.add_argument( '--config', action='store', dest='config', default=b'$XDG_CONFIG_DIR/mopidy/mopidy.conf', help='config files to use, colon seperated, later files override') - parser.add_option( + parser.add_argument( '-o', '--option', - action='append', dest='overrides', type='config_override', + action='append', dest='overrides', type=config_override_type, help='`section/key=value` values to override config options') - return parser.parse_args(args=mopidy_args)[0] + return parser.parse_args(args=mopidy_args) -def show_config_callback(option, opt, value, parser): - # TODO: don't use callback for this as --config or -o set after - # --show-config will be ignored. - files = getattr(parser.values, 'config', b'').split(b':') - overrides = getattr(parser.values, 'overrides', []) +def show_config_task(args): + files = vars(args).get('config', b'').split(b':') + overrides = vars(args).get('overrides', []) extensions = ext.load_extensions() config, errors = config_lib.load(files, extensions, overrides) diff --git a/mopidy/utils/deps.py b/mopidy/utils/deps.py index 7258d8da..780b0fdb 100644 --- a/mopidy/utils/deps.py +++ b/mopidy/utils/deps.py @@ -14,13 +14,8 @@ import pkg_resources from . import formatting -def show_deps_optparse_callback(*args): - """ - Prints a list of all dependencies. - - Called by optparse when Mopidy is run with the :option:`--show-deps` - option. - """ +def show_deps_task(): + """Prints a list of all dependencies and exits.""" print format_dependency_list() sys.exit(0) From 1fb9634e47b13a8af5693208c0f470930de397a9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Apr 2013 21:27:01 +0200 Subject: [PATCH 2/8] argparse: Convert mopidy-scan command --- mopidy/scanner.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/mopidy/scanner.py b/mopidy/scanner.py index 61257add..41b42347 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -1,8 +1,8 @@ from __future__ import unicode_literals +import argparse import datetime import logging -import optparse import os import sys @@ -33,7 +33,7 @@ from mopidy.utils import log, path, versioning def main(): - options = parse_options() + args = parse_args() # TODO: support config files and overrides (shared from main?) config_files = [b'/etc/mopidy/mopidy.conf', b'$XDG_CONFIG_DIR/mopidy/mopidy.conf'] @@ -43,7 +43,7 @@ def main(): # Initial config without extensions to bootstrap logging. logging_config, _ = config_lib.load(config_files, [], config_overrides) log.setup_root_logger() - log.setup_console_logging(logging_config, options.verbosity_level) + log.setup_console_logging(logging_config, args.verbosity_level) extensions = ext.load_extensions() config, errors = config_lib.load( @@ -87,18 +87,20 @@ def main(): logging.info('Done writing tag cache') -def parse_options(): - parser = optparse.OptionParser( +def parse_args(): + parser = argparse.ArgumentParser() + parser.add_argument( + '--version', action='version', version='Mopidy %s' % versioning.get_version()) - parser.add_option( + parser.add_argument( '-q', '--quiet', action='store_const', const=0, dest='verbosity_level', help='less output (warning level)') - parser.add_option( + parser.add_argument( '-v', '--verbose', action='count', default=1, dest='verbosity_level', help='more output (debug level)') - return parser.parse_args(args=mopidy_args)[0] + return parser.parse_args(args=mopidy_args) def translator(data): From f7ef080671d9e1c1a4b5c903fa107d63b6d65e81 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Apr 2013 21:29:10 +0200 Subject: [PATCH 3/8] docs: Add argparse conversion to changelog --- docs/changelog.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index c35cde49..6241f748 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,14 @@ v0.15.0 (UNRELEASED) - Mopidy no longer supports Python 2.6. Currently, the only Python version supported by Mopidy is Python 2.7. (Fixes: :issue:`344`) +**Command line options** + +- Converted from the optparse to the argparse library for handling command line + options. + +- :option:`mopidy --show-config` will now take into consideration any + :option:`mopidy --option` arguments appearing later on the command line. + v0.14.1 (2013-04-28) ==================== From a301906fe710bf4b5dba32879b02a4ba0ae197d1 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Apr 2013 22:52:11 +0200 Subject: [PATCH 4/8] commands: Move --show-{config,deps} handlers to new module --- mopidy/__main__.py | 30 ++++-------------------------- mopidy/commands.py | 35 +++++++++++++++++++++++++++++++++++ mopidy/utils/deps.py | 7 ------- 3 files changed, 39 insertions(+), 33 deletions(-) create mode 100644 mopidy/commands.py diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 26a02094..1f9b185c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -24,11 +24,11 @@ sys.path.insert( 0, os.path.abspath(os.path.join(os.path.dirname(__file__), '../'))) -from mopidy import ext +from mopidy import commands, ext from mopidy.audio import Audio from mopidy import config as config_lib from mopidy.core import Core -from mopidy.utils import deps, log, path, process, versioning +from mopidy.utils import log, path, process, versioning logger = logging.getLogger('mopidy.main') @@ -39,9 +39,9 @@ def main(): args = parse_args() if args.show_config: - show_config_task(args) + commands.show_config(args) if args.show_deps: - deps.show_deps_task() + commands.show_deps() config_files = args.config.split(b':') config_overrides = args.overrides @@ -173,28 +173,6 @@ def parse_args(): return parser.parse_args(args=mopidy_args) -def show_config_task(args): - files = vars(args).get('config', b'').split(b':') - overrides = vars(args).get('overrides', []) - - extensions = ext.load_extensions() - config, errors = config_lib.load(files, extensions, overrides) - - # Clear out any config for disabled extensions. - for extension in extensions: - if not ext.validate_extension(extension): - config[extension.ext_name] = {b'enabled': False} - errors[extension.ext_name] = { - b'enabled': b'extension disabled its self.'} - elif not config[extension.ext_name]['enabled']: - config[extension.ext_name] = {b'enabled': False} - errors[extension.ext_name] = { - b'enabled': b'extension disabled by config.'} - - print config_lib.format(config, extensions, errors) - sys.exit(0) - - def check_old_locations(): dot_mopidy_dir = path.expand_path(b'~/.mopidy') if os.path.isdir(dot_mopidy_dir): diff --git a/mopidy/commands.py b/mopidy/commands.py new file mode 100644 index 00000000..7c21b013 --- /dev/null +++ b/mopidy/commands.py @@ -0,0 +1,35 @@ +from __future__ import unicode_literals + +import sys + +from mopidy import config as config_lib, ext +from mopidy.utils import deps + + +def show_config(args): + """Prints the effective config and exits.""" + files = vars(args).get('config', b'').split(b':') + overrides = vars(args).get('overrides', []) + + extensions = ext.load_extensions() + config, errors = config_lib.load(files, extensions, overrides) + + # Clear out any config for disabled extensions. + for extension in extensions: + if not ext.validate_extension(extension): + config[extension.ext_name] = {b'enabled': False} + errors[extension.ext_name] = { + b'enabled': b'extension disabled its self.'} + elif not config[extension.ext_name]['enabled']: + config[extension.ext_name] = {b'enabled': False} + errors[extension.ext_name] = { + b'enabled': b'extension disabled by config.'} + + print config_lib.format(config, extensions, errors) + sys.exit(0) + + +def show_deps(): + """Prints a list of all dependencies and exits.""" + print deps.format_dependency_list() + sys.exit(0) diff --git a/mopidy/utils/deps.py b/mopidy/utils/deps.py index 780b0fdb..99a22d3c 100644 --- a/mopidy/utils/deps.py +++ b/mopidy/utils/deps.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals import functools import os import platform -import sys import pygst pygst.require('0.10') @@ -14,12 +13,6 @@ import pkg_resources from . import formatting -def show_deps_task(): - """Prints a list of all dependencies and exits.""" - print format_dependency_list() - sys.exit(0) - - def format_dependency_list(adapters=None): if adapters is None: dist_names = set([ From df2abde2588893dbf84e6e0be5eb47006931e6ab Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 29 Apr 2013 22:57:44 +0200 Subject: [PATCH 5/8] commands: Move argument parser to commands module --- mopidy/__main__.py | 50 ++-------------------------------------------- mopidy/commands.py | 46 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index 1f9b185c..c0189d0f 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -import argparse import logging import os import signal @@ -28,7 +27,7 @@ from mopidy import commands, ext from mopidy.audio import Audio from mopidy import config as config_lib from mopidy.core import Core -from mopidy.utils import log, path, process, versioning +from mopidy.utils import log, path, process logger = logging.getLogger('mopidy.main') @@ -37,7 +36,7 @@ def main(): signal.signal(signal.SIGTERM, process.exit_handler) signal.signal(signal.SIGUSR1, pykka.debug.log_thread_tracebacks) - args = parse_args() + args = commands.parser.parse_args(args=mopidy_args) if args.show_config: commands.show_config(args) if args.show_deps: @@ -128,51 +127,6 @@ def check_config_errors(errors): sys.exit(1) -def config_override_type(value): - try: - return config_lib.parse_override(value) - except ValueError: - raise argparse.ArgumentTypeError( - '%s must have the format section/key=value' % value) - - -def parse_args(): - parser = argparse.ArgumentParser() - parser.add_argument( - '--version', action='version', - version='Mopidy %s' % versioning.get_version()) - parser.add_argument( - '-q', '--quiet', - action='store_const', const=0, dest='verbosity_level', - help='less output (warning level)') - parser.add_argument( - '-v', '--verbose', - action='count', default=1, dest='verbosity_level', - help='more output (debug level)') - parser.add_argument( - '--save-debug-log', - action='store_true', dest='save_debug_log', - help='save debug log to "./mopidy.log"') - parser.add_argument( - '--show-config', - action='store_true', dest='show_config', - help='show current config') - parser.add_argument( - '--show-deps', - action='store_true', dest='show_deps', - help='show dependencies and their versions') - parser.add_argument( - '--config', - action='store', dest='config', - default=b'$XDG_CONFIG_DIR/mopidy/mopidy.conf', - help='config files to use, colon seperated, later files override') - parser.add_argument( - '-o', '--option', - action='append', dest='overrides', type=config_override_type, - help='`section/key=value` values to override config options') - return parser.parse_args(args=mopidy_args) - - def check_old_locations(): dot_mopidy_dir = path.expand_path(b'~/.mopidy') if os.path.isdir(dot_mopidy_dir): diff --git a/mopidy/commands.py b/mopidy/commands.py index 7c21b013..a942e263 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -1,9 +1,53 @@ from __future__ import unicode_literals +import argparse import sys from mopidy import config as config_lib, ext -from mopidy.utils import deps +from mopidy.utils import deps, versioning + + +def config_override_type(value): + try: + return config_lib.parse_override(value) + except ValueError: + raise argparse.ArgumentTypeError( + '%s must have the format section/key=value' % value) + + +parser = argparse.ArgumentParser() +parser.add_argument( + '--version', action='version', + version='Mopidy %s' % versioning.get_version()) +parser.add_argument( + '-q', '--quiet', + action='store_const', const=0, dest='verbosity_level', + help='less output (warning level)') +parser.add_argument( + '-v', '--verbose', + action='count', default=1, dest='verbosity_level', + help='more output (debug level)') +parser.add_argument( + '--save-debug-log', + action='store_true', dest='save_debug_log', + help='save debug log to "./mopidy.log"') +parser.add_argument( + '--show-config', + action='store_true', dest='show_config', + help='show current config') +parser.add_argument( + '--show-deps', + action='store_true', dest='show_deps', + help='show dependencies and their versions') +parser.add_argument( + '--config', + action='store', dest='config', + default=b'$XDG_CONFIG_DIR/mopidy/mopidy.conf', + help='config files to use, colon seperated, later files override') +parser.add_argument( + '-o', '--option', + action='append', dest='overrides', type=config_override_type, + help='`section/key=value` values to override config options') def show_config(args): From 5e4f22bd1772e25c16502d87d7c996875aed2c36 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 30 Apr 2013 23:23:39 +0200 Subject: [PATCH 6/8] commands: Use argparse to split config files into a list --- mopidy/__main__.py | 8 +++----- mopidy/commands.py | 14 ++++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/mopidy/__main__.py b/mopidy/__main__.py index c0189d0f..0118395c 100644 --- a/mopidy/__main__.py +++ b/mopidy/__main__.py @@ -42,9 +42,6 @@ def main(): if args.show_deps: commands.show_deps() - config_files = args.config.split(b':') - config_overrides = args.overrides - loop = gobject.MainLoop() enabled_extensions = [] # Make sure it is defined before the finally block logging_initialized = False @@ -54,7 +51,8 @@ def main(): try: # Initial config without extensions to bootstrap logging. - logging_config, _ = config_lib.load(config_files, [], config_overrides) + logging_config, _ = config_lib.load( + args.config_files, [], args.config_overrides) # TODO: setup_logging needs defaults in-case config values are None log.setup_logging( @@ -64,7 +62,7 @@ def main(): installed_extensions = ext.load_extensions() config, config_errors = config_lib.load( - config_files, installed_extensions, config_overrides) + args.config_files, installed_extensions, args.config_overrides) # Filter out disabled extensions and remove any config errors for them. for extension in installed_extensions: diff --git a/mopidy/commands.py b/mopidy/commands.py index a942e263..a5d821b4 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -7,6 +7,10 @@ from mopidy import config as config_lib, ext from mopidy.utils import deps, versioning +def config_files_type(value): + return value.split(b':') + + def config_override_type(value): try: return config_lib.parse_override(value) @@ -41,22 +45,20 @@ parser.add_argument( help='show dependencies and their versions') parser.add_argument( '--config', - action='store', dest='config', + action='store', dest='config_files', type=config_files_type, default=b'$XDG_CONFIG_DIR/mopidy/mopidy.conf', help='config files to use, colon seperated, later files override') parser.add_argument( '-o', '--option', - action='append', dest='overrides', type=config_override_type, + action='append', dest='config_overrides', type=config_override_type, help='`section/key=value` values to override config options') def show_config(args): """Prints the effective config and exits.""" - files = vars(args).get('config', b'').split(b':') - overrides = vars(args).get('overrides', []) - extensions = ext.load_extensions() - config, errors = config_lib.load(files, extensions, overrides) + config, errors = config_lib.load( + args.config_files, extensions, args.config_overrides) # Clear out any config for disabled extensions. for extension in extensions: From 69caea2ef97403f5de4e57d6c64b71655f6e80df Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 30 Apr 2013 23:25:02 +0200 Subject: [PATCH 7/8] command: Move override parsing into module --- mopidy/commands.py | 4 +++- mopidy/config/__init__.py | 7 ------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/mopidy/commands.py b/mopidy/commands.py index a5d821b4..c5ca0236 100644 --- a/mopidy/commands.py +++ b/mopidy/commands.py @@ -13,7 +13,9 @@ def config_files_type(value): def config_override_type(value): try: - return config_lib.parse_override(value) + section, remainder = value.split(b'/', 1) + key, value = remainder.split(b'=', 1) + return (section.strip(), key.strip(), value.strip()) except ValueError: raise argparse.ArgumentTypeError( '%s must have the format section/key=value' % value) diff --git a/mopidy/config/__init__.py b/mopidy/config/__init__.py index e4723b15..0d9b9e7a 100644 --- a/mopidy/config/__init__.py +++ b/mopidy/config/__init__.py @@ -140,13 +140,6 @@ def _format(config, comments, schemas, display): return b'\n'.join(output) -def parse_override(override): - """Parse ``section/key=value`` command line overrides""" - section, remainder = override.split(b'/', 1) - key, value = remainder.split(b'=', 1) - return (section.strip(), key.strip(), value.strip()) - - class Proxy(collections.Mapping): def __init__(self, data): self._data = data From 94ab12b13cf407e5f3b9914862209d97f749c33b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 30 Apr 2013 23:42:53 +0200 Subject: [PATCH 8/8] commands: Update override parsing tests --- tests/commands_test.py | 44 +++++++++++++++++++++++++++++++++++++ tests/config/config_test.py | 28 ----------------------- 2 files changed, 44 insertions(+), 28 deletions(-) create mode 100644 tests/commands_test.py diff --git a/tests/commands_test.py b/tests/commands_test.py new file mode 100644 index 00000000..35fd0de5 --- /dev/null +++ b/tests/commands_test.py @@ -0,0 +1,44 @@ +from __future__ import unicode_literals + +import argparse +import unittest + +from mopidy import commands + + +class ConfigOverrideTypeTest(unittest.TestCase): + def test_valid_override(self): + expected = (b'section', b'key', b'value') + self.assertEqual( + expected, commands.config_override_type(b'section/key=value')) + self.assertEqual( + expected, commands.config_override_type(b'section/key=value ')) + self.assertEqual( + expected, commands.config_override_type(b'section/key =value')) + self.assertEqual( + expected, commands.config_override_type(b'section /key=value')) + + def test_valid_override_is_bytes(self): + section, key, value = commands.config_override_type( + b'section/key=value') + self.assertIsInstance(section, bytes) + self.assertIsInstance(key, bytes) + self.assertIsInstance(value, bytes) + + def test_empty_override(self): + expected = ('section', 'key', '') + self.assertEqual( + expected, commands.config_override_type(b'section/key=')) + self.assertEqual( + expected, commands.config_override_type(b'section/key= ')) + + def test_invalid_override(self): + self.assertRaises( + argparse.ArgumentTypeError, + commands.config_override_type, b'section/key') + self.assertRaises( + argparse.ArgumentTypeError, + commands.config_override_type, b'section=') + self.assertRaises( + argparse.ArgumentTypeError, + commands.config_override_type, b'section') diff --git a/tests/config/config_test.py b/tests/config/config_test.py index a792839a..c40baa87 100644 --- a/tests/config/config_test.py +++ b/tests/config/config_test.py @@ -106,31 +106,3 @@ class ValidateTest(unittest.TestCase): self.assertEqual({'foo': {'bar': 'bad'}}, errors) # TODO: add more tests - - -class ParseOverrideTest(unittest.TestCase): - def test_valid_override(self): - expected = (b'section', b'key', b'value') - self.assertEqual(expected, config.parse_override(b'section/key=value')) - self.assertEqual( - expected, config.parse_override(b'section/key=value ')) - self.assertEqual( - expected, config.parse_override(b'section/key =value')) - self.assertEqual( - expected, config.parse_override(b'section /key=value')) - - def test_valid_override_is_bytes(self): - section, key, value = config.parse_override(b'section/key=value') - self.assertIsInstance(section, bytes) - self.assertIsInstance(key, bytes) - self.assertIsInstance(value, bytes) - - def test_empty_override(self): - expected = ('section', 'key', '') - self.assertEqual(expected, config.parse_override(b'section/key=')) - self.assertEqual(expected, config.parse_override(b'section/key= ')) - - def test_invalid_override(self): - self.assertRaises(ValueError, config.parse_override, b'section/key') - self.assertRaises(ValueError, config.parse_override, b'section=') - self.assertRaises(ValueError, config.parse_override, b'section')