From 4c219d8d507f14b7aed8fa571a5a60a4b89bedf8 Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Wed, 7 Oct 2026 11:02:04 -0400 Subject: [PATCH] allow unquoted comma-containing favorite queries to be saved in ~/.myclirc. ConfigObj returns a list when it sees a comma, so mycli was capable of saving an item it could not read back on restart. The fix is for values in the favorite_queries stanza to be treated differently, and never to be read in as a list. Favorite queries are all triple-quoted on save, and there is an on-the-fly hack to the reader to triple-quote any favorite from past saves which are not triple-quoted. There could be other stanzas of the config file which should have the same treatment. Shipping this with one minor limitation noted in comments: saving SQL with _both_ possible triple-quote delimiters as values internal to the SQL text is not possible. --- changelog.md | 1 + mycli/config.py | 74 +++++++- .../special_commands/favorite_queries.py | 5 +- mycli_test/pytests/test_config.py | 55 +++++- mycli_test/pytests/test_favoritequeries.py | 166 +++++++++++++++++- .../pytests/test_special_io_commands.py | 2 +- 6 files changed, 287 insertions(+), 16 deletions(-) diff --git a/changelog.md b/changelog.md index 25994b1c9..723508f5c 100644 --- a/changelog.md +++ b/changelog.md @@ -4,6 +4,7 @@ Upcoming (TBD) Bug Fixes -------- * Let the `beep_after_seconds` slow query alert happen before paged output. +* Allow comma-containing favorite queries in `~/.myclirc`. Internal diff --git a/mycli/config.py b/mycli/config.py index b19c77fa3..21600aaaa 100644 --- a/mycli/config.py +++ b/mycli/config.py @@ -14,24 +14,84 @@ logger = logging.getLogger(__name__) +class FavoriteQueryPreservingConfigObj(ConfigObj): + """When reading, quote SQL text on the fly which ConfigObj would otherwise interpret as a list.""" + + # Buglet: the use of _get_triple_quote() does not allow values which + # contain _both_ possible triple-quote delimiters to be read. + def _parse(self, infile: list[str]) -> None: + if not self.list_values or self.unrepr: + super()._parse(infile) + return + + lines = infile.copy() + in_favorites = False + index = 0 + while index < len(lines): + line = lines[index] + if not line.strip() or line.lstrip().startswith('#'): + index += 1 + continue + section = self._sectionmarker.match(line) + if section is not None: + _, opening, name, closing, _ = section.groups() + in_favorites = opening.count('[') == closing.count(']') == 1 and self._unquote(name) == 'favorite_queries' + else: + entry = self._keyword.match(line) + if entry is not None: + indent, key, value = entry.groups() + if value.startswith(('"""', "'''")): + # Skip complete multiline values, including section-like SQL. + try: + _, _, index = self._multiline(value, lines, index, len(lines) - 1) + except SyntaxError: + break + elif in_favorites and not value.startswith(('"', "'")): + match = self._nolistvalue.match(value) + if match is not None: + sql, comment = match.groups() + lines[index] = f'{indent}{key} = {self._get_triple_quote(sql) % sql} {comment or ""}' + index += 1 + + super()._parse(lines) + + +class TripleQuotedConfigValue(str): + """A value that must retain triple quoting when written.""" + + quote: str | None + + def __new__(cls, value: str, quote: str | None = None) -> 'TripleQuotedConfigValue': + instance = super().__new__(cls, value) + instance.quote = quote + return instance + + class LimiitedQuotePreservingConfigObj(ConfigObj): """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. + Preserve existing quoting and add triple quotes for new multiline values. """ + # Buglet: the use of _get_triple_quote() does not allow values which + # contain _both_ possible triple-quote delimiters to be saved. def __init__(self, *args, **kwargs): ConfigObj.__init__(self, *args, **kwargs) def _unquote(self, value): return value - def _quote(self, value, multiline=True): + def _multiline(self, value: str, infile: list[str], cur_index: int, maxline: int) -> tuple[str, str | None, int]: + parsed, comment, end_index = super()._multiline(value, infile, cur_index, maxline) + return TripleQuotedConfigValue(parsed, value[:3]), comment, end_index + + def _quote(self, value: str, multiline: bool = True) -> str: + if isinstance(value, TripleQuotedConfigValue): + if value.quote is not None: + return f'{value.quote}{value}{value.quote}' + return self._get_triple_quote(value) % value if '\n' in value: - return f"'''{value}'''" + return self._get_triple_quote(value) % value return value @@ -70,7 +130,7 @@ def read_config_file( if preserve_quotes: config = LimiitedQuotePreservingConfigObj(f, interpolation=False, encoding="utf8", list_values=False) else: - config = ConfigObj(f, interpolation=False, encoding="utf8", list_values=list_values) + config = FavoriteQueryPreservingConfigObj(f, interpolation=False, encoding="utf8", list_values=list_values) except ConfigObjError as e: if raise_errors: raise diff --git a/mycli/packages/special_commands/favorite_queries.py b/mycli/packages/special_commands/favorite_queries.py index be2f0765d..dc427a177 100644 --- a/mycli/packages/special_commands/favorite_queries.py +++ b/mycli/packages/special_commands/favorite_queries.py @@ -10,7 +10,7 @@ from jinja2 import meta, nodes from jinja2.sandbox import SandboxedEnvironment -from mycli.config import log, read_config_file, read_config_files +from mycli.config import TripleQuotedConfigValue, log, read_config_file, read_config_files logger = logging.getLogger(__name__) @@ -283,6 +283,7 @@ def reload(self) -> None: def _clean_query(self, query: str | None) -> str | None: if not query: return query + query = query.lstrip(' \t\n\r') query = query.rstrip(' \t\n\r') query = query.removesuffix(';') @@ -317,7 +318,7 @@ def save(self, name: str, query: str) -> None: 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, query) + self._set_query(config, name, TripleQuotedConfigValue(query)) try: config.write() except Exception: diff --git a/mycli_test/pytests/test_config.py b/mycli_test/pytests/test_config.py index ab721afdd..142d87f39 100644 --- a/mycli_test/pytests/test_config.py +++ b/mycli_test/pytests/test_config.py @@ -16,6 +16,7 @@ from mycli import config as config_module from mycli.config import ( LimiitedQuotePreservingConfigObj, + TripleQuotedConfigValue, _remove_pad, create_default_config, get_mylogin_cnf_path, @@ -181,6 +182,33 @@ def test_quote_preserving_config_retains_quotes_and_quotes_multiline_values() -> assert config._quote('first line\nsecond line') == "'''first line\nsecond line'''" +@pytest.mark.parametrize('quote', ['"""', "'''"]) +@pytest.mark.parametrize('value', ["SELECT 1, '#tag', 2", "SELECT 1,\n'#tag'"]) +def test_quote_preserving_config_retains_triple_quoted_values(quote: str, value: str) -> None: + text = f'[favorite_queries]\nq = {quote}{value}{quote}\n' + config = read_config_file(StringIO(text), preserve_quotes=True) + assert isinstance(config, LimiitedQuotePreservingConfigObj) + output = BytesIO() + config.write(output) + assert output.getvalue().decode('utf-8') == text + + +def test_quote_preserving_config_quotes_new_marked_value() -> None: + config = read_config_file(StringIO('[favorite_queries]\n'), preserve_quotes=True) + assert config is not None + query = "SELECT 1, '#tag', 2" + config['favorite_queries']['q'] = TripleQuotedConfigValue(query) + output = BytesIO() + + config.write(output) + + text = output.getvalue().decode('utf-8') + assert text == f"[favorite_queries]\nq = '''{query}'''\n" + reloaded = read_config_file(StringIO(text), raise_errors=True) + assert reloaded is not None + assert reloaded['favorite_queries']['q'] == query + + 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'}}) @@ -266,7 +294,7 @@ def test_read_config_file_permission_error(monkeypatch, caplog) -> None: def raise_oserror(*_args, **_kwargs): raise OSError(13, 'denied', '/tmp/test.cnf') - monkeypatch.setattr(config_module, 'ConfigObj', raise_oserror) + monkeypatch.setattr(config_module, 'FavoriteQueryPreservingConfigObj', raise_oserror) with caplog.at_level(logging.WARNING, logger='mycli.config'): assert read_config_file('/tmp/test.cnf') is None @@ -281,13 +309,36 @@ def test_read_config_file_can_raise_parse_errors(tmp_path) -> None: read_config_file(str(invalid_path), raise_errors=True) +@pytest.mark.parametrize('value', ['"""unterminated', "'''closed''' trailing"]) +def test_read_config_file_raises_for_invalid_multiline_value(value: str) -> None: + contents = f'[favorite_queries]\nvalid = SELECT 1\nbroken = {value}\n' + + with pytest.raises(ConfigObjError) as exc_info: + read_config_file(StringIO(contents), raise_errors=True) + + assert exc_info.value.line_number == 3 + assert exc_info.value.config['favorite_queries']['valid'] == 'SELECT 1' + + +def test_read_config_file_recovers_from_invalid_multiline_value(caplog: pytest.LogCaptureFixture) -> None: + contents = '[favorite_queries]\nvalid = SELECT 1\nbroken = """unterminated\n' + + with caplog.at_level(logging.WARNING, logger='mycli.config'): + config = read_config_file(StringIO(contents)) + + assert config is not None + assert config['favorite_queries'] == {'valid': 'SELECT 1'} + assert 'Unable to parse line 3' in caplog.text + assert 'Using successfully parsed config values.' in caplog.text + + def test_read_config_file_can_raise_io_errors(monkeypatch) -> None: error = OSError(13, 'denied', '/tmp/test.cnf') def raise_oserror(*_args, **_kwargs): raise error - monkeypatch.setattr(config_module, 'ConfigObj', raise_oserror) + monkeypatch.setattr(config_module, 'FavoriteQueryPreservingConfigObj', raise_oserror) with pytest.raises(OSError) as exc_info: read_config_file('/tmp/test.cnf', raise_errors=True) diff --git a/mycli_test/pytests/test_favoritequeries.py b/mycli_test/pytests/test_favoritequeries.py index 3f12025db..52acd96cc 100644 --- a/mycli_test/pytests/test_favoritequeries.py +++ b/mycli_test/pytests/test_favoritequeries.py @@ -1,13 +1,171 @@ from collections.abc import Mapping +from io import StringIO import logging from pathlib import Path import pytest +from mycli.config import read_config_file from mycli.packages.special_commands import favorite_queries as favorite_queries_module +from mycli.packages.special_commands.dsn_aliases import DsnAliases from mycli.packages.special_commands.favorite_queries import FavoriteQueries, FavoriteQueryReloadError +@pytest.mark.parametrize( + 'query', + [ + "SELECT 1, 'hello', 2", + "SELECT 'a, b', 2", + "SELECT 1, 'x,y', 2", + 'SELECT 1, "x,y", 2', + 'SELECT 1, 2', + "SELECT 1, 'hello' AS greeting", + "SELECT CONCAT('a', 'b')", + ], +) +def test_unquoted_comma_queries_preserve_sql(query: str) -> None: + config = read_config_file(StringIO(f'[favorite_queries]\nq = {query}\n')) + assert config is not None + assert FavoriteQueries(config).get('q') == query + + +@pytest.mark.parametrize('header', ['[ favorite_queries ]', '[\tfavorite_queries\t]', '[ "favorite_queries" ]']) +def test_spaced_favorite_section_preserves_sql(header: str) -> None: + query = "SELECT 1, 'hello' AS greeting" + config = read_config_file(StringIO(f'{header}\nq = {query}\n'), raise_errors=True) + assert config is not None + assert FavoriteQueries(config).get('q') == query + + +def test_nested_spaced_favorite_section_keeps_list_parsing() -> None: + config = read_config_file(StringIO('[main]\n[[ favorite_queries ]]\nitems = a, b\n'), raise_errors=True) + assert config is not None + assert config['main']['favorite_queries']['items'] == ['a', 'b'] + + +@pytest.mark.parametrize( + 'query', + [ + 'SELECT 1', + "SELECT 1, 'hello' AS greeting", + "SELECT CONCAT('a', 'b')", + "SELECT 1, '#tag', 2", + 'SELECT "#tag"', + '''SELECT '#tag', "value"''', + "SELECT 1,\n'#tag'", + 'SELECT \'"""\' AS quoted', + ], +) +def test_save_and_reload_quoted_query(tmp_path: Path, query: str) -> None: + user_file = tmp_path / 'myclirc' + user_file.write_text('', encoding='utf-8') + favorites = FavoriteQueries(DummyConfig(), str(user_file)) + favorites.save('q', query) + assert favorites.get('q') == query + assert user_file.read_text(encoding='utf-8').split('q = ', 1)[1].startswith(('"', "'")) + + favorites.reload() + + assert favorites.get('q') == query + + +@pytest.mark.parametrize('operation', ['save_favorite', 'delete_favorite', 'save_dsn', 'delete_dsn']) +def test_config_updates_preserve_saved_favorite_quoting(tmp_path: Path, operation: str) -> None: + path = tmp_path / 'myclirc' + path.write_text('[favorite_queries]\nother = SELECT 3\n[alias_dsn]\nother = mysql://localhost\n', encoding='utf-8') + config = read_config_file(str(path)) + assert config is not None + favorites = FavoriteQueries(config, str(path)) + query = "SELECT 1, '#tag', 2" + favorites.save('tags', query) + aliases = DsnAliases(config, config_file=str(path)) + + if operation == 'save_favorite': + favorites.save('other', 'SELECT 4') + elif operation == 'delete_favorite': + favorites.delete('other') + elif operation == 'save_dsn': + aliases.save('other', 'mysql://localhost/mysql') + else: + aliases.delete('other') + + favorites.reload() + assert favorites.get('tags') == query + + +def test_unquoted_comma_query_with_terminator() -> None: + config = read_config_file(StringIO("[favorite_queries]\nq = SELECT 1, 'hello';\n"), raise_errors=True) + assert config is not None + assert config['favorite_queries']['q'] == "SELECT 1, 'hello';" + + +def test_multiline_query_section_text_does_not_change_list_parsing() -> None: + config = read_config_file( + StringIO('[favorite_queries]\nq = """SELECT 1,\n[main]\n2"""\nother = SELECT CONCAT(\'a\', \'b\')\n[main]\nitems = a, b\n'), + raise_errors=True, + ) + assert config is not None + assert config['favorite_queries']['q'] == 'SELECT 1,\n[main]\n2' + assert config['favorite_queries']['other'] == "SELECT CONCAT('a', 'b')" + assert config['main']['items'] == ['a', 'b'] + + +def test_comma_query_preserves_sql_with_config_comment() -> None: + config = read_config_file(StringIO("[favorite_queries]\nq = SELECT 1, 'hello', 2 # comment\n")) + assert config is not None + assert FavoriteQueries(config).get('q') == "SELECT 1, 'hello', 2" + + +@pytest.mark.parametrize('quote', ['"""', "'''"]) +@pytest.mark.parametrize('indent', ['', ' ', '\t']) +def test_commented_multiline_example_does_not_skip_favorites(quote: str, indent: str) -> None: + query = "SELECT 1, 'hello' AS greeting" + config = read_config_file( + StringIO(f'{indent}# example = {quote}\n\n[favorite_queries]\nq = {query}\n'), + raise_errors=True, + ) + assert config is not None + assert FavoriteQueries(config).get('q') == query + + +def test_favorite_query_comment_text_is_preserved() -> None: + comment = ' # example = SELECT 1, 2' + config = read_config_file(StringIO(f'[favorite_queries]\n{comment}\nq = SELECT 3\n'), raise_errors=True) + assert config is not None + assert config['favorite_queries'].comments['q'] == [comment] + + +@pytest.mark.parametrize('wrapper', ['"', "'", '"""', "'''"]) +def test_quoted_comma_query_keeps_config_quote_semantics(wrapper: str) -> None: + config = read_config_file(StringIO(f'[favorite_queries]\nq = {wrapper}SELECT 1, 2{wrapper}\n')) + assert config is not None + assert FavoriteQueries(config).get('q') == 'SELECT 1, 2' + + +def test_favorite_sql_preservation_does_not_change_other_lists() -> None: + config = read_config_file(StringIO('[main]\nitems = a, b\n[favorite_queries]\nq = SELECT 1, 2\n')) + assert config is not None + assert config['main']['items'] == ['a', 'b'] + assert config['favorite_queries']['q'] == 'SELECT 1, 2' + + +def test_reload_preserves_unquoted_comma_query(tmp_path: Path) -> None: + user_file = tmp_path / 'myclirc' + query = "SELECT 1, 'x,y', 2" + user_file.write_text(f'[favorite_queries]\nq = {query}\n', encoding='utf-8') + favorites = FavoriteQueries(DummyConfig(), str(user_file)) + + favorites.reload() + + assert favorites.get('q') == query + + +def test_partial_config_preserves_unquoted_comma_query() -> None: + config = read_config_file(StringIO("[broken\n[favorite_queries]\nq = SELECT 1, 'hello', 2\n")) + assert config is not None + assert FavoriteQueries(config).get('q') == "SELECT 1, 'hello', 2" + + class DummyConfig(dict): def __init__(self, initial: Mapping[str, object] | None = None) -> None: super().__init__(initial or {}) @@ -260,7 +418,7 @@ def test_reload_shared_failure_warns_and_uses_user_favorites( [ (b'[favorite_queries]\ninvalid = \xff\n', 'unable to read user configuration'), (b'favorite_queries = invalid\n', r'invalid \[favorite_queries\] section'), - (b'[favorite_queries]\ninvalid = select 1, select 2\n', r'invalid \[favorite_queries\] section'), + (b'[favorite_queries]\n[[invalid]]\nquery = select 1\n', r'invalid \[favorite_queries\] section'), ], ) def test_reload_invalid_config_preserves_runtime_favorites( @@ -452,7 +610,7 @@ def test_save_preserves_user_config_comments_and_excludes_merged_values(tmp_path [favorite_queries] # Existing favorite. existing = select 1 -new = select 2 +new = '''select 2''' # User footer. """ ) @@ -477,7 +635,7 @@ def test_save_overwrites_favorite_without_removing_its_comment(tmp_path: Path) - config_file.read_text(encoding='utf-8') == """[favorite_queries] # Keep this explanation. -report = select 2 +report = '''select 2''' """ ) assert merged_config['favorite_queries']['report'] == 'select 2' @@ -543,7 +701,7 @@ def test_save_shared_favorite_override_writes_only_user_config(tmp_path: Path) - favorites.save('report', 'select 2') assert shared_file.read_text(encoding='utf-8') == shared_contents - assert config_file.read_text(encoding='utf-8') == '# User config.\n[favorite_queries]\nreport = select 2\n' + assert config_file.read_text(encoding='utf-8') == "# User config.\n[favorite_queries]\nreport = '''select 2'''\n" assert favorites.get('report') == 'select 2' diff --git a/mycli_test/pytests/test_special_io_commands.py b/mycli_test/pytests/test_special_io_commands.py index 25dcbe568..ccee39050 100644 --- a/mycli_test/pytests/test_special_io_commands.py +++ b/mycli_test/pytests/test_special_io_commands.py @@ -1233,7 +1233,7 @@ def test_favorite_edit_command_creates_local_override_for_shared_query(monkeypat assert io_commands.favorite(arg='edit report') == [SQLResult(status='report: Edited.')] assert shared_file.read_text(encoding='utf-8') == shared_contents - assert 'report = select 2' in config_file.read_text(encoding='utf-8') + assert "report = '''select 2'''" in config_file.read_text(encoding='utf-8') assert favorite_queries.get('report') == 'select 2'