From bb57f024861398cfef913017ad153cd184202f09 Mon Sep 17 00:00:00 2001 From: Bob Swift Date: Fri, 12 Nov 2021 11:06:59 -0700 Subject: [PATCH] Add functions for creating unique numbered script/profile titles: - Add utility function unique_numbered_title() -- submitted by zas - Add utility function to extract base title to allow creating unique numbered title for titles that have been copied. --- picard/config_upgrade.py | 5 +- picard/const/__init__.py | 3 +- picard/ui/options/profiles.py | 7 ++- picard/ui/scripteditor.py | 20 ++++++- picard/ui/widgets/profilelistwidget.py | 15 +++-- picard/ui/widgets/scriptlistwidget.py | 17 +++--- picard/util/__init__.py | 40 ++++++++++++- test/test_config_upgrade.py | 5 +- test/test_util_get_base_title.py | 80 ++++++++++++++++++++++++++ test/test_util_uniqnum_title.py | 62 ++++++++++++++++++++ 10 files changed, 228 insertions(+), 26 deletions(-) create mode 100644 test/test_util_get_base_title.py create mode 100644 test/test_util_uniqnum_title.py diff --git a/picard/config_upgrade.py b/picard/config_upgrade.py index 550fc0b19..35347684f 100644 --- a/picard/config_upgrade.py +++ b/picard/config_upgrade.py @@ -41,9 +41,10 @@ from picard.config import ( ) from picard.const import ( DEFAULT_FILE_NAMING_FORMAT, - DEFAULT_NUMBERED_SCRIPT_NAME, + DEFAULT_SCRIPT_NAME, ) from picard.const.sys import IS_FROZEN +from picard.util import unique_numbered_title # TO ADD AN UPGRADE HOOK: @@ -228,7 +229,7 @@ def upgrade_to_v1_4_0_dev_6(config): if old_script_text_option in _s: old_script_text = _s.value(old_script_text_option, TextOption, "") if old_script_text: - old_script = (0, _(DEFAULT_NUMBERED_SCRIPT_NAME) % 1, _s["enable_tagger_scripts"], old_script_text) + old_script = (0, unique_numbered_title(_(DEFAULT_SCRIPT_NAME), list_of_scripts), _s["enable_tagger_scripts"], old_script_text) list_of_scripts.append(old_script) _s["list_of_scripts"] = list_of_scripts _s.remove(old_enabled_option) diff --git a/picard/const/__init__.py b/picard/const/__init__.py index 14198d6a5..d6b9287bf 100644 --- a/picard/const/__init__.py +++ b/picard/const/__init__.py @@ -186,10 +186,9 @@ DEFAULT_FILE_NAMING_FORMAT = "$if2(%albumartist%,%artist%)/\n" \ "%title%" -DEFAULT_NUMBERED_SCRIPT_NAME = N_("My script %d") DEFAULT_SCRIPT_NAME = N_("My script") DEFAULT_COVER_IMAGE_FILENAME = "cover" -DEFAULT_NUMBERED_PROFILE_NAME = N_("My profile %d") DEFAULT_PROFILE_NAME = N_("My profile") +DEFAULT_COPY_TEXT = N_("(copy)") SCRIPT_LANGUAGE_VERSION = '1.1' diff --git a/picard/ui/options/profiles.py b/picard/ui/options/profiles.py index d5eb76f1e..a5dffe1db 100644 --- a/picard/ui/options/profiles.py +++ b/picard/ui/options/profiles.py @@ -33,8 +33,10 @@ from picard.config import ( SettingConfigSection, get_config, ) +from picard.const import DEFAULT_COPY_TEXT from picard.profile import UserProfileGroups from picard.script import get_file_naming_script_presets +from picard.util import get_base_title from picard.ui.moveable_list_view import MoveableListView from picard.ui.options import ( @@ -371,7 +373,7 @@ class ProfilesOptionsPage(OptionsPage): QtWidgets.QMessageBox.Ok, self ).exec_() - item.setText(_("Unnamed profile")) + item.setText(self.ui.profile_list.unique_profile_name()) elif text != item.text(): # Remove leading and trailing spaces from new title. item.setText(text) @@ -420,7 +422,8 @@ class ProfilesOptionsPage(OptionsPage): id = str(uuid.uuid4()) settings = deepcopy(self.profile_settings[self.current_profile_id]) self.profile_settings[id] = settings - name = _("%s (copy)") % item.name + base_title = "%s %s" % (get_base_title(item.name), _(DEFAULT_COPY_TEXT)) + name = self.ui.profile_list.unique_profile_name(base_title) self.ui.profile_list.add_profile(name=name, profile_id=id) self.update_config_overrides() self.reload_all_page_settings() diff --git a/picard/ui/scripteditor.py b/picard/ui/scripteditor.py index 509bfa967..c605d4bcf 100644 --- a/picard/ui/scripteditor.py +++ b/picard/ui/scripteditor.py @@ -40,7 +40,9 @@ from picard.config import ( get_config, ) from picard.const import ( + DEFAULT_COPY_TEXT, DEFAULT_FILE_NAMING_FORMAT, + DEFAULT_SCRIPT_NAME, PICARD_URLS, ) from picard.file import File @@ -55,7 +57,9 @@ from picard.script.serializer import ( ScriptImportExportError, ) from picard.util import ( + get_base_title, icontheme, + unique_numbered_title, webbrowser2, ) from picard.util.settingsoverride import SettingsOverride @@ -739,11 +743,19 @@ class ScriptEditorDialog(PicardDialog, SingletonDialog): self.select_script(skip_check=True) self.restore_selected_script_index = idx + def new_script_name(self, base_title=None): + """Get new unique script name. + """ + default_title = base_title if base_title is not None else _(DEFAULT_SCRIPT_NAME) + existing_titles = set(script['title'] for script in self.naming_scripts.values()) + return unique_numbered_title(default_title, existing_titles) + def new_script(self): """Add a new (empty) script to the script selection combo box and script list. """ if self.unsaved_changes_confirmation(): - script_item = FileNamingScript(script=DEFAULT_FILE_NAMING_FORMAT).to_dict() + title = self.new_script_name() + script_item = FileNamingScript(title=title, script=DEFAULT_FILE_NAMING_FORMAT).to_dict() self._insert_item(script_item) def copy_script(self): @@ -752,8 +764,10 @@ class ScriptEditorDialog(PicardDialog, SingletonDialog): if self.unsaved_changes_confirmation(): selected = self.ui.preset_naming_scripts.currentIndex() script_item = self.ui.preset_naming_scripts.itemData(selected) - new_item = FileNamingScript.create_from_dict(script_dict=script_item).copy().to_dict() - self._insert_item(new_item) + new_item = FileNamingScript.create_from_dict(script_dict=script_item).copy() + base_title = "%s %s" % (get_base_title(script_item['title']), _(DEFAULT_COPY_TEXT)) + new_item.title = self.new_script_name(base_title) + self._insert_item(new_item.to_dict()) def update_script_in_settings(self): """Sends a save signal to trigger processing in the parent. diff --git a/picard/ui/widgets/profilelistwidget.py b/picard/ui/widgets/profilelistwidget.py index b42288023..c098869ee 100644 --- a/picard/ui/widgets/profilelistwidget.py +++ b/picard/ui/widgets/profilelistwidget.py @@ -28,10 +28,8 @@ from PyQt5 import ( QtWidgets, ) -from picard.const import ( - DEFAULT_NUMBERED_PROFILE_NAME, - DEFAULT_PROFILE_NAME, -) +from picard.const import DEFAULT_PROFILE_NAME +from picard.util import unique_numbered_title from picard.ui import HashableListWidgetItem @@ -58,10 +56,15 @@ class ProfileListWidget(QtWidgets.QListWidget): else: super().keyPressEvent(event) + def unique_profile_name(self, base_name=None): + if base_name is None: + base_name = _(DEFAULT_PROFILE_NAME) + existing_titles = [self.item(i).name for i in range(self.count())] + return unique_numbered_title(base_name, existing_titles) + def add_profile(self, name=None, profile_id=""): if name is None: - count = self.count() - name = _(DEFAULT_NUMBERED_PROFILE_NAME) % (count + 1) + name = self.unique_profile_name() list_item = ProfileListWidgetItem(name=name, profile_id=profile_id) list_item.setCheckState(QtCore.Qt.Checked) self.insertItem(0, list_item) diff --git a/picard/ui/widgets/scriptlistwidget.py b/picard/ui/widgets/scriptlistwidget.py index 2a3966dd8..7951bf199 100644 --- a/picard/ui/widgets/scriptlistwidget.py +++ b/picard/ui/widgets/scriptlistwidget.py @@ -29,10 +29,8 @@ from PyQt5 import ( QtWidgets, ) -from picard.const import ( - DEFAULT_NUMBERED_SCRIPT_NAME, - DEFAULT_SCRIPT_NAME, -) +from picard.const import DEFAULT_SCRIPT_NAME +from picard.util import unique_numbered_title from picard.ui import HashableListWidgetItem @@ -63,9 +61,12 @@ class ScriptListWidget(QtWidgets.QListWidget): else: super().keyPressEvent(event) + def unique_script_name(self): + existing_titles = [self.item(i).name for i in range(self.count())] + return unique_numbered_title(_(DEFAULT_SCRIPT_NAME), existing_titles) + def add_script(self): - count = self.count() - numbered_name = _(DEFAULT_NUMBERED_SCRIPT_NAME) % (count + 1) + numbered_name = self.unique_script_name() list_item = ScriptListWidgetItem(name=numbered_name) list_item.setCheckState(QtCore.Qt.Checked) self.addItem(list_item) @@ -88,8 +89,8 @@ class ScriptListWidget(QtWidgets.QListWidget): def item_changed(self, item): if not item.name.strip(): - # Replace empty script name with default. - item.setText(_(DEFAULT_SCRIPT_NAME)) + # Replace empty script name with unique numbered name. + item.setText(self.unique_script_name()) class ScriptListWidgetItem(HashableListWidgetItem): diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 73a1770db..8c6448b0b 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -59,7 +59,10 @@ from dateutil.parser import parse from PyQt5 import QtCore from picard import log -from picard.const import MUSICBRAINZ_SERVERS +from picard.const import ( + DEFAULT_COPY_TEXT, + MUSICBRAINZ_SERVERS, +) from picard.const.sys import ( FROZEN_TEMP_PATH, IS_FROZEN, @@ -891,3 +894,38 @@ def wildcards_to_regex_pattern(pattern): regex.append('\\[') regex.append(wildcards_to_regex_pattern(''.join(group[1:]))) return ''.join(regex) + + +def unique_numbered_title(default_title, existing_titles): + """Generate a new unique and numbered title + based on given default title and existing titles + """ + escaped = re.escape(default_title) + regex = re.compile('^' + escaped + '(?: \\((\\d+)\\))?$') + count = 0 + for title in existing_titles: + m = regex.match(title) + if m: + num = m.group(1) + if num is not None: + count = max(count, int(num)) + else: + count += 1 + return "{0} ({1})".format(default_title, count + 1) + + +def get_base_title_with_suffix(title, suffix): + """Extract the base portion of a title, + removing the suffix and number portion from the end. + """ + escaped_suffix = re.escape(suffix) + re_text = "^(.*)\\s+" + escaped_suffix + "(\\s+\\(\\d*\\))?$" + match_obj = re.fullmatch(re_text, title) + return match_obj.group(1) if match_obj else title + + +def get_base_title(title): + """Extract the base portion of a title, using the standard suffix. + """ + suffix = _(DEFAULT_COPY_TEXT) + return get_base_title_with_suffix(title, suffix) diff --git a/test/test_config_upgrade.py b/test/test_config_upgrade.py index 7a424578c..c8f7c9fb2 100644 --- a/test/test_config_upgrade.py +++ b/test/test_config_upgrade.py @@ -59,8 +59,9 @@ from picard.config_upgrade import ( ) from picard.const import ( DEFAULT_FILE_NAMING_FORMAT, - DEFAULT_NUMBERED_SCRIPT_NAME, + DEFAULT_SCRIPT_NAME, ) +from picard.util import unique_numbered_title class TestPicardConfigUpgrades(TestPicardConfigCommon): @@ -188,7 +189,7 @@ class TestPicardConfigUpgrades(TestPicardConfigCommon): self.assertNotIn('tagger_script', self.config.setting) self.assertTrue(self.config.setting['enable_tagger_scripts']) - self.assertEqual([(0, DEFAULT_NUMBERED_SCRIPT_NAME % 1, True, 'abc')], self.config.setting['list_of_scripts']) + self.assertEqual([(0, unique_numbered_title(DEFAULT_SCRIPT_NAME, []), True, 'abc')], self.config.setting['list_of_scripts']) def test_upgrade_to_v1_4_0_dev_7(self): BoolOption('setting', 'embed_only_one_front_image', False) diff --git a/test/test_util_get_base_title.py b/test/test_util_get_base_title.py new file mode 100644 index 000000000..bac05136b --- /dev/null +++ b/test/test_util_get_base_title.py @@ -0,0 +1,80 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# +# Copyright (C) 2021 Bob Swift +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + +from test.picardtestcase import PicardTestCase + +from picard.util import get_base_title_with_suffix + + +class GetBaseTitle(PicardTestCase): + def test_base_title_0(self): + # Test with no matching suffix + test_title = 'title' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, 'title') + + def test_base_title_1(self): + # Test with matching suffix but no number section + test_title = 'title (copy)' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, 'title') + + def test_base_title_2(self): + # Test with matching suffix and number + test_title = 'title (copy) (1)' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, 'title') + + def test_base_title_3(self): + # Test with missing space between suffix and number section + test_title = 'title (copy)(1)' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, test_title) + + def test_base_title_4(self): + # Test with missing space between suffix and number section (and missing number) + test_title = 'title (copy)()' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, test_title) + + def test_base_title_5(self): + # Test with missing number + test_title = 'title (copy) ()' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, 'title') + + def test_base_title_6(self): + # Test with invalid number + test_title = 'title (copy) (x)' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, test_title) + + def test_base_title_7(self): + # Test with extra character after number section + test_title = 'title (copy) (1)x' + title = get_base_title_with_suffix(test_title, '(copy)') + self.assertEqual(title, test_title) + + def test_base_title_8(self): + # Test escaping of suffix + test_title = 'title (copy) (1)' + title = get_base_title_with_suffix(test_title, '(c?py)') + self.assertEqual(title, test_title) diff --git a/test/test_util_uniqnum_title.py b/test/test_util_uniqnum_title.py new file mode 100644 index 000000000..c64f9b53d --- /dev/null +++ b/test/test_util_uniqnum_title.py @@ -0,0 +1,62 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# +# Copyright (C) 2021 Laurent Monin +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + +from test.picardtestcase import PicardTestCase + +from picard.util import unique_numbered_title + + +class UniqueNumberedTitle(PicardTestCase): + def test_existing_titles_0(self): + title = unique_numbered_title('title', []) + self.assertEqual(title, 'title (1)') + + def test_existing_titles_1(self): + title = unique_numbered_title('title', ['title']) + self.assertEqual(title, 'title (2)') + + def test_existing_titles_2(self): + title = unique_numbered_title('title', ['title', 'title (2)']) + self.assertEqual(title, 'title (3)') + + def test_existing_titles_3(self): + title = unique_numbered_title('title', ['title (1)', 'title (2)']) + self.assertEqual(title, 'title (3)') + + def test_existing_titles_4(self): + title = unique_numbered_title('title', ['title', 'title']) + self.assertEqual(title, 'title (3)') + + def test_existing_titles_5(self): + title = unique_numbered_title('title', ['x title', 'title y']) + self.assertEqual(title, 'title (1)') + + def test_existing_titles_6(self): + title = unique_numbered_title('title', ['title (n)']) + self.assertEqual(title, 'title (1)') + + def test_existing_titles_7(self): + title = unique_numbered_title('title', ['title ()']) + self.assertEqual(title, 'title (1)') + + def test_existing_titles_8(self): + title = unique_numbered_title('title', ['title(2)']) + self.assertEqual(title, 'title (1)')