Fix the root cause of all the book list edit cell problems

The root cause was that Qt doesnt support sharing a delegate between
multiple views. So give the main view and pin view their own delegate
instances. Hopefully I didnt break anything else.

src/calibre/gui2/library/delegates.py

@@ -93,15 +93,7 @@ class UpdateEditorGeometry:
             new_width += r.width()
 
         # Compute the maximum we can show if we consume the entire viewport
-        pin_view = self.table_widget.pin_view
+        max_width = self.parent().viewport().rect().width()
-        is_pin_view, p = False, editor.parent()
-        while p is not None:
-            if p is pin_view:
-                is_pin_view = True
-                break
-            p = p.parent()
-
-        max_width = (pin_view if is_pin_view else self.table_widget).viewport().rect().width()
         # What we have to display might not fit. If so, adjust down
         new_width = new_width if new_width < max_width else max_width
 
@@ -215,15 +207,6 @@ class StyledItemDelegate(QStyledItemDelegate):
     ignore_kb_mods_on_edit = False
 
     def createEditor(self, parent, option, index):
-        current_indices = [self.table_widget.currentIndex()]
-        if hasattr(self.table_widget, 'pin_view'):
-            current_indices.append(self.table_widget.pin_view.currentIndex())
-        if index not in current_indices:
-            idx = self.table_widget.currentIndex()
-            print(f'createEditor idx err: delegate={self.__class__.__name__}. '
-                  f'cur idx=({idx.row()}, {idx.column()}), '
-                  f'given idx=({index.row()}, {index.column()})')
-            return None
         e = self.create_editor(parent, option, index)
         return e
 
@@ -249,7 +232,6 @@ class RatingDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
     def __init__(self, *args, **kwargs):
         StyledItemDelegate.__init__(self, *args)
         self.is_half_star = kwargs.get('is_half_star', False)
-        self.table_widget = args[0]
         self.rf = QFont(rating_font())
         self.em = Qt.TextElideMode.ElideMiddle
         delta = 0
@@ -295,7 +277,6 @@ class DateDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
     def __init__(self, parent, tweak_name='gui_timestamp_display_format',
             default_format='dd MMM yyyy'):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
         self.tweak_name = tweak_name
         self.format = tweaks[self.tweak_name]
         if self.format is None:
@@ -331,7 +312,6 @@ class PubDateDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
     def __init__(self, *args, **kwargs):
         StyledItemDelegate.__init__(self, *args, **kwargs)
         self.format = tweaks['gui_pubdate_display_format']
-        self.table_widget = args[0]
         if self.format is None:
             self.format = 'MMM yyyy'
 
@@ -370,7 +350,6 @@ class TextDelegate(StyledItemDelegate, UpdateEditorGeometry, EditableTextDelegat
         auto-complete will be used.
         '''
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
         self.auto_complete_function = None
 
     def set_auto_complete_function(self, f):
@@ -417,10 +396,10 @@ class CompleteDelegate(StyledItemDelegate, UpdateEditorGeometry, EditableTextDel
         self.sep = sep
         self.items_func_name = items_func_name
         self.space_before_sep = space_before_sep
-        self.table_widget = parent
 
-    def set_database(self, db):
+    @property
-        self.db = db
+    def db(self):
+        return self.parent().model().db
 
     def create_editor(self, parent, option, index):
         if self.db and hasattr(self.db, self.items_func_name):
@@ -464,7 +443,6 @@ class LanguagesDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
 
     def create_editor(self, parent, option, index):
         editor = LanguagesEdit(parent=parent)
@@ -491,7 +469,6 @@ class CcDateDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
 
     def set_format(self, _format):
         if not _format:
@@ -541,7 +518,6 @@ class CcTextDelegate(StyledItemDelegate, UpdateEditorGeometry, EditableTextDeleg
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
 
     def create_editor(self, parent, option, index):
         m = index.model()
@@ -589,7 +565,6 @@ class CcLongTextDelegate(StyledItemDelegate):  # {{{
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
         self.document = QTextDocument()
         self.is_editable_with_tab = False
 
@@ -617,7 +592,6 @@ class CcMarkdownDelegate(StyledItemDelegate):  # {{{
 
     def __init__(self, parent):
         super().__init__(parent)
-        self.table_widget = parent
         self.document = QTextDocument()
         self.is_editable_with_tab = False
 
@@ -671,7 +645,6 @@ class CcNumberDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
 
     def create_editor(self, parent, option, index):
         m = index.model()
@@ -722,7 +695,6 @@ class CcEnumDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
         self.longest_text = ''
 
     def create_editor(self, parent, option, index):
@@ -770,7 +742,6 @@ class CcCommentsDelegate(StyledItemDelegate):  # {{{
 
     def __init__(self, parent):
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
         self.document = QTextDocument()
         self.is_editable_with_tab = False
 
@@ -833,7 +804,6 @@ class CcBoolDelegate(StyledItemDelegate, UpdateEditorGeometry):  # {{{
         '''
         self.nuke_option_data = False
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
 
     def create_editor(self, parent, option, index):
         editor = DelegateCB(parent)
@@ -896,7 +866,6 @@ class CcTemplateDelegate(StyledItemDelegate):  # {{{
         Delegate for composite custom_columns.
         '''
         StyledItemDelegate.__init__(self, parent)
-        self.table_widget = parent
         self.disallow_edit = gprefs['edit_metadata_templates_only_F2_on_booklist']
         self.is_editable_with_tab = False
 

src/calibre/gui2/library/views.py

@@ -11,7 +11,6 @@ from collections import OrderedDict
 from functools import partial
 
 from qt.core import (
-    QAbstractItemDelegate,
     QAbstractItemView,
     QDialog,
     QDialogButtonBox,
@@ -68,7 +67,7 @@ from calibre.gui2.library.delegates import (
     TextDelegate,
 )
 from calibre.gui2.library.models import BooksModel, DeviceBooksModel
-from calibre.gui2.pin_columns import PinTableView
+from calibre.gui2.pin_columns import CustomEditTabbingBehavior, PinTableView
 from calibre.gui2.preferences.create_custom_column import CreateNewCustomColumn
 from calibre.utils.config import prefs, tweaks
 from calibre.utils.icu import primary_sort_key
@@ -357,7 +356,7 @@ class AdjustColumnSize(QDialog):  # {{{
 
 
 @setup_dnd_interface
-class BooksView(QTableView):  # {{{
+class BooksView(QTableView, CustomEditTabbingBehavior):  # {{{
 
     files_dropped = pyqtSignal(object)
     books_dropped = pyqtSignal(object)
@@ -418,28 +417,30 @@ class BooksView(QTableView):  # {{{
             wv.setWordWrap(False)
         self.refresh_grid()
 
-        self.rating_delegate = RatingDelegate(self)
+        def create_delegates(view):
-        self.half_rating_delegate = RatingDelegate(self, is_half_star=True)
+            view.rating_delegate = RatingDelegate(view)
-        self.timestamp_delegate = DateDelegate(self)
+            view.half_rating_delegate = RatingDelegate(view, is_half_star=True)
-        self.pubdate_delegate = PubDateDelegate(self)
+            view.timestamp_delegate = DateDelegate(view)
-        self.last_modified_delegate = DateDelegate(self,
+            view.pubdate_delegate = PubDateDelegate(view)
-                tweak_name='gui_last_modified_display_format')
+            view.last_modified_delegate = DateDelegate(view, tweak_name='gui_last_modified_display_format')
-        self.languages_delegate = LanguagesDelegate(self)
+            view.languages_delegate = LanguagesDelegate(view)
-        self.tags_delegate = CompleteDelegate(self, ',', 'all_tag_names')
+            view.tags_delegate = CompleteDelegate(view, ',', 'all_tag_names')
-        self.authors_delegate = CompleteDelegate(self, '&', 'all_author_names', True)
+            view.authors_delegate = CompleteDelegate(view, '&', 'all_author_names', True)
-        self.cc_names_delegate = CompleteDelegate(self, '&', 'all_custom', True)
+            view.cc_names_delegate = CompleteDelegate(view, '&', 'all_custom', True)
-        self.series_delegate = SeriesDelegate(self)
+            view.series_delegate = SeriesDelegate(view)
-        self.publisher_delegate = TextDelegate(self)
+            view.publisher_delegate = TextDelegate(view)
-        self.text_delegate = TextDelegate(self)
+            view.text_delegate = TextDelegate(view)
-        self.cc_text_delegate = CcTextDelegate(self)
+            view.cc_text_delegate = CcTextDelegate(view)
-        self.cc_series_delegate = CcSeriesDelegate(self)
+            view.cc_series_delegate = CcSeriesDelegate(view)
-        self.cc_longtext_delegate = CcLongTextDelegate(self)
+            view.cc_longtext_delegate = CcLongTextDelegate(view)
-        self.cc_markdown_delegate = CcMarkdownDelegate(self)
+            view.cc_markdown_delegate = CcMarkdownDelegate(view)
-        self.cc_enum_delegate = CcEnumDelegate(self)
+            view.cc_enum_delegate = CcEnumDelegate(view)
-        self.cc_bool_delegate = CcBoolDelegate(self)
+            view.cc_bool_delegate = CcBoolDelegate(view)
-        self.cc_comments_delegate = CcCommentsDelegate(self)
+            view.cc_comments_delegate = CcCommentsDelegate(view)
-        self.cc_template_delegate = CcTemplateDelegate(self)
+            view.cc_template_delegate = CcTemplateDelegate(view)
-        self.cc_number_delegate = CcNumberDelegate(self)
+            view.cc_number_delegate = CcNumberDelegate(view)
+
+        create_delegates(self), create_delegates(self.pin_view)
         self.display_parent = parent
         self._model = modelcls(self)
         self.setModel(self._model)
@@ -1124,11 +1125,6 @@ class BooksView(QTableView):  # {{{
         self.alternate_views.set_database(db)
         self.save_state()
         self._model.set_database(db)
-        self.tags_delegate.set_database(db)
-        self.cc_names_delegate.set_database(db)
-        self.authors_delegate.set_database(db)
-        self.series_delegate.set_auto_complete_function(db.all_series)
-        self.publisher_delegate.set_auto_complete_function(db.all_publishers)
         self.alternate_views.set_database(db, stage=1)
 
     def marked_changed(self, old_marked, current_marked):
@@ -1172,55 +1168,56 @@ class BooksView(QTableView):  # {{{
                         self.last_modified_delegate, self.languages_delegate, self.half_rating_delegate):
                     vw.setItemDelegateForColumn(i, vw.itemDelegate())
 
-        cm = self.column_map
+        def set_delegates(view):
+            cm = view.column_map
 
-        def set_item_delegate(colhead, delegate):
+            def set_item_delegate(colhead, delegate):
-            idx = cm.index(colhead)
+                idx = view.column_map.index(colhead)
-            self.setItemDelegateForColumn(idx, delegate)
+                view.setItemDelegateForColumn(idx, delegate)
-            self.pin_view.setItemDelegateForColumn(idx, delegate)
 
-        for colhead in cm:
+            for colhead in cm:
-            if self._model.is_custom_column(colhead):
+                if self._model.is_custom_column(colhead):
-                cc = self._model.custom_columns[colhead]
+                    cc = self._model.custom_columns[colhead]
-                if cc['datatype'] == 'datetime':
+                    if cc['datatype'] == 'datetime':
-                    delegate = CcDateDelegate(self)
+                        delegate = CcDateDelegate(view)
-                    delegate.set_format(cc['display'].get('date_format',''))
+                        delegate.set_format(cc['display'].get('date_format',''))
-                    set_item_delegate(colhead, delegate)
+                        set_item_delegate(colhead, delegate)
-                elif cc['datatype'] == 'comments':
+                    elif cc['datatype'] == 'comments':
-                    ctype = cc['display'].get('interpret_as', 'html')
+                        ctype = cc['display'].get('interpret_as', 'html')
-                    if ctype == 'short-text':
+                        if ctype == 'short-text':
-                        set_item_delegate(colhead, self.cc_text_delegate)
+                            set_item_delegate(colhead, view.cc_text_delegate)
-                    elif ctype == 'long-text':
+                        elif ctype == 'long-text':
-                        set_item_delegate(colhead, self.cc_longtext_delegate)
+                            set_item_delegate(colhead, view.cc_longtext_delegate)
-                    elif ctype == 'markdown':
+                        elif ctype == 'markdown':
-                        set_item_delegate(colhead, self.cc_markdown_delegate)
+                            set_item_delegate(colhead, view.cc_markdown_delegate)
-                    else:
-                        set_item_delegate(colhead, self.cc_comments_delegate)
-                elif cc['datatype'] == 'text':
-                    if cc['is_multiple']:
-                        if cc['display'].get('is_names', False):
-                            set_item_delegate(colhead, self.cc_names_delegate)
                         else:
-                            set_item_delegate(colhead, self.tags_delegate)
+                            set_item_delegate(colhead, view.cc_comments_delegate)
-                    else:
+                    elif cc['datatype'] == 'text':
-                        set_item_delegate(colhead, self.cc_text_delegate)
+                        if cc['is_multiple']:
-                elif cc['datatype'] == 'series':
+                            if cc['display'].get('is_names', False):
-                    set_item_delegate(colhead, self.cc_series_delegate)
+                                set_item_delegate(colhead, view.cc_names_delegate)
-                elif cc['datatype'] in ('int', 'float'):
+                            else:
-                    set_item_delegate(colhead, self.cc_number_delegate)
+                                set_item_delegate(colhead, view.tags_delegate)
-                elif cc['datatype'] == 'bool':
+                        else:
-                    set_item_delegate(colhead, self.cc_bool_delegate)
+                            set_item_delegate(colhead, view.cc_text_delegate)
-                elif cc['datatype'] == 'rating':
+                    elif cc['datatype'] == 'series':
-                    d = self.half_rating_delegate if cc['display'].get('allow_half_stars', False) else self.rating_delegate
+                        set_item_delegate(colhead, view.cc_series_delegate)
-                    set_item_delegate(colhead, d)
+                    elif cc['datatype'] in ('int', 'float'):
-                elif cc['datatype'] == 'composite':
+                        set_item_delegate(colhead, view.cc_number_delegate)
-                    set_item_delegate(colhead, self.cc_template_delegate)
+                    elif cc['datatype'] == 'bool':
-                elif cc['datatype'] == 'enumeration':
+                        set_item_delegate(colhead, view.cc_bool_delegate)
-                    set_item_delegate(colhead, self.cc_enum_delegate)
+                    elif cc['datatype'] == 'rating':
-            else:
+                        d = view.half_rating_delegate if cc['display'].get('allow_half_stars', False) else view.rating_delegate
-                dattr = colhead+'_delegate'
+                        set_item_delegate(colhead, d)
-                delegate = colhead if hasattr(self, dattr) else 'text'
+                    elif cc['datatype'] == 'composite':
-                set_item_delegate(colhead, getattr(self, delegate+'_delegate'))
+                        set_item_delegate(colhead, view.cc_template_delegate)
+                    elif cc['datatype'] == 'enumeration':
+                        set_item_delegate(colhead, view.cc_enum_delegate)
+                else:
+                    dattr = colhead+'_delegate'
+                    delegate = colhead if hasattr(view, dattr) else 'text'
+                    set_item_delegate(colhead, getattr(view, delegate+'_delegate'))
+        set_delegates(self), set_delegates(self.pin_view)
 
         self.restore_state()
         self.set_ondevice_column_visibility()
@@ -1637,77 +1634,9 @@ class BooksView(QTableView):  # {{{
     def close(self):
         self._model.close()
 
-    def is_index_editable_with_tab(self, index) -> bool:
-        if not index.isValid():
-            return False
-        col = self.column_map[index.column()]
-        m = self.model()
-        if m.is_custom_column(col):
-            # Don't try to open editors implemented by dialogs such as
-            # markdown, composites and comments
-            return self.itemDelegateForIndex(index).is_editable_with_tab
-        return bool(m.flags(index) & Qt.ItemFlag.ItemIsEditable)
-
-    def closeEditor(self, editor, hint):
-        # We want to implement our own go to next/previous cell behavior
-        orig = self.currentIndex()
-        do_move = False
-        delta = 1
-        if hint is QAbstractItemDelegate.EndEditHint.EditNextItem:
-            do_move = True
-        elif hint is QAbstractItemDelegate.EndEditHint.EditPreviousItem:
-            do_move = True
-            delta = -1
-        super().closeEditor(editor, QAbstractItemDelegate.EndEditHint.NoHint if do_move else hint)
-        if do_move:
-            # Need to invoke after event loop tick otherwise
-            # mirror_selection_between_views causes issues
-            QTimer.singleShot(0, lambda: self.edit_next_cell(orig, delta))
-
     def on_current_row_change(self, current, previous):
         self._model.current_changed(current, previous)
 
-    def edit(self, index, trigger=QAbstractItemView.EditTrigger.AllEditTriggers, event=None):
-        edited = super().edit(index, trigger, event)
-        return edited
-
-    def edit_next_cell(self, current, delta=1):
-        m = self.model()
-        row = current.row()
-        idx = m.index(row, current.column(), current.parent())
-        while True:
-            col = idx.column() + delta
-            if col < 0:
-                if row <= 0:
-                    return
-                row -= 1
-                col += len(self.column_map)
-            if col >= len(self.column_map):
-                if row >= len(self.column_map) - 1:
-                    return
-                row += 1
-                col -= len(self.column_map)
-            if col < 0 or col >= len(self.column_map):
-                return
-            colname = self.column_map[col]
-            idx = m.index(row, col, current.parent())
-            if m.is_custom_column(colname):
-                if self.itemDelegateForIndex(idx).is_editable_with_tab:
-                    # Don't try to open editors implemented by dialogs such as
-                    # markdown, composites and comments
-                    break
-            elif m.flags(idx) & Qt.ItemFlag.ItemIsEditable:
-                break
-
-        if idx.isValid():
-            # Tell the delegate to ignore keyboard modifiers in case
-            # Shift-Tab is being used to move the cell.
-            d = self.itemDelegateForIndex(idx)
-            if d is not None:
-                d.ignore_kb_mods_on_edit = True
-            self.setCurrentIndex(idx)
-            self.edit(idx)
-
     def set_editable(self, editable, supports_backloading):
         self._model.set_editable(editable)
 
@@ -1853,4 +1782,7 @@ class DeviceBooksView(BooksView):  # {{{
         h.setSortIndicator(
             h.sortIndicatorSection(), Qt.SortOrder.AscendingOrder if h.sortIndicatorOrder() == Qt.SortOrder.DescendingOrder else Qt.SortOrder.DescendingOrder)
 
+    def closeEditor(self, editor, hint):
+        return super().closeEditor(editor, hint)
+
 # }}}

src/calibre/gui2/pin_columns.py

@@ -2,25 +2,72 @@
 # License: GPLv3 Copyright: 2018, Kovid Goyal <kovid at kovidgoyal.net>
 
 
-from qt.core import QAbstractItemView, QSplitter, QTableView
+from qt.core import QAbstractItemDelegate, QSplitter, Qt, QTableView
 
 from calibre.gui2 import gprefs
 from calibre.gui2.library import DEFAULT_SORT
 
 
-class PinTableView(QTableView):
+class CustomEditTabbingBehavior:
+
+    def closeEditor(self, editor, hint):
+        # We want to implement our own go to next/previous cell behavior
+        orig = self.currentIndex()
+        delta = 0
+        if hint is QAbstractItemDelegate.EndEditHint.EditNextItem:
+            delta = 1
+        elif hint is QAbstractItemDelegate.EndEditHint.EditPreviousItem:
+            delta = -1
+        QTableView.closeEditor(self, editor, QAbstractItemDelegate.EndEditHint.NoHint if delta else hint)
+        if not delta:
+            return
+        current = self.currentIndex()
+        m = self.model()
+        row = current.row()
+        idx = m.index(row, current.column(), current.parent())
+        while True:
+            col = idx.column() + delta
+            if col < 0:
+                if row <= 0:
+                    return
+                row -= 1
+                col += len(self.column_map)
+            if col >= len(self.column_map):
+                if row >= len(self.column_map) - 1:
+                    return
+                row += 1
+                col -= len(self.column_map)
+            if col < 0 or col >= len(self.column_map):
+                return
+            colname = self.column_map[col]
+            idx = m.index(row, col, current.parent())
+            if m.is_custom_column(colname):
+                if self.itemDelegateForIndex(idx).is_editable_with_tab:
+                    # Don't try to open editors implemented by dialogs such as
+                    # markdown, composites and comments
+                    break
+            elif m.flags(idx) & Qt.ItemFlag.ItemIsEditable:
+                break
+
+        if idx.isValid():
+            # Tell the delegate to ignore keyboard modifiers in case
+            # Shift-Tab is being used to move the cell.
+            d = self.itemDelegateForIndex(idx)
+            if d is not None:
+                d.ignore_kb_mods_on_edit = True
+            self.setCurrentIndex(idx)
+            self.edit(idx)
+
+
+class PinTableView(QTableView, CustomEditTabbingBehavior):
+
+    disable_save_state = False
 
     def __init__(self, books_view, parent=None):
         QTableView.__init__(self, parent)
         self.books_view = books_view
         self.verticalHeader().close()
         self.splitter = None
-        self.disable_save_state = False
-
-    def edit(self, index, trigger=QAbstractItemView.EditTrigger.AllEditTriggers, event=None):
-        if not self.isVisible():
-            return False
-        return super().edit(index, trigger, event)
 
     @property
     def column_map(self):