From aaee5e4f375dba4bfd39f709dc8935984f78d523 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Wed, 9 Mar 2022 14:06:22 +0100 Subject: [PATCH] PICARD-993: Have different error icons for different file errors Separate icons for file not found, access denied and anything else --- picard/file.py | 29 +++++++++++++++++++++++---- picard/ui/itemviews.py | 14 ++++++++++++-- picard/util/__init__.py | 15 ++++++++++++++ test/test_utils.py | 43 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 6 deletions(-) diff --git a/picard/file.py b/picard/file.py index 240e2f759..db8d5ae5e 100644 --- a/picard/file.py +++ b/picard/file.py @@ -44,6 +44,10 @@ from collections import Counter +from enum import ( + Enum, + auto, +) import fnmatch from functools import partial import os @@ -73,6 +77,7 @@ from picard.plugin import ( ) from picard.script import get_file_naming_script from picard.util import ( + any_exception_isinstance, decode_filename, emptydir, find_best_match, @@ -95,6 +100,13 @@ from picard.util.tags import PRESERVED_TAGS from picard.ui.item import Item +class FileErrorType(Enum): + + UNKNOWN = auto() + NOTFOUND = auto() + NOACCESS = auto() + + class File(QtCore.QObject, Item): metadata_images_changed = QtCore.pyqtSignal() @@ -144,6 +156,7 @@ class File(QtCore.QObject, Item): self.base_filename = os.path.basename(filename) self._state = File.UNDEFINED self.state = File.PENDING + self.error_type = FileErrorType.UNKNOWN self.orig_metadata = Metadata() self.metadata = Metadata() @@ -189,6 +202,16 @@ class File(QtCore.QObject, Item): copy[name] = self.format_specific_metadata(metadata, name, settings) return copy + def _set_error(self, error): + self.state = File.ERROR + self.error_append(str(error)) + if any_exception_isinstance(error, FileNotFoundError): + self.error_type = FileErrorType.NOTFOUND + elif any_exception_isinstance(error, PermissionError): + self.error_type = FileErrorType.NOACCESS + else: + self.error_type = FileErrorType.UNKNOWN + def load(self, callback): thread.run_task( partial(self._load_check, self.filename), @@ -215,8 +238,7 @@ class File(QtCore.QObject, Item): return config = get_config() if error is not None: - self.state = self.ERROR - self.error_append(str(error)) + self._set_error(error) # If loading failed, force format guessing and try loading again from picard.formats.util import guess_format @@ -390,8 +412,7 @@ class File(QtCore.QObject, Item): return old_filename = new_filename = self.filename if error is not None: - self.state = File.ERROR - self.error_append(str(error)) + self._set_error(error) else: self.filename = new_filename = result self.base_filename = os.path.basename(new_filename) diff --git a/picard/ui/itemviews.py b/picard/ui/itemviews.py index 04746e0dc..9030e8e52 100644 --- a/picard/ui/itemviews.py +++ b/picard/ui/itemviews.py @@ -70,7 +70,10 @@ from picard.config import ( Option, get_config, ) -from picard.file import File +from picard.file import ( + File, + FileErrorType, +) from picard.plugin import ExtensionPoint from picard.track import ( NonAlbumTrack, @@ -258,6 +261,8 @@ class MainPanel(QtWidgets.QSplitter): FileItem.icon_file = QtGui.QIcon(":/images/file.png") FileItem.icon_file_pending = QtGui.QIcon(":/images/file-pending.png") FileItem.icon_error = icontheme.lookup('dialog-error', icontheme.ICON_SIZE_MENU) + FileItem.icon_error_not_found = icontheme.lookup('error-not-found', icontheme.ICON_SIZE_MENU) + FileItem.icon_error_no_access = icontheme.lookup('error-no-access', icontheme.ICON_SIZE_MENU) FileItem.icon_saved = QtGui.QIcon(":/images/track-saved.png") FileItem.icon_fingerprint = icontheme.lookup('fingerprint', icontheme.ICON_SIZE_MENU) FileItem.icon_fingerprint_gray = icontheme.lookup('fingerprint-gray', icontheme.ICON_SIZE_MENU) @@ -1130,7 +1135,12 @@ class FileItem(TreeItem): @staticmethod def decide_file_icon(file): if file.state == File.ERROR: - return FileItem.icon_error + if file.error_type == FileErrorType.NOTFOUND: + return FileItem.icon_error_not_found + elif file.error_type == FileErrorType.NOACCESS: + return FileItem.icon_error_no_access + else: + return FileItem.icon_error elif isinstance(file.parent, Track): if file.state == File.NORMAL: return FileItem.icon_saved diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 95517bb7f..a9df13bad 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -1037,3 +1037,18 @@ def get_base_title(title): """ suffix = _(DEFAULT_COPY_TEXT) return get_base_title_with_suffix(title, suffix) + + +def iter_exception_chain(err): + """Iterate over the exception chain. + Yields this exception and all __context__ and __cause__ exceptions""" + yield err + if hasattr(err, '__context__'): + yield from iter_exception_chain(err.__context__) + if hasattr(err, '__cause__'): + yield from iter_exception_chain(err.__cause__) + + +def any_exception_isinstance(error, type_): + """Returns True, if any exception in the exception chain is instance of type_.""" + return any(isinstance(err, type_) for err in iter_exception_chain(error)) diff --git a/test/test_utils.py b/test/test_utils.py index 16518eb93..83d15856c 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -52,10 +52,12 @@ from picard.const.sys import ( ) from picard.util import ( album_artist_from_path, + any_exception_isinstance, build_qurl, extract_year_from_date, find_best_match, is_absolute_path, + iter_exception_chain, iter_files_from_objects, iter_unique, limited_join, @@ -781,3 +783,44 @@ class SystemSupportsLongPathsTest(PicardTestCase): self.assertTrue(system_supports_long_paths()) mock_open_key.assert_called_once() mock_query_value.assert_called_once() + + +class IterExceptionChainTest(PicardTestCase): + + def test_iter_exception_chain(self): + e1 = Mock(name='e1') + e2 = Mock(name='e2') + e3 = Mock(name='e3') + e4 = Mock(name='e4') + e5 = Mock(name='e5') + e1.__context__ = e2 + e2.__context__ = e3 + e2.__cause__ = e4 + e1.__cause__ = e5 + self.assertEqual([e1, e2, e3, e4, e5], list(iter_exception_chain(e1))) + + +class AnyExceptionIsinstanceTest(PicardTestCase): + + def test_any_exception_isinstance_itself(self): + ex = RuntimeError() + self.assertTrue(any_exception_isinstance(ex, RuntimeError)) + + def test_any_exception_isinstance_context(self): + ex = Mock() + self.assertFalse(any_exception_isinstance(ex, RuntimeError)) + ex.__context__ = RuntimeError() + self.assertTrue(any_exception_isinstance(ex, RuntimeError)) + + def test_any_exception_isinstance_cause(self): + ex = Mock() + self.assertFalse(any_exception_isinstance(ex, RuntimeError)) + ex.__cause__ = RuntimeError() + self.assertTrue(any_exception_isinstance(ex, RuntimeError)) + + def test_any_exception_isinstance_nested(self): + ex = Mock() + self.assertFalse(any_exception_isinstance(ex, RuntimeError)) + ex.__cause__ = Mock() + ex.__cause__.__context__ = RuntimeError() + self.assertTrue(any_exception_isinstance(ex, RuntimeError))