diff --git a/changelog.md b/changelog.md index 200369ae..f2d6c5f7 100644 --- a/changelog.md +++ b/changelog.md @@ -11,6 +11,7 @@ Bugfixes * Don't allow saving favorite queries to rewrite comments in `~/.myclirc`. * Don't allow saving favorite queries to rewrite quoting in `~/.myclirc`. * Don't allow saving named DSNs to rewrite comments in `~/.myclirc`. +* Don't allow saving named DSNs to rewrite quoting in `~/.myclirc`. Documentation diff --git a/mycli/config.py b/mycli/config.py index c90b78bf..5df3c8c1 100644 --- a/mycli/config.py +++ b/mycli/config.py @@ -15,7 +15,13 @@ class LimiitedQuotePreservingConfigObj(ConfigObj): - """Useful for saving individual items without modifying the whole file.""" + """Useful for saving individual items without modifying the whole file. + + Triplequotes must be manually added for multiline values, and despite the + name of the class, could change from double to single in style. If we + don't do this, multiline triplequoted strings lose their quotes entirely, + resulting in unreadable files. + """ def __init__(self, *args, **kwargs): ConfigObj.__init__(self, *args, **kwargs) @@ -24,6 +30,8 @@ def _unquote(self, value): return value def _quote(self, value, multiline=True): + if '\n' in value: + return f"'''{value}'''" return value diff --git a/mycli/packages/special/dsn_aliases.py b/mycli/packages/special/dsn_aliases.py index 8086d1ff..eadce77c 100644 --- a/mycli/packages/special/dsn_aliases.py +++ b/mycli/packages/special/dsn_aliases.py @@ -101,7 +101,7 @@ def _config_for_write(self) -> Any: if self.config_file is None: return self.config - config = read_config_file(self.config_file) + config = read_config_file(self.config_file, preserve_quotes=True) if config is None: raise OSError(f"Unable to read config file '{os.path.expanduser(self.config_file)}'.") return config diff --git a/mycli/packages/special/favoritequeries.py b/mycli/packages/special/favoritequeries.py index 8b578bfa..ad480491 100644 --- a/mycli/packages/special/favoritequeries.py +++ b/mycli/packages/special/favoritequeries.py @@ -145,14 +145,10 @@ def get(self, name) -> str | None: def save(self, name: str, query: str) -> None: config = self._config_for_write() query = query.rstrip(' \t\n\r;') - if '\n' in query: - manually_quoted_query = f"'''{query}'''" - else: - manually_quoted_query = query config.encoding = "utf-8" section_existed = self.section_name in config previous_query = config.get(self.section_name, {}).get(name, MISSING) - self._set_query(config, name, manually_quoted_query) + self._set_query(config, name, query) try: config.write() except Exception: @@ -165,7 +161,6 @@ def save(self, name: str, query: str) -> None: raise if config is not self.config: - # use the unquoted query for the current session self._set_query(self.config, name, query) def delete(self, name: str) -> str: diff --git a/test/pytests/test_config.py b/test/pytests/test_config.py index 3abc1cce..d02a3f8e 100644 --- a/test/pytests/test_config.py +++ b/test/pytests/test_config.py @@ -14,6 +14,7 @@ from mycli import config as config_module from mycli.config import ( + LimiitedQuotePreservingConfigObj, _remove_pad, create_default_config, get_mylogin_cnf_path, @@ -21,6 +22,7 @@ open_mylogin_cnf, read_and_decrypt_mylogin_cnf, read_config_file, + read_config_files, str_to_bool, strip_matching_quotes, write_default_config, @@ -169,6 +171,71 @@ def test_read_config_file_list_values_off(): assert config["main"]["weather"] == "'cloudy with a chance of meatballs'" +def test_quote_preserving_config_retains_quotes_and_quotes_multiline_values() -> None: + config = read_config_file(StringIO('[main]\nquoted = "value"\n'), preserve_quotes=True) + + assert isinstance(config, LimiitedQuotePreservingConfigObj) + assert config['main']['quoted'] == '"value"' + assert config._quote('one line') == 'one line' + assert config._quote('first line\nsecond line') == "'''first line\nsecond line'''" + + +def test_read_config_files_merges_files_in_order(monkeypatch: pytest.MonkeyPatch) -> None: + defaults = config_module.ConfigObj({'main': {'default': 'yes', 'color': 'default'}}) + first = config_module.ConfigObj({'main': {'color': 'blue'}}) + first.filename = '/tmp/first.cnf' + second = config_module.ConfigObj({'main': {'color': 'green'}}) + second.filename = '/tmp/second.cnf' + files = ['first.cnf', 'missing.cnf', 'second.cnf'] + create_calls: list[bool] = [] + read_calls: list[tuple[str, bool]] = [] + + def create_default_config(list_values: bool = True) -> config_module.ConfigObj: + create_calls.append(list_values) + return defaults + + def read_config_file(path: str, list_values: bool = True) -> config_module.ConfigObj | None: + read_calls.append((path, list_values)) + return {'first.cnf': first, 'missing.cnf': None, 'second.cnf': second}[path] + + monkeypatch.setattr(config_module, 'create_default_config', create_default_config) + monkeypatch.setattr(config_module, 'read_config_file', read_config_file) + + config = read_config_files(files, list_values=False) + + assert files == ['first.cnf', 'missing.cnf', 'second.cnf'] + assert create_calls == [False] + assert read_calls == [('first.cnf', False), ('missing.cnf', False), ('second.cnf', False)] + assert config['main'] == {'default': 'yes', 'color': 'green'} + assert config.filename == '/tmp/second.cnf' + + +def test_read_config_files_can_ignore_package_defaults(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + config_module, + 'create_default_config', + lambda **_kwargs: pytest.fail('package defaults should not be read'), + ) + + config = read_config_files([], ignore_package_defaults=True) + + assert config == {} + + +def test_read_config_files_can_ignore_user_options(monkeypatch: pytest.MonkeyPatch) -> None: + defaults = config_module.ConfigObj({'main': {'default': 'yes'}}) + monkeypatch.setattr(config_module, 'create_default_config', lambda **_kwargs: defaults) + monkeypatch.setattr( + config_module, + 'read_config_file', + lambda *_args, **_kwargs: pytest.fail('user options should not be read'), + ) + + config = read_config_files(['user.cnf'], ignore_user_options=True) + + assert config is defaults + + def test_log_prints_to_stderr_when_root_logger(capsys) -> None: fake_logger = SimpleNamespace(parent=SimpleNamespace(name='root'), log=lambda level, message: None) diff --git a/test/pytests/test_dsn_aliases.py b/test/pytests/test_dsn_aliases.py index b2eee41f..fdc411e6 100644 --- a/test/pytests/test_dsn_aliases.py +++ b/test/pytests/test_dsn_aliases.py @@ -314,7 +314,7 @@ def test_save_does_not_update_runtime_config_when_user_config_cannot_be_read( ) -> None: merged_config = DummyConfig({'alias_dsn': {'existing': 'mysql://existing/db'}}) aliases = DsnAliases(merged_config, config_file='~/.myclirc') - monkeypatch.setattr(dsn_aliases_module, 'read_config_file', lambda _path: None) + monkeypatch.setattr(dsn_aliases_module, 'read_config_file', lambda _path, **kwargs: None) with pytest.raises(OSError, match=r"Unable to read config file '.*/\.myclirc'\."): aliases.save('new', 'mysql://new/db') diff --git a/test/pytests/test_favoritequeries.py b/test/pytests/test_favoritequeries.py index 29a193ce..64174bd1 100644 --- a/test/pytests/test_favoritequeries.py +++ b/test/pytests/test_favoritequeries.py @@ -77,28 +77,6 @@ def test_save_updates_existing_section_and_writes_config() -> None: assert config.write_calls == 1 -def test_save_quotes_multiline_query_for_disk_and_keeps_runtime_query_unquoted( - monkeypatch: pytest.MonkeyPatch, -) -> None: - write_config = DummyConfig() - runtime_config = DummyConfig() - favorites = FavoriteQueries(runtime_config, '/tmp/myclirc') - read_calls: list[tuple[str, bool]] = [] - - def read_config_file(path: str, preserve_quotes: bool = False) -> DummyConfig: - read_calls.append((path, preserve_quotes)) - return write_config - - monkeypatch.setattr(favoritequeries_module, 'read_config_file', read_config_file) - - favorites.save('report', 'select 1;\nselect 2;\n') - - assert read_calls == [('/tmp/myclirc', True)] - assert write_config['favorite_queries']['report'] == "'''select 1;\nselect 2'''" - assert write_config.write_calls == 1 - assert runtime_config['favorite_queries']['report'] == 'select 1;\nselect 2' - - def test_delete_removes_existing_favorite_and_writes_config() -> None: config = DummyConfig({'favorite_queries': {'demo': 'select 1'}}) favorites = FavoriteQueries(config)