diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index b19a83e73..1efc96856 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -408,7 +408,7 @@ class Config(QObject): def read_yaml(self): """Read the YAML settings from self._yaml.""" self._yaml.load() - for name, value in self._yaml.values.items(): + for name, value in self._yaml: self._set_value(self.get_opt(name), value) def get_opt(self, name): @@ -462,7 +462,7 @@ class Config(QObject): """ self._set_value(self.get_opt(name), value) if save_yaml: - self._yaml.values[name] = value + self._yaml[name] = value def set_str(self, name, value, *, save_yaml=False): """Set the given setting from a string. @@ -476,7 +476,7 @@ class Config(QObject): value)) self._set_value(opt, converted) if save_yaml: - self._yaml.values[name] = converted + self._yaml[name] = converted def update_mutables(self, *, save_yaml=False): """Update mutable settings if they changed. diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 9e66f4ffe..e26a03454 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -83,7 +83,8 @@ class YamlConfig: def __init__(self): self._filename = os.path.join(standarddir.config(auto=True), 'autoconfig.yml') - self.values = {} + self._values = {} + self._dirty = None def init_save_manager(self, save_manager): """Make sure the config gets saved properly. @@ -93,9 +94,25 @@ class YamlConfig: """ save_manager.add_saveable('yaml-config', self._save) + def __getitem__(self, name): + return self._values[name] + + def __setitem__(self, name, value): + self._dirty = True + self._values[name] = value + + def __contains__(self, name): + return name in self._values + + def __iter__(self): + return iter(self._values.items()) + def _save(self): - """Save the changed settings to the YAML file.""" - data = {'config_version': self.VERSION, 'global': self.values} + """Save the settings to the YAML file if they've changed.""" + if not self._dirty: + return + + data = {'config_version': self.VERSION, 'global': self._values} with qtutils.savefile_open(self._filename) as f: f.write(textwrap.dedent(""" # DO NOT edit this file by hand, qutebrowser will overwrite it. @@ -105,12 +122,12 @@ class YamlConfig: utils.yaml_dump(data, f) def load(self): - """Load self.values from the configured YAML file.""" + """Load configuration from the configured YAML file.""" try: with open(self._filename, 'r', encoding='utf-8') as f: yaml_data = utils.yaml_load(f) except FileNotFoundError: - return + return {} except OSError as e: desc = configexc.ConfigErrorDesc("While reading", e) raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) @@ -136,7 +153,8 @@ class YamlConfig: "'global' object is not a dict") raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) - self.values = global_obj + self._values = global_obj + self._dirty = False class ConfigAPI: diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py index 1b386c201..0bab0ce96 100644 --- a/tests/helpers/stubs.py +++ b/tests/helpers/stubs.py @@ -414,12 +414,24 @@ class FakeYamlConfig: """Fake configfiles.YamlConfig object.""" def __init__(self): - self.values = {} self.loaded = False + self._values = {} def load(self): self.loaded = True + def __contains__(self, item): + return item in self._values + + def __iter__(self): + return iter(self._values.items()) + + def __setitem__(self, key, value): + self._values[key] = value + + def __getitem__(self, key): + return self._values[key] + class StatusBarCommandStub(QLineEdit): diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index b5be51546..6dff3c30a 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -317,9 +317,9 @@ class TestSetConfigCommand: assert config_stub.get(option) == new_value if temp: - assert option not in config_stub._yaml.values + assert option not in config_stub._yaml else: - assert config_stub._yaml.values[option] == new_value + assert config_stub._yaml[option] == new_value @pytest.mark.parametrize('temp', [True, False]) def test_set_temp_override(self, commands, config_stub, temp): @@ -335,7 +335,7 @@ class TestSetConfigCommand: commands.set(0, 'url.auto_search', 'never', temp=True) assert config_stub.val.url.auto_search == 'never' - assert config_stub._yaml.values['url.auto_search'] == 'dns' + assert config_stub._yaml['url.auto_search'] == 'dns' def test_set_print(self, config_stub, commands, message_mock): """Run ':set -p url.auto_search never'. @@ -357,7 +357,7 @@ class TestSetConfigCommand: assert not config_stub.val.auto_save.session commands.set(0, 'auto_save.session!') assert config_stub.val.auto_save.session - assert config_stub._yaml.values['auto_save.session'] + assert config_stub._yaml['auto_save.session'] def test_set_toggle_nonbool(self, commands, config_stub): """Run ':set url.auto_search!'. @@ -439,7 +439,7 @@ class TestSetConfigCommand: config_stub.set_obj(opt, initial) commands.set(0, opt, 'green', 'magenta', 'blue', 'yellow') assert config_stub.get(opt) == expected - assert config_stub._yaml.values[opt] == expected + assert config_stub._yaml[opt] == expected class TestBindConfigCommand: @@ -464,7 +464,7 @@ class TestBindConfigCommand: commands.bind('a', command) assert keyconf.get_command('a', 'normal') == command - yaml_bindings = config_stub._yaml.values['bindings.commands']['normal'] + yaml_bindings = config_stub._yaml['bindings.commands']['normal'] assert yaml_bindings['a'] == command @pytest.mark.parametrize('key, mode, expected', [ @@ -565,7 +565,7 @@ class TestBindConfigCommand: commands.unbind(key) assert keyconf.get_command(key, 'normal') is None - yaml_bindings = config_stub._yaml.values['bindings.commands']['normal'] + yaml_bindings = config_stub._yaml['bindings.commands']['normal'] if key in 'bc': # Custom binding assert normalized not in yaml_bindings @@ -612,7 +612,7 @@ class TestConfig: def test_read_yaml(self, conf): assert not conf._yaml.loaded - conf._yaml.values['content.plugins'] = True + conf._yaml['content.plugins'] = True conf.read_yaml() @@ -620,7 +620,7 @@ class TestConfig: assert conf._values['content.plugins'] is True def test_read_yaml_invalid(self, conf): - conf._yaml.values['foo.bar'] = True + conf._yaml['foo.bar'] = True with pytest.raises(configexc.NoOptionError): conf.read_yaml() @@ -743,9 +743,9 @@ class TestConfig: meth(option, value, save_yaml=save_yaml) assert conf._values[option] is True if save_yaml: - assert conf._yaml.values[option] is True + assert conf._yaml[option] is True else: - assert option not in conf._yaml.values + assert option not in conf._yaml @pytest.mark.parametrize('method', ['set_obj', 'set_str']) def test_set_invalid(self, conf, qtbot, method): diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index b7dba3b18..c18baa7de 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -68,22 +68,24 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): yaml.load() if insert: - yaml.values['tabs.show'] = 'never' + yaml['tabs.show'] = 'never' yaml._save() - text = autoconfig.read_text('utf-8') - lines = text.splitlines() - print(lines) - - assert lines[0].startswith('# DO NOT edit this file by hand,') - assert 'config_version: {}'.format(yaml.VERSION) in lines - - if old_config is None and not insert: - assert 'global: {}' in lines + if not insert and old_config is None: + lines = [] else: + text = autoconfig.read_text('utf-8') + lines = text.splitlines() + + if insert: + assert lines[0].startswith('# DO NOT edit this file by hand,') + assert 'config_version: {}'.format(yaml.VERSION) in lines + assert 'global:' in lines + print(lines) + # WORKAROUND for https://github.com/PyCQA/pylint/issues/574 if 'magenta' in (old_config or ''): # pylint: disable=superfluous-parens assert ' colors.hints.fg: magenta' in lines @@ -91,6 +93,58 @@ def test_yaml_config(fake_save_manager, config_tmpdir, old_config, insert): assert ' tabs.show: never' in lines +@pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', +]) +@pytest.mark.parametrize('key, value', [ + ('colors.hints.fg', 'green'), + ('colors.hints.bg', None), + ('confirm_quit', True), + ('confirm_quit', False), +]) +def test_yaml_config_changed(fake_save_manager, config_tmpdir, old_config, key, value): + autoconfig = config_tmpdir / 'autoconfig.yml' + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') + + yaml = configfiles.YamlConfig() + yaml.load() + + yaml[key] = value + assert key in yaml + assert yaml[key] == value + + yaml._save() + + yaml = configfiles.YamlConfig() + yaml.load() + + assert key in yaml + assert yaml[key] == value + + +@pytest.mark.parametrize('old_config', [ + None, + 'global:\n colors.hints.fg: magenta', +]) +def test_yaml_config_unchanged(fake_save_manager, config_tmpdir, old_config): + autoconfig = config_tmpdir / 'autoconfig.yml' + mtime = None + if old_config is not None: + autoconfig.write_text(old_config, 'utf-8') + mtime = autoconfig.stat().mtime + + yaml = configfiles.YamlConfig() + yaml.load() + yaml._save() + + if old_config is None: + assert not autoconfig.exists() + else: + assert autoconfig.stat().mtime == mtime + + @pytest.mark.parametrize('line, text, exception', [ ('%', 'While parsing', 'while scanning a directive'), ('global: 42', 'While loading data', "'global' object is not a dict"),