From 99ced5cc7def450181cc62fb7190be5ef4697eba Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 27 Jun 2022 19:00:40 +0200 Subject: [PATCH] feat(import): Move logfile import logic to `--from-logfile` argument As requested here: https://github.com/beetbox/beets/pull/4387#pullrequestreview-1020040380 --- beets/ui/commands.py | 77 +++++++++++++++++++++++++++--------------- docs/guides/tagger.rst | 4 +-- docs/reference/cli.rst | 4 ++- 3 files changed, 54 insertions(+), 31 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index b0921728dd..0defc57758 100755 --- a/beets/ui/commands.py +++ b/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 or []: + 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, diff --git a/docs/guides/tagger.rst b/docs/guides/tagger.rst index 5184f433fd..d47ee3c4ae 100644 --- a/docs/guides/tagger.rst +++ b/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 diff --git a/docs/reference/cli.rst b/docs/reference/cli.rst index dad407489b..3726fb3732 100644 --- a/docs/reference/cli.rst +++ b/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