From c47221555f2f2dae727b236805dc344f3dcd71fe Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Mon, 16 Feb 2015 11:55:26 +0100 Subject: [PATCH] Add beets.util.interactive_open() find cmd + execute interactive_open() takes a target and an optional command, if it does not receive a command then it uses open_anything(). It parses command and lexes it with shlex.split(), revieling the client from that task. "config -e" command uses it, and gives a better error message in case of problem. "play" plugin uses it as well, as side-effect being that the command is now interactive, as requested in issue #1321. Fix issue #1321. --- beets/ui/commands.py | 25 +++++++------------------ beets/util/__init__.py | 24 ++++++++++++++++++++++++ beetsplug/play.py | 24 +++++++----------------- docs/changelog.rst | 2 ++ docs/plugins/play.rst | 4 ++-- test/test_config_command.py | 5 +++-- test/test_util.py | 13 +++++++++++++ 7 files changed, 58 insertions(+), 39 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 98658a57d..d6fb726ec 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -21,7 +21,6 @@ from __future__ import (division, absolute_import, print_function, import os import re -import shlex import beets from beets import ui @@ -1494,24 +1493,14 @@ def config_edit(): """ path = config.user_config_path() - if 'EDITOR' in os.environ: - editor = os.environ['EDITOR'].encode('utf8') - try: - editor = [e.decode('utf8') for e in shlex.split(editor)] - except ValueError: # Malformed shell tokens. - editor = [editor] - args = editor + [path] - args.insert(1, args[0]) - else: - base = util.open_anything() - args = [base, base, path] - + editor = os.environ.get('EDITOR') try: - os.execlp(*args) - except OSError: - raise ui.UserError("Could not edit configuration. Please " - "set the EDITOR environment variable.") - + util.interactive_open(path, editor) + except OSError as exc: + message = "Could not edit configuration: {0}".format(exc) + if not editor: + message += ". Please set the EDITOR environment variable" + raise ui.UserError(message) config_cmd = ui.Subcommand('config', help='show or edit the user configuration') diff --git a/beets/util/__init__.py b/beets/util/__init__.py index b072d11a5..2d6968e13 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -26,6 +26,7 @@ from collections import defaultdict import traceback import subprocess import platform +import shlex MAX_FILENAME_LENGTH = 200 @@ -697,3 +698,26 @@ def open_anything(): else: # Assume Unix base_cmd = 'xdg-open' return base_cmd + + +def interactive_open(target, command=None): + """Open `target` file with `command` or, in not available, ask the OS to + deal with it. + + The executed program will have stdin, stdout and stderr. + OSError may be raised, it is left to the caller to catch them. + """ + if command: + command = command.encode('utf8') + try: + command = [c.decode('utf8') + for c in shlex.split(command)] + except ValueError: # Malformed shell tokens. + command = [command] + command.insert(0, command[0]) # for argv[0] + else: + base_cmd = open_anything() + command = [base_cmd, base_cmd] + + command.append(target) + return os.execlp(*command) diff --git a/beetsplug/play.py b/beetsplug/play.py index dabce2b43..db3348210 100644 --- a/beetsplug/play.py +++ b/beetsplug/play.py @@ -23,7 +23,6 @@ from beets import config from beets import ui from beets import util from os.path import relpath -import shlex from tempfile import NamedTemporaryFile @@ -60,10 +59,6 @@ class PlayPlugin(BeetsPlugin): relative_to = config['play']['relative_to'].get() if relative_to: relative_to = util.normpath(relative_to) - if command_str: - command = shlex.split(command_str) - else: - command = [util.open_anything()] # Preform search by album and add folders rather than tracks to # playlist. @@ -114,17 +109,12 @@ class PlayPlugin(BeetsPlugin): m3u.write(item + b'\n') m3u.close() - command.append(m3u.name) - - # Invoke the command and log the output. - output = util.command_output(command) - if output: - self._log.debug(u'Output of {0}: {1}', - util.displayable_path(command[0]), - output.decode('utf8', 'ignore')) - else: - self._log.debug(u'no output') - ui.print_(u'Playing {0} {1}.'.format(len(selection), item_type)) - util.remove(m3u.name) + try: + util.interactive_open(m3u.name, command_str) + except OSError as exc: + raise ui.UserError("Could not play the music playlist: " + "{0}".format(exc)) + finally: + util.remove(m3u.name) diff --git a/docs/changelog.rst b/docs/changelog.rst index 329770102..d68b92812 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,8 @@ Changelog Features: +* :doc:`/plugins/play` gives full interaction with the command invoked. + :bug:`1321` * The summary shown to compare duplicate albums during import now displays the old and new filesizes. :bug:`1291` * The colors used are now configurable via the new config option ``colors``, diff --git a/docs/plugins/play.rst b/docs/plugins/play.rst index 325656aaa..939e3bec4 100644 --- a/docs/plugins/play.rst +++ b/docs/plugins/play.rst @@ -26,8 +26,8 @@ would on the command-line):: play: command: /usr/bin/command --option1 --option2 some_other_option -Enable beets' verbose logging to see the command's output if you need to -debug. +While playing you'll be able to interact with the player if it is a +command-line oriented, and you'll get its output in real time. Configuration ------------- diff --git a/test/test_config_command.py b/test/test_config_command.py index 82417f14f..7289daf13 100644 --- a/test/test_config_command.py +++ b/test/test_config_command.py @@ -92,10 +92,11 @@ class ConfigCommandTest(unittest.TestCase, TestHelper): def test_config_editor_not_found(self): with self.assertRaises(ui.UserError) as user_error: with patch('os.execlp') as execlp: - execlp.side_effect = OSError() + execlp.side_effect = OSError('here is problem') self.run_command('config', '-e') self.assertIn('Could not edit configuration', - unicode(user_error.exception.args[0])) + unicode(user_error.exception)) + self.assertIn('here is problem', unicode(user_error.exception)) def test_edit_invalid_config_file(self): self.lib = Library(':memory:') diff --git a/test/test_util.py b/test/test_util.py index c38dcdf83..20c7708d5 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -20,6 +20,8 @@ import sys import re import os +from mock import patch + from test._common import unittest from test import _common from beets import util @@ -36,6 +38,17 @@ class UtilTest(unittest.TestCase): with _common.system_mock('Tagada'): self.assertEqual(util.open_anything(), 'xdg-open') + @patch('os.execlp') + @patch('beets.util.open_anything') + def test_interactive_open(self, mock_open, mock_execlp): + mock_open.return_value = 'tagada' + util.interactive_open('foo') + mock_execlp.assert_called_once_with('tagada', 'tagada', 'foo') + mock_execlp.reset_mock() + + util.interactive_open('foo', 'bar') + mock_execlp.assert_called_once_with('bar', 'bar', 'foo') + def test_sanitize_unix_replaces_leading_dot(self): with _common.platform_posix(): p = util.sanitize_path(u'one/.two/three')