mirror of
https://github.com/beetbox/beets.git
synced 2026-02-12 10:22:13 +01:00
Clean up convert --pretend (#891)
There were a number of problems with the changes to the util melange: - It used print rather than logging, and its string formatting was probably not Unicode-ready. - The shell-command-like print lines were not quite compatible, which makes their general usefulness questionable. - Used an unsafe/leaky global variable for mkdirall. - Used deprecated sets.Set. Seemed better just to add this to the plugin where we need it so it's easier to see where this goes. It also seems unnecessary to me to print `mkdir -p` commands. They just clutter up the output for me when I really just want to see the transcoding commands.
This commit is contained in:
Adrian Sampson
parent
12a375f4ed
commit
e7f1ff0e3f
3 changed files with 42 additions and 51 deletions
|
|
@ -24,7 +24,6 @@ from collections import defaultdict
|
|||
import traceback
|
||||
import subprocess
|
||||
import platform
|
||||
from sets import Set
|
||||
|
||||
|
||||
MAX_FILENAME_LENGTH = 200
|
||||
|
|
@ -202,25 +201,10 @@ def sorted_walk(path, ignore=(), logger=None):
|
|||
yield res
|
||||
|
||||
|
||||
# We don't create directories on dry-runs, but we must pretend they exist
|
||||
directories_created = Set()
|
||||
|
||||
|
||||
def mkdirall(path, pretend=False):
|
||||
def mkdirall(path):
|
||||
"""Make all the enclosing directories of path (like mkdir -p on the
|
||||
parent).
|
||||
"""
|
||||
|
||||
if pretend:
|
||||
# directory = syspath(ancestry(path)[-1]) # "dirname"
|
||||
# This seems cleaner but MAY have differences on symlinks (leading to
|
||||
# an equivalent result)
|
||||
directory = os.path.dirname(path)
|
||||
if directory not in directories_created:
|
||||
directories_created.add(directory)
|
||||
# This is not a "raw" translation, but it's brief one
|
||||
print("mkdir -p '%s'" % (directory))
|
||||
return
|
||||
for ancestor in ancestry(path):
|
||||
if not os.path.isdir(syspath(ancestor)):
|
||||
try:
|
||||
|
|
@ -404,13 +388,10 @@ def samefile(p1, p2):
|
|||
return shutil._samefile(syspath(p1), syspath(p2))
|
||||
|
||||
|
||||
def remove(path, soft=True, pretend=False):
|
||||
def remove(path, soft=True):
|
||||
"""Remove the file. If `soft`, then no error will be raised if the
|
||||
file does not exist.
|
||||
"""
|
||||
if pretend:
|
||||
print("rm '%s'" % (path))
|
||||
return
|
||||
path = syspath(path)
|
||||
if soft and not os.path.exists(path):
|
||||
return
|
||||
|
|
@ -420,15 +401,12 @@ def remove(path, soft=True, pretend=False):
|
|||
raise FilesystemError(exc, 'delete', (path,), traceback.format_exc())
|
||||
|
||||
|
||||
def copy(path, dest, replace=False, pretend=False):
|
||||
def copy(path, dest, replace=False):
|
||||
"""Copy a plain file. Permissions are not copied. If `dest` already
|
||||
exists, raises a FilesystemError unless `replace` is True. Has no
|
||||
effect if `path` is the same as `dest`. Paths are translated to
|
||||
system paths before the syscall.
|
||||
"""
|
||||
if pretend:
|
||||
print("cp '%s' '%s'" % (path, dest))
|
||||
return
|
||||
if samefile(path, dest):
|
||||
return
|
||||
path = syspath(path)
|
||||
|
|
@ -442,7 +420,7 @@ def copy(path, dest, replace=False, pretend=False):
|
|||
traceback.format_exc())
|
||||
|
||||
|
||||
def move(path, dest, replace=False, pretend=False):
|
||||
def move(path, dest, replace=False):
|
||||
"""Rename a file. `dest` may not be a directory. If `dest` already
|
||||
exists, raises an OSError unless `replace` is True. Has no effect if
|
||||
`path` is the same as `dest`. If the paths are on different
|
||||
|
|
@ -450,9 +428,6 @@ def move(path, dest, replace=False, pretend=False):
|
|||
instead, in which case metadata will *not* be preserved. Paths are
|
||||
translated to system paths.
|
||||
"""
|
||||
if pretend:
|
||||
print("mv '%s' '%s'" % (path, dest))
|
||||
return
|
||||
if samefile(path, dest):
|
||||
return
|
||||
path = syspath(path)
|
||||
|
|
@ -643,7 +618,7 @@ def cpu_count():
|
|||
return 1
|
||||
|
||||
|
||||
def command_output(cmd, shell=False, pretend=False):
|
||||
def command_output(cmd, shell=False):
|
||||
"""Runs the command and returns its output after it has exited.
|
||||
|
||||
``cmd`` is a list of arguments starting with the command names. If
|
||||
|
|
@ -658,9 +633,6 @@ def command_output(cmd, shell=False, pretend=False):
|
|||
Python 2.6 and which can have problems if lots of output is sent to
|
||||
stderr.
|
||||
"""
|
||||
if pretend:
|
||||
print(cmd)
|
||||
return
|
||||
with open(os.devnull, 'wb') as devnull:
|
||||
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=devnull,
|
||||
close_fds=platform.system() != 'Windows',
|
||||
|
|
|
|||
|
|
@ -90,7 +90,7 @@ def encode(command, source, dest, pretend=False):
|
|||
quiet = config['convert']['quiet'].get()
|
||||
|
||||
if not quiet and not pretend:
|
||||
log.info(u'Started encoding {0}'.format(util.displayable_path(source)))
|
||||
log.info(u'Encoding {0}'.format(util.displayable_path(source)))
|
||||
|
||||
command = Template(command).safe_substitute({
|
||||
'source': pipes.quote(source),
|
||||
|
|
@ -100,10 +100,12 @@ def encode(command, source, dest, pretend=False):
|
|||
log.debug(u'convert: executing: {0}'
|
||||
.format(util.displayable_path(command)))
|
||||
|
||||
if pretend:
|
||||
log.info(command)
|
||||
return
|
||||
|
||||
try:
|
||||
util.command_output(command, shell=True, pretend=pretend)
|
||||
if pretend:
|
||||
return
|
||||
util.command_output(command, shell=True)
|
||||
except subprocess.CalledProcessError:
|
||||
# Something went wrong (probably Ctrl+C), remove temporary files
|
||||
log.info(u'Encoding {0} failed. Cleaning up...'
|
||||
|
|
@ -152,8 +154,9 @@ def convert_item(dest_dir, keep_new, path_formats, command, ext,
|
|||
# Ensure that only one thread tries to create directories at a
|
||||
# time. (The existence check is not atomic with the directory
|
||||
# creation inside this function.)
|
||||
with _fs_lock:
|
||||
util.mkdirall(dest, pretend)
|
||||
if not pretend:
|
||||
with _fs_lock:
|
||||
util.mkdirall(dest)
|
||||
|
||||
if os.path.exists(util.syspath(dest)):
|
||||
log.info(u'Skipping {0} (target file exists)'.format(
|
||||
|
|
@ -162,14 +165,29 @@ def convert_item(dest_dir, keep_new, path_formats, command, ext,
|
|||
continue
|
||||
|
||||
if keep_new:
|
||||
log.info(u'Moving to {0}'.
|
||||
format(util.displayable_path(original)))
|
||||
util.move(item.path, original, pretend)
|
||||
if pretend:
|
||||
log.info(u'mv {0} {1}'.format(
|
||||
util.displayable_path(item.path),
|
||||
util.displayable_path(original),
|
||||
))
|
||||
else:
|
||||
log.info(u'Moving to {0}'.format(
|
||||
util.displayable_path(original))
|
||||
)
|
||||
util.move(item.path, original)
|
||||
|
||||
if not should_transcode(item):
|
||||
# No transcoding necessary.
|
||||
log.info(u'Copying {0}'.format(util.displayable_path(item.path)))
|
||||
util.copy(original, converted, pretend)
|
||||
if pretend:
|
||||
log.info(u'cp {0} {1}'.format(
|
||||
util.displayable_path(original),
|
||||
util.displayable_path(converted),
|
||||
))
|
||||
else:
|
||||
# No transcoding necessary.
|
||||
log.info(u'Copying {0}'.format(
|
||||
util.displayable_path(item.path))
|
||||
)
|
||||
util.copy(original, converted)
|
||||
else:
|
||||
try:
|
||||
encode(command, original, converted, pretend)
|
||||
|
|
@ -177,8 +195,7 @@ def convert_item(dest_dir, keep_new, path_formats, command, ext,
|
|||
continue
|
||||
|
||||
if pretend:
|
||||
# Should we add support for tagging and after_convert plugins?
|
||||
continue # A yield is used at the start of the loop
|
||||
continue
|
||||
|
||||
# Write tags from the database to the converted file.
|
||||
item.write(path=converted)
|
||||
|
|
@ -238,7 +255,7 @@ def convert_func(lib, opts, args):
|
|||
command, ext = get_format(opts.format)
|
||||
|
||||
pretend = opts.pretend if opts.pretend is not None else \
|
||||
config['convert']['pretend'].get()
|
||||
config['convert']['pretend'].get(bool)
|
||||
|
||||
if not pretend:
|
||||
ui.commands.list_items(lib, ui.decargs(args), opts.album, None)
|
||||
|
|
@ -299,7 +316,7 @@ class ConvertPlugin(BeetsPlugin):
|
|||
def commands(self):
|
||||
cmd = ui.Subcommand('convert', help='convert to external location')
|
||||
cmd.parser.add_option('-p', '--pretend', action='store_true',
|
||||
help='only show what would happen')
|
||||
help='show actions but do nothing')
|
||||
cmd.parser.add_option('-a', '--album', action='store_true',
|
||||
help='choose albums instead of tracks')
|
||||
cmd.parser.add_option('-t', '--threads', action='store', type='int',
|
||||
|
|
|
|||
|
|
@ -67,8 +67,10 @@ Little improvements and fixes:
|
|||
import.
|
||||
* :doc:`/plugins/chroma`: A new ``auto`` configuration option disables
|
||||
fingerprinting on import. Thanks to ddettrittus.
|
||||
* :doc:`/plugins/convert`: Add ``--format`` option to select the
|
||||
transoding command from the command-line.
|
||||
* :doc:`/plugins/convert`: A new ``--format`` option to can select the
|
||||
transcoding preset from the command-line.
|
||||
* :doc:`/plugins/convert`: Transcoding presets can now omit their filename
|
||||
extensions (extensions default to the name of the preset).
|
||||
* A new :ref:`asciify-paths` configuration option replaces all non-ASCII
|
||||
characters in paths.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue