diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 3dbb72dc8..3f24af721 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -73,6 +73,9 @@ def run(args): log.init.debug("Initializing directories...") standarddir.init(args) + log.init.debug("Initializing config...") + config.early_init() + global qApp qApp = Application(args) qApp.setOrganizationName("qutebrowser") @@ -408,6 +411,7 @@ def _init_modules(args, crash_handler): save_manager = savemanager.SaveManager(qApp) objreg.register('save-manager', save_manager) save_manager.add_saveable('version', _save_version) + config.late_init(save_manager) log.init.debug("Initializing network...") networkmanager.init() @@ -419,10 +423,6 @@ def _init_modules(args, crash_handler): readline_bridge = readline.ReadlineBridge() objreg.register('readline-bridge', readline_bridge) - log.init.debug("Initializing config...") - config.init(qApp) - save_manager.init_autosave() - log.init.debug("Initializing sql...") try: sql.init(os.path.join(standarddir.data(), 'history.sqlite')) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 9907eb205..89eb4fad3 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -39,6 +39,8 @@ key_instance = None # Keeping track of all change filters to validate them later. _change_filters = [] +# Errors which happened during init, so we can show a message box. +_init_errors = [] class change_filter: # pylint: disable=invalid-name @@ -382,6 +384,14 @@ class Config(QObject): self._mutables = [] self._yaml = yaml_config + def init_save_manager(self, save_manager): + """Make sure the config gets saved properly. + + We do this outside of __init__ because the config gets created before + the save_manager exists. + """ + self._yaml.init_save_manager(save_manager) + def _set_value(self, opt, value): """Set the given option to the given value.""" if objects.backend is not None: @@ -634,18 +644,14 @@ class StyleSheetObserver(QObject): instance.changed.connect(self._update_stylesheet) -def init(parent=None): - """Initialize the config. - - Args: - parent: The parent to pass to QObjects which get initialized. - """ +def early_init(): + """Initialize the part of the config which works without a QApplication.""" configdata.init() yaml_config = configfiles.YamlConfig() global val, instance, key_instance - instance = Config(yaml_config=yaml_config, parent=parent) + instance = Config(yaml_config=yaml_config) val = ConfigContainer(instance) key_instance = KeyConfig(instance) @@ -666,12 +672,7 @@ def init(parent=None): raise configexc.ConfigFileErrors('config.py', config_api.errors) except configexc.ConfigFileErrors as e: log.config.exception("Error while loading config.py") - errbox = msgbox.msgbox(parent=None, - title="Error while reading config", - text=e.to_html(), - icon=QMessageBox.Warning, - plain_text=False) - errbox.exec_() + _init_errors.append(e) try: if getattr(config_api, 'load_autoconfig', True): @@ -683,12 +684,23 @@ def init(parent=None): desc = configexc.ConfigErrorDesc("Error", e) raise configexc.ConfigFileErrors('autoconfig.yml', [desc]) except configexc.ConfigFileErrors as e: - log.config.exception("Error while loading autoconfig.yml") + log.config.exception("Error while loading config.py") + _init_errors.append(e) + + configfiles.init() + + +def late_init(save_manager): + """Initialize the rest of the config after the QApplication is created.""" + global _init_errors + for err in _init_errors: errbox = msgbox.msgbox(parent=None, title="Error while reading config", - text=e.to_html(), + text=err.to_html(), icon=QMessageBox.Warning, plain_text=False) errbox.exec_() + _init_errors = [] - configfiles.init() + instance.init_save_manager(save_manager) + objreg.get('state-config').init_save_manager(save_manager) diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 2bdb5d263..c9af13d95 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -39,7 +39,6 @@ class StateConfig(configparser.ConfigParser): def __init__(self): super().__init__() - save_manager = objreg.get('save-manager') self._filename = os.path.join(standarddir.data(), 'state') self.read(self._filename, encoding='utf-8') for sect in ['general', 'geometry']: @@ -49,6 +48,13 @@ class StateConfig(configparser.ConfigParser): pass # See commit a98060e020a4ba83b663813a4b9404edb47f28ad. self['general'].pop('fooled', None) + + def init_save_manager(self, save_manager): + """Make sure the config gets saved properly. + + We do this outside of __init__ because the config gets created before + the save_manager exists. + """ save_manager.add_saveable('state-config', self._save) def _save(self): @@ -68,12 +74,18 @@ class YamlConfig: VERSION = 1 def __init__(self): - save_manager = objreg.get('save-manager') self._filename = os.path.join(standarddir.config(auto=True), 'autoconfig.yml') - save_manager.add_saveable('yaml-config', self._save) self.values = {} + def init_save_manager(self, save_manager): + """Make sure the config gets saved properly. + + We do this outside of __init__ because the config gets created before + the save_manager exists. + """ + save_manager.add_saveable('yaml-config', self._save) + def _save(self): """Save the changed settings to the YAML file.""" data = {'config_version': self.VERSION, 'global': self.values} diff --git a/qutebrowser/misc/savemanager.py b/qutebrowser/misc/savemanager.py index 926bf1230..ddda5325b 100644 --- a/qutebrowser/misc/savemanager.py +++ b/qutebrowser/misc/savemanager.py @@ -113,19 +113,12 @@ class SaveManager(QObject): self.saveables = collections.OrderedDict() self._save_timer = usertypes.Timer(self, name='save-timer') self._save_timer.timeout.connect(self.autosave) + self._set_autosave_interval() + config.instance.changed.connect(self._set_autosave_interval) def __repr__(self): return utils.get_repr(self, saveables=self.saveables) - def init_autosave(self): - """Initialize auto-saving. - - We don't do this in __init__ because the config needs to be initialized - first, but the config needs the save manager. - """ - self._set_autosave_interval() - config.instance.changed.connect(self._set_autosave_interval) - @config.change_filter('auto_save.interval') def _set_autosave_interval(self): """Set the auto-save interval.""" diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 4e4816e7c..c6e30f16d 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -873,6 +873,7 @@ def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, monkeypatch.setattr(config, 'instance', None) monkeypatch.setattr(config, 'key_instance', None) monkeypatch.setattr(config, '_change_filters', []) + monkeypatch.setattr(config, '_init_errors', []) yield for obj in ['config-commands', 'state-config']: try: @@ -885,8 +886,8 @@ def init_patch(qapp, fake_save_manager, monkeypatch, config_tmpdir, @pytest.mark.parametrize('config_py', [True, 'error', False]) @pytest.mark.parametrize('invalid_yaml', ['42', 'unknown', False]) # pylint: disable=too-many-branches -def test_init(init_patch, fake_save_manager, config_tmpdir, mocker, caplog, - load_autoconfig, config_py, invalid_yaml): +def test_early_init(init_patch, config_tmpdir, caplog, + load_autoconfig, config_py, invalid_yaml): # Prepare files autoconfig_file = config_tmpdir / 'autoconfig.yml' config_py_file = config_tmpdir / 'config.py' @@ -910,36 +911,32 @@ def test_init(init_patch, fake_save_manager, config_tmpdir, mocker, caplog, config_py_file.write_text('\n'.join(config_py_lines), 'utf-8', ensure=True) - msgbox_mock = mocker.patch('qutebrowser.config.config.msgbox.msgbox', - autospec=True) - with caplog.at_level(logging.ERROR): - config.init() + config.early_init() # Check error messages expected_errors = [] if config_py == 'error': - expected_errors.append("Errors occurred while reading config.py:") + expected_errors.append( + "Errors occurred while reading config.py:\n" + " While setting 'foo': No option 'foo'") if invalid_yaml and (load_autoconfig or not config_py): - expected_errors.append("Errors occurred while reading autoconfig.yml:") - if expected_errors: - assert len(expected_errors) == len(msgbox_mock.call_args_list) - comparisons = zip( - expected_errors, - [call[1]['text'] for call in msgbox_mock.call_args_list]) - for expected, actual in comparisons: - assert actual.strip().startswith(expected) - else: - assert not msgbox_mock.called + error = "Errors occurred while reading autoconfig.yml:\n" + if invalid_yaml == '42': + error += " While loading data: Toplevel object is not a dict" + elif invalid_yaml == 'unknown': + error += " Error: No option 'colors.foobar'" + else: + assert False, invalid_yaml + expected_errors.append(error) + + actual_errors = [str(err) for err in config._init_errors] + assert actual_errors == expected_errors # Make sure things have been init'ed objreg.get('config-commands') assert isinstance(config.instance, config.Config) assert isinstance(config.key_instance, config.KeyConfig) - fake_save_manager.add_saveable.assert_any_call( - 'state-config', unittest.mock.ANY) - fake_save_manager.add_saveable.assert_any_call( - 'yaml-config', unittest.mock.ANY) # Check config values if config_py and load_autoconfig and not invalid_yaml: @@ -955,7 +952,33 @@ def test_init(init_patch, fake_save_manager, config_tmpdir, mocker, caplog, assert config.instance._values == {'colors.hints.fg': 'magenta'} -def test_init_invalid_change_filter(init_patch): +def test_early_init_invalid_change_filter(init_patch): config.change_filter('foobar') with pytest.raises(configexc.NoOptionError): - config.init() + config.early_init() + + +@pytest.mark.parametrize('errors', [True, False]) +def test_late_init(init_patch, monkeypatch, fake_save_manager, mocker, errors): + config.early_init() + if errors: + err = configexc.ConfigErrorDesc("Error text", Exception("Exception")) + errs = configexc.ConfigFileErrors("config.py", [err]) + monkeypatch.setattr(config, '_init_errors', [errs]) + msgbox_mock = mocker.patch('qutebrowser.config.config.msgbox.msgbox', + autospec=True) + + config.late_init(fake_save_manager) + + fake_save_manager.add_saveable.assert_any_call( + 'state-config', unittest.mock.ANY) + fake_save_manager.add_saveable.assert_any_call( + 'yaml-config', unittest.mock.ANY) + if errors: + assert len(msgbox_mock.call_args_list) == 1 + _call_posargs, call_kwargs = msgbox_mock.call_args_list[0] + text = call_kwargs['text'].strip() + assert text.startswith('Errors occurred while reading config.py:') + assert 'Error text: Exception' in text + else: + assert not msgbox_mock.called