From f554e2e4a0f3c01c7fc3ae17e2fa4d77074f4b84 Mon Sep 17 00:00:00 2001
From: Dietrich Daroch <dietr1ch@acm.org>
Date: Mon, 28 Jul 2014 20:13:15 -0400
Subject: [PATCH 01/16] [Improvement] --pretend option for the convert plugin

Partially resolves #877 showing:
  - Directory creation
  - Copies
  - Deletes
  - Moves
  - Encodings

Information about tagging and plugins on _after_convert_ is not
currently shown. That requires changing the plugins to support the
pretend option, so a lot of work may be needed and it doesn't seem to be
helpful enough for me.
---
 beets/util/__init__.py | 35 ++++++++++++++++++++++++++++++-----
 beetsplug/convert.py   | 41 +++++++++++++++++++++++++++--------------
 2 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/beets/util/__init__.py b/beets/util/__init__.py
index 428de312a..6b8cf6647 100644
--- a/beets/util/__init__.py
+++ b/beets/util/__init__.py
@@ -24,6 +24,7 @@ from collections import defaultdict
 import traceback
 import subprocess
 import platform
+from sets import Set
 
 
 MAX_FILENAME_LENGTH = 200
@@ -201,10 +202,22 @@ def sorted_walk(path, ignore=(), logger=None):
             yield res
 
 
-def mkdirall(path):
+#We don't create directories on dry-runs, but we must pretend they exist
+directories_created = Set()
+def mkdirall(path, pretend=False):
     """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:
@@ -388,10 +401,13 @@ def samefile(p1, p2):
     return shutil._samefile(syspath(p1), syspath(p2))
 
 
-def remove(path, soft=True):
+def remove(path, soft=True, pretend=False):
     """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
@@ -401,12 +417,15 @@ def remove(path, soft=True):
         raise FilesystemError(exc, 'delete', (path,), traceback.format_exc())
 
 
-def copy(path, dest, replace=False):
+def copy(path, dest, replace=False, pretend=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)
@@ -420,7 +439,7 @@ def copy(path, dest, replace=False):
                               traceback.format_exc())
 
 
-def move(path, dest, replace=False):
+def move(path, dest, replace=False, pretend=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
@@ -428,6 +447,9 @@ def move(path, dest, replace=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)
@@ -618,7 +640,7 @@ def cpu_count():
         return 1
 
 
-def command_output(cmd, shell=False):
+def command_output(cmd, shell=False, pretend=False):
     """Runs the command and returns its output after it has exited.
 
     ``cmd`` is a list of arguments starting with the command names.  If
@@ -633,6 +655,9 @@ def command_output(cmd, shell=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',
diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index 9892cc001..8fce6d19f 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -83,7 +83,7 @@ def get_format():
         )
 
 
-def encode(source, dest):
+def encode(source, dest, pretend=False):
     """Encode ``source`` to ``dest`` using the command from ``get_format()``.
 
     Raises an ``ui.UserError`` if the command was not found and a
@@ -92,7 +92,7 @@ def encode(source, dest):
     """
     quiet = config['convert']['quiet'].get()
 
-    if not quiet:
+    if not quiet and not pretend:
         log.info(u'Started encoding {0}'.format(util.displayable_path(source)))
 
     command, _ = get_format()
@@ -105,7 +105,9 @@ def encode(source, dest):
               .format(util.displayable_path(command)))
 
     try:
-        util.command_output(command, shell=True)
+        util.command_output(command, shell=True, pretend=pretend)
+        if pretend:
+            return
     except subprocess.CalledProcessError:
         # Something went wrong (probably Ctrl+C), remove temporary files
         log.info(u'Encoding {0} failed. Cleaning up...'
@@ -118,7 +120,7 @@ def encode(source, dest):
             u'convert: could invoke ffmpeg: {0}'.format(exc)
         )
 
-    if not quiet:
+    if not quiet and not pretend:
         log.info(u'Finished encoding {0}'.format(
             util.displayable_path(source))
         )
@@ -134,7 +136,7 @@ def should_transcode(item):
         item.bitrate >= 1000 * maxbr
 
 
-def convert_item(dest_dir, keep_new, path_formats):
+def convert_item(dest_dir, keep_new, path_formats, pretend=False):
     while True:
         item = yield
         dest = _destination(dest_dir, item, keep_new, path_formats)
@@ -149,7 +151,7 @@ def convert_item(dest_dir, keep_new, path_formats):
         # time. (The existence check is not atomic with the directory
         # creation inside this function.)
         with _fs_lock:
-            util.mkdirall(dest)
+            util.mkdirall(dest, pretend)
 
         # When keeping the new file in the library, we first move the
         # current (pristine) file to the destination. We'll then copy it
@@ -157,7 +159,7 @@ def convert_item(dest_dir, keep_new, path_formats):
         if keep_new:
             log.info(u'Moving to {0}'.
                      format(util.displayable_path(dest)))
-            util.move(item.path, dest)
+            util.move(item.path, dest, pretend)
             original = dest
             _, ext = get_format()
             converted = os.path.splitext(item.path)[0] + ext
@@ -168,13 +170,17 @@ def convert_item(dest_dir, keep_new, path_formats):
         if not should_transcode(item):
             # No transcoding necessary.
             log.info(u'Copying {0}'.format(util.displayable_path(item.path)))
-            util.copy(original, converted)
+            util.copy(original, converted, pretend)
         else:
             try:
-                encode(original, converted)
+                encode(original, converted, pretend)
             except subprocess.CalledProcessError:
                 continue
 
+        if pretend:
+            #Should we add support for tagging and after_convert plugins?
+            continue #A yield is used at the start of the loop
+
         # Write tags from the database to the converted file.
         item.write(path=converted)
 
@@ -229,17 +235,21 @@ def convert_func(lib, opts, args):
     else:
         path_formats = ui.get_path_formats(config['convert']['paths'])
 
-    ui.commands.list_items(lib, ui.decargs(args), opts.album, None)
+    pretend = opts.pretend if opts.pretend is not None else \
+        config['convert']['pretend'].get()
 
-    if not ui.input_yn("Convert? (Y/n)"):
-        return
+    if not pretend:
+        ui.commands.list_items(lib, ui.decargs(args), opts.album, None)
+
+        if not ui.input_yn("Convert? (Y/n)"):
+            return
 
     if opts.album:
         items = (i for a in lib.albums(ui.decargs(args)) for i in a.items())
     else:
         items = iter(lib.items(ui.decargs(args)))
-    convert = [convert_item(dest, keep_new, path_formats)
-               for i in range(threads)]
+    convert = [convert_item(dest, keep_new, path_formats, pretend)
+               for _ in range(threads)]
     pipe = util.pipeline.Pipeline([items, convert])
     pipe.run_parallel()
 
@@ -249,6 +259,7 @@ class ConvertPlugin(BeetsPlugin):
         super(ConvertPlugin, self).__init__()
         self.config.add({
             u'dest': None,
+            u'pretend': False,
             u'threads': util.cpu_count(),
             u'format': u'mp3',
             u'formats': {
@@ -295,6 +306,8 @@ 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')
         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',

From ea4832e212c97f4d9c763767ca2ea402d6da00f3 Mon Sep 17 00:00:00 2001
From: Dietrich Daroch <dietr1ch@acm.org>
Date: Wed, 30 Jul 2014 14:35:19 -0400
Subject: [PATCH 02/16] [PEP8] I didn't had a pep8 checker on vim :c

---
 beets/util/__init__.py | 13 ++++++++-----
 beetsplug/convert.py   |  4 ++--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/beets/util/__init__.py b/beets/util/__init__.py
index 6b8cf6647..e91a12f91 100644
--- a/beets/util/__init__.py
+++ b/beets/util/__init__.py
@@ -202,21 +202,24 @@ def sorted_walk(path, ignore=(), logger=None):
             yield res
 
 
-#We don't create directories on dry-runs, but we must pretend they exist
+# We don't create directories on dry-runs, but we must pretend they exist
 directories_created = Set()
+
+
 def mkdirall(path, pretend=False):
     """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 = 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) )
+            # 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)):
diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index 8fce6d19f..8bf048039 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -178,8 +178,8 @@ def convert_item(dest_dir, keep_new, path_formats, pretend=False):
                 continue
 
         if pretend:
-            #Should we add support for tagging and after_convert plugins?
-            continue #A yield is used at the start of the loop
+            # Should we add support for tagging and after_convert plugins?
+            continue  # A yield is used at the start of the loop
 
         # Write tags from the database to the converted file.
         item.write(path=converted)

From b27409684e113d5c8f02b5ac4936f17dcf9174e5 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 5 Aug 2014 10:45:32 +0200
Subject: [PATCH 03/16] convert: Add --format option

This option allows the user to specify the format on the command line
instead of editing the configuration.

The commit also includes some refactoring. In particular adding
arguments to functions to avoid dependence on global state.

Doc and Changelog in next commit
---
 beetsplug/convert.py | 118 ++++++++++++++++++++++---------------------
 test/test_convert.py |  24 +++++++--
 2 files changed, 82 insertions(+), 60 deletions(-)

diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index 9892cc001..37395db1a 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -37,26 +37,22 @@ ALIASES = {
 }
 
 
-def _destination(dest_dir, item, keep_new, path_formats):
-    """Return the path under `dest_dir` where the file should be placed
-    (possibly after conversion).
+def replace_ext(path, ext):
+    """Return the path with its extension replaced by `ext`.
+
+    The new extension must not contain a leading dot.
     """
-    dest = item.destination(basedir=dest_dir, path_formats=path_formats)
-    if keep_new:
-        # When we're keeping the converted file, no extension munging
-        # occurs.
-        return dest
-    else:
-        # Otherwise, replace the extension.
-        _, ext = get_format()
-        return os.path.splitext(dest)[0] + ext
+    return os.path.splitext(path)[0] + '.' + ext
 
 
-def get_format():
-    """Get the currently configured format command and extension.
+def get_format(format=None):
+    """Return the command tempate and the extension from the config.
     """
-    format = config['convert']['format'].get(unicode).lower()
+    if not format:
+        format = config['convert']['format'].get(unicode).lower()
     format = ALIASES.get(format, format)
+    # TODO extension may default to format so this doesn't have to be a
+    # dictionary
     format_info = config['convert']['formats'][format].get(dict)
 
     # Convenience and backwards-compatibility shortcuts.
@@ -74,7 +70,7 @@ def get_format():
     try:
         return (
             format_info['command'].encode('utf8'),
-            (u'.' + format_info['extension']).encode('utf8'),
+            format_info['extension'].encode('utf8'),
         )
     except KeyError:
         raise ui.UserError(
@@ -83,11 +79,10 @@ def get_format():
         )
 
 
-def encode(source, dest):
-    """Encode ``source`` to ``dest`` using the command from ``get_format()``.
+def encode(command, source, dest):
+    """Encode `source` to `dest` using command template `command`.
 
-    Raises an ``ui.UserError`` if the command was not found and a
-    ``subprocess.CalledProcessError`` if the command exited with a
+    Raises `subprocess.CalledProcessError` if the command exited with a
     non-zero status code.
     """
     quiet = config['convert']['quiet'].get()
@@ -95,7 +90,6 @@ def encode(source, dest):
     if not quiet:
         log.info(u'Started encoding {0}'.format(util.displayable_path(source)))
 
-    command, _ = get_format()
     command = Template(command).safe_substitute({
         'source': pipes.quote(source),
         'dest':   pipes.quote(dest),
@@ -115,7 +109,7 @@ def encode(source, dest):
         raise
     except OSError as exc:
         raise ui.UserError(
-            u'convert: could invoke ffmpeg: {0}'.format(exc)
+            u"convert: could invoke '{0}': {0}".format(command, exc)
         )
 
     if not quiet:
@@ -134,16 +128,21 @@ def should_transcode(item):
         item.bitrate >= 1000 * maxbr
 
 
-def convert_item(dest_dir, keep_new, path_formats):
+def convert_item(dest_dir, keep_new, path_formats, command, ext):
     while True:
         item = yield
-        dest = _destination(dest_dir, item, keep_new, path_formats)
+        dest = item.destination(basedir=dest_dir, path_formats=path_formats)
 
-        if os.path.exists(util.syspath(dest)):
-            log.info(u'Skipping {0} (target file exists)'.format(
-                util.displayable_path(item.path)
-            ))
-            continue
+        # When keeping the new file in the library, we first move the
+        # current (pristine) file to the destination. We'll then copy it
+        # back to its old path or transcode it to a new path.
+        if keep_new:
+            original = dest
+            converted = replace_ext(item.path, ext)
+        else:
+            original = item.path
+            dest = replace_ext(dest, ext)
+            converted = dest
 
         # Ensure that only one thread tries to create directories at a
         # time. (The existence check is not atomic with the directory
@@ -151,19 +150,16 @@ def convert_item(dest_dir, keep_new, path_formats):
         with _fs_lock:
             util.mkdirall(dest)
 
-        # When keeping the new file in the library, we first move the
-        # current (pristine) file to the destination. We'll then copy it
-        # back to its old path or transcode it to a new path.
+        if os.path.exists(util.syspath(dest)):
+            log.info(u'Skipping {0} (target file exists)'.format(
+                util.displayable_path(item.path)
+            ))
+            continue
+
         if keep_new:
             log.info(u'Moving to {0}'.
-                     format(util.displayable_path(dest)))
-            util.move(item.path, dest)
-            original = dest
-            _, ext = get_format()
-            converted = os.path.splitext(item.path)[0] + ext
-        else:
-            original = item.path
-            converted = dest
+                     format(util.displayable_path(original)))
+            util.move(item.path, original)
 
         if not should_transcode(item):
             # No transcoding necessary.
@@ -171,7 +167,7 @@ def convert_item(dest_dir, keep_new, path_formats):
             util.copy(original, converted)
         else:
             try:
-                encode(original, converted)
+                encode(command, original, converted)
             except subprocess.CalledProcessError:
                 continue
 
@@ -198,12 +194,12 @@ def convert_on_import(lib, item):
     library.
     """
     if should_transcode(item):
-        _, ext = get_format()
+        command, ext = get_format()
         fd, dest = tempfile.mkstemp(ext)
         os.close(fd)
         _temp_files.append(dest)  # Delete the transcode later.
         try:
-            encode(item.path, dest)
+            encode(command, item.path, dest)
         except subprocess.CalledProcessError:
             return
         item.path = dest
@@ -213,21 +209,24 @@ def convert_on_import(lib, item):
 
 
 def convert_func(lib, opts, args):
-    dest = opts.dest if opts.dest is not None else \
-        config['convert']['dest'].get()
-
-    if not dest:
+    if not opts.dest:
+        opts.dest = config['convert']['dest'].get()
+    if not opts.dest:
         raise ui.UserError('no convert destination set')
+    opts.dest = util.bytestring_path(opts.dest)
 
-    dest = util.bytestring_path(dest)
-    threads = opts.threads if opts.threads is not None else \
-        config['convert']['threads'].get(int)
-    keep_new = opts.keep_new
+    if not opts.threads:
+        opts.threads = config['convert']['threads'].get(int)
 
-    if not config['convert']['paths']:
-        path_formats = ui.get_path_formats()
-    else:
+    if config['convert']['paths']:
         path_formats = ui.get_path_formats(config['convert']['paths'])
+    else:
+        path_formats = ui.get_path_formats()
+
+    if not opts.format:
+        opts.format = config['convert']['format'].get(unicode).lower()
+
+    command, ext = get_format(opts.format)
 
     ui.commands.list_items(lib, ui.decargs(args), opts.album, None)
 
@@ -238,9 +237,12 @@ def convert_func(lib, opts, args):
         items = (i for a in lib.albums(ui.decargs(args)) for i in a.items())
     else:
         items = iter(lib.items(ui.decargs(args)))
-    convert = [convert_item(dest, keep_new, path_formats)
-               for i in range(threads)]
-    pipe = util.pipeline.Pipeline([items, convert])
+    convert_stages = []
+    for i in range(opts.threads):
+        convert_stages.append(
+            convert_item(opts.dest, opts.keep_new, path_formats, command, ext)
+        )
+    pipe = util.pipeline.Pipeline([items, convert_stages])
     pipe.run_parallel()
 
 
@@ -305,6 +307,8 @@ class ConvertPlugin(BeetsPlugin):
                               and move the old files')
         cmd.parser.add_option('-d', '--dest', action='store',
                               help='set the destination directory')
+        cmd.parser.add_option('-f', '--format', action='store', dest='format',
+                              help='set the destination directory')
         cmd.func = convert_func
         return [cmd]
 
diff --git a/test/test_convert.py b/test/test_convert.py
index 21de94696..ebe30ad8c 100644
--- a/test/test_convert.py
+++ b/test/test_convert.py
@@ -72,9 +72,21 @@ class ConvertCliTest(unittest.TestCase, TestHelper):
         self.load_plugins('convert')
 
         self.convert_dest = os.path.join(self.temp_dir, 'convert_dest')
-        self.config['convert']['dest'] = str(self.convert_dest)
-        self.config['convert']['command'] = u'cp $source $dest'
-        self.config['convert']['paths']['default'] = u'converted'
+        self.config['convert'] = {
+            'dest': self.convert_dest,
+            'paths': {'default': 'converted'},
+            'format': 'mp3',
+            'formats': {
+                'mp3': {
+                    'command': 'cp $source $dest',
+                    'extension': 'mp3',
+                },
+                'opus': {
+                    'command': 'cp $source $dest',
+                    'extension': 'opus',
+                }
+            }
+        }
 
     def tearDown(self):
         self.unload_plugins()
@@ -95,6 +107,12 @@ class ConvertCliTest(unittest.TestCase, TestHelper):
         self.item.load()
         self.assertEqual(os.path.splitext(self.item.path)[1], '.mp3')
 
+    def test_format_option(self):
+        with control_stdin('y'):
+            self.run_command('convert', '--format', 'opus', self.item.path)
+            converted = os.path.join(self.convert_dest, 'converted.opus')
+        self.assertTrue(os.path.isfile(converted))
+
     def test_embed_album_art(self):
         self.config['convert']['embed'] = True
         image_path = os.path.join(_common.RSRC, 'image-2x3.jpg')

From c2822a5b90d97f0bdbf6a996b2e489b73f29ec73 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 5 Aug 2014 11:50:06 +0200
Subject: [PATCH 04/16] Documentation and changelog for b2740968

---
 docs/changelog.rst       |  2 +
 docs/plugins/convert.rst | 94 ++++++++++++++++++++--------------------
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 26e095cd0..74ce6195b 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -67,6 +67,8 @@ 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.
 
 
 1.3.6 (May 10, 2014)
diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst
index 5eff02f1e..bf5d62ddb 100644
--- a/docs/plugins/convert.rst
+++ b/docs/plugins/convert.rst
@@ -4,23 +4,17 @@ Convert Plugin
 The ``convert`` plugin lets you convert parts of your collection to a
 directory of your choice, transcoding audio and embedding album art along the
 way. It can transcode to and from any format using a configurable command
-line. It will skip files that are already present in the target directory.
-Converted files follow the same path formats as your library.
-
-.. _FFmpeg: http://ffmpeg.org
+line.
 
 
 Installation
 ------------
 
-First, enable the ``convert`` plugin (see :doc:`/plugins/index`).
+Enable the ``convert`` plugin in your configuration (see
+:doc:`/plugins/index`).  By default, the plugin depends on `FFmpeg`_ to
+transcode the audio, so you might want to install it.
 
-To transcode music, this plugin requires the ``ffmpeg`` command-line
-tool. If its executable is in your path, it  will be found automatically
-by the plugin. Otherwise, configure the plugin to locate the executable::
-
-    convert:
-        ffmpeg: /usr/bin/ffmpeg
+.. _FFmpeg: http://ffmpeg.org
 
 
 Usage
@@ -28,11 +22,22 @@ Usage
 
 To convert a part of your collection, run ``beet convert QUERY``. This
 will display all items matching ``QUERY`` and ask you for confirmation before
-starting the conversion. The ``-a`` (or ``--album``) option causes the command
+starting the conversion. The command will then transcode all the
+matching files to the destination directory given by the ``-d``
+(``--dest``) option or the ``dest`` configuration. The path layout
+mirrors that of your library, but it may be customized through the
+``paths`` configuration.
+
+The plugin uses a command-line program to transcode the audio. With the
+``-f`` (``--format``) option you can choose the transcoding command
+and customize the available commands
+:ref:`through the configuration <convert-format-config>`.
+
+The ``-a`` (or ``--album``) option causes the command
 to match albums instead of tracks.
 
-The ``-t`` (``--threads``) and ``-d`` (``--dest``) options allow you to specify
-or overwrite the respective configuration options.
+The ``-t`` (``--threads``) option allows you to specify or overwrite
+the respective configuration option.
 
 By default, the command places converted files into the destination directory
 and leaves your library pristine. To instead back up your original files into
@@ -48,7 +53,7 @@ The plugin offers several configuration options, all of which live under the
 
 * ``dest`` sets the directory the files will be converted (or copied) to.
   A destination is required---you either have to provide it in the config file
-  or on the command line using the ``-d`` flag.
+  or on the command-line using the ``-d`` flag.
 * ``embed`` indicates whether or not to embed album art in converted items.
   Default: true.
 * If you set ``max_bitrate``, all lossy files with a higher bitrate will be
@@ -69,39 +74,14 @@ The plugin offers several configuration options, all of which live under the
   encoding. By default, the plugin will detect the number of processors
   available and use them all.
 
-These config options control the transcoding process:
+.. _convert-format-config:
 
-* ``format`` is the name of the audio file format to transcode to. Files that
-  are already in the format (and are below the maximum bitrate) will not be
-  transcoded. The plugin includes default commands for the formats MP3, AAC,
-  ALAC, FLAC, Opus, Vorbis, and Windows Media; the default is MP3. If you want
-  to use a different format (or customize the transcoding options), use the
-  options below.
-* ``extension`` is the filename extension to be used for newly transcoded
-  files. This is implied by the ``format`` option, but you can set it yourself
-  if you're using a different format.
-* ``command`` is the command line to use to transcode audio. A default
-  command, usually using an FFmpeg invocation, is implied by the ``format``
-  option. The tokens ``$source`` and ``$dest`` in the command are replaced
-  with the paths to the existing and new file. For example, the command
-  ``ffmpeg -i $source -y -aq 4 $dest`` transcodes to MP3 using FFmpeg at the
-  V4 quality level.
+Configuring the transcoding command
+```````````````````````````````````
 
-Here's an example configuration::
-
-    convert:
-        embed: false
-        format: aac
-        max_bitrate: 200
-        dest: /home/user/MusicForPhone
-        threads: 4
-        paths:
-            default: $albumartist/$title
-
-If you have several formats you want to switch between, you can list them
-under the ``formats`` key and refer to them using the ``format`` option. Each
-key under ``formats`` should contain values for ``command`` and ``extension``
-as described above::
+You can customize the transcoding command through the ``formats`` map
+and select a command with the ``--format`` command-line option or the
+``format`` configuration.::
 
     convert:
         format: speex
@@ -112,3 +92,25 @@ as described above::
             wav:
                 command: ffmpeg -i $source -y -acodec pcm_s16le $dest
                 extension: wav
+
+In this example ``beet convert`` will use the *speex* command by
+default. To convert the audio to `wav`, run ``beet convert -f wav``.
+
+Each entry in the ``formats`` map consists of a key (the name of the
+format) as well as the command and the extension.  ``extension`` is the
+filename extension to be used for newly transcoded files.
+``command`` is the command-line to use to transcode audio. The tokens
+``$source`` and ``$dest`` in the command are replaced with the paths to
+the existing and new file.
+
+The plugin in comes with default commands for the most common audio
+formats: `mp3`, `alac`, `flac`, `aac`, `opus`, `ogg`, `wmv`. For
+details have a look at the output of ``beet config -d``.
+
+For a one-command-fits-all solution use the ``convert.command`` and
+``convert.extension`` options. If these are set the formats are ignored
+and the given command is used for all conversions.::
+
+    convert:
+        command: ffmpeg -i $source -y -vn -aq 2 $dest
+        extension: mp3

From 29e4fde57165e7dfc1482d566934d4bd125fa7d3 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Tue, 5 Aug 2014 12:06:35 +0200
Subject: [PATCH 05/16] convert: Simplify format configuration.

We don't have to specify the extension. By default it is the same as
the format name.
---
 beetsplug/convert.py     | 71 ++++++++++++++++------------------------
 docs/plugins/convert.rst | 16 ++++-----
 test/test_convert.py     |  9 ++---
 3 files changed, 40 insertions(+), 56 deletions(-)

diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index 37395db1a..8113c86ca 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -25,6 +25,7 @@ import pipes
 from beets import ui, util, plugins, config
 from beets.plugins import BeetsPlugin
 from beetsplug.embedart import embed_item
+from beets.util.confit import ConfigTypeError
 
 log = logging.getLogger('beets')
 _fs_lock = threading.Lock()
@@ -51,32 +52,33 @@ def get_format(format=None):
     if not format:
         format = config['convert']['format'].get(unicode).lower()
     format = ALIASES.get(format, format)
-    # TODO extension may default to format so this doesn't have to be a
-    # dictionary
-    format_info = config['convert']['formats'][format].get(dict)
-
-    # Convenience and backwards-compatibility shortcuts.
-    keys = config['convert'].keys()
-    if 'command' in keys:
-        format_info['command'] = config['convert']['command'].get(unicode)
-    elif 'opts' in keys:
-        # Undocumented option for backwards compatibility with < 1.3.1.
-        format_info['command'] = u'ffmpeg -i $source -y {0} $dest'.format(
-            config['convert']['opts'].get(unicode)
-        )
-    if 'extension' in keys:
-        format_info['extension'] = config['convert']['extension'].get(unicode)
 
     try:
-        return (
-            format_info['command'].encode('utf8'),
-            format_info['extension'].encode('utf8'),
-        )
+        format_info = config['convert']['formats'][format].get(dict)
+        command = format_info['command']
+        extension = format_info['extension']
     except KeyError:
         raise ui.UserError(
             u'convert: format {0} needs "command" and "extension" fields'
             .format(format)
         )
+    except ConfigTypeError:
+        command = config['convert']['formats'][format].get(str)
+        extension = format
+
+    # Convenience and backwards-compatibility shortcuts.
+    keys = config['convert'].keys()
+    if 'command' in keys:
+        command = config['convert']['command'].get(unicode)
+    elif 'opts' in keys:
+        # Undocumented option for backwards compatibility with < 1.3.1.
+        command = u'ffmpeg -i $source -y {0} $dest'.format(
+            config['convert']['opts'].get(unicode)
+        )
+    if 'extension' in keys:
+        extension = config['convert']['extension'].get(unicode)
+
+    return (command.encode('utf8'), extension.encode('utf8'))
 
 
 def encode(command, source, dest):
@@ -263,29 +265,14 @@ class ConvertPlugin(BeetsPlugin):
                     u'command': u'ffmpeg -i $source -y -vn -acodec alac $dest',
                     u'extension': u'm4a',
                 },
-                u'flac': {
-                    u'command': u'ffmpeg -i $source -y -vn -acodec flac $dest',
-                    u'extension': u'flac',
-                },
-                u'mp3': {
-                    u'command': u'ffmpeg -i $source -y -vn -aq 2 $dest',
-                    u'extension': u'mp3',
-                },
-                u'opus': {
-                    u'command': u'ffmpeg -i $source -y -vn -acodec libopus '
-                                u'-ab 96k $dest',
-                    u'extension': u'opus',
-                },
-                u'ogg': {
-                    u'command': u'ffmpeg -i $source -y -vn -acodec libvorbis '
-                                u'-aq 2 $dest',
-                    u'extension': u'ogg',
-                },
-                u'windows media': {
-                    u'command': u'ffmpeg -i $source -y -vn -acodec wmav2 '
-                                u'-vn $dest',
-                    u'extension': u'wma',
-                },
+                u'flac': u'ffmpeg -i $source -y -vn -acodec flac $dest',
+                u'mp3': u'ffmpeg -i $source -y -vn -aq 2 $dest',
+                u'opus':
+                    u'ffmpeg -i $source -y -vn -acodec libopus -ab 96k $dest',
+                u'ogg':
+                    u'ffmpeg -i $source -y -vn -acodec libvorbis -aq 2 $dest',
+                u'wma':
+                    u'ffmpeg -i $source -y -vn -acodec wmav2 -vn $dest',
             },
             u'max_bitrate': 500,
             u'auto': False,
diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst
index bf5d62ddb..485c80400 100644
--- a/docs/plugins/convert.rst
+++ b/docs/plugins/convert.rst
@@ -89,19 +89,19 @@ and select a command with the ``--format`` command-line option or the
             speex:
                 command: ffmpeg -i $source -y -acodec speex $dest
                 extension: spx
-            wav:
-                command: ffmpeg -i $source -y -acodec pcm_s16le $dest
-                extension: wav
+            wav: ffmpeg -i $source -y -acodec pcm_s16le $dest
 
 In this example ``beet convert`` will use the *speex* command by
 default. To convert the audio to `wav`, run ``beet convert -f wav``.
+This will also use the format key (`wav`) as the file extension.
 
 Each entry in the ``formats`` map consists of a key (the name of the
-format) as well as the command and the extension.  ``extension`` is the
-filename extension to be used for newly transcoded files.
-``command`` is the command-line to use to transcode audio. The tokens
-``$source`` and ``$dest`` in the command are replaced with the paths to
-the existing and new file.
+format) as well as the command and the possibly the file extension.
+``extension`` is the filename extension to be used for newly transcoded
+files.  If only the command is given as a string, the file extension
+defaults to the format’s name. ``command`` is the command-line to use
+to transcode audio. The tokens ``$source`` and ``$dest`` in the command
+are replaced with the paths to the existing and new file.
 
 The plugin in comes with default commands for the most common audio
 formats: `mp3`, `alac`, `flac`, `aac`, `opus`, `ogg`, `wmv`. For
diff --git a/test/test_convert.py b/test/test_convert.py
index ebe30ad8c..24ed44748 100644
--- a/test/test_convert.py
+++ b/test/test_convert.py
@@ -77,13 +77,10 @@ class ConvertCliTest(unittest.TestCase, TestHelper):
             'paths': {'default': 'converted'},
             'format': 'mp3',
             'formats': {
-                'mp3': {
-                    'command': 'cp $source $dest',
-                    'extension': 'mp3',
-                },
+                'mp3': 'cp $source $dest',
                 'opus': {
                     'command': 'cp $source $dest',
-                    'extension': 'opus',
+                    'extension': 'ops',
                 }
             }
         }
@@ -110,7 +107,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper):
     def test_format_option(self):
         with control_stdin('y'):
             self.run_command('convert', '--format', 'opus', self.item.path)
-            converted = os.path.join(self.convert_dest, 'converted.opus')
+            converted = os.path.join(self.convert_dest, 'converted.ops')
         self.assertTrue(os.path.isfile(converted))
 
     def test_embed_album_art(self):

From 76c7ba918604457f0caf739e978cdaf4e566871f Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Wed, 6 Aug 2014 17:53:44 +0200
Subject: [PATCH 06/16] Add asciify_paths configuration option

---
 beets/config_default.yaml     |  1 +
 beets/library.py              |  7 +++++++
 docs/changelog.rst            |  3 +++
 docs/reference/config.rst     | 16 ++++++++++++++++
 docs/reference/pathformat.rst |  3 ++-
 test/test_library.py          | 12 ++++++++++++
 6 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/beets/config_default.yaml b/beets/config_default.yaml
index 58136d56f..689ab44b1 100644
--- a/beets/config_default.yaml
+++ b/beets/config_default.yaml
@@ -32,6 +32,7 @@ replace:
     '\s+$': ''
     '^\s+': ''
 path_sep_replace: _
+asciify_paths: false
 art_filename: cover
 max_filename_length: 0
 
diff --git a/beets/library.py b/beets/library.py
index 95a02c3b8..8496a844e 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -571,8 +571,13 @@ class Item(LibModel):
             subpath = unicodedata.normalize('NFD', subpath)
         else:
             subpath = unicodedata.normalize('NFC', subpath)
+
+        if beets.config['asciify_paths']:
+            subpath = unidecode(subpath)
+
         # Truncate components and remove forbidden characters.
         subpath = util.sanitize_path(subpath, self._db.replacements)
+
         # Encode for the filesystem.
         if not fragment:
             subpath = bytestring_path(subpath)
@@ -830,6 +835,8 @@ class Album(LibModel):
 
         filename_tmpl = Template(beets.config['art_filename'].get(unicode))
         subpath = self.evaluate_template(filename_tmpl, True)
+        if beets.config['asciify_paths']:
+            subpath = unidecode(subpath)
         subpath = util.sanitize_path(subpath,
                                      replacements=self._db.replacements)
         subpath = bytestring_path(subpath)
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 74ce6195b..bfc6da537 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -69,6 +69,9 @@ Little improvements and fixes:
   fingerprinting on import. Thanks to ddettrittus.
 * :doc:`/plugins/convert`: Add ``--format`` option to select the
   transoding command from the command-line.
+* Add :ref:`asciify-paths` configuration option to replace non-ASCII
+  characters in paths.
+
 
 
 1.3.6 (May 10, 2014)
diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index bf9fb6daa..b703d9c70 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -119,6 +119,22 @@ compatibility with Windows-influenced network filesystems like Samba).
 Trailing dots and trailing whitespace, which can cause problems on Windows
 clients, are also removed.
 
+.. _asciify-paths:
+
+asciify_paths
+~~~~~~~~~~~~~
+
+Works like a specialized ``replace`` configuration.  If set to ``yes``,
+all non-ASCII characters in paths created by beets are converted to
+their ASCII equivalents.  For example, if your path template for
+singletons is ``singletons/$title`` and the title of a track is "Café",
+then the track will be saved as ``singletons/Cafe.mp3``.  The changes
+take place before applying the ``replace`` configuration. Uses the
+mapping provided by the `unidecode module`_.  Defaults to ``no``.
+
+.. _unidecode module: http://pypi.python.org/pypi/Unidecode
+
+
 .. _art-filename:
 
 art_filename
diff --git a/docs/reference/pathformat.rst b/docs/reference/pathformat.rst
index 005d4ed63..8efe54cd2 100644
--- a/docs/reference/pathformat.rst
+++ b/docs/reference/pathformat.rst
@@ -69,7 +69,8 @@ These functions are built in to beets:
   nothing if ``falsetext`` is left off).
 * ``%asciify{text}``: Convert non-ASCII characters to their ASCII equivalents.
   For example, "café" becomes "cafe". Uses the mapping provided by the
-  `unidecode module`_.
+  `unidecode module`_. See the :ref:`asciify-paths` configuration
+  option.
 * ``%aunique{identifiers,disambiguators}``: Provides a unique string to
   disambiguate similar albums in the database. See :ref:`aunique`, below.
 * ``%time{date_time,format}``: Return the date and time in any format accepted
diff --git a/test/test_library.py b/test/test_library.py
index c96d01c27..aac58b9e9 100644
--- a/test/test_library.py
+++ b/test/test_library.py
@@ -136,6 +136,10 @@ class DestinationTest(_common.TestCase):
         super(DestinationTest, self).tearDown()
         self.lib._connection().close()
 
+        # Reset config if it was changed in test cases
+        config.clear()
+        config.read(user=False, defaults=True)
+
     def test_directory_works_with_trailing_slash(self):
         self.lib.directory = 'one/'
         self.lib.path_formats = [('default', 'two')]
@@ -447,6 +451,14 @@ class DestinationTest(_common.TestCase):
         dest = self.i.destination(platform='linux2', fragment=True)
         self.assertEqual(dest, u'foo.caf\xe9')
 
+    def test_asciify_and_replace(self):
+        config['asciify_paths'] = True
+        self.lib.replacements = [(re.compile(u'"'), u'q')]
+        self.lib.directory = 'lib'
+        self.lib.path_formats = [('default', '$title')]
+        self.i.title = u'\u201c\u00f6\u2014\u00cf\u201d'
+        self.assertEqual(self.i.destination(), np('lib/qo--Iq'))
+
 
 class ItemFormattedMappingTest(_common.LibTestCase):
     def test_formatted_item_value(self):

From f1388eb23db0e292af90007f78b1abb9c2f422e2 Mon Sep 17 00:00:00 2001
From: Thomas Scholtes <thomas-scholtes@gmx.de>
Date: Wed, 6 Aug 2014 18:00:18 +0200
Subject: [PATCH 07/16] docs: Add note for 'replace' config and unicode

---
 docs/reference/config.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index b703d9c70..d7ca4b090 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -119,6 +119,12 @@ compatibility with Windows-influenced network filesystems like Samba).
 Trailing dots and trailing whitespace, which can cause problems on Windows
 clients, are also removed.
 
+Note that paths might contain special characters such as typographical
+quotes (``“”``). With the configuration above, those will not be
+replaced as they don’t match the typewriter quote (``"``). You either
+have to add them to the replacement list or use the
+:ref:`asciify-paths` configuration option below.
+
 .. _asciify-paths:
 
 asciify_paths

From d5910b4e856debc531bcd1fc59649b27be2f039a Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 10 Aug 2014 16:18:15 -0700
Subject: [PATCH 08/16] Docs tweaks

---
 docs/changelog.rst        |  2 +-
 docs/reference/config.rst | 17 ++++++++++-------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index bfc6da537..edc3919c9 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -69,7 +69,7 @@ Little improvements and fixes:
   fingerprinting on import. Thanks to ddettrittus.
 * :doc:`/plugins/convert`: Add ``--format`` option to select the
   transoding command from the command-line.
-* Add :ref:`asciify-paths` configuration option to replace non-ASCII
+* A new :ref:`asciify-paths` configuration option replaces all non-ASCII
   characters in paths.
 
 
diff --git a/docs/reference/config.rst b/docs/reference/config.rst
index d7ca4b090..04deddd37 100644
--- a/docs/reference/config.rst
+++ b/docs/reference/config.rst
@@ -121,8 +121,8 @@ clients, are also removed.
 
 Note that paths might contain special characters such as typographical
 quotes (``“”``). With the configuration above, those will not be
-replaced as they don’t match the typewriter quote (``"``). You either
-have to add them to the replacement list or use the
+replaced as they don't match the typewriter quote (``"``). To also strip these
+special characters, you can either add them to the replacement list or use the
 :ref:`asciify-paths` configuration option below.
 
 .. _asciify-paths:
@@ -130,13 +130,16 @@ have to add them to the replacement list or use the
 asciify_paths
 ~~~~~~~~~~~~~
 
-Works like a specialized ``replace`` configuration.  If set to ``yes``,
-all non-ASCII characters in paths created by beets are converted to
-their ASCII equivalents.  For example, if your path template for
+Convert all non-ASCII characters in paths to ASCII equivalents.
+
+For example, if your path template for
 singletons is ``singletons/$title`` and the title of a track is "Café",
 then the track will be saved as ``singletons/Cafe.mp3``.  The changes
-take place before applying the ``replace`` configuration. Uses the
-mapping provided by the `unidecode module`_.  Defaults to ``no``.
+take place before applying the :ref:`replace` configuration and are roughly
+equivalent to wrapping all your path templates in the ``%asciify{}``
+:ref:`template function <template-functions>`.
+
+Default: ``no``.
 
 .. _unidecode module: http://pypi.python.org/pypi/Unidecode
 

From e7f1ff0e3fa82a8b901d57f7dad51f5429c7e78a Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 10 Aug 2014 16:42:13 -0700
Subject: [PATCH 09/16] 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.
---
 beets/util/__init__.py | 38 +++++---------------------------
 beetsplug/convert.py   | 49 ++++++++++++++++++++++++++++--------------
 docs/changelog.rst     |  6 ++++--
 3 files changed, 42 insertions(+), 51 deletions(-)

diff --git a/beets/util/__init__.py b/beets/util/__init__.py
index e91a12f91..428de312a 100644
--- a/beets/util/__init__.py
+++ b/beets/util/__init__.py
@@ -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',
diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index a94f4c284..2b1646b2c 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -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',
diff --git a/docs/changelog.rst b/docs/changelog.rst
index edc3919c9..01307b6d8 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -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.
 

From 60c59ca96ae7f5a612d4f84e00a2fa7b79b1b2aa Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 10 Aug 2014 16:51:42 -0700
Subject: [PATCH 10/16] Docs/changelog for #891

---
 docs/changelog.rst       | 6 +++++-
 docs/plugins/convert.rst | 7 ++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 01307b6d8..a7d20d923 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -12,7 +12,8 @@ New stuff
   IFF chunks.
 * A new :ref:`required` configuration option for the importer skips matches
   that are missing certain data. Thanks to oprietop.
-* The new :doc:`/plugins/bpm` lets you manually measure the tempo of a playing song. Thanks to aroquen.
+* The new :doc:`/plugins/bpm` lets you manually measure the tempo of a playing
+  song. Thanks to aroquen.
 
 Little improvements and fixes:
 
@@ -73,6 +74,9 @@ Little improvements and fixes:
   extensions (extensions default to the name of the preset).
 * A new :ref:`asciify-paths` configuration option replaces all non-ASCII
   characters in paths.
+* :doc:`/plugins/convert`: A new ``--pretend`` option lets you preview the
+  commands the plugin will execute without actually taking any action. Thanks
+  to Dietrich Daroch.
 
 
 
diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst
index 485c80400..b94f874d7 100644
--- a/docs/plugins/convert.rst
+++ b/docs/plugins/convert.rst
@@ -36,14 +36,15 @@ and customize the available commands
 The ``-a`` (or ``--album``) option causes the command
 to match albums instead of tracks.
 
-The ``-t`` (``--threads``) option allows you to specify or overwrite
-the respective configuration option.
-
 By default, the command places converted files into the destination directory
 and leaves your library pristine. To instead back up your original files into
 the destination directory and keep converted files in your library, use the
 ``-k`` (or ``--keep-new``) option.
 
+To test your configuration without taking any actions, use the ``--pretend``
+flag. The plugin will print out the commands it will run instead of executing
+them.
+
 
 Configuration
 -------------

From 945e30d1555c090e449c2919826ecba6b4feb5a2 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Sun, 10 Aug 2014 17:18:10 -0700
Subject: [PATCH 11/16] Fix music-crawl error messages (thanks, derwin)

Due to the new exception nesting stuff, we were catching and emitting
exceptions where none was necessary: specifically, when the file was non-music
(which is expected, especially when there are images).
---
 beets/autotag/__init__.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/beets/autotag/__init__.py b/beets/autotag/__init__.py
index b6e2c8f23..86c104b84 100644
--- a/beets/autotag/__init__.py
+++ b/beets/autotag/__init__.py
@@ -52,16 +52,19 @@ def albums_in_dir(path):
         for filename in files:
             try:
                 i = library.Item.from_path(os.path.join(root, filename))
-            except mediafile.FileTypeError:
-                pass
-            except mediafile.UnreadableFileError:
-                log.warn(u'unreadable file: {0}'.format(
-                    displayable_path(filename))
-                )
             except library.ReadError as exc:
-                log.error(u'error reading {0}: {1}'.format(
-                    displayable_path(filename), exc
-                ))
+                if isinstance(exc.reason, mediafile.FileTypeError):
+                    # Silently ignore non-music files.
+                    pass
+                elif isinstance(exc.reason, mediafile.UnreadableFileError):
+                    log.warn(u'unreadable file: {0}'.format(
+                        displayable_path(filename))
+                    )
+                else:
+                    log.error(u'error reading {0}: {1}'.format(
+                        displayable_path(filename),
+                        exc,
+                    ))
             else:
                 items.append(i)
 

From af3bdd8a66985c5614f94185a1856b9ced74657c Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Mon, 11 Aug 2014 16:52:37 -0700
Subject: [PATCH 12/16] echonest: Log on retries (for mersault_)

---
 beetsplug/echonest.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/beetsplug/echonest.py b/beetsplug/echonest.py
index 773e97eb5..e183eb8ef 100644
--- a/beetsplug/echonest.py
+++ b/beetsplug/echonest.py
@@ -137,6 +137,9 @@ class EchonestMetadataPlugin(plugins.BeetsPlugin):
             except pyechonest.util.EchoNestAPIError as e:
                 if e.code == 3:
                     # reached access limit per minute
+                    log.debug(u'echonest: rate-limited on try {0}; '
+                              u'waiting {1} seconds'
+                              .format(i + 1, RETRY_INTERVAL))
                     time.sleep(RETRY_INTERVAL)
                 elif e.code == 5:
                     # specified identifier does not exist

From 64fc3539cd8e1a6577629f8e26787aefaf90dd29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stig=20Inge=20Lea=20Bj=C3=B8rnsen?= <stiginge@pvv.org>
Date: Thu, 14 Aug 2014 00:44:19 +0200
Subject: [PATCH 13/16] Correct the textual description of a date query

The the date query syntax `2008-12..2009-10-11` covers the interval [2008-12-01T00:00:00, 2009-10-12T00:00:00).
---
 docs/reference/query.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/reference/query.rst b/docs/reference/query.rst
index cf981bed2..464b842dc 100644
--- a/docs/reference/query.rst
+++ b/docs/reference/query.rst
@@ -156,7 +156,7 @@ Find all items added before the year 2010::
 
     $ beet ls 'added:..2009'
 
-Find all items added on 2008-12-01 but before 2009-10-12::
+Find all items added on or after 2008-12-01 but before 2009-10-12::
 
     $ beet ls 'added:2008-12..2009-10-11'
 

From 3771134716fc4edcc1428b40053834d40660541d Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Wed, 13 Aug 2014 21:54:43 -0700
Subject: [PATCH 14/16] Add zsh completion script by @vapniks (#862)

---
 docs/reference/cli.rst |  13 ++-
 extra/_beet            | 242 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 253 insertions(+), 2 deletions(-)
 create mode 100644 extra/_beet

diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst
index 1b16d1980..391dfa0e6 100644
--- a/docs/reference/cli.rst
+++ b/docs/reference/cli.rst
@@ -17,12 +17,17 @@ Command-Line Interface
 
         beet COMMAND [ARGS...]
 
-    Beets also offers command line completion via the `completion`_
-    command.  The rest of this document describes the available
+    The rest of this document describes the available
     commands. If you ever need a quick list of what's available, just
     type ``beet help`` or ``beet help COMMAND`` for help with a specific
     command.
 
+    Beets also offers shell completion. For bash, see the `completion`_
+    command; for zsh, see the accompanying `completion script`_ for the
+    ``beet`` command.
+
+
+
 
 Commands
 --------
@@ -399,6 +404,10 @@ Completion of plugin commands only works for those plugins
 that were enabled when running ``beet completion``. If you add a plugin
 later on you will want to re-generate the script.
 
+If you use zsh, take a look instead at the included `completion script`_.
+
+.. _completion script: https://github.com/sampsyo/beets/blob/master/extra/_beet
+
 
 .. only:: man
 
diff --git a/extra/_beet b/extra/_beet
new file mode 100644
index 000000000..1987ed57c
--- /dev/null
+++ b/extra/_beet
@@ -0,0 +1,242 @@
+#compdef beet
+
+# Completion for beets music library manager and MusicBrainz tagger: http://beets.radbox.org/
+
+# useful: argument to _regex_arguments for matching any word
+local matchany=/$'[^\0]##\0'/
+
+# Deal with completions for querying and modifying fields..
+local fieldargs matchquery matchmodify
+local -a fields
+# get list of all fields
+fields=(`beet fields | grep -G '^  ' | sort -u | colrm 1 2`)
+# regexps for matching query and modify terms on the command line
+matchquery=/"(${(j/|/)fields[@]})"$':[^\0]##\0'/
+matchmodify=/"(${(j/|/)fields[@]})"$'(=[^\0]##|!)\0'/
+# Function for getting unique values for field from database (you may need to change the path to the database).
+function _beet_field_values()
+{
+    local -a output fieldvals
+    local library="$(beet config|grep library|cut -f 2 -d ' ')"
+    output=$(sqlite3 ${~library} "select distinct $1 from items;")
+    case $1
+        in
+        lyrics)
+            fieldvals=
+            ;;
+        *)
+            fieldvals=("${(f)output[@]}")
+            ;;
+    esac
+    compadd -P \" -S \" -M 'm:{[:lower:][:upper:]}={[:upper:][:lower:]}' -Q -a fieldvals
+}
+# store call to _values function for completing query terms
+# (first build arguments for completing field values)
+local field
+for field in "${fields[@]}"
+do
+    fieldargs="$fieldargs '$field:::{_beet_field_values $field}'"
+done
+local queryelem modifyelem
+queryelem="_values -S : 'query field (add an extra : to match by regexp)' '::' $fieldargs"
+# store call to _values function for completing modify terms (no need to complete field values)
+modifyelem="_values -S = 'modify field (replace = with ! to remove field)' $(echo "'${^fields[@]}:: '")"
+# Create completion function for queries
+_regex_arguments _beet_query "$matchany" \# \( "$matchquery" ":query:query string:$queryelem" \) \
+    \( "$matchquery" ":query:query string:$queryelem" \) \#
+# store regexps for completing lists of queries and modifications
+local -a query modify
+query=( \( "$matchquery" ":query:query string:{_beet_query}" \) \( "$matchquery" ":query:query string:{_beet_query}" \) \# )
+modify=( \( "$matchmodify" ":modify:modify string:$modifyelem" \) \( "$matchmodify" ":modify:modify string:$modifyelem" \) \# )
+
+# arguments to _regex_arguments for completing files and directories
+local -a files dirs 
+files=("$matchany" ':file:file:_files')
+dirs=("$matchany" ':dir:directory:_dirs')
+
+# Individual options used by subcommands, and global options (must be single quoted).
+# Its much faster if these are hard-coded rather generated using _beet_subcmd_options
+local helpopt formatopt albumopt dontmoveopt writeopt nowriteopt pretendopt pathopt destopt copyopt nocopyopt
+local inferopt noinferopt resumeopt noresumeopt nopromptopt logopt individualopt confirmopt retagopt skipopt noskipopt
+local flatopt groupopt editopt defaultopt noconfirmopt exactopt removeopt configopt debugopt
+helpopt='-h:show this help message and exit'
+formatopt='-f:print with custom format:$matchany'
+albumopt='-a:match albums instead of tracks'
+dontmoveopt='-M:dont move files in library'
+writeopt='-w:write new metadata to files tags (default)'
+nowriteopt='-W:dont write metadata (opposite of -w)'
+pretendopt='-p:show all changes but do nothing'
+pathopt='-p:print paths for matched items or albums'
+destopt='-d:destination music directory:$dirs'
+copyopt='-c:copy tracks into library directory (default)'
+nocopyopt='-C:dont copy tracks (opposite of -c)'
+inferopt='-a:infer tags for imported files (default)'
+noinferopt='-A:dont infer tags for imported files (opposite of -a)'
+resumeopt='-p:resume importing if interrupted'
+noresumeopt='-P:do not try to resume importing'
+nopromptopt='-q:never prompt for input, skip albums instead'
+logopt='-l:file to log untaggable albums for later review:$files'
+individualopt='-s:import individual tracks instead of full albums'
+confirmopt='-t:always confirm all actions'
+retagopt='-L:retag items matching a query:${query[@]}'
+skipopt='-i:skip already-imported directories'
+noskipopt='-I:do not skip already-imported directories'
+flatopt='--flat:import an entire tree as a single album'
+groupopt='-g:group tracks in a folder into seperate albums' 
+editopt='-e:edit user configuration with $EDITOR'
+defaultopt='-d:include the default configuration'
+copynomoveopt='-c:copy instead of moving'
+noconfirmopt='-y:skip confirmation'
+exactopt='-e:get exact file sizes'
+removeopt='-d:also remove files from disk'
+configopt='-c:path to configuration file:$files'
+debugopt='-v:print debugging information'
+libopt='-l:library database file to use:$files'
+
+# This function takes a beet subcommand as its first argument, and then uses _regex_words to set ${reply[@]}
+# to an array containing arguments for the _regex_arguments function.
+function _beet_subcmd_options()
+{
+    local shortopt optarg optdesc
+    local -a regex_words
+    regex_words=()
+    for i in ${${(f)"$(beet help $1 | awk '/^ +-/{if(x)print x;x=$0;next}/^ *$/{if(x) exit}{if(x) x=x$0}END{print x}')"}[@]}
+    do
+        shortopt="${i[(w)1]/,/}"
+        optarg="${$(echo ${i[(w)2]}|grep -o '[A-Z]\+')[(w)1]}"
+        optdesc="${${${${${i[(w)2,-1]/[A-Z, ]#--[a-z]##[=A-Z]# #/}//:/-}//\[/(}//\]/)}//\'/}"
+        case $optarg
+            in
+            ("")
+                if [[ "$1" == "import" && "$shortopt" == "-L" ]]; then
+                    regex_words+=("$shortopt:$optdesc:\${query[@]}")
+                else
+                    regex_words+=("$shortopt:$optdesc")
+                fi
+                ;;
+            (LOG)
+                regex_words+=("$shortopt:$optdesc:\$files")
+                ;;
+            (CONFIG)
+                local -a configfile
+                configfile=("$matchany" ':file:config file:{_files -g *.yaml}')
+                regex_words+=("$shortopt:$optdesc:\$configfile")
+                ;;
+            (LIB|LIBRARY)
+                local -a libfile
+                libfile=("$matchany" ':file:database file:{_files -g *.db}')
+                regex_words+=("$shortopt:$optdesc:\$libfile")
+                ;;
+            (DIR|DIRECTORY)
+                regex_words+=("$shortopt:$optdesc:\$dirs")
+                ;;
+            (SOURCE)
+                if [[ $1 -eq lastgenre ]]; then
+                    local -a lastgenresource
+                    lastgenresource=(/$'(artist|album|track)\0'/ ':source:genre source:(artist album track)')
+                    regex_words+=("$shortopt:$optdesc:\$lastgenresource")
+                else
+                    regex_words+=("$shortopt:$optdesc:\$matchany")
+                fi
+                ;;
+            (*)
+                regex_words+=("$shortopt:$optdesc:\$matchany")
+                ;;
+        esac
+    done
+    _regex_words options "$1 options" "${regex_words[@]}"
+}
+
+# Now build the arguments to _regex_arguments for each subcommand.
+local -a options regex_words_subcmds regex_words_help
+local subcmd cmddesc
+for i in ${${(f)"$(beet help | awk 'f;/Commands:/{f=1}')"[@]}[@]}
+do
+    subcmd="${i[(w)1]}"
+    cmddesc="${${${${${i[(w)2,-1]##\(*\) #}//:/-}//\[/(}//\]/)}//\'/}"
+    case $subcmd
+        in
+        (config)
+            _regex_words options "config options" "$helpopt" "$pathopt"  "$editopt" "$defaultopt"
+            options=("${reply[@]}")
+            ;;
+        (import)
+            _regex_words options "import options" "$helpopt" "$writeopt" "$nowriteopt" "$copyopt" "$nocopyopt" "$inferopt" \
+                "$noinferopt" "$resumeopt" "$noresumeopt" "$nopromptopt" "$logopt" "$individualopt" "$confirmopt" "$retagopt" \
+                "$skipopt" "$noskipopt" "$flatopt" "$groupopt"
+            options=( "${reply[@]}" \# "${files[@]}" \# )
+            ;;
+        (list)
+            _regex_words options "list options" "$helpopt" "$pathopt" "$albumopt" "$formatopt"
+            options=( "$reply[@]" \# "${query[@]}" )
+            ;;
+        (modify)
+            _regex_words options "modify options" "$helpopt" "$dontmoveopt" "$writeopt" "$nowriteopt" "$albumopt" \
+                "$noconfirmopt" "$formatopt"
+            options=( "${reply[@]}" \# "${query[@]}" "${modify[@]}" )
+            ;;
+        (move)
+            _regex_words options "move options" "$helpopt" "$albumopt" "$destopt" "$copynomoveopt"
+            options=( "${reply[@]}" \# "${query[@]}")
+            ;;
+        (remove)
+            _regex_words options "remove options" "$helpopt" "$albumopt" "$removeopt"
+            options=( "${reply[@]}" \# "${query[@]}" )
+            ;;
+        (stats)
+            _regex_words options "stats options" "$helpopt" "$exactopt" 
+            options=( "${reply[@]}" \# "${query[@]}" )
+            ;;
+        (update)
+            _regex_words options "update options" "$helpopt" "$albumopt" "$dontmoveopt" "$pretendopt" "$formatopt"
+            options=( "${reply[@]}" \# "${query[@]}" )
+            ;;
+        (write)
+            _regex_words options "write options" "$helpopt" "$pretendopt"
+            options=( "${reply[@]}" \# "${query[@]}" )
+            ;;
+        (fields|migrate|version)
+            options=()
+            ;;
+        (help)
+            # The help subcommand is treated separately
+            continue 
+            ;;
+        (*) # completions for plugin commands are generated using _beet_subcmd_options 
+            _beet_subcmd_options "$subcmd"
+            options=( \( "${reply[@]}" \# "${query[@]}" \) )
+            ;;
+    esac
+    # Create variable for holding option for this subcommand, and assign to it (needs to have a unique name).
+    typeset -a opts_for_$subcmd
+    set -A opts_for_$subcmd ${options[@]} # Assignment MUST be done using set (other methods fail).
+    regex_words_subcmds+=("$subcmd:$cmddesc:\${(@)opts_for_$subcmd}")
+    # Add to regex_words args for help subcommand
+    regex_words_help+=("$subcmd:$cmddesc")
+done
+
+local -a opts_for_help
+_regex_words subcmds "subcommands" "${regex_words_help[@]}"
+opts_for_help=("${reply[@]}")
+regex_words_subcmds+=('help:show help:$opts_for_help')
+
+# Argument for global options
+local -a globalopts
+_regex_words options "global options" "$configopt" "$debugopt" "$libopt" "$helpopt" "$destopt"
+globalopts=("${reply[@]}")
+
+# Create main completion function
+#local -a subcmds
+_regex_words subcmds "subcommands" "${regex_words_subcmds[@]}"
+subcmds=("${reply[@]}")
+_regex_arguments _beet "$matchany" \( "${globalopts[@]}" \# \) "${subcmds[@]}"
+
+# Set tag-order so that options are completed separately from arguments
+zstyle ":completion:${curcontext}:" tag-order '! options'
+
+# Execute the completion function
+_beet "$@"
+
+# Local Variables: 
+# mode:shell-script
+# End:

From 7de6259c1d445de7972469f88fba484a443cb090 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 15 Aug 2014 12:09:18 -0700
Subject: [PATCH 15/16] MediaFile: make id3v23 a constructor parameter

For #899, we need to change MediaFile's behavior (pre-write) based on whether
we're doing ID3v2.3 or not. So we need a field on the object, not a parameter
to `save()`.
---
 beets/library.py            |  5 +++--
 beets/mediafile.py          | 18 +++++++++++-------
 beetsplug/embedart.py       |  5 +++--
 beetsplug/scrub.py          |  5 +++--
 test/test_mediafile_edge.py | 16 ++++++++--------
 5 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/beets/library.py b/beets/library.py
index 8496a844e..59d681043 100644
--- a/beets/library.py
+++ b/beets/library.py
@@ -393,7 +393,8 @@ class Item(LibModel):
         else:
             path = normpath(path)
         try:
-            mediafile = MediaFile(syspath(path))
+            mediafile = MediaFile(syspath(path),
+                                  id3v23=beets.config['id3v23'].get(bool))
         except (OSError, IOError) as exc:
             raise ReadError(self.path, exc)
 
@@ -401,7 +402,7 @@ class Item(LibModel):
 
         mediafile.update(self)
         try:
-            mediafile.save(id3v23=beets.config['id3v23'].get(bool))
+            mediafile.save()
         except (OSError, IOError, MutagenError) as exc:
             raise WriteError(self.path, exc)
 
diff --git a/beets/mediafile.py b/beets/mediafile.py
index 24152ec35..c1be35d49 100644
--- a/beets/mediafile.py
+++ b/beets/mediafile.py
@@ -1222,9 +1222,12 @@ class MediaFile(object):
     """Represents a multimedia file on disk and provides access to its
     metadata.
     """
-    def __init__(self, path):
+    def __init__(self, path, id3v23=False):
         """Constructs a new `MediaFile` reflecting the file at path. May
         throw `UnreadableFileError`.
+
+        By default, MP3 files are saved with ID3v2.4 tags. You can use
+        the older ID3v2.3 standard by specifying the `id3v23` option.
         """
         self.path = path
 
@@ -1296,18 +1299,19 @@ class MediaFile(object):
         else:
             raise FileTypeError(path, type(self.mgfile).__name__)
 
-        # add a set of tags if it's missing
+        # Add a set of tags if it's missing.
         if self.mgfile.tags is None:
             self.mgfile.add_tags()
 
-    def save(self, id3v23=False):
-        """Write the object's tags back to the file.
+        # Set the ID3v2.3 flag only for MP3s.
+        self.id3v23 = id3v23 and self.type == 'mp3'
 
-        By default, MP3 files are saved with ID3v2.4 tags. You can use
-        the older ID3v2.3 standard by specifying the `id3v23` option.
+    def save(self):
+        """Write the object's tags back to the file.
         """
+        # Possibly save the tags to ID3v2.3.
         kwargs = {}
-        if id3v23 and self.type == 'mp3':
+        if self.id3v23:
             id3 = self.mgfile
             if hasattr(id3, 'tags'):
                 # In case this is an MP3 object, not an ID3 object.
diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py
index 0322af6c7..46e10a61d 100644
--- a/beetsplug/embedart.py
+++ b/beetsplug/embedart.py
@@ -183,11 +183,12 @@ def clear(lib, query):
     for item in lib.items(query):
         log.info(u'%s - %s' % (item.artist, item.title))
         try:
-            mf = mediafile.MediaFile(syspath(item.path))
+            mf = mediafile.MediaFile(syspath(item.path),
+                                     config['id3v23'].get(bool))
         except mediafile.UnreadableFileError as exc:
             log.error(u'Could not clear art from {0}: {1}'.format(
                 displayable_path(item.path), exc
             ))
             continue
         mf.art = None
-        mf.save(config['id3v23'].get(bool))
+        mf.save()
diff --git a/beetsplug/scrub.py b/beetsplug/scrub.py
index 0b0c69e2f..af1a77370 100644
--- a/beetsplug/scrub.py
+++ b/beetsplug/scrub.py
@@ -68,7 +68,8 @@ class ScrubPlugin(BeetsPlugin):
 
                 # Get album art if we need to restore it.
                 if opts.write:
-                    mf = mediafile.MediaFile(item.path)
+                    mf = mediafile.MediaFile(item.path,
+                                             config['id3v23'].get(bool))
                     art = mf.art
 
                 # Remove all tags.
@@ -82,7 +83,7 @@ class ScrubPlugin(BeetsPlugin):
                         log.info('restoring art')
                         mf = mediafile.MediaFile(item.path)
                         mf.art = art
-                        mf.save(config['id3v23'].get(bool))
+                        mf.save()
 
             scrubbing = False
 
diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index b0b45da0c..cb32b92ae 100644
--- a/test/test_mediafile_edge.py
+++ b/test/test_mediafile_edge.py
@@ -269,21 +269,21 @@ class SoundCheckTest(unittest.TestCase):
 
 
 class ID3v23Test(unittest.TestCase, TestHelper):
-    def _make_test(self, ext='mp3'):
+    def _make_test(self, ext='mp3', id3v23=False):
         self.create_temp_dir()
         src = os.path.join(_common.RSRC, 'full.{0}'.format(ext))
         self.path = os.path.join(self.temp_dir, 'test.{0}'.format(ext))
         shutil.copy(src, self.path)
-        return beets.mediafile.MediaFile(self.path)
+        return beets.mediafile.MediaFile(self.path, id3v23=id3v23)
 
     def _delete_test(self):
         self.remove_temp_dir()
 
     def test_v24_year_tag(self):
-        mf = self._make_test()
+        mf = self._make_test(id3v23=False)
         try:
             mf.year = 2013
-            mf.save(id3v23=False)
+            mf.save()
             frame = mf.mgfile['TDRC']
             self.assertTrue('2013' in str(frame))
             self.assertTrue('TYER' not in mf.mgfile)
@@ -291,10 +291,10 @@ class ID3v23Test(unittest.TestCase, TestHelper):
             self._delete_test()
 
     def test_v23_year_tag(self):
-        mf = self._make_test()
+        mf = self._make_test(id3v23=True)
         try:
             mf.year = 2013
-            mf.save(id3v23=True)
+            mf.save()
             frame = mf.mgfile['TYER']
             self.assertTrue('2013' in str(frame))
             self.assertTrue('TDRC' not in mf.mgfile)
@@ -302,10 +302,10 @@ class ID3v23Test(unittest.TestCase, TestHelper):
             self._delete_test()
 
     def test_v23_on_non_mp3_is_noop(self):
-        mf = self._make_test('m4a')
+        mf = self._make_test('m4a', id3v23=True)
         try:
             mf.year = 2013
-            mf.save(id3v23=True)
+            mf.save()
         finally:
             self._delete_test()
 

From 09b0e1c75d66186bab1dab32c438b428e9e5d73a Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 15 Aug 2014 12:38:41 -0700
Subject: [PATCH 16/16] Add failing test for #899

---
 test/test_mediafile_edge.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/test/test_mediafile_edge.py b/test/test_mediafile_edge.py
index cb32b92ae..0347d3836 100644
--- a/test/test_mediafile_edge.py
+++ b/test/test_mediafile_edge.py
@@ -309,6 +309,31 @@ class ID3v23Test(unittest.TestCase, TestHelper):
         finally:
             self._delete_test()
 
+    def test_v24_image_encoding(self):
+        mf = self._make_test(id3v23=False)
+        try:
+            mf.images = [beets.mediafile.Image(b'test data')]
+            mf.save()
+            frame = mf.mgfile.tags.getall('APIC')[0]
+            self.assertEqual(frame.encoding, 3)
+        finally:
+            self._delete_test()
+
+    @unittest.skip
+    def test_v23_image_encoding(self):
+        """For compatibility with OS X/iTunes (and strict adherence to
+        the standard), ID3v2.3 tags need to use an inferior text
+        encoding: UTF-8 is not supported.
+        """
+        mf = self._make_test(id3v23=True)
+        try:
+            mf.images = [beets.mediafile.Image(b'test data')]
+            mf.save()
+            frame = mf.mgfile.tags.getall('APIC')[0]
+            self.assertEqual(frame.encoding, 1)
+        finally:
+            self._delete_test()
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)