From 64710d0aea4da9734ba73f8082c5e54679f929e3 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 27 Apr 2024 17:30:30 +0200 Subject: [PATCH 01/27] Better handling of exceptions while loading option pages - without this patch, Picard crashes, and/or the options display gets corrupted - with this patch, a fake page is loaded instead, displaying the message, but avoiding a crash - it will be important for pages created by plugins --- picard/ui/options/dialog.py | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 52fa0a4b57..772d10d2a9 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -61,6 +61,7 @@ ) from picard.ui.options import ( # noqa: F401 # pylint: disable=unused-import OptionsCheckError, + OptionsPage, _pages as page_classes, advanced, cdlookup, @@ -93,6 +94,29 @@ from picard.ui.util import StandardButton +class ErrorOptionsPage(OptionsPage): + + def __init__(self, parent=None, errmsg='', from_cls=None): + # copy properties from failing page + self.NAME = from_cls.NAME + self.TITLE = from_cls.TITLE + self.PARENT = from_cls.PARENT + self.SORT_ORDER = from_cls.SORT_ORDER + self.ACTIVE = from_cls.ACTIVE + self.HELP_URL = from_cls.HELP_URL + + super().__init__(parent) + + msg = _( + "Error while loading option page '%s':\n" + "\n" + "%s\n" + "\n" + "Please report this issue." + ) % (_(self.TITLE), errmsg) + self.ui = QtWidgets.QLabel(msg, self) + + class OptionsDialog(PicardDialog, SingletonDialog): suspend_signals = False @@ -155,11 +179,16 @@ def __init__(self, default_page=None, parent=None): self.pages = [] for Page in page_classes: try: - page = Page(self.ui.pages_stack) - page.set_dialog(self) - self.pages.append(page) - except Exception: + page = Page() + except Exception as e: log.exception("Failed initializing options page %r", Page) + # create an empty page with the error message in place of the failing page + # this approach still allows subpages of the failing page to load + page = ErrorOptionsPage(from_cls=Page, errmsg=str(e)) + self.ui.pages_stack.addWidget(page) + page.set_dialog(self) + self.pages.append(page) + self.item_to_page = {} self.page_to_item = {} self.default_item = None From a48d1cf3564afbf4f36ed3c7bf35f2870c81449c Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 27 Apr 2024 18:36:28 +0200 Subject: [PATCH 02/27] No need to add widget to pages_stack in add_pages(), as it is done before it is called - not sure why we had this, but removing one or the other doesn't change anything and works --- picard/ui/options/dialog.py | 1 - 1 file changed, 1 deletion(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 772d10d2a9..ebf7b64b97 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -130,7 +130,6 @@ def add_pages(self, parent, default_page, parent_item): if page.ACTIVE: self.item_to_page[item] = page self.page_to_item[page.NAME] = item - self.ui.pages_stack.addWidget(page) profile_groups_order(page.NAME) else: item.setFlags(QtCore.Qt.ItemFlag.ItemIsEnabled) From b892157d8634d89f9deb0cf9c955131eb1894090 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 27 Apr 2024 18:54:11 +0200 Subject: [PATCH 03/27] Append (error) to page title and add a tooltip indicating it failed to load --- picard/ui/options/__init__.py | 2 ++ picard/ui/options/dialog.py | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/picard/ui/options/__init__.py b/picard/ui/options/__init__.py index b7c9e99e11..f3d7122917 100644 --- a/picard/ui/options/__init__.py +++ b/picard/ui/options/__init__.py @@ -55,6 +55,8 @@ class OptionsPage(QtWidgets.QWidget): STYLESHEET_ERROR = "QWidget { background-color: #f55; color: white; font-weight:bold; padding: 2px; }" STYLESHEET = "QLabel { qproperty-wordWrap: true; }" + error = None + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.tagger = QtCore.QCoreApplication.instance() diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index ebf7b64b97..35d8c20607 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -113,7 +113,8 @@ def __init__(self, parent=None, errmsg='', from_cls=None): "%s\n" "\n" "Please report this issue." - ) % (_(self.TITLE), errmsg) + ) % (_(from_cls.TITLE), errmsg) + self.error = _("This page failed to load") self.ui = QtWidgets.QLabel(msg, self) @@ -126,7 +127,12 @@ def add_pages(self, parent, default_page, parent_item): items = [] for foo, bar, page in sorted(pages): item = HashableTreeWidgetItem(parent_item) - item.setText(0, _(page.TITLE)) + if page.error: + title = _("%s (error)") % _(page.TITLE) + item.setToolTip(0, page.error) + else: + title = _(page.TITLE) + item.setText(0, title) if page.ACTIVE: self.item_to_page[item] = page self.page_to_item[page.NAME] = item From b26d5608ebed2f635fdb8c71873b834f71cff418 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 27 Apr 2024 18:59:42 +0200 Subject: [PATCH 04/27] Avoid a possible exception if the current item is an inactive page To replicate the issue: - select a page - exit Picard - set the page ACTIVE attribute to False - load Picard - open Options again --- picard/ui/options/dialog.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 35d8c20607..5ca86461f1 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -226,8 +226,14 @@ def __init__(self, default_page=None, parent=None): self.installEventFilter(self) self.highlight_enabled_profile_options() - current_page = self.item_to_page[self.ui.pages_tree.currentItem()] - self.set_profiles_button_and_highlight(current_page) + + try: + current_item = self.ui.pages_tree.currentItem() + current_page = self.item_to_page[current_item] + self.set_profiles_button_and_highlight(current_page) + except KeyError: + # selected page became inactive, not available + pass def load_all_pages(self): for page in self.pages: From 866c0bd7afd1cabaa01df3381112e661a80af895 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 27 Apr 2024 22:28:25 +0200 Subject: [PATCH 05/27] Improve error message and add a link to bug tracker --- picard/const/__init__.py | 1 + picard/ui/options/dialog.py | 23 +++++++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/picard/const/__init__.py b/picard/const/__init__.py index 5dc2bb1792..e912834bb5 100644 --- a/picard/const/__init__.py +++ b/picard/const/__init__.py @@ -86,6 +86,7 @@ 'chromaprint': "https://acoustid.org/chromaprint#download", 'acoustid_apikey': "https://acoustid.org/api-key", 'acoustid_track': "https://acoustid.org/track/", + 'bug_tracker': "https://tickets.metabrainz.org/projects/PICARD", } # Various Artists MBID diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 5ca86461f1..52bf659c37 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -44,6 +44,7 @@ SettingConfigSection, get_config, ) +from picard.const import PICARD_URLS from picard.i18n import ( N_, gettext as _, @@ -110,12 +111,26 @@ def __init__(self, parent=None, errmsg='', from_cls=None): msg = _( "Error while loading option page '%s':\n" "\n" - "%s\n" - "\n" - "Please report this issue." + "%s" ) % (_(from_cls.TITLE), errmsg) self.error = _("This page failed to load") - self.ui = QtWidgets.QLabel(msg, self) + + layout = QtWidgets.QVBoxLayout(self) + widget = QtWidgets.QLabel() + widget.setTextFormat(QtCore.Qt.TextFormat.PlainText) + widget.setText(msg) + widget.setWordWrap(True) + layout.addWidget(widget) + + widget = QtWidgets.QLabel( + _('Please report this issue on the Picard bug tracker.') + % PICARD_URLS['bug_tracker'] + ) + widget.setOpenExternalLinks(True) + layout.addWidget(widget) + + layout.addStretch() + self.ui = layout class OptionsDialog(PicardDialog, SingletonDialog): From a7c172e6c120191aa188af256e937af2e8e34bec Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sat, 27 Apr 2024 22:45:59 +0200 Subject: [PATCH 06/27] Introduce OptionsPage.loaded and make code more resistant to page loading failures - if the page fails during initialization `error` property is set - if the page fails during `load()` `loaded` property is False - test `page.loaded` before trying to use it or call methods from it --- picard/ui/options/__init__.py | 1 + picard/ui/options/dialog.py | 56 +++++++++++++++++++++-------------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/picard/ui/options/__init__.py b/picard/ui/options/__init__.py index f3d7122917..ea84badbaa 100644 --- a/picard/ui/options/__init__.py +++ b/picard/ui/options/__init__.py @@ -55,6 +55,7 @@ class OptionsPage(QtWidgets.QWidget): STYLESHEET_ERROR = "QWidget { background-color: #f55; color: white; font-weight:bold; padding: 2px; }" STYLESHEET = "QLabel { qproperty-wordWrap: true; }" + loaded = False error = None def __init__(self, *args, **kwargs): diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 52bf659c37..b916c3fa04 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -227,38 +227,43 @@ def __init__(self, default_page=None, parent=None): self.restoreWindowState() self.finished.connect(self.saveWindowState) - - self.load_all_pages() self.ui.pages_tree.setCurrentItem(self.default_item) - self.profile_page = self.get_page('profiles') - self.profile_page.signal_refresh.connect(self.update_from_profile_changes) + self.load_all_pages() self.maintenance_page = self.get_page('maintenance') - self.maintenance_page.signal_reload.connect(self.load_all_pages) + if self.maintenance_page.loaded: + self.maintenance_page.signal_reload.connect(self.load_all_pages) self.first_enter = True self.installEventFilter(self) - self.highlight_enabled_profile_options() - - try: - current_item = self.ui.pages_tree.currentItem() - current_page = self.item_to_page[current_item] - self.set_profiles_button_and_highlight(current_page) - except KeyError: - # selected page became inactive, not available - pass + self.profile_page = self.get_page('profiles') + if self.profile_page.loaded: + self.profile_page.signal_refresh.connect(self.update_from_profile_changes) + self.highlight_enabled_profile_options() + try: + current_item = self.ui.pages_tree.currentItem() + current_page = self.item_to_page[current_item] + self.set_profiles_button_and_highlight(current_page) + except KeyError: + # selected page became inactive, not available + pass def load_all_pages(self): for page in self.pages: + if page.error: + continue try: page.load() + page.loaded = True except Exception: log.exception("Failed loading options page %r", page) self.disable_page(page.NAME) def show_attached_profiles_dialog(self): + if not self.profile_page.loaded: + return window_title = _("Profiles Attached to Options") items = self.ui.pages_tree.selectedItems() if not items: @@ -292,6 +297,8 @@ def update_from_profile_changes(self): def get_working_profile_data(self): profile_page = self.get_page('profiles') + if not profile_page.loaded: + return working_profiles = profile_page._clean_and_get_all_profiles() if working_profiles is None: working_profiles = [] @@ -354,6 +361,8 @@ def get_page(self, name): return self.item_to_page[self.page_to_item[name]] def page_has_attached_profiles(self, page, enabled_profiles_only=False): + if not page.loaded: + return False option_group = profile_groups_group_from_page(page) if not option_group: return False @@ -409,15 +418,16 @@ def accept(self): log.exception("Failed checking options page %r", page) self._show_page_error(page, e) return - self.profile_page.save() - for page in self.pages: - try: - if page != self.profile_page: - page.save() - except Exception as e: - log.exception("Failed saving options page %r", page) - self._show_page_error(page, e) - return + if self.profile_page.loaded: + self.profile_page.save() + for page in self.pages: + try: + if page != self.profile_page: + page.save() + except Exception as e: + log.exception("Failed saving options page %r", page) + self._show_page_error(page, e) + return super().accept() def _show_page_error(self, page, error): From 4346b771528ac5f5a243f3998a4206358faa1e73 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 10:37:04 +0200 Subject: [PATCH 07/27] Link to Troubleshooting documentation instead of Bug Tracker Suggested by rdswift --- picard/const/__init__.py | 1 - picard/ui/options/dialog.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/picard/const/__init__.py b/picard/const/__init__.py index e912834bb5..5dc2bb1792 100644 --- a/picard/const/__init__.py +++ b/picard/const/__init__.py @@ -86,7 +86,6 @@ 'chromaprint': "https://acoustid.org/chromaprint#download", 'acoustid_apikey': "https://acoustid.org/api-key", 'acoustid_track': "https://acoustid.org/track/", - 'bug_tracker': "https://tickets.metabrainz.org/projects/PICARD", } # Various Artists MBID diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index b916c3fa04..274cd02fac 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -123,8 +123,8 @@ def __init__(self, parent=None, errmsg='', from_cls=None): layout.addWidget(widget) widget = QtWidgets.QLabel( - _('Please report this issue on the Picard bug tracker.') - % PICARD_URLS['bug_tracker'] + _('Please see Troubleshooting documentation and eventually report a bug.') + % PICARD_URLS['troubleshooting'] ) widget.setOpenExternalLinks(True) layout.addWidget(widget) From 063e2cc538cb9a738511f96c70e520bea81bee7a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 11:06:06 +0200 Subject: [PATCH 08/27] Call setCurrentWidget() from switch_page(), it makes more sense --- picard/ui/options/dialog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 274cd02fac..4b68cfb04a 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -381,7 +381,6 @@ def set_profiles_button_and_highlight(self, page): self.ui.attached_profiles_button.setDisabled(False) else: self.ui.attached_profiles_button.setDisabled(True) - self.ui.pages_stack.setCurrentWidget(page) def switch_page(self): items = self.ui.pages_tree.selectedItems() @@ -390,6 +389,7 @@ def switch_page(self): page = self.item_to_page[items[0]] config.persist['options_last_active_page'] = page.NAME self.set_profiles_button_and_highlight(page) + self.ui.pages_stack.setCurrentWidget(page) def disable_page(self, name): item = self.page_to_item[name] From d47968bce193a2970427228d82ca32716f72c2dc Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 11:07:46 +0200 Subject: [PATCH 09/27] Do not try to use profiles if profiles option page failed to load --- picard/ui/options/dialog.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 4b68cfb04a..9fba4861b7 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -296,16 +296,15 @@ def update_from_profile_changes(self): self.highlight_enabled_profile_options(load_settings=True) def get_working_profile_data(self): - profile_page = self.get_page('profiles') - if not profile_page.loaded: - return - working_profiles = profile_page._clean_and_get_all_profiles() + working_profiles = self.profile_page._clean_and_get_all_profiles() if working_profiles is None: working_profiles = [] - working_settings = profile_page.profile_settings + working_settings = self.profile_page.profile_settings return working_profiles, working_settings def highlight_enabled_profile_options(self, load_settings=False): + if not self.profile_page.loaded: + return working_profiles, working_settings = self.get_working_profile_data() from picard.ui.colors import interface_colors as colors fg_color = colors.get_color('profile_hl_fg') @@ -361,7 +360,7 @@ def get_page(self, name): return self.item_to_page[self.page_to_item[name]] def page_has_attached_profiles(self, page, enabled_profiles_only=False): - if not page.loaded: + if not page.loaded or not self.profile_page.loaded: return False option_group = profile_groups_group_from_page(page) if not option_group: From 25a36ee2818382c132f817e2fbd4fce48d1ced59 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 11:17:17 +0200 Subject: [PATCH 10/27] Disable Restore Defaults for pages that failed to load --- picard/ui/options/dialog.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 9fba4861b7..2001601434 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -384,11 +384,12 @@ def set_profiles_button_and_highlight(self, page): def switch_page(self): items = self.ui.pages_tree.selectedItems() if items: - config = get_config() page = self.item_to_page[items[0]] - config.persist['options_last_active_page'] = page.NAME self.set_profiles_button_and_highlight(page) + self.ui.reset_button.setDisabled(not page.loaded) self.ui.pages_stack.setCurrentWidget(page) + config = get_config() + config.persist['options_last_active_page'] = page.NAME def disable_page(self, name): item = self.page_to_item[name] @@ -464,15 +465,18 @@ def restore_all_defaults(self): self.suspend_signals = True for page in self.pages: try: - page.restore_defaults() + if page.loaded: + page.restore_defaults() except Exception as e: log.error("Failed restoring all defaults for page %r: %s", page, e) self.highlight_enabled_profile_options(load_settings=False) self.suspend_signals = False def restore_page_defaults(self): - self.ui.pages_stack.currentWidget().restore_defaults() - self.highlight_enabled_profile_options(load_settings=False) + current_page = self.ui.pages_stack.currentWidget() + if current_page.loaded: + current_page.restore_defaults() + self.highlight_enabled_profile_options(load_settings=False) def confirm_reset(self): msg = _("You are about to reset your options for this page.") From 054ba32d6cf20af9bd69943f4d533127da229fb4 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 11:27:29 +0200 Subject: [PATCH 11/27] Introduce generator properties to iterate initialized and loaded pages --- picard/ui/options/dialog.py | 40 ++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 2001601434..fd9af18ac4 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -250,10 +250,16 @@ def __init__(self, default_page=None, parent=None): # selected page became inactive, not available pass + @property + def initialized_pages(self): + yield from (page for page in self.pages if not page.error) + + @property + def loaded_pages(self): + yield from (page for page in self.pages if page.loaded) + def load_all_pages(self): - for page in self.pages: - if page.error: - continue + for page in self.initialized_pages: try: page.load() page.loaded = True @@ -310,7 +316,7 @@ def highlight_enabled_profile_options(self, load_settings=False): fg_color = colors.get_color('profile_hl_fg') bg_color = colors.get_color('profile_hl_bg') - for page in self.pages: + for page in self.loaded_pages: option_group = profile_groups_group_from_page(page) if option_group: if load_settings: @@ -408,7 +414,7 @@ def help_url(self): return url def accept(self): - for page in self.pages: + for page in self.loaded_pages: try: page.check() except OptionsCheckError as e: @@ -418,16 +424,19 @@ def accept(self): log.exception("Failed checking options page %r", page) self._show_page_error(page, e) return + if self.profile_page.loaded: self.profile_page.save() - for page in self.pages: - try: - if page != self.profile_page: - page.save() - except Exception as e: - log.exception("Failed saving options page %r", page) - self._show_page_error(page, e) - return + + for page in self.loaded_pages: + try: + if page != self.profile_page: + page.save() + except Exception as e: + log.exception("Failed saving options page %r", page) + self._show_page_error(page, e) + return + super().accept() def _show_page_error(self, page, error): @@ -463,10 +472,9 @@ def restoreWindowState(self): def restore_all_defaults(self): self.suspend_signals = True - for page in self.pages: + for page in self.loaded_pages: try: - if page.loaded: - page.restore_defaults() + page.restore_defaults() except Exception as e: log.error("Failed restoring all defaults for page %r: %s", page, e) self.highlight_enabled_profile_options(load_settings=False) From e9ef17f3321080475613330a8c64b8e44703c0dc Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 11:29:07 +0200 Subject: [PATCH 12/27] Simplify code, no need to make a special case of profile_page.save() --- picard/ui/options/dialog.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index fd9af18ac4..b143962da9 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -425,13 +425,9 @@ def accept(self): self._show_page_error(page, e) return - if self.profile_page.loaded: - self.profile_page.save() - for page in self.loaded_pages: try: - if page != self.profile_page: - page.save() + page.save() except Exception as e: log.exception("Failed saving options page %r", page) self._show_page_error(page, e) From b387c91468fae467b8dd031d50c4a73a9207c745 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 11:39:17 +0200 Subject: [PATCH 13/27] Run `page.set_dialog()` in the try/except block as it may be redefined And add a parameter to pass it to `ErrorOptionsPage`. It looks code duplication, but it isn't, as an `OptionsPage` subclass can redefine `set_dialog()`. --- picard/ui/options/dialog.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index b143962da9..90698751b3 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -97,7 +97,7 @@ class ErrorOptionsPage(OptionsPage): - def __init__(self, parent=None, errmsg='', from_cls=None): + def __init__(self, parent=None, errmsg='', from_cls=None, dialog=None): # copy properties from failing page self.NAME = from_cls.NAME self.TITLE = from_cls.TITLE @@ -132,6 +132,8 @@ def __init__(self, parent=None, errmsg='', from_cls=None): layout.addStretch() self.ui = layout + self.dialog = dialog + class OptionsDialog(PicardDialog, SingletonDialog): @@ -200,13 +202,13 @@ def __init__(self, default_page=None, parent=None): for Page in page_classes: try: page = Page() + page.set_dialog(self) except Exception as e: log.exception("Failed initializing options page %r", Page) # create an empty page with the error message in place of the failing page # this approach still allows subpages of the failing page to load - page = ErrorOptionsPage(from_cls=Page, errmsg=str(e)) + page = ErrorOptionsPage(from_cls=Page, errmsg=str(e), dialog=self) self.ui.pages_stack.addWidget(page) - page.set_dialog(self) self.pages.append(page) self.item_to_page = {} From 200028f2c7b32e3363500f790343d0b782c09ae9 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 13:33:10 +0200 Subject: [PATCH 14/27] Improve ErrorOptionsPage display --- picard/ui/options/dialog.py | 41 ++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 90698751b3..07a8c3dede 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -108,27 +108,36 @@ def __init__(self, parent=None, errmsg='', from_cls=None, dialog=None): super().__init__(parent) - msg = _( - "Error while loading option page '%s':\n" - "\n" - "%s" - ) % (_(from_cls.TITLE), errmsg) self.error = _("This page failed to load") - layout = QtWidgets.QVBoxLayout(self) - widget = QtWidgets.QLabel() - widget.setTextFormat(QtCore.Qt.TextFormat.PlainText) - widget.setText(msg) - widget.setWordWrap(True) - layout.addWidget(widget) - - widget = QtWidgets.QLabel( - _('Please see Troubleshooting documentation and eventually report a bug.') + title_widget = QtWidgets.QLabel( + _("Error while loading option page '%s':") + % _(from_cls.TITLE) + ) + + error_widget = QtWidgets.QLabel() + error_widget.setTextFormat(QtCore.Qt.TextFormat.PlainText) + error_widget.setText(errmsg) + error_widget.setWordWrap(True) + error_widget.setFrameShape(QtWidgets.QFrame.Shape.StyledPanel) + error_widget.setFrameShadow(QtWidgets.QFrame.Shadow.Sunken) + error_widget.setLineWidth(1) + error_widget.setTextInteractionFlags( + QtCore.Qt.TextInteractionFlag.TextSelectableByKeyboard + | QtCore.Qt.TextInteractionFlag.TextSelectableByMouse + ) + + report_bug_widget = QtWidgets.QLabel( + _('Please see Troubleshooting documentation' + ' and eventually report a bug.') % PICARD_URLS['troubleshooting'] ) - widget.setOpenExternalLinks(True) - layout.addWidget(widget) + report_bug_widget.setOpenExternalLinks(True) + layout = QtWidgets.QVBoxLayout(self) + layout.addWidget(title_widget) + layout.addWidget(error_widget) + layout.addWidget(report_bug_widget) layout.addStretch() self.ui = layout From b80f4d698e38eca3a3750d4956a2eaae46de6e71 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 18:32:43 +0200 Subject: [PATCH 15/27] Replace OptionsPage error property with clearer initialized property - drop tooltip, as error is clear displayed in the page anyway - technically those exceptions happen during `__init__()` not `load()`, so that's less ambiguous --- picard/ui/options/__init__.py | 2 +- picard/ui/options/dialog.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/picard/ui/options/__init__.py b/picard/ui/options/__init__.py index ea84badbaa..40d00b9e95 100644 --- a/picard/ui/options/__init__.py +++ b/picard/ui/options/__init__.py @@ -55,8 +55,8 @@ class OptionsPage(QtWidgets.QWidget): STYLESHEET_ERROR = "QWidget { background-color: #f55; color: white; font-weight:bold; padding: 2px; }" STYLESHEET = "QLabel { qproperty-wordWrap: true; }" + initialized = False loaded = False - error = None def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 07a8c3dede..6baf258683 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -108,10 +108,10 @@ def __init__(self, parent=None, errmsg='', from_cls=None, dialog=None): super().__init__(parent) - self.error = _("This page failed to load") + self.error = _("This page failed to initialize") title_widget = QtWidgets.QLabel( - _("Error while loading option page '%s':") + _("Error while initializing option page '%s':") % _(from_cls.TITLE) ) @@ -153,9 +153,8 @@ def add_pages(self, parent, default_page, parent_item): items = [] for foo, bar, page in sorted(pages): item = HashableTreeWidgetItem(parent_item) - if page.error: + if not page.initialized: title = _("%s (error)") % _(page.TITLE) - item.setToolTip(0, page.error) else: title = _(page.TITLE) item.setText(0, title) @@ -212,6 +211,7 @@ def __init__(self, default_page=None, parent=None): try: page = Page() page.set_dialog(self) + page.initialized = True except Exception as e: log.exception("Failed initializing options page %r", Page) # create an empty page with the error message in place of the failing page @@ -263,7 +263,7 @@ def __init__(self, default_page=None, parent=None): @property def initialized_pages(self): - yield from (page for page in self.pages if not page.error) + yield from (page for page in self.pages if page.initialized) @property def loaded_pages(self): From dd91bddd3b31c18790ef7413d41f5a042278a157 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 18:43:07 +0200 Subject: [PATCH 16/27] add_pages(): drop unused variables foo & bar - create a generator - use sorted() key property to define order --- picard/ui/options/dialog.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 6baf258683..83ccf7647a 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -149,9 +149,13 @@ class OptionsDialog(PicardDialog, SingletonDialog): suspend_signals = False def add_pages(self, parent, default_page, parent_item): - pages = [(p.SORT_ORDER, p.NAME, p) for p in self.pages if p.PARENT == parent] + def pages(): + for p in self.pages: + if p.PARENT == parent: + yield p + items = [] - for foo, bar, page in sorted(pages): + for page in sorted(pages(), key=lambda p: (p.SORT_ORDER, p.NAME)): item = HashableTreeWidgetItem(parent_item) if not page.initialized: title = _("%s (error)") % _(page.TITLE) From 7152db11cbb7ca11d535f33b866dd97a0521e186 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 18:48:20 +0200 Subject: [PATCH 17/27] Rename `page_to_item` to `pagename_to_item` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That's clearer, as we use `page` for `OptionsPage`, but also for `OptionsPage.NAME`. This dict clearly converts a page's name to an item. --- picard/ui/options/dialog.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 83ccf7647a..e9d60dad1f 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -164,7 +164,7 @@ def pages(): item.setText(0, title) if page.ACTIVE: self.item_to_page[item] = page - self.page_to_item[page.NAME] = item + self.pagename_to_item[page.NAME] = item profile_groups_order(page.NAME) else: item.setFlags(QtCore.Qt.ItemFlag.ItemIsEnabled) @@ -225,7 +225,7 @@ def __init__(self, default_page=None, parent=None): self.pages.append(page) self.item_to_page = {} - self.page_to_item = {} + self.pagename_to_item = {} self.default_item = None if not default_page: default_page = config.persist['options_last_active_page'] @@ -378,7 +378,7 @@ def eventFilter(self, object, event): return False def get_page(self, name): - return self.item_to_page[self.page_to_item[name]] + return self.item_to_page[self.pagename_to_item[name]] def page_has_attached_profiles(self, page, enabled_profiles_only=False): if not page.loaded or not self.profile_page.loaded: @@ -413,7 +413,7 @@ def switch_page(self): config.persist['options_last_active_page'] = page.NAME def disable_page(self, name): - item = self.page_to_item[name] + item = self.pagename_to_item[name] item.setDisabled(True) @property @@ -422,7 +422,7 @@ def help_url(self): url = current_page.HELP_URL # If URL is empty, use the first non empty parent help URL. while current_page.PARENT and not url: - current_page = self.item_to_page[self.page_to_item[current_page.PARENT]] + current_page = self.item_to_page[self.pagename_to_item[current_page.PARENT]] url = current_page.HELP_URL if not url: url = 'doc_options' # key in PICARD_URLS @@ -453,12 +453,12 @@ def accept(self): def _show_page_error(self, page, error): if not isinstance(error, OptionsCheckError): error = OptionsCheckError(_("Unexpected error"), str(error)) - self.ui.pages_tree.setCurrentItem(self.page_to_item[page.NAME]) + self.ui.pages_tree.setCurrentItem(self.pagename_to_item[page.NAME]) page.display_error(error) def saveWindowState(self): expanded_pages = [] - for page, item in self.page_to_item.items(): + for page, item in self.pagename_to_item.items(): index = self.ui.pages_tree.indexFromItem(item) is_expanded = self.ui.pages_tree.isExpanded(index) expanded_pages.append((page, is_expanded)) @@ -476,7 +476,7 @@ def restoreWindowState(self): else: for page, is_expanded in pages_tree_state: try: - item = self.page_to_item[page] + item = self.pagename_to_item[page] except KeyError: continue item.setExpanded(is_expanded) From 499fe18cfbf57ad875144a474070b045f2cac224 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 18:50:21 +0200 Subject: [PATCH 18/27] Use existing method `get_page()` --- picard/ui/options/dialog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index e9d60dad1f..aa7b6605fc 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -422,7 +422,7 @@ def help_url(self): url = current_page.HELP_URL # If URL is empty, use the first non empty parent help URL. while current_page.PARENT and not url: - current_page = self.item_to_page[self.pagename_to_item[current_page.PARENT]] + current_page = self.get_page(current_page.PARENT) url = current_page.HELP_URL if not url: url = 'doc_options' # key in PICARD_URLS From 07e6efac0009bafa37db0fc72d45a20ff1def3de Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 18:59:26 +0200 Subject: [PATCH 19/27] Use pagename for ... page's names page was referring either to a page object or its name --- picard/ui/options/dialog.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index aa7b6605fc..5990afb413 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -148,10 +148,10 @@ class OptionsDialog(PicardDialog, SingletonDialog): suspend_signals = False - def add_pages(self, parent, default_page, parent_item): + def add_pages(self, parent_pagename, default_pagename, parent_item): def pages(): for p in self.pages: - if p.PARENT == parent: + if p.PARENT == parent_pagename: yield p items = [] @@ -168,11 +168,11 @@ def pages(): profile_groups_order(page.NAME) else: item.setFlags(QtCore.Qt.ItemFlag.ItemIsEnabled) - self.add_pages(page.NAME, default_page, item) - if page.NAME == default_page: + self.add_pages(page.NAME, default_pagename, item) + if page.NAME == default_pagename: self.default_item = item items.append(item) - if not self.default_item and not parent: + if not self.default_item and not parent_pagename: self.default_item = items[0] def __init__(self, default_page=None, parent=None): @@ -228,8 +228,8 @@ def __init__(self, default_page=None, parent=None): self.pagename_to_item = {} self.default_item = None if not default_page: - default_page = config.persist['options_last_active_page'] - self.add_pages(None, default_page, self.ui.pages_tree) + default_pagename = config.persist['options_last_active_page'] + self.add_pages(None, default_pagename, self.ui.pages_tree) # work-around to set optimal option pane width self.ui.pages_tree.expandAll() @@ -377,8 +377,8 @@ def eventFilter(self, object, event): self.activateWindow() return False - def get_page(self, name): - return self.item_to_page[self.pagename_to_item[name]] + def get_page(self, pagename): + return self.item_to_page[self.pagename_to_item[pagename]] def page_has_attached_profiles(self, page, enabled_profiles_only=False): if not page.loaded or not self.profile_page.loaded: @@ -412,8 +412,8 @@ def switch_page(self): config = get_config() config.persist['options_last_active_page'] = page.NAME - def disable_page(self, name): - item = self.pagename_to_item[name] + def disable_page(self, pagename): + item = self.pagename_to_item[pagename] item.setDisabled(True) @property @@ -458,10 +458,10 @@ def _show_page_error(self, page, error): def saveWindowState(self): expanded_pages = [] - for page, item in self.pagename_to_item.items(): + for pagename, item in self.pagename_to_item.items(): index = self.ui.pages_tree.indexFromItem(item) is_expanded = self.ui.pages_tree.isExpanded(index) - expanded_pages.append((page, is_expanded)) + expanded_pages.append((pagename, is_expanded)) config = get_config() config.persist['options_pages_tree_state'] = expanded_pages config.setting.set_profiles_override() @@ -474,9 +474,9 @@ def restoreWindowState(self): if not pages_tree_state: self.ui.pages_tree.expandAll() else: - for page, is_expanded in pages_tree_state: + for pagename, is_expanded in pages_tree_state: try: - item = self.pagename_to_item[page] + item = self.pagename_to_item[pagename] except KeyError: continue item.setExpanded(is_expanded) From 14c145dda45ef8244e640ae4674d4dda6fd65618 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 19:17:49 +0200 Subject: [PATCH 20/27] Reduce code redundancy a bit as switch_page calls set_profiles_button_and_highlight - ensure signal is connected before setting the current item - ensure profiles stuff is ready before setting current item --- picard/ui/options/dialog.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 5990afb413..c959770403 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -238,32 +238,25 @@ def __init__(self, default_page=None, parent=None): self.ui.pages_tree.setHeaderLabels([""]) self.ui.pages_tree.header().hide() - self.ui.pages_tree.itemSelectionChanged.connect(self.switch_page) self.restoreWindowState() self.finished.connect(self.saveWindowState) - self.ui.pages_tree.setCurrentItem(self.default_item) self.load_all_pages() + self.first_enter = True + self.installEventFilter(self) self.maintenance_page = self.get_page('maintenance') if self.maintenance_page.loaded: self.maintenance_page.signal_reload.connect(self.load_all_pages) - self.first_enter = True - self.installEventFilter(self) - self.profile_page = self.get_page('profiles') if self.profile_page.loaded: self.profile_page.signal_refresh.connect(self.update_from_profile_changes) self.highlight_enabled_profile_options() - try: - current_item = self.ui.pages_tree.currentItem() - current_page = self.item_to_page[current_item] - self.set_profiles_button_and_highlight(current_page) - except KeyError: - # selected page became inactive, not available - pass + + self.ui.pages_tree.itemSelectionChanged.connect(self.switch_page) + self.ui.pages_tree.setCurrentItem(self.default_item) # this will call switch_page @property def initialized_pages(self): From 25964df99a633cd9601b59ce3b5cee3954040b1e Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 19:26:31 +0200 Subject: [PATCH 21/27] Use a tuple instead of a generator Suggested by phw --- picard/ui/options/dialog.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index c959770403..4998878a5a 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -149,13 +149,9 @@ class OptionsDialog(PicardDialog, SingletonDialog): suspend_signals = False def add_pages(self, parent_pagename, default_pagename, parent_item): - def pages(): - for p in self.pages: - if p.PARENT == parent_pagename: - yield p - + pages = (p for p in self.pages if p.PARENT == parent_pagename) items = [] - for page in sorted(pages(), key=lambda p: (p.SORT_ORDER, p.NAME)): + for page in sorted(pages, key=lambda p: (p.SORT_ORDER, p.NAME)): item = HashableTreeWidgetItem(parent_item) if not page.initialized: title = _("%s (error)") % _(page.TITLE) From 99a059e53d323dcee3f687f7ff7f6ff939bc1b27 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 19:40:25 +0200 Subject: [PATCH 22/27] Minor change in try/except, add debug output if restoring expanded state fails --- picard/ui/options/dialog.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 4998878a5a..e3298d316b 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -466,9 +466,9 @@ def restoreWindowState(self): for pagename, is_expanded in pages_tree_state: try: item = self.pagename_to_item[pagename] - except KeyError: - continue - item.setExpanded(is_expanded) + item.setExpanded(is_expanded) + except KeyError as e: + log.debug("Failed restoring expanded state: %s", e) def restore_all_defaults(self): self.suspend_signals = True From 0411483d1c523fc95aca9cf259a4b10847539358 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 23:00:53 +0200 Subject: [PATCH 23/27] OptionsDialog.maintenance_page is used nowhere, use a local variable instead --- picard/ui/options/dialog.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index e3298d316b..8f213aeb4f 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -242,9 +242,9 @@ def __init__(self, default_page=None, parent=None): self.first_enter = True self.installEventFilter(self) - self.maintenance_page = self.get_page('maintenance') - if self.maintenance_page.loaded: - self.maintenance_page.signal_reload.connect(self.load_all_pages) + maintenance_page = self.get_page('maintenance') + if maintenance_page.loaded: + maintenance_page.signal_reload.connect(self.load_all_pages) self.profile_page = self.get_page('profiles') if self.profile_page.loaded: From c482b2e1224c6632af2311aab88979857c7458ba Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 23:04:51 +0200 Subject: [PATCH 24/27] Do not create a OptionsDialog.profile_page property, use get_page() instead --- picard/ui/options/dialog.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 8f213aeb4f..1a148a7093 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -246,9 +246,9 @@ def __init__(self, default_page=None, parent=None): if maintenance_page.loaded: maintenance_page.signal_reload.connect(self.load_all_pages) - self.profile_page = self.get_page('profiles') - if self.profile_page.loaded: - self.profile_page.signal_refresh.connect(self.update_from_profile_changes) + profile_page = self.get_page('profiles') + if profile_page.loaded: + profile_page.signal_refresh.connect(self.update_from_profile_changes) self.highlight_enabled_profile_options() self.ui.pages_tree.itemSelectionChanged.connect(self.switch_page) @@ -272,7 +272,8 @@ def load_all_pages(self): self.disable_page(page.NAME) def show_attached_profiles_dialog(self): - if not self.profile_page.loaded: + profile_page = self.get_page('profiles') + if not profile_page.loaded: return window_title = _("Profiles Attached to Options") items = self.ui.pages_tree.selectedItems() @@ -289,8 +290,8 @@ def show_attached_profiles_dialog(self): message_box.setStandardButtons(QtWidgets.QMessageBox.StandardButton.Ok) return message_box.exec() - override_profiles = self.profile_page._clean_and_get_all_profiles() - override_settings = self.profile_page.profile_settings + override_profiles = profile_page._clean_and_get_all_profiles() + override_settings = profile_page.profile_settings profile_dialog = AttachedProfilesDialog( option_group, parent=self, @@ -306,14 +307,16 @@ def update_from_profile_changes(self): self.highlight_enabled_profile_options(load_settings=True) def get_working_profile_data(self): - working_profiles = self.profile_page._clean_and_get_all_profiles() + profile_page = self.get_page('profiles') + working_profiles = profile_page._clean_and_get_all_profiles() if working_profiles is None: working_profiles = [] - working_settings = self.profile_page.profile_settings + working_settings = profile_page.profile_settings return working_profiles, working_settings def highlight_enabled_profile_options(self, load_settings=False): - if not self.profile_page.loaded: + profile_page = self.get_page('profiles') + if not profile_page.loaded: return working_profiles, working_settings = self.get_working_profile_data() from picard.ui.colors import interface_colors as colors @@ -370,7 +373,8 @@ def get_page(self, pagename): return self.item_to_page[self.pagename_to_item[pagename]] def page_has_attached_profiles(self, page, enabled_profiles_only=False): - if not page.loaded or not self.profile_page.loaded: + profile_page = self.get_page('profiles') + if not page.loaded or not profile_page.loaded: return False option_group = profile_groups_group_from_page(page) if not option_group: From a20b71df1a1d71b40d02f8dee698b4795778a0e7 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 23:44:41 +0200 Subject: [PATCH 25/27] Split show_attached_profiles_dialog() in smaller bits --- picard/ui/options/dialog.py | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 1a148a7093..5f58f5e4b8 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -272,24 +272,30 @@ def load_all_pages(self): self.disable_page(page.NAME) def show_attached_profiles_dialog(self): - profile_page = self.get_page('profiles') - if not profile_page.loaded: - return - window_title = _("Profiles Attached to Options") items = self.ui.pages_tree.selectedItems() if not items: return page = self.item_to_page[items[0]] option_group = profile_groups_group_from_page(page) - if not option_group: - message_box = QtWidgets.QMessageBox(self) - message_box.setIcon(QtWidgets.QMessageBox.Icon.Information) - message_box.setWindowModality(QtCore.Qt.WindowModality.WindowModal) - message_box.setWindowTitle(window_title) - message_box.setText(_("The options on this page are not currently available to be managed using profiles.")) - message_box.setStandardButtons(QtWidgets.QMessageBox.StandardButton.Ok) - return message_box.exec() + if option_group: + self.display_attached_profiles(option_group) + else: + self.display_simple_message_box( + _("Profiles Attached to Options"), + _("The options on this page are not currently available to be managed using profiles."), + ) + def display_simple_message_box(self, window_title, message): + message_box = QtWidgets.QMessageBox(self) + message_box.setIcon(QtWidgets.QMessageBox.Icon.Information) + message_box.setWindowModality(QtCore.Qt.WindowModality.WindowModal) + message_box.setWindowTitle(window_title) + message_box.setText(message) + message_box.setStandardButtons(QtWidgets.QMessageBox.StandardButton.Ok) + message_box.exec() + + def display_attached_profiles(self, option_group): + profile_page = self.get_page('profiles') override_profiles = profile_page._clean_and_get_all_profiles() override_settings = profile_page.profile_settings profile_dialog = AttachedProfilesDialog( From 26c566573a05dc46a9cf465dba08aa23b8e2eca0 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 23:45:25 +0200 Subject: [PATCH 26/27] Do not try to check profiles page if the current page isn't loaded --- picard/ui/options/dialog.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 5f58f5e4b8..5598a9b24b 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -379,8 +379,10 @@ def get_page(self, pagename): return self.item_to_page[self.pagename_to_item[pagename]] def page_has_attached_profiles(self, page, enabled_profiles_only=False): + if not page.loaded: + return False profile_page = self.get_page('profiles') - if not page.loaded or not profile_page.loaded: + if not profile_page.loaded: return False option_group = profile_groups_group_from_page(page) if not option_group: From b10efe3c734d1ddc005646b2e1b9fac3832c401a Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Sun, 28 Apr 2024 23:46:15 +0200 Subject: [PATCH 27/27] No need to check profile_page here, the button leading here will be disabled --- picard/ui/options/dialog.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/picard/ui/options/dialog.py b/picard/ui/options/dialog.py index 5598a9b24b..c411c5e59c 100644 --- a/picard/ui/options/dialog.py +++ b/picard/ui/options/dialog.py @@ -321,9 +321,6 @@ def get_working_profile_data(self): return working_profiles, working_settings def highlight_enabled_profile_options(self, load_settings=False): - profile_page = self.get_page('profiles') - if not profile_page.loaded: - return working_profiles, working_settings = self.get_working_profile_data() from picard.ui.colors import interface_colors as colors fg_color = colors.get_color('profile_hl_fg')