Fix #783065 (Manage authors dialog too wide). Metadata plugboards: Make matching hierarchical and improve error messages

src/calibre/gui2/dialogs/edit_authors_dialog.ui

@@ -78,22 +78,22 @@
     </widget>
    </item>
    <item>
-    <layout class="QHBoxLayout" name="horizontalLayout">
+    <layout class="QGridLayout">
-     <item>
+     <item row="0" column="0">
       <widget class="QPushButton" name="sort_by_author">
        <property name="text">
         <string>Sort by author</string>
        </property>
       </widget>
      </item>
-     <item>
+     <item row="0" column="1">
       <widget class="QPushButton" name="sort_by_author_sort">
        <property name="text">
         <string>Sort by author sort</string>
        </property>
       </widget>
      </item>
-     <item>
+     <item row="1" column="0">
       <widget class="QPushButton" name="recalc_author_sort">
        <property name="toolTip">
         <string>Reset all the author sort values to a value automatically
@@ -105,7 +105,7 @@ generated can be controlled via Preferences-&gt;Advanced-&gt;Tweaks</string>
        </property>
       </widget>
      </item>
-     <item>
+     <item row="1" column="1">
       <widget class="QPushButton" name="auth_sort_to_author">
        <property name="toolTip">
         <string>Copy author sort to author for every author. You typically use this button
@@ -116,20 +116,7 @@ after changing Preferences-&gt;Advanced-&gt;Tweaks-&gt;Author sort name algorith
        </property>
       </widget>
      </item>
-     <item>
+     <item row="1" column="2">
-      <spacer name="horizontalSpacer_3">
-       <property name="orientation">
-        <enum>Qt::Horizontal</enum>
-       </property>
-       <property name="sizeHint" stdset="0">
-        <size>
-         <width>40</width>
-         <height>20</height>
-        </size>
-       </property>
-      </spacer>
-     </item>
-     <item>
       <widget class="QDialogButtonBox" name="buttonBox">
        <property name="sizePolicy">
         <sizepolicy hsizetype="Minimum" vsizetype="Fixed">

src/calibre/gui2/preferences/plugboard.py

@@ -5,17 +5,20 @@ __license__   = 'GPL v3'
 __copyright__ = '2010, Kovid Goyal <kovid@kovidgoyal.net>'
 __docformat__ = 'restructuredtext en'
 
+import copy
+
 from PyQt4.Qt import Qt, QLineEdit, QComboBox, SIGNAL, QListWidgetItem
 
 from calibre.customize.ui import is_disabled
-from calibre.gui2 import error_dialog
+from calibre.gui2 import error_dialog, question_dialog
 from calibre.gui2.device import device_name_for_plugboards
 from calibre.gui2.dialogs.template_dialog import TemplateDialog
 from calibre.gui2.preferences import ConfigWidgetBase, test_widget
 from calibre.gui2.preferences.plugboard_ui import Ui_Form
 from calibre.customize.ui import metadata_writers, device_plugins
 from calibre.library.save_to_disk import plugboard_any_format_value, \
-                        plugboard_any_device_value, plugboard_save_to_disk_value
+                    plugboard_any_device_value, plugboard_save_to_disk_value, \
+                    find_plugboard
 from calibre.library.server.content import plugboard_content_server_value, \
                                         plugboard_content_server_formats
 from calibre.utils.formatter import validation_formatter
@@ -46,9 +49,6 @@ class ConfigWidget(ConfigWidgetBase, Ui_Form):
     def genesis(self, gui):
         self.gui = gui
         self.db = gui.library_view.model().db
-        self.current_plugboards = self.db.prefs.get('plugboards',{})
-        self.current_device = None
-        self.current_format = None
 
     def initialize(self):
         def field_cmp(x, y):
@@ -64,6 +64,10 @@ class ConfigWidget(ConfigWidgetBase, Ui_Form):
 
         ConfigWidgetBase.initialize(self)
 
+        self.current_plugboards = copy.deepcopy(self.db.prefs.get('plugboards',{}))
+        self.current_device = None
+        self.current_format = None
+
         if self.gui.device_manager.connected_device is not None:
             self.device_label.setText(_('Device currently connected: ') +
                     self.gui.device_manager.connected_device.__class__.__name__)
@@ -196,51 +200,66 @@ class ConfigWidget(ConfigWidgetBase, Ui_Form):
             return
         self.clear_fields(edit_boxes=True)
         self.current_device = unicode(txt)
-        error = False
-        if self.current_format == plugboard_any_format_value:
-            # user specified any format.
-            for f in self.current_plugboards:
-                devs = set(self.current_plugboards[f])
-                if self.current_device != plugboard_save_to_disk_value and \
-                        plugboard_any_device_value in devs:
-                    # specific format/any device in list. conflict.
-                    # note: any device does not match save_to_disk
-                    error = True
-                    break
-                if self.current_device in devs:
-                    # specific format/current device in list. conflict
-                    error = True
-                    break
-                if self.current_device == plugboard_any_device_value:
-                    # any device and a specific device already there. conflict
-                    error = True
-                    break
-        else:
-            # user specified specific format.
-            for f in self.current_plugboards:
-                devs = set(self.current_plugboards[f])
-                if f == plugboard_any_format_value and \
-                                self.current_device in devs:
-                    # any format/same device in list. conflict.
-                    error = True
-                    break
-                if f == self.current_format and self.current_device in devs:
-                    # current format/current device in list. conflict
-                    error = True
-                    break
-                if f == self.current_format and plugboard_any_device_value in devs:
-                    # current format/any device in list. conflict
-                    error = True
-                    break
 
-        if error:
+        if self.current_format in self.current_plugboards and \
+                self.current_device in self.current_plugboards[self.current_format]:
             error_dialog(self, '',
-                     _('That format and device already has a plugboard or '
+                     _('That format and device already has a plugboard.'),
-                       'conflicts with another plugboard.'),
                      show=True)
             self.new_device.setCurrentIndex(0)
             return
-        if self.current_device in self.device_to_formats_map:
+
+        # If we have a specific format/device combination, check if a more
+        # general combination matches.
+        if self.current_format != plugboard_any_format_value and \
+                self.current_device != plugboard_any_device_value:
+            if find_plugboard(self.current_device, self.current_format,
+                      self.current_plugboards):
+                if not question_dialog(self.gui,
+                        _('Possibly override plugboard?'),
+                        _('A more general plugboard already exists for '
+                          'that format and device. '
+                          'Are you sure you want to add the new plugboard?')):
+                    self.new_device.setCurrentIndex(0)
+                    return
+
+        # If we have a specific format, check if we are adding a possibly-
+        # covered plugboard
+        if self.current_format != plugboard_any_format_value:
+            if self.current_format in self.current_plugboards:
+                if self.current_device == plugboard_any_device_value:
+                    if not question_dialog(self.gui,
+                               _('Add possibly overridden plugboard?'),
+                               _('More specific device plugboards exist for '
+                                 'that format. '
+                                 'Are you sure you want to add the new plugboard?')):
+                        self.new_device.setCurrentIndex(0)
+                        return
+        # We are adding an 'any format' entry. Check if we are adding a specific
+        # device and if so, does some other plugboard match that device.
+        elif self.current_device != plugboard_any_device_value:
+            for fmt in self.current_plugboards:
+                if find_plugboard(self.current_device, fmt, self.current_plugboards):
+                    if not question_dialog(self.gui,
+                            _('Really add plugboard?'),
+                            _('A different plugboard matches that format and '
+                              'device combination. '
+                              'Are you sure you want to add the new plugboard?')):
+                        self.new_device.setCurrentIndex(0)
+                        return
+        # We are adding an any format/any device entry, which will be overridden
+        # by any other entry. Ask if such entries exist.
+        elif len(self.current_plugboards):
+            if not question_dialog(self.gui,
+                       _('Add possibly overridden plugboard?'),
+                       _('More specific format and device plugboards '
+                         'already exist. '
+                         'Are you sure you want to add the new plugboard?')):
+                self.new_device.setCurrentIndex(0)
+                return
+
+        if self.current_format != plugboard_any_format_value and \
+                    self.current_device in self.device_to_formats_map:
             allowable_formats = self.device_to_formats_map[self.current_device]
             if self.current_format not in allowable_formats:
                 error_dialog(self, '',

src/calibre/library/save_to_disk.py

@@ -56,16 +56,17 @@ for x in FORMAT_ARG_DESCS:
 def find_plugboard(device_name, format, plugboards):
     cpb = None
     if format in plugboards:
-        cpb = plugboards[format]
+        pb = plugboards[format]
-    elif plugboard_any_format_value in plugboards:
+        if device_name in pb:
-        cpb = plugboards[plugboard_any_format_value]
+            cpb = pb[device_name]
-    if cpb is not None:
+        elif plugboard_any_device_value in pb:
-        if device_name in cpb:
+            cpb = pb[plugboard_any_device_value]
-            cpb = cpb[device_name]
+    if not cpb and plugboard_any_format_value in plugboards:
-        elif plugboard_any_device_value in cpb:
+        pb = plugboards[plugboard_any_format_value]
-            cpb = cpb[plugboard_any_device_value]
+        if device_name in pb:
-        else:
+            cpb = pb[device_name]
-            cpb = None
+        elif plugboard_any_device_value in pb:
+            cpb = pb[plugboard_any_device_value]
     if DEBUG:
         prints('Device using plugboard', format, device_name, cpb)
     return cpb