From 764683da58624a53420bbf64c37ba483551a934e Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Mon, 16 Feb 2015 10:31:21 +0100 Subject: [PATCH 1/2] Fix bug #1422116: different characters that sort intermixed break tag browser first letter categorization. Also make rating and count category sorts order the list by the tag's sort value if the numeric values are the same. This turns it into a semi-stable sort when switching from name to (for example) count. --- src/calibre/db/cache.py | 6 ++++-- src/calibre/db/categories.py | 23 +++++++++++++---------- src/calibre/gui2/tag_browser/model.py | 11 +++++++---- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/src/calibre/db/cache.py b/src/calibre/db/cache.py index 92e6512b90..bffb792bfe 100644 --- a/src/calibre/db/cache.py +++ b/src/calibre/db/cache.py @@ -941,11 +941,13 @@ class Cache(object): return self._search_api(self, query, restriction, virtual_fields=virtual_fields, book_ids=book_ids) @api - def get_categories(self, sort='name', book_ids=None, icon_map=None, already_fixed=None): + def get_categories(self, sort='name', book_ids=None, icon_map=None, already_fixed=None, + first_letter_sort=False): ' Used internally to implement the Tag Browser ' try: with self.safe_read_lock: - return get_categories(self, sort=sort, book_ids=book_ids, icon_map=icon_map) + return get_categories(self, sort=sort, book_ids=book_ids, icon_map=icon_map, + first_letter_sort=first_letter_sort) except InvalidLinkTable as err: bad_field = err.field_name if bad_field == already_fixed: diff --git a/src/calibre/db/categories.py b/src/calibre/db/categories.py index 2e445f53f2..45c9e5711f 100644 --- a/src/calibre/db/categories.py +++ b/src/calibre/db/categories.py @@ -15,7 +15,7 @@ from future_builtins import map from calibre.ebooks.metadata import author_to_author_sort from calibre.library.field_metadata import TagsIcons from calibre.utils.config_base import tweaks -from calibre.utils.icu import sort_key +from calibre.utils.icu import sort_key, collation_order CATEGORY_SORTS = ('name', 'popularity', 'rating') # This has to be a tuple not a set @@ -116,19 +116,22 @@ def clean_user_categories(dbcache): pass return new_cats -def sort_categories(items, sort): - reverse = True +def sort_categories(items, sort, first_letter_sort=False): if sort == 'popularity': - key=attrgetter('count') + key=lambda x:(-getattr(x, 'count', 0), sort_key(x.sort or x.name)) elif sort == 'rating': - key=attrgetter('avg_rating') + key=lambda x:(-getattr(x, 'avg_rating', 0.0), sort_key(x.sort or x.name)) else: - key=lambda x:sort_key(x.sort or x.name) - reverse=False - items.sort(key=key, reverse=reverse) + if first_letter_sort: + key=lambda x:(collation_order(icu_upper(x.sort or x.name or ' ')), + sort_key(x.sort or x.name)) + else: + key=lambda x:sort_key(x.sort or x.name) + items.sort(key=key) return items -def get_categories(dbcache, sort='name', book_ids=None, icon_map=None): +def get_categories(dbcache, sort='name', book_ids=None, icon_map=None, + first_letter_sort=False): if icon_map is not None and type(icon_map) != TagsIcons: raise TypeError('icon_map passed to get_categories must be of type TagIcons') if sort not in CATEGORY_SORTS: @@ -170,7 +173,7 @@ def get_categories(dbcache, sort='name', book_ids=None, icon_map=None): cat['is_multiple'] and cat['display'].get('is_names', False)): for item in cats: item.sort = author_to_author_sort(item.sort) - sort_categories(cats, sort) + sort_categories(cats, sort, first_letter_sort=first_letter_sort) categories[category] = cats # Needed for legacy databases that have multiple ratings that diff --git a/src/calibre/gui2/tag_browser/model.py b/src/calibre/gui2/tag_browser/model.py index edcd7eac4d..57f9aca620 100644 --- a/src/calibre/gui2/tag_browser/model.py +++ b/src/calibre/gui2/tag_browser/model.py @@ -865,16 +865,19 @@ class TagsModel(QAbstractItemModel): # {{{ # Get the categories if self.db.data.get_base_restriction() or self.db.data.get_search_restriction(): try: - data = self.db.get_categories(sort=sort, + data = self.db.new_api.get_categories(sort=sort, icon_map=self.category_icon_map, - ids=self.db.search('', return_matches=True, sort_results=False)) + book_ids=self.db.search('', return_matches=True, sort_results=False), + first_letter_sort = self.collapse_model == 'first letter') except: import traceback traceback.print_exc() - data = self.db.get_categories(sort=sort, icon_map=self.category_icon_map) + data = self.db.new_api.get_categories(sort=sort, icon_map=self.category_icon_map, + first_letter_sort = self.collapse_model == 'first letter') self.restriction_error.emit() else: - data = self.db.get_categories(sort=sort, icon_map=self.category_icon_map) + data = self.db.new_api.get_categories(sort=sort, icon_map=self.category_icon_map, + first_letter_sort = self.collapse_model == 'first letter') # Reconstruct the user categories, putting them into metadata self.db.field_metadata.remove_dynamic_categories() From af49a1ca4a2dc932e41507e35baf00de4a9a5b91 Mon Sep 17 00:00:00 2001 From: Charles Haley Date: Mon, 16 Feb 2015 10:33:57 +0100 Subject: [PATCH 2/2] Fix problem applying a search restriction if the VL menu has never been opened. See http://www.mobileread.com/forums/showthread.php?p=3047621#post3047621 --- src/calibre/gui2/search_restriction_mixin.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/calibre/gui2/search_restriction_mixin.py b/src/calibre/gui2/search_restriction_mixin.py index eb3af9f7ec..4273d7c6d7 100644 --- a/src/calibre/gui2/search_restriction_mixin.py +++ b/src/calibre/gui2/search_restriction_mixin.py @@ -341,6 +341,7 @@ class SearchRestrictionMixin(object): self.ar_menu = QMenu(_('Additional restriction')) self.edit_menu = QMenu(_('Edit Virtual Library')) self.rm_menu = QMenu(_('Remove Virtual Library')) + self.search_restriction_list_built = False def add_virtual_library(self, db, name, search): virt_libs = db.prefs.get('virtual_libraries', {}) @@ -498,6 +499,7 @@ class SearchRestrictionMixin(object): return name[0:MAX_VIRTUAL_LIBRARY_NAME_LENGTH].strip() def build_search_restriction_list(self): + self.search_restriction_list_built = True from calibre.gui2.ui import get_gui m = self.ar_menu m.clear() @@ -539,6 +541,8 @@ class SearchRestrictionMixin(object): self.apply_search_restriction(index) def apply_named_search_restriction(self, name): + if not self.search_restriction_list_built: + self.build_search_restriction_list() if not name: r = 0 else: @@ -549,6 +553,8 @@ class SearchRestrictionMixin(object): self.apply_search_restriction(r) def apply_text_search_restriction(self, search): + if not self.search_restriction_list_built: + self.build_search_restriction_list() search = unicode(search) if not search: self.search_restriction.setCurrentIndex(0) @@ -567,6 +573,8 @@ class SearchRestrictionMixin(object): self._apply_search_restriction(search, self._trim_restriction_name(s)) def apply_search_restriction(self, i): + if not self.search_restriction_list_built: + self.build_search_restriction_list() if i == 1: self.apply_text_search_restriction(unicode(self.search.currentText())) elif i == 2 and unicode(self.search_restriction.currentText()).startswith('*'):