feat(import): Move logfile import logic to --from-logfile argument

As requested here: beetbox#4387 (review)

beets/ui/commands.py

@@ -933,31 +933,6 @@ def import_files(lib, paths, query):
     """Import the files in the given list of paths or matching the
     query.
     """
-    # Check the user-specified directories.
-    paths_to_import = []
-    for path in paths:
-        normalized_path = syspath(normpath(path))
-        if not os.path.exists(normalized_path):
-            raise ui.UserError('no such file or directory: {}'.format(
-                displayable_path(path)))
-
-        # Read additional paths from logfile.
-        if os.path.isfile(normalized_path):
-            try:
-                paths_to_import.extend(list(
-                    _paths_from_logfile(normalized_path)))
-            except ValueError:
-                # We could raise an error here, but it's possible that the user
-                # tried to import a media file instead of a logfile. In that
-                # case, logging a warning would be confusing, so we skip this
-                # here.
-                pass
-            else:
-                # Don't re-add the log file to the list of paths to import.
-                continue
-
-        paths_to_import.append(paths)
-
     # Check parameter consistency.
     if config['import']['quiet'] and config['import']['timid']:
         raise ui.UserError("can't be both quiet and timid")
@@ -978,11 +953,11 @@ def import_files(lib, paths, query):
             config['import']['quiet']:
         config['import']['resume'] = False
 
-    session = TerminalImportSession(lib, loghandler, paths_to_import, query)
+    session = TerminalImportSession(lib, loghandler, paths, query)
     session.run()
 
     # Emit event.
-    plugins.send('import', lib=lib, paths=paths_to_import)
+    plugins.send('import', lib=lib, paths=paths)
 
 
 def import_func(lib, opts, args):
@@ -999,7 +974,23 @@ def import_func(lib, opts, args):
     else:
         query = None
         paths = args
-        if not paths:
+
+        # The paths from the logfiles go into a separate list to allow handling
+        # errors differently from user-specified paths.
+        paths_from_logfiles = []
+        for logfile in opts.from_logfiles:
+            try:
+                paths_from_logfiles.extend(list(_paths_from_logfile(syspath(normpath(logfile)))))
+            except ValueError as err:
+                raise ui.UserError('malformed logfile {}'.format(
+                    util.displayable_path(logfile),
+                )) from err
+            except IOError as err:
+                raise ui.UserError('unreadable logfile {}'.format(
+                    util.displayable_path(logfile),
+                )) from err
+
+        if not paths and not paths_from_logfiles:
             raise ui.UserError('no path specified')
 
         # On Python 2, we used to get filenames as raw bytes, which is
@@ -1008,6 +999,31 @@ def import_func(lib, opts, args):
         # filename.
         paths = [p.encode(util.arg_encoding(), 'surrogateescape')
                  for p in paths]
+        paths_from_logfiles = [p.encode(util.arg_encoding(), 'surrogateescape')
+                               for p in paths_from_logfiles]
+
+        # Check the user-specified directories.
+        for path in paths:
+            if not os.path.exists(syspath(normpath(path))):
+                raise ui.UserError('no such file or directory: {}'.format(
+                    displayable_path(path)))
+
+        # Check the directories from the logfiles, but don't throw an error in
+        # case those paths don't exist. Maybe some of those paths have already
+        # been imported and moved separately, so logging a warning should
+        # suffice.
+        for path in paths_from_logfiles:
+            if not os.path.exists(syspath(normpath(path))):
+                log.warning('No such file or directory: {}'.format(
+                    displayable_path(path)))
+                continue
+
+            paths.append(paths_from_logfiles)
+
+        # If all paths were read from a logfile, and none of them exist, throw
+        # an error
+        if not paths:
+            raise ui.UserError('none of the paths are importable')
 
     import_files(lib, paths, query)
 
@@ -1100,6 +1116,11 @@ def import_func(lib, opts, args):
     metavar='ID',
     help='restrict matching to a specific metadata backend ID'
 )
+import_cmd.parser.add_option(
+    '--from-logfile', dest='from_logfiles', action='append',
+    metavar='PATH',
+    help='read skipped paths from an existing logfile'
+)
 import_cmd.parser.add_option(
     '--set', dest='set_fields', action='callback',
     callback=_store_dict,

docs/guides/tagger.rst

@@ -103,8 +103,8 @@ command-line options you should know:
 * ``beet import -l LOGFILE``: write a message to ``LOGFILE`` every time you skip
   an album or choose to take its tags "as-is" (see below) or the album is
   skipped as a duplicate; this lets you come back later and reexamine albums
-  that weren't tagged successfully. Run ``beet import LOGFILE`` rerun the
-  importer on such paths from the logfile.
+  that weren't tagged successfully. Run ``beet import --from-logfile=LOGFILE``
+  rerun the importer on such paths from the logfile.
 
 * ``beet import -q``: quiet mode. Never prompt for input and, instead,
   conservatively skip any albums that need your opinion. The ``-ql`` combination

docs/reference/cli.rst

@@ -86,7 +86,9 @@ Optional command flags:
   that weren't tagged successfully---either because they're not in the
   MusicBrainz database or because something's wrong with the files. Use the
   ``-l`` option to specify a filename to log every time you skip an album
-  or import it "as-is" or an album gets skipped as a duplicate.
+  or import it "as-is" or an album gets skipped as a duplicate. You can later
+  review the file manually or import skipped paths from the logfile
+  automatically by using the ``--from-logfile LOGFILE`` argument.
 
 * Relatedly, the ``-q`` (quiet) option can help with large imports by
   autotagging without ever bothering to ask for user input. Whenever the