From 42f6cfb38a68bc6633f61093d8a6d4e52fa0df1f Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Mon, 25 Mar 2019 09:35:48 +0100 Subject: [PATCH] Use tuples of integers and heapq instead strings/sort/list/dict for menu ordering - old code was a bit tricky, concatenating special prefix to name to order - use a heapq so entries are sorted on insert - rename few local variables to make them more explicit - use "constants" ORDER_BEFORE and ORDER_AFTER instead of "0", "?" - review behavior with countries/formats & preferences, making the code easier to understand - simplify "grouping": we add a separator after each group --- picard/ui/itemviews.py | 49 +++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/picard/ui/itemviews.py b/picard/ui/itemviews.py index 39602d9ed..722b74678 100644 --- a/picard/ui/itemviews.py +++ b/picard/ui/itemviews.py @@ -18,6 +18,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. from functools import partial +from heapq import heappush, heappop import os import re @@ -356,33 +357,41 @@ class BaseTreeView(QtWidgets.QTreeWidget): versions = obj.release_group.versions - albumtracks = obj.get_num_total_files() if obj.get_num_total_files() else len(obj.tracks) + album_tracks_count = obj.get_num_total_files() or len(obj.tracks) preferred_countries = set(config.setting["preferred_release_countries"]) preferred_formats = set(config.setting["preferred_release_formats"]) - matches = ("trackmatch", "countrymatch", "formatmatch") - priorities = {} - for version in versions: - priority = { - "trackmatch": "0" if version['totaltracks'] == albumtracks else "?", - "countrymatch": "0" if len(preferred_countries) == 0 or preferred_countries & set(version['countries'] or '') else "?", - "formatmatch": "0" if len(preferred_formats) == 0 or preferred_formats & set(version['formats'] or '') else "?", - } - priorities[version['id']] = "".join(priority[k] for k in matches) - versions.sort(key=lambda version: priorities[version['id']] + version['name']) + ORDER_BEFORE, ORDER_AFTER = 0, 1 - priority = normal = False + alternatives = [] for version in versions: - if not normal and "?" in priorities[version['id']]: - if priority: + trackmatch = countrymatch = formatmatch = ORDER_BEFORE + if version['totaltracks'] != album_tracks_count: + trackmatch = ORDER_AFTER + if preferred_countries: + countries = set(version['countries']) + if not countries or not countries.intersection(preferred_countries): + countrymatch = ORDER_AFTER + if preferred_formats: + formats = set(version['formats']) + if not formats or not formats.intersection(preferred_formats): + formatmatch = ORDER_AFTER + group = (trackmatch, countrymatch, formatmatch) + # order by group, name, and id on push + heappush(alternatives, (group, version['name'], version['id'])) + + prev_group = None + while alternatives: + group, action_text, release_id = heappop(alternatives) + if group != prev_group: + if prev_group is not None: releases_menu.addSeparator() - normal = True - else: - priority = True - action = releases_menu.addAction(version["name"]) + prev_group = group + action = releases_menu.addAction(action_text) action.setCheckable(True) - if obj.id == version["id"]: + if obj.id == release_id: action.setChecked(True) - action.triggered.connect(partial(obj.switch_release_version, version["id"])) + action.triggered.connect(partial(obj.switch_release_version, release_id)) + versions_count = len(versions) if versions_count > 1: releases_menu.setTitle(_("&Other versions (%d)") % versions_count)