From 26b5f56f046e862e8d10b3f47a67c04441355af8 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 21 Mar 2019 21:12:25 +0100 Subject: [PATCH 1/3] Sort settings --- picard/ui/options/renaming.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/picard/ui/options/renaming.py b/picard/ui/options/renaming.py index 35fed7d3d..e400b7fc7 100644 --- a/picard/ui/options/renaming.py +++ b/picard/ui/options/renaming.py @@ -139,14 +139,14 @@ class RenamingOptionsPage(OptionsPage): def _example_to_filename(self, file): settings = { - 'windows_compatibility': self.ui.windows_compatibility.isChecked(), 'ascii_filenames': self.ui.ascii_filenames.isChecked(), - 'rename_files': self.ui.rename_files.isChecked(), - 'move_files': self.ui.move_files.isChecked(), - 'use_va_format': False, # TODO remove - 'file_naming_format': self.ui.file_naming_format.toPlainText(), - 'move_files_to': os.path.normpath(self.ui.move_files_to.text()), 'clear_existing_tags': config.setting['clear_existing_tags'], + 'file_naming_format': self.ui.file_naming_format.toPlainText(), + 'move_files': self.ui.move_files.isChecked(), + 'move_files_to': os.path.normpath(self.ui.move_files_to.text()), + 'rename_files': self.ui.rename_files.isChecked(), + 'use_va_format': False, # TODO remove + 'windows_compatibility': self.ui.windows_compatibility.isChecked(), } try: if config.setting["enable_tagger_scripts"]: From 25ceef01e4ce55f5eea6af43667f73268da1b522 Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 21 Mar 2019 21:13:04 +0100 Subject: [PATCH 2/3] Remove unused (and obsolete) use_va_format from settings --- picard/ui/options/renaming.py | 1 - 1 file changed, 1 deletion(-) diff --git a/picard/ui/options/renaming.py b/picard/ui/options/renaming.py index e400b7fc7..1a1ce30f3 100644 --- a/picard/ui/options/renaming.py +++ b/picard/ui/options/renaming.py @@ -145,7 +145,6 @@ class RenamingOptionsPage(OptionsPage): 'move_files': self.ui.move_files.isChecked(), 'move_files_to': os.path.normpath(self.ui.move_files_to.text()), 'rename_files': self.ui.rename_files.isChecked(), - 'use_va_format': False, # TODO remove 'windows_compatibility': self.ui.windows_compatibility.isChecked(), } try: From bc11a330abf81d94f4cb29abcf6130f1543ddade Mon Sep 17 00:00:00 2001 From: Laurent Monin Date: Thu, 21 Mar 2019 21:47:44 +0100 Subject: [PATCH 3/3] Introduce SettingsOverride class to fix this more elegantly - basically it returns config.setting[key] when key isn't in the internal dict - it protects config.setting from any write, since __setitem__ only write to self._dict - del settings['xxx'] will remove 'xxx' from internal dict, but still allow access to config.setting - it can be used for tests - minimal implementation based on MutableMapping - add tests for it --- picard/ui/options/renaming.py | 7 ++-- picard/util/settingsoverride.py | 63 +++++++++++++++++++++++++++++++++ test/test_settingsoverride.py | 51 ++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 picard/util/settingsoverride.py create mode 100644 test/test_settingsoverride.py diff --git a/picard/ui/options/renaming.py b/picard/ui/options/renaming.py index 1a1ce30f3..42b261ba8 100644 --- a/picard/ui/options/renaming.py +++ b/picard/ui/options/renaming.py @@ -33,6 +33,7 @@ from picard.script import ( ScriptError, ScriptParser, ) +from picard.util.settingsoverride import SettingsOverride from picard.ui.options import ( OptionsCheckError, @@ -138,15 +139,15 @@ class RenamingOptionsPage(OptionsPage): self.update_examples() def _example_to_filename(self, file): - settings = { + settings = SettingsOverride(config.setting, { 'ascii_filenames': self.ui.ascii_filenames.isChecked(), - 'clear_existing_tags': config.setting['clear_existing_tags'], 'file_naming_format': self.ui.file_naming_format.toPlainText(), 'move_files': self.ui.move_files.isChecked(), 'move_files_to': os.path.normpath(self.ui.move_files_to.text()), 'rename_files': self.ui.rename_files.isChecked(), 'windows_compatibility': self.ui.windows_compatibility.isChecked(), - } + }) + try: if config.setting["enable_tagger_scripts"]: for s_pos, s_name, s_enabled, s_text in config.setting["list_of_scripts"]: diff --git a/picard/util/settingsoverride.py b/picard/util/settingsoverride.py new file mode 100644 index 000000000..3ce7098f0 --- /dev/null +++ b/picard/util/settingsoverride.py @@ -0,0 +1,63 @@ +# -*- coding: utf-8 -*- +# +# Picard, the next-generation MusicBrainz tagger +# Copyright (C) 2019 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 collections.abc import MutableMapping + + +class SettingsOverride(MutableMapping): + """ This class can be used to override config temporarly + Basically it returns config[key] if key isn't found in internal dict + + Typical usage: + + settings = SettingsOverride(config.setting) + settings["option"] = "value" + """ + + def __init__(self, orig_settings, *args, **kwargs): + self.orig_settings = orig_settings + self._dict = dict() + for k, v in dict(*args, **kwargs).items(): + self[k] = v + + def __getitem__(self, key): + try: + return self._dict[key] + except KeyError: + return self.orig_settings[key] + + def __setitem__(self, key, value): + self._dict[key] = value + + def __delitem__(self, key): + try: + del self._dict[key] + except KeyError: + pass + + def __len__(self): + return len(self._dict) + + def __iter__(self): + return iter(self._dict) + + def __repr__(self): + d = self.orig_settings.copy() + d.update(self._dict) + return repr(d) diff --git a/test/test_settingsoverride.py b/test/test_settingsoverride.py new file mode 100644 index 000000000..81f87b832 --- /dev/null +++ b/test/test_settingsoverride.py @@ -0,0 +1,51 @@ +from test.picardtestcase import PicardTestCase + +from picard import config +from picard.util.settingsoverride import SettingsOverride + + +class SettingsOverrideTest(PicardTestCase): + + def setUp(self): + self.config = {'key1': 'origval1', 'key2': 'origval2'} + config.setting = self.config.copy() + self.new_settings = {'key1': 'newval2'} + + def test_read_orig_settings(self): + override = SettingsOverride(config.setting, self.new_settings) + self.assertEqual(override['key1'], 'newval2') + self.assertEqual(override['key2'], 'origval2') + with self.assertRaises(KeyError): + x = override['key3'] + + def test_read_orig_settings_kw(self): + override = SettingsOverride(config.setting, key1='newval2') + self.assertEqual(override['key1'], 'newval2') + self.assertEqual(override['key2'], 'origval2') + + def test_write_orig_settings(self): + override = SettingsOverride(config.setting, self.new_settings) + override['key1'] = 'newval3' + self.assertEqual(override['key1'], 'newval3') + self.assertEqual(config.setting['key1'], 'origval1') + + override['key2'] = 'newval4' + self.assertEqual(override['key2'], 'newval4') + self.assertEqual(config.setting['key2'], 'origval2') + + override['key3'] = 'newval5' + self.assertEqual(override['key3'], 'newval5') + with self.assertRaises(KeyError): + x = config.setting['key3'] + + def test_del_orig_settings(self): + override = SettingsOverride(config.setting, self.new_settings) + + override['key1'] = 'newval3' + self.assertEqual(override['key1'], 'newval3') + del override['key1'] + self.assertEqual(override['key1'], 'origval1') + + self.assertEqual(override['key2'], 'origval2') + del override['key2'] + self.assertEqual(override['key2'], 'origval2')