From 0c5d16b8228fd931a1f1191993724e39160fb81a Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Wed, 3 Feb 2021 20:10:44 +0100 Subject: Only warn about autoconfig for the top-level config.py If we are e.g. loading a secondary file via config.source() before config.load_autoconfig(False) has been called in the main one, we don't care about the warning. Neither should :config-source fail if we started with the warning for the main config. Fixes #6099 --- qutebrowser/config/config.py | 1 - qutebrowser/config/configfiles.py | 26 +++++++++++++++++++++----- qutebrowser/config/configinit.py | 2 +- tests/unit/config/test_configfiles.py | 32 ++++++++++++++++++++++++++------ tests/unit/config/test_configinit.py | 13 +++++++++++++ 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index a91f72c82..c644725b5 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -278,7 +278,6 @@ class Config(QObject): self._init_values() self.yaml_loaded = False self.config_py_loaded = False - self.warn_autoconfig = True def _init_values(self) -> None: """Populate the self._values dict.""" diff --git a/qutebrowser/config/configfiles.py b/qutebrowser/config/configfiles.py index 975ea6b4a..498c5572f 100644 --- a/qutebrowser/config/configfiles.py +++ b/qutebrowser/config/configfiles.py @@ -591,17 +591,24 @@ class ConfigAPI: Attributes: _config: The main Config object to use. _keyconfig: The KeyConfig object. + _warn_autoconfig: Whether to warn if autoconfig.yml wasn't loaded. errors: Errors which occurred while setting options. configdir: The qutebrowser config directory, as pathlib.Path. datadir: The qutebrowser data directory, as pathlib.Path. """ - def __init__(self, conf: config.Config, keyconfig: config.KeyConfig): + def __init__( + self, + conf: config.Config, + keyconfig: config.KeyConfig, + warn_autoconfig: bool, + ): self._config = conf self._keyconfig = keyconfig self.errors: List[configexc.ConfigErrorDesc] = [] self.configdir = pathlib.Path(standarddir.config()) self.datadir = pathlib.Path(standarddir.data()) + self._warn_autoconfig = warn_autoconfig @contextlib.contextmanager def _handle_error(self, action: str, name: str) -> Iterator[None]: @@ -624,7 +631,7 @@ class ConfigAPI: def finalize(self) -> None: """Do work which needs to be done after reading config.py.""" - if self._config.warn_autoconfig: + if self._warn_autoconfig: desc = configexc.ConfigErrorDesc( "autoconfig loading not specified", ("Your config.py should call either `config.load_autoconfig()`" @@ -635,7 +642,7 @@ class ConfigAPI: def load_autoconfig(self, load_config: bool = True) -> None: """Load the autoconfig.yml file which is used for :set/:bind/etc.""" - self._config.warn_autoconfig = False + self._warn_autoconfig = False if load_config: with self._handle_error('reading', 'autoconfig.yml'): read_autoconfig() @@ -815,18 +822,27 @@ class ConfigPyWriter: yield '' -def read_config_py(filename: str, raising: bool = False) -> None: +def read_config_py( + filename: str, + raising: bool = False, + warn_autoconfig: bool = False, +) -> None: """Read a config.py file. Arguments; filename: The name of the file to read. raising: Raise exceptions happening in config.py. This is needed during tests to use pytest's inspection. + warn_autoconfig: Whether to warn if config.load_autoconfig() wasn't specified. """ assert config.instance is not None assert config.key_instance is not None - api = ConfigAPI(config.instance, config.key_instance) + api = ConfigAPI( + config.instance, + config.key_instance, + warn_autoconfig=warn_autoconfig, + ) container = config.ConfigContainer(config.instance, configapi=api) basename = os.path.basename(filename) diff --git a/qutebrowser/config/configinit.py b/qutebrowser/config/configinit.py index 8b2b12227..7e20487db 100644 --- a/qutebrowser/config/configinit.py +++ b/qutebrowser/config/configinit.py @@ -62,7 +62,7 @@ def early_init(args: argparse.Namespace) -> None: try: if os.path.exists(config_file): - configfiles.read_config_py(config_file) + configfiles.read_config_py(config_file, warn_autoconfig=True) else: configfiles.read_autoconfig() except configexc.ConfigFileErrors as e: diff --git a/tests/unit/config/test_configfiles.py b/tests/unit/config/test_configfiles.py index fdd12d308..cd8a91423 100644 --- a/tests/unit/config/test_configfiles.py +++ b/tests/unit/config/test_configfiles.py @@ -729,22 +729,28 @@ class ConfPy: def __init__(self, tmpdir, filename: str = "config.py"): self._file = tmpdir / filename self.filename = str(self._file) - config.instance.warn_autoconfig = False def write(self, *lines): text = '\n'.join(lines) self._file.write_text(text, 'utf-8', ensure=True) - def read(self, error=False): + def read(self, error=False, warn_autoconfig=False): """Read the config.py via configfiles and check for errors.""" if error: with pytest.raises(configexc.ConfigFileErrors) as excinfo: - configfiles.read_config_py(self.filename) + configfiles.read_config_py( + self.filename, + warn_autoconfig=warn_autoconfig, + ) errors = excinfo.value.errors assert len(errors) == 1 return errors[0] else: - configfiles.read_config_py(self.filename, raising=True) + configfiles.read_config_py( + self.filename, + raising=True, + warn_autoconfig=warn_autoconfig, + ) return None def write_qbmodule(self): @@ -1025,9 +1031,8 @@ class TestConfigPy: def test_load_autoconfig_warning(self, confpy): confpy.write('') - config.instance.warn_autoconfig = True with pytest.raises(configexc.ConfigFileErrors) as excinfo: - configfiles.read_config_py(confpy.filename) + configfiles.read_config_py(confpy.filename, warn_autoconfig=True) assert len(excinfo.value.errors) == 1 error = excinfo.value.errors[0] assert error.text == "autoconfig loading not specified" @@ -1194,6 +1199,21 @@ class TestConfigPy: assert error.text == "Error while reading doesnotexist.py" assert isinstance(error.exception, FileNotFoundError) + @pytest.mark.parametrize('reverse', [True, False]) + def test_source_warn_autoconfig(self, tmpdir, confpy, reverse): + subfile = tmpdir / 'config' / 'subfile.py' + subfile.write_text("c.content.javascript.enabled = False", + encoding='utf-8') + lines = [ + "config.source('subfile.py')", + "config.load_autoconfig(False)", + ] + if reverse: + lines.reverse() + + confpy.write(*lines) + confpy.read(warn_autoconfig=True) + class TestConfigPyWriter: diff --git a/tests/unit/config/test_configinit.py b/tests/unit/config/test_configinit.py index 4a7788874..1945f7f8a 100644 --- a/tests/unit/config/test_configinit.py +++ b/tests/unit/config/test_configinit.py @@ -204,6 +204,19 @@ class TestEarlyInit: assert dump == '\n'.join(expected) + def test_autoconfig_warning(self, init_patch, args, config_tmpdir, caplog): + """Test the warning shown for missing autoconfig loading.""" + config_py_file = config_tmpdir / 'config.py' + config_py_file.ensure() + + with caplog.at_level(logging.ERROR): + configinit.early_init(args) + + # Check error messages + assert len(configinit._init_errors.errors) == 1 + error = configinit._init_errors.errors[0] + assert str(error).startswith("autoconfig loading not specified") + @pytest.mark.parametrize('byte', [ b'\x00', # configparser.Error b'\xda', # UnicodeDecodeError -- cgit v1.2.3-54-g00ecf