Don't rewrite ~/.myclirc quotes on /dsn save

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

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
 
 

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

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:

test/pytests/test_config.py

@@ -14,13 +14,15 @@
 
 from mycli import config as config_module
 from mycli.config import (
+    LimiitedQuotePreservingConfigObj,
     _remove_pad,
     create_default_config,
     get_mylogin_cnf_path,
     log,
     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)
 

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')

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)