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

@@ -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'))

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

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

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:
 

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',

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'")