From a115dc7557eacb5e64401db5c41e8f651642ec4a Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Tue, 4 Aug 2026 06:38:11 -0400 Subject: [PATCH] don't rewrite ~/.myclirc quotes on /dsn save The user's own quoting, or lack of quoting, could be rewritten if saving or deleting a DSN alias via the REPL. If the dotfile is kept under revision control, this can create needless churn. Moves implementation of manual triplequoting from favorite queries to the ConfigObj subclass. The previous implementation only guaranteed maintaining triplequotes within favorite queries, during a favorite queries save. There is one limitation explained in the docstring: the style of any triplequoted multiline value cannot be maintained perfectly. --- changelog.md | 1 + mycli/config.py | 10 +++- mycli/packages/special/dsn_aliases.py | 2 +- mycli/packages/special/favoritequeries.py | 7 +-- test/pytests/test_config.py | 67 +++++++++++++++++++++++ test/pytests/test_dsn_aliases.py | 2 +- test/pytests/test_favoritequeries.py | 22 -------- 7 files changed, 80 insertions(+), 31 deletions(-) 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)