From dd4294de0316109c0e66fce4bf2b0fcfef72a6ad Mon Sep 17 00:00:00 2001 From: Ryan Farley Date: Tue, 19 Sep 2017 16:26:02 -0500 Subject: [PATCH] fix #2979: use dictionary for mutable tracking Using a dictionary instead of a list keeps only one working copy, allowing consistency in between calls of update_mutables() --- qutebrowser/config/config.py | 28 ++++++++++++++++------------ tests/unit/config/test_config.py | 2 +- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 88312facd..2a3e79b79 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -368,7 +368,7 @@ class Config(QObject): Attributes: _values: A dict mapping setting names to their values. - _mutables: A list of mutable objects to be checked for changes. + _mutables: A dictionary of mutable objects to be checked for changes. _yaml: A YamlConfig object or None. Signals: @@ -430,19 +430,23 @@ class Config(QObject): If mutable=True is set, watch the returned object for mutations. """ opt = self.get_opt(name) - # We always return a copy of the value stored internally, so the + obj = None + # If we allow mutation, there is a chance that prior mutations already + # entered the mutable dictionary and thus further copies are unneeded + # until update_mutables() is called + if name in self._mutables and mutable: + obj = self._mutables[name][1] + # Otherwise, we return a copy of the value stored internally, so the # internal value can never be changed by mutating the object returned. - obj = copy.deepcopy(self._values.get(name, opt.default)) - # Then we watch the returned object for changes. - if isinstance(obj, (dict, list)): - if mutable: - if name not in self._mutables: - self._mutables[name] = (copy.deepcopy(obj), obj) - else: - obj = self._mutables[name][1] else: - # Shouldn't be mutable (and thus hashable) - assert obj.__hash__ is not None, obj + obj = copy.deepcopy(self._values.get(name, opt.default)) + # Then we watch the returned object for changes. + if isinstance(obj, (dict, list)): + if mutable: + self._mutables[name] = (copy.deepcopy(obj), obj) + else: + # Shouldn't be mutable (and thus hashable) + assert obj.__hash__ is not None, obj return obj def get_str(self, name): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index ab9e31fa8..b5be51546 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -706,7 +706,7 @@ class TestConfig: assert obj == new if mutable: - assert conf._mutables == ( old, new) + assert conf._mutables[option] == (old, new) if mutable and mutated: # Now let's update