From e96b5a7eba2bb7a76ada3b18fe93ae8cac046852 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Tue, 23 Mar 2021 21:55:51 -0500 Subject: [PATCH 01/10] Check files during import, show checker warnings and errors along with tagging choices. --- beets/importer.py | 1 + beets/ui/commands.py | 23 ++++++++++------- beetsplug/badfiles.py | 60 +++++++++++++++++++++++++++++++++++-------- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 3220b260f..c16350c68 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -472,6 +472,7 @@ class ImportTask(BaseImportTask): def __init__(self, toppath, paths, items): super(ImportTask, self).__init__(toppath, paths, items) self.choice_flag = None + self.skip_summary_judgement = False self.cur_album = None self.cur_artist = None self.candidates = [] diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 4d010f4b1..8d169129a 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -698,14 +698,18 @@ class TerminalImportSession(importer.ImportSession): print_(displayable_path(task.paths, u'\n') + u' ({0} items)'.format(len(task.items))) - # Take immediate action if appropriate. - action = _summary_judgment(task.rec) - if action == importer.action.APPLY: - match = task.candidates[0] - show_change(task.cur_artist, task.cur_album, match) - return match - elif action is not None: - return action + # Call here so plugins have the option to skip `_summary_judgement` + choices = self._get_choices(task) + + if not task.skip_summary_judgement: + # Take immediate action if appropriate. + action = _summary_judgment(task.rec) + if action == importer.action.APPLY: + match = task.candidates[0] + show_change(task.cur_artist, task.cur_album, match) + return match + elif action is not None: + return action # Loop until we have a choice. while True: @@ -713,7 +717,6 @@ class TerminalImportSession(importer.ImportSession): # `choose_candidate` may be an `importer.action`, an # `AlbumMatch` object for a specific selection, or a # `PromptChoice`. - choices = self._get_choices(task) choice = choose_candidate( task.candidates, False, task.rec, task.cur_artist, task.cur_album, itemcount=len(task.items), choices=choices @@ -742,6 +745,8 @@ class TerminalImportSession(importer.ImportSession): assert isinstance(choice, autotag.AlbumMatch) return choice + choices = self._get_choices(task) + def choose_item(self, task): """Ask the user for a choice about tagging a single item. Returns either an action constant or a TrackMatch object. diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index 36b45de3a..c54190eaf 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -30,6 +30,7 @@ from beets.plugins import BeetsPlugin from beets.ui import Subcommand from beets.util import displayable_path, par_map from beets import ui +from beets import importer class CheckerCommandException(Exception): @@ -54,6 +55,11 @@ class BadFiles(BeetsPlugin): super(BadFiles, self).__init__() self.verbose = False + self.register_listener('import_task_start', + self.on_import_task_start) + self.register_listener('before_choose_candidate', + self.on_before_choose_candidate) + def run_command(self, cmd): self._log.debug(u"running command: {}", displayable_path(list2cmdline(cmd))) @@ -115,7 +121,7 @@ class BadFiles(BeetsPlugin): if not checker: self._log.error(u"no checker specified in the config for {}", ext) - return + return [] path = item.path if not isinstance(path, six.text_type): path = item.path.decode(sys.getfilesystemencoding()) @@ -130,25 +136,59 @@ class BadFiles(BeetsPlugin): ) else: self._log.error(u"error invoking {}: {}", e.checker, e.msg) - return + return [] + + error_lines = [] + if status > 0: - ui.print_(u"{}: checker exited with status {}" - .format(ui.colorize('text_error', dpath), status)) + error_lines.append(u"{}: checker exited with status {}" + .format(ui.colorize('text_error', dpath), status)) for line in output: - ui.print_(u" {}".format(line)) + error_lines.append(u" {}".format(line)) + elif errors > 0: - ui.print_(u"{}: checker found {} errors or warnings" - .format(ui.colorize('text_warning', dpath), errors)) + error_lines.append(u"{}: checker found {} errors or warnings" + .format(ui.colorize('text_warning', dpath), errors)) for line in output: - ui.print_(u" {}".format(line)) + error_lines.append(u" {}".format(line)) elif self.verbose: - ui.print_(u"{}: ok".format(ui.colorize('text_success', dpath))) + error_lines.append(u"{}: ok".format(ui.colorize('text_success', dpath))) + + return error_lines + + def on_import_task_start(self, task, session): + checks_failed = [] + + for item in task.items: + error_lines = self.check_item(item) + if error_lines: + checks_failed.append(error_lines) + + if checks_failed: + task._badfiles_checks_failed = checks_failed + task.skip_summary_judgement = True + + def on_before_choose_candidate(self, task, session): + if hasattr(task, '_badfiles_checks_failed'): + ui.print_('{} one or more files failed checks:' + .format(ui.colorize('text_warning', 'BAD'))) + for error in task._badfiles_checks_failed: + for error_line in error: + ui.print_(error_line) + + ui.print_() + def command(self, lib, opts, args): # Get items from arguments items = lib.items(ui.decargs(args)) self.verbose = opts.verbose - par_map(self.check_item, items) + + def check_and_print(item): + for error_line in self.check_item(item): + ui.print_(error_line) + + par_map(check_and_print, items) def commands(self): bad_command = Subcommand('bad', From 2c9f3d6464d851236bef8a9173ec8e17d0ea9983 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Wed, 24 Mar 2021 16:51:58 -0500 Subject: [PATCH 02/10] Add `check_on_import` config for badfiles. --- beetsplug/badfiles.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index c54190eaf..c9edbc8c8 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -157,6 +157,9 @@ class BadFiles(BeetsPlugin): return error_lines def on_import_task_start(self, task, session): + if not self.config['check_on_import'].get(False): + return + checks_failed = [] for item in task.items: From 2f8a8e2f8a5fc7859328196a3f14adb5c9ee2a4e Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Wed, 24 Mar 2021 16:55:28 -0500 Subject: [PATCH 03/10] Add docs for badfiles' `check_on_import` option. --- docs/plugins/badfiles.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/plugins/badfiles.rst b/docs/plugins/badfiles.rst index a59dbd0d1..796f991e1 100644 --- a/docs/plugins/badfiles.rst +++ b/docs/plugins/badfiles.rst @@ -17,6 +17,7 @@ install yourself: You can also add custom commands for a specific extension, like this:: badfiles: + check_on_import: yes commands: ogg: myoggchecker --opt1 --opt2 flac: flac --test --warnings-as-errors --silent @@ -25,6 +26,10 @@ Custom commands will be run once for each file of the specified type, with the path to the file as the last argument. Commands must return a status code greater than zero for a file to be considered corrupt. +You can run the checkers when importing files by using the `check_on_import` +option. When on, checkers will be run against every imported file and warnings +and errors will be presented when selecting a tagging option. + .. _mp3val: http://mp3val.sourceforge.net/ .. _flac: https://xiph.org/flac/ From 227d420efbfd08764b55abf28aff7188160b6344 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Wed, 24 Mar 2021 17:04:17 -0500 Subject: [PATCH 04/10] Add entry to changelog. --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index d7c6f682d..421c1e964 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -210,6 +210,8 @@ Other new things: * The :doc:`/plugins/aura` has arrived! * :doc:`plugins/fetchart`: The new ``max_filesize`` option for fetchart can be used to target a maximum image filesize. +* :doc:`/plugins/badfiles`: Checkers can now be run during import with the + ``check_on_import`` config option. Fixes: From 9c17d9a9243b82adbc1d4f3b96f42172840bd07c Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Wed, 24 Mar 2021 17:29:16 -0500 Subject: [PATCH 05/10] linting --- beetsplug/badfiles.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index c9edbc8c8..44c1ce812 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -30,7 +30,6 @@ from beets.plugins import BeetsPlugin from beets.ui import Subcommand from beets.util import displayable_path, par_map from beets import ui -from beets import importer class CheckerCommandException(Exception): @@ -141,18 +140,21 @@ class BadFiles(BeetsPlugin): error_lines = [] if status > 0: - error_lines.append(u"{}: checker exited with status {}" - .format(ui.colorize('text_error', dpath), status)) + error_lines.append( + u"{}: checker exited with status {}" + .format(ui.colorize('text_error', dpath), status)) for line in output: error_lines.append(u" {}".format(line)) elif errors > 0: - error_lines.append(u"{}: checker found {} errors or warnings" - .format(ui.colorize('text_warning', dpath), errors)) + error_lines.append( + u"{}: checker found {} errors or warnings" + .format(ui.colorize('text_warning', dpath), errors)) for line in output: error_lines.append(u" {}".format(line)) elif self.verbose: - error_lines.append(u"{}: ok".format(ui.colorize('text_success', dpath))) + error_lines.append( + u"{}: ok".format(ui.colorize('text_success', dpath))) return error_lines @@ -163,7 +165,7 @@ class BadFiles(BeetsPlugin): checks_failed = [] for item in task.items: - error_lines = self.check_item(item) + error_lines = self.check_item(item) if error_lines: checks_failed.append(error_lines) @@ -181,7 +183,6 @@ class BadFiles(BeetsPlugin): ui.print_() - def command(self, lib, opts, args): # Get items from arguments items = lib.items(ui.decargs(args)) From 1d4a2c3e2ae348d6d3884bf18823cdbe0e268662 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Sat, 27 Mar 2021 11:37:26 -0500 Subject: [PATCH 06/10] Add a new hook for badfiles pre-import user interaction, instead of flagging the task. --- beets/importer.py | 1 - beets/plugins.py | 16 ++++++++++++++++ beets/ui/commands.py | 28 +++++++++++++++------------- beetsplug/badfiles.py | 20 ++++++++++++++++---- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 491383622..b7bfdb156 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -472,7 +472,6 @@ class ImportTask(BaseImportTask): def __init__(self, toppath, paths, items): super(ImportTask, self).__init__(toppath, paths, items) self.choice_flag = None - self.skip_summary_judgement = False self.cur_album = None self.cur_artist = None self.candidates = [] diff --git a/beets/plugins.py b/beets/plugins.py index 23f938169..402a6db17 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -501,6 +501,22 @@ def send(event, **arguments): return results +def send_seq(event, **arguments): + """Like `send` but passes the result of the previous event handler to the + next, and returns only the result of the last non-None event handler. + + `event` is the name of the event to send, all other named arguments + are passed along to the handlers. + """ + log.debug(u'Sequentially sending event: {0}', event) + previous = None + for handler in event_handlers()[event]: + result = handler(previous=previous, **arguments) + previous = result or previous + + return previous + + def feat_tokens(for_artist=True): """Return a regular expression that matches phrases like "featuring" that separate a main artist or a song title from secondary artists. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 8d169129a..c8f85a999 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -698,18 +698,21 @@ class TerminalImportSession(importer.ImportSession): print_(displayable_path(task.paths, u'\n') + u' ({0} items)'.format(len(task.items))) - # Call here so plugins have the option to skip `_summary_judgement` - choices = self._get_choices(task) + # Let plugins display info or prompt the user before we go through the + # process of selecting candidate. + action = plugins.send_seq('import_task_before_choice', + session=self, task=task) + if action is not None: + return action - if not task.skip_summary_judgement: - # Take immediate action if appropriate. - action = _summary_judgment(task.rec) - if action == importer.action.APPLY: - match = task.candidates[0] - show_change(task.cur_artist, task.cur_album, match) - return match - elif action is not None: - return action + # Take immediate action if appropriate. + action = _summary_judgment(task.rec) + if action == importer.action.APPLY: + match = task.candidates[0] + show_change(task.cur_artist, task.cur_album, match) + return match + elif action is not None: + return action # Loop until we have a choice. while True: @@ -717,6 +720,7 @@ class TerminalImportSession(importer.ImportSession): # `choose_candidate` may be an `importer.action`, an # `AlbumMatch` object for a specific selection, or a # `PromptChoice`. + choices = self._get_choices(task) choice = choose_candidate( task.candidates, False, task.rec, task.cur_artist, task.cur_album, itemcount=len(task.items), choices=choices @@ -745,8 +749,6 @@ class TerminalImportSession(importer.ImportSession): assert isinstance(choice, autotag.AlbumMatch) return choice - choices = self._get_choices(task) - def choose_item(self, task): """Ask the user for a choice about tagging a single item. Returns either an action constant or a TrackMatch object. diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index 44c1ce812..0fede8f4b 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -30,6 +30,7 @@ from beets.plugins import BeetsPlugin from beets.ui import Subcommand from beets.util import displayable_path, par_map from beets import ui +from beets import importer class CheckerCommandException(Exception): @@ -56,8 +57,8 @@ class BadFiles(BeetsPlugin): self.register_listener('import_task_start', self.on_import_task_start) - self.register_listener('before_choose_candidate', - self.on_before_choose_candidate) + self.register_listener('import_task_before_choice', + self.on_import_task_before_choice) def run_command(self, cmd): self._log.debug(u"running command: {}", @@ -171,9 +172,8 @@ class BadFiles(BeetsPlugin): if checks_failed: task._badfiles_checks_failed = checks_failed - task.skip_summary_judgement = True - def on_before_choose_candidate(self, task, session): + def on_import_task_before_choice(self, task, session, previous): if hasattr(task, '_badfiles_checks_failed'): ui.print_('{} one or more files failed checks:' .format(ui.colorize('text_warning', 'BAD'))) @@ -182,6 +182,18 @@ class BadFiles(BeetsPlugin): ui.print_(error_line) ui.print_() + ui.print_('What would you like to do?') + + sel = ui.input_options(['aBort', 'skip', 'continue']) + + if sel == 's': + return importer.action.SKIP + elif sel == 'c': + return None + elif sel == 'b': + raise importer.ImportAbort() + else: + raise Exception('Unexpected selection: {}'.format(sel)) def command(self, lib, opts, args): # Get items from arguments From 6c4cb82c0317f5d11c99cbbe4a331e6c68302b33 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Sat, 27 Mar 2021 12:03:56 -0500 Subject: [PATCH 07/10] badfiles: Respect any previous decision to skip an import and don't re-prompt the user. --- beetsplug/badfiles.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index 0fede8f4b..c68d7224e 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -174,6 +174,10 @@ class BadFiles(BeetsPlugin): task._badfiles_checks_failed = checks_failed def on_import_task_before_choice(self, task, session, previous): + # Already skipping, so no need to notify the user of anything + if previous == importer.action.SKIP: + return None + if hasattr(task, '_badfiles_checks_failed'): ui.print_('{} one or more files failed checks:' .format(ui.colorize('text_warning', 'BAD'))) From 39b5a7636c33fcffe0001f8038b1dc28f0af28d5 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Sat, 27 Mar 2021 12:04:25 -0500 Subject: [PATCH 08/10] Add line in dev docs for new hook. --- docs/dev/plugins.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index a6aa3d6d7..8a75bc142 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -200,6 +200,14 @@ The events currently available are: import pipeline stage is a better choice (see :ref:`plugin-stage`). Parameters: ``task`` and ``session``. +* `import_task_before_choice`: called after candidate search for an import task + before any decision is made about how/if to import or tag. Can be used to + present information about the task or initiate interaction with the user + before importing occurs. Return an importer action to take a specific action. + Parameters: ``task``, ``session``, and ``previous`` (which is the action + specified by the previous event handler for the same task, allowing an early + exist if e.g. a previous plugin has already selected to skip the import) + * `import_task_choice`: called after a decision has been made about an import task. This event can be used to initiate further interaction with the user. Use ``task.choice_flag`` to determine or change the action to be From 79616b42ed5111f3a69420200da5b0ad08b5201f Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Sun, 28 Mar 2021 16:53:01 -0500 Subject: [PATCH 09/10] Use simpler approach of asserting that at most one handler of `import_task_before_choice` returns an action. --- beets/plugins.py | 16 ---------------- beets/ui/commands.py | 14 ++++++++++---- beetsplug/badfiles.py | 6 +----- docs/dev/plugins.rst | 5 ++--- 4 files changed, 13 insertions(+), 28 deletions(-) diff --git a/beets/plugins.py b/beets/plugins.py index 402a6db17..23f938169 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -501,22 +501,6 @@ def send(event, **arguments): return results -def send_seq(event, **arguments): - """Like `send` but passes the result of the previous event handler to the - next, and returns only the result of the last non-None event handler. - - `event` is the name of the event to send, all other named arguments - are passed along to the handlers. - """ - log.debug(u'Sequentially sending event: {0}', event) - previous = None - for handler in event_handlers()[event]: - result = handler(previous=previous, **arguments) - previous = result or previous - - return previous - - def feat_tokens(for_artist=True): """Return a regular expression that matches phrases like "featuring" that separate a main artist or a song title from secondary artists. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index c8f85a999..9d3e3dda8 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -700,10 +700,16 @@ class TerminalImportSession(importer.ImportSession): # Let plugins display info or prompt the user before we go through the # process of selecting candidate. - action = plugins.send_seq('import_task_before_choice', - session=self, task=task) - if action is not None: - return action + results = plugins.send('import_task_before_choice', + session=self, task=task) + actions = [action for action in results if action] + + if len(actions) == 1: + return actions[0] + elif len(actions) > 1: + raise Exception( + u'Only one handler for `import_task_before_choice` may return ' + u'an action.') # Take immediate action if appropriate. action = _summary_judgment(task.rec) diff --git a/beetsplug/badfiles.py b/beetsplug/badfiles.py index c68d7224e..5eaecc054 100644 --- a/beetsplug/badfiles.py +++ b/beetsplug/badfiles.py @@ -173,11 +173,7 @@ class BadFiles(BeetsPlugin): if checks_failed: task._badfiles_checks_failed = checks_failed - def on_import_task_before_choice(self, task, session, previous): - # Already skipping, so no need to notify the user of anything - if previous == importer.action.SKIP: - return None - + def on_import_task_before_choice(self, task, session): if hasattr(task, '_badfiles_checks_failed'): ui.print_('{} one or more files failed checks:' .format(ui.colorize('text_warning', 'BAD'))) diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 8a75bc142..d81461f4d 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -204,9 +204,8 @@ The events currently available are: before any decision is made about how/if to import or tag. Can be used to present information about the task or initiate interaction with the user before importing occurs. Return an importer action to take a specific action. - Parameters: ``task``, ``session``, and ``previous`` (which is the action - specified by the previous event handler for the same task, allowing an early - exist if e.g. a previous plugin has already selected to skip the import) + Only one handler may return a non-None result. + Parameters: ``task`` and ``session`` * `import_task_choice`: called after a decision has been made about an import task. This event can be used to initiate further interaction with the user. From c8fa1b28094110a63f757a9a3966545ac6043a42 Mon Sep 17 00:00:00 2001 From: Ryan Lanny Jenkins Date: Mon, 29 Mar 2021 18:12:30 -0500 Subject: [PATCH 10/10] Raise PluginConflictException instead of the generic Exception for new hook. --- beets/ui/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 9d3e3dda8..38687b3a9 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -707,7 +707,7 @@ class TerminalImportSession(importer.ImportSession): if len(actions) == 1: return actions[0] elif len(actions) > 1: - raise Exception( + raise plugins.PluginConflictException( u'Only one handler for `import_task_before_choice` may return ' u'an action.')