From abbd69f6040ea3cd6fc7b9c00906da69e89f29fa Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 11 Oct 2017 07:13:51 +0200 Subject: [PATCH] Implement deleting/renaming values in configdata.yml This is needed for #3077, but also is used for the deletion in #2847 now. See #2772. --- qutebrowser/config/configdata.py | 36 +++++++++++++++++++--- qutebrowser/config/configdata.yml | 3 ++ qutebrowser/config/configfiles.py | 20 ++++++++++--- tests/unit/config/test_configdata.py | 43 +++++++++++++++++++++++++-- tests/unit/config/test_configfiles.py | 31 ++++++++++++++++++- tests/unit/config/test_configinit.py | 4 +++ 6 files changed, 126 insertions(+), 11 deletions(-) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index d58168ab3..ccce4c377 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -31,6 +31,7 @@ from qutebrowser.config import configtypes from qutebrowser.utils import usertypes, qtutils, utils DATA = None +MIGRATIONS = None @attr.s @@ -49,6 +50,15 @@ class Option: description = attr.ib() +@attr.s +class Migrations: + + """Nigrated options in configdata.yml.""" + + renamed = attr.ib(default=attr.Factory(dict)) + deleted = attr.ib(default=attr.Factory(list)) + + def _raise_invalid_node(name, what, node): """Raise an exception for an invalid configdata YAML node. @@ -172,14 +182,27 @@ def _read_yaml(yaml_data): yaml_data: The YAML string to parse. Return: - A dict mapping option names to Option elements. + A tuple with two elements: + - A dict mapping option names to Option elements. + - A Migrations object. """ parsed = {} + migrations = Migrations() data = utils.yaml_load(yaml_data) keys = {'type', 'default', 'desc', 'backend'} for name, option in data.items(): + if set(option.keys()) == {'renamed'}: + migrations.renamed[name] = option['renamed'] + continue + if set(option.keys()) == {'deleted'}: + value = option['deleted'] + if value is not True: + raise ValueError("Invalid deleted value: {}".format(value)) + migrations.deleted.append(name) + continue + if not set(option.keys()).issubset(keys): raise ValueError("Invalid keys {} for {}".format( option.keys(), name)) @@ -200,7 +223,12 @@ def _read_yaml(yaml_data): if key2.startswith(key1 + '.'): raise ValueError("Shadowing keys {} and {}".format(key1, key2)) - return parsed + # Make sure rename targets actually exist. + for old, new in migrations.renamed.items(): + if new not in parsed: + raise ValueError("Renaming {} to unknown {}".format(old, new)) + + return parsed, migrations @functools.lru_cache(maxsize=256) @@ -211,5 +239,5 @@ def is_valid_prefix(prefix): def init(): """Initialize configdata from the YAML file.""" - global DATA - DATA = _read_yaml(utils.read_file('config/configdata.yml')) + global DATA, MIGRATIONS + DATA, MIGRATIONS = _read_yaml(utils.read_file('config/configdata.yml')) diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index f500a2f0f..3f69d91b3 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -1252,6 +1252,9 @@ tabs.width.indicator: minval: 0 desc: Width of the progress indicator (0 to disable). +tabs.width.pinned: + deleted: true + tabs.wrap: default: true type: Bool diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 7be756b2d..6439f9688 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -33,7 +33,7 @@ from PyQt5.QtCore import pyqtSignal, QObject, QSettings import qutebrowser from qutebrowser.config import configexc, config, configdata -from qutebrowser.utils import standarddir, utils, qtutils +from qutebrowser.utils import standarddir, utils, qtutils, log # The StateConfig instance @@ -162,11 +162,23 @@ class YamlConfig(QObject): "'global' object is not a dict") raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) - # Delete unknown values - # (e.g. options which were removed from configdata.yml) + # Handle unknown/renamed keys for name in list(global_obj): - if name not in configdata.DATA: + if name in configdata.MIGRATIONS.renamed: + new_name = configdata.MIGRATIONS.renamed[name] + log.config.debug("Renaming {} to {}".format(name, new_name)) + global_obj[new_name] = global_obj[name] del global_obj[name] + elif name in configdata.MIGRATIONS.deleted: + log.config.debug("Removing {}".format(name)) + del global_obj[name] + elif name in configdata.DATA: + pass + else: + desc = configexc.ConfigErrorDesc( + "While loading options", + "Unknown option {}".format(name)) + raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) self._values = global_obj self._dirty = False diff --git a/tests/unit/config/test_configdata.py b/tests/unit/config/test_configdata.py index d4677e9b3..82c3fedc9 100644 --- a/tests/unit/config/test_configdata.py +++ b/tests/unit/config/test_configdata.py @@ -57,7 +57,7 @@ def test_is_valid_prefix(monkeypatch): class TestReadYaml: def test_valid(self): - data = textwrap.dedent(""" + yaml_data = textwrap.dedent(""" test1: type: Bool default: true @@ -69,7 +69,7 @@ class TestReadYaml: backend: QtWebKit desc: Hello World 2 """) - data = configdata._read_yaml(data) + data, _migrations = configdata._read_yaml(yaml_data) assert data.keys() == {'test1', 'test2'} assert data['test1'].description == "Hello World" assert data['test2'].default == "foo" @@ -113,6 +113,45 @@ class TestReadYaml: else: configdata._read_yaml(data) + def test_rename(self): + yaml_data = textwrap.dedent(""" + test: + renamed: test_new + + test_new: + type: Bool + default: true + desc: Hello World + """) + data, migrations = configdata._read_yaml(yaml_data) + assert data.keys() == {'test_new'} + assert migrations.renamed == {'test': 'test_new'} + + def test_rename_unknown_target(self): + yaml_data = textwrap.dedent(""" + test: + renamed: test2 + """) + with pytest.raises(ValueError, match='Renaming test to unknown test2'): + configdata._read_yaml(yaml_data) + + def test_delete(self): + yaml_data = textwrap.dedent(""" + test: + deleted: true + """) + data, migrations = configdata._read_yaml(yaml_data) + assert not data.keys() + assert migrations.deleted == ['test'] + + def test_delete_invalid_value(self): + yaml_data = textwrap.dedent(""" + test: + deleted: false + """) + with pytest.raises(ValueError, match='Invalid deleted value: False'): + configdata._read_yaml(yaml_data) + class TestParseYamlType: diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index 6339784e9..44b8cde95 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -119,16 +119,45 @@ class TestYaml: 'yaml-config', unittest.mock.ANY, unittest.mock.ANY) def test_unknown_key(self, yaml, config_tmpdir): - """An unknown setting should be deleted.""" + """An unknown setting should show an error.""" autoconfig = config_tmpdir / 'autoconfig.yml' autoconfig.write_text('global:\n hello: world', encoding='utf-8') + with pytest.raises(configexc.ConfigFileErrors) as excinfo: + yaml.load() + + assert len(excinfo.value.errors) == 1 + error = excinfo.value.errors[0] + assert error.text == "While loading options" + assert str(error.exception) == "Unknown option hello" + + def test_deleted_key(self, monkeypatch, yaml, config_tmpdir): + """A key marked as deleted should be removed.""" + autoconfig = config_tmpdir / 'autoconfig.yml' + autoconfig.write_text('global:\n hello: world', encoding='utf-8') + + monkeypatch.setattr(configdata.MIGRATIONS, 'deleted', ['hello']) + yaml.load() yaml._save() lines = autoconfig.read_text('utf-8').splitlines() assert ' hello:' not in lines + def test_renamed_key(self, monkeypatch, yaml, config_tmpdir): + """A key marked as renamed should be renamed properly.""" + autoconfig = config_tmpdir / 'autoconfig.yml' + autoconfig.write_text('global:\n old: value', encoding='utf-8') + + monkeypatch.setattr(configdata.MIGRATIONS, 'renamed', {'old': 'new'}) + + yaml.load() + yaml._save() + + lines = autoconfig.read_text('utf-8').splitlines() + assert ' old:' not in lines + assert ' new:' not in lines + @pytest.mark.parametrize('old_config', [ None, 'global:\n colors.hints.fg: magenta', diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index c210f3122..9c4ffc859 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -149,6 +149,10 @@ class TestEarlyInit: error = ("Error{}: Invalid value 'True' - expected a value of " "type str but got bool.".format(suffix)) expected_errors.append(error) + elif invalid_yaml == 'unknown': + error = ("While loading options{}: Unknown option " + "colors.foobar".format(suffix)) + expected_errors.append(error) if config_py == 'error': expected_errors.append("While setting 'foo': No option 'foo'")