From 5bd40e732c00707944af086f30a347d50303594e Mon Sep 17 00:00:00 2001
From: Florian Bruhin <git@the-compiler.org>
Date: Thu, 17 Apr 2014 19:02:58 +0200
Subject: [PATCH] Do validation and reading from configparser correctly

---
 qutebrowser/config/config.py   | 28 +++++++++++++++++++---------
 qutebrowser/config/sections.py | 32 ++++++++------------------------
 qutebrowser/config/value.py    |  5 +++--
 3 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py
index ede791507..78183e9e5 100644
--- a/qutebrowser/config/config.py
+++ b/qutebrowser/config/config.py
@@ -120,12 +120,9 @@ class Config(QObject):
         self._configdir = configdir
         self._interpolation = ExtendedInterpolation()
         self._proxies = {}
-        for secname, section in self.sections.items():
+        for secname in self.sections.keys():
             self._proxies[secname] = SectionProxy(self, secname)
-            try:
-                section.from_cp(self._configparser[secname])
-            except KeyError:
-                pass
+        self._from_cp(self._configparser)
 
     def __getitem__(self, key):
         """Get a section from the config."""
@@ -198,6 +195,18 @@ class Config(QObject):
             lines.append(keyval)
         return lines
 
+    def _from_cp(self, cp):
+        """Read the config from a configparser instance.
+
+        Args:
+            cp: The configparser instande to read the values from.
+        """
+        for secname in self.sections.keys():
+            if secname not in cp:
+                continue
+            for k, v in cp[secname].items():
+                self.set('conf', secname, k, v)
+
     def has_option(self, section, option):
         """Check if option exists in section.
 
@@ -312,15 +321,16 @@ class Config(QObject):
             changed: If the config was changed.
             style_changed: When style caches need to be invalidated.
         """
-        if value:
-            value = self._interpolation.before_set(self, section, option,
-                                                   value)
+        value = self._interpolation.before_set(self, section, option, value)
         try:
             sect = self.sections[section]
         except KeyError:
             raise NoSectionError(section)
+        mapping = {key: val.value for key, val in sect.values.items()}
+        interpolated = self._interpolation.before_get(self, section, option,
+                                                      value, mapping)
         try:
-            sect.setv(layer, option, value)
+            sect.setv(layer, option, value, interpolated)
         except KeyError:
             raise NoOptionError(option, section)
         else:
diff --git a/qutebrowser/config/sections.py b/qutebrowser/config/sections.py
index 7b0563eef..15935a5b5 100644
--- a/qutebrowser/config/sections.py
+++ b/qutebrowser/config/sections.py
@@ -17,7 +17,6 @@
 
 """Setting sections used for qutebrowser."""
 
-import logging
 from collections import OrderedDict, ChainMap
 
 from qutebrowser.config.value import SettingValue
@@ -78,7 +77,7 @@ class Section:
         """Get value keys."""
         return self.values.keys()
 
-    def setv(self, layer, key, value):
+    def setv(self, layer, key, value, interpolated):
         """Set the value on a layer.
 
         Args:
@@ -86,13 +85,10 @@ class Section:
                    ValueLayers dict.
             key: The key of the element to set.
             value: The value to set.
+            interpolated: The interpolated value, for checking.
         """
         raise NotImplementedError
 
-    def from_cp(self, sect):
-        """Initialize the values from a configparser section."""
-        raise NotImplementedError
-
     def dump_userconfig(self):
         """Dump the part of the config which was changed by the user.
 
@@ -124,13 +120,8 @@ class KeyValue(Section):
             self.values[k] = v
             self.descriptions[k] = desc
 
-    def setv(self, layer, key, value):
-        self.values[key].setv(layer, value)
-
-    def from_cp(self, sect):
-        for k, v in sect.items():
-            logging.debug("'{}' = '{}'".format(k, v))
-            self.values[k].setv('conf', v)
+    def setv(self, layer, key, value, interpolated):
+        self.values[key].setv(layer, value, interpolated)
 
     def dump_userconfig(self):
         changed = []
@@ -183,22 +174,15 @@ class ValueList(Section):
         self.values = ChainMap(self.layers['temp'], self.layers['conf'],
                                self.layers['default'])
 
-    def setv(self, layer, key, value):
+    def setv(self, layer, key, value, interpolated):
+        self.keytype().validate(key)
         if key in self.layers[layer]:
-            self.layers[layer][key].setv(layer, value)
+            self.layers[layer][key].setv(layer, value, interpolated)
         else:
             val = SettingValue(self.valtype)
-            val.setv(layer, value)
+            val.setv(layer, value, interpolated)
             self.layers[layer][key] = val
 
-    def from_cp(self, sect):
-        keytype = self.keytype()
-        valtype = self.valtype()
-        for k, v in sect.items():
-            keytype.validate(k)
-            valtype.validate(v)
-            self.setv('conf', k, v)
-
     def dump_userconfig(self):
         changed = []
         mapping = ChainMap(self.layers['temp'], self.layers['conf'])
diff --git a/qutebrowser/config/value.py b/qutebrowser/config/value.py
index 7227da171..5a12177c5 100644
--- a/qutebrowser/config/value.py
+++ b/qutebrowser/config/value.py
@@ -90,13 +90,14 @@ class SettingValue:
         v = self.value
         return self.typ.transform(v)
 
-    def setv(self, layer, value):
+    def setv(self, layer, value, interpolated):
         """Set the value on a layer.
 
         Args:
             layer: The layer to set the value on, an element name of the
                    ValueLayers dict.
             value: The value to set.
+            interpolated: The interpolated value, for typechecking.
         """
-        self.typ.validate(value)
+        self.typ.validate(interpolated)
         self._values[layer] = value