From b9fe05265ddcc4a12c5daa1abfe113b9bc37e8fd Mon Sep 17 00:00:00 2001 From: Justin Raymond Date: Fri, 3 Apr 2026 11:25:18 -0400 Subject: [PATCH] Move log and history to \$XDG_STATE_HOME per XDG spec Log and history are state files (persist across restarts, not user-configuration), so they belong in \$XDG_STATE_HOME (~/.local/state/pgcli/) rather than \$XDG_CONFIG_HOME (~/.config/pgcli/). On first launch after upgrade, existing files at the old location are silently migrated to the new location. Errors during migration are logged rather than raised. Fixes #1497 Co-Authored-By: Claude Sonnet 4.6 --- AUTHORS | 1 + changelog.rst | 1 + pgcli/config.py | 31 ++++++++++++++++++++ pgcli/main.py | 8 +++-- pgcli/pgclirc | 4 +-- tests/test_config.py | 70 +++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 110 insertions(+), 5 deletions(-) diff --git a/AUTHORS b/AUTHORS index 79c650c5d..252718486 100644 --- a/AUTHORS +++ b/AUTHORS @@ -146,6 +146,7 @@ Contributors: * Charbel Jacquin (charbeljc) * Devadathan M B (devadathanmb) * Charalampos Stratakis + * Justin Raymond (jrraymond) Creator: -------- diff --git a/changelog.rst b/changelog.rst index da6c611f1..340a106d0 100644 --- a/changelog.rst +++ b/changelog.rst @@ -3,6 +3,7 @@ Upcoming (TBD) Features: --------- +* Move log and history files to ``$XDG_STATE_HOME`` (default ``~/.local/state/pgcli/``) per the XDG Base Directory Specification. Existing files at the old location (``~/.config/pgcli/``) are automatically migrated on first launch. * Add support for `\\T` prompt escape sequence to display transaction status (similar to psql's `%x`). * Add cursor shape support for vi mode. When ``vi = True``, the terminal cursor now reflects the current editing mode: beam in INSERT, block in NORMAL, underline in REPLACE. diff --git a/pgcli/config.py b/pgcli/config.py index 2b44a7bb7..62fc33305 100644 --- a/pgcli/config.py +++ b/pgcli/config.py @@ -1,3 +1,4 @@ +import logging import shutil import os import platform @@ -6,6 +7,8 @@ from typing import TextIO from configobj import ConfigObj +logger = logging.getLogger(__name__) + def config_location(): if "XDG_CONFIG_HOME" in os.environ: @@ -16,6 +19,16 @@ def config_location(): return expanduser("~/.config/pgcli/") +def state_location(): + if "XDG_STATE_HOME" in os.environ: + return "%s/pgcli/" % expanduser(os.environ["XDG_STATE_HOME"]) + elif platform.system() == "Windows": + # No XDG equivalent on Windows; use the same directory as config. + return config_location() + else: + return expanduser("~/.local/state/pgcli/") + + def load_config(usr_cfg, def_cfg=None): # avoid config merges when possible. For writing, we need an umerged config instance. # see https://github.com/dbcli/pgcli/issues/1240 and https://github.com/DiffSK/configobj/issues/171 @@ -29,6 +42,24 @@ def load_config(usr_cfg, def_cfg=None): return cfg +def migrate_file(old_path, new_path): + """Move old_path to new_path if old exists and new does not. + + Silently does nothing if old_path does not exist or new_path already exists. + Logs an error if the move fails. + """ + old_path = expanduser(old_path) + new_path = expanduser(new_path) + if not os.path.exists(old_path) or os.path.exists(new_path): + return + try: + ensure_dir_exists(new_path) + shutil.move(old_path, new_path) + logger.debug("Migrated %r to %r.", old_path, new_path) + except OSError as e: + logger.error("Failed to migrate %r to %r: %s", old_path, new_path, e) + + def ensure_dir_exists(path): parent_dir = expanduser(dirname(path)) os.makedirs(parent_dir, exist_ok=True) diff --git a/pgcli/main.py b/pgcli/main.py index 9ce15135d..e8b6c059f 100644 --- a/pgcli/main.py +++ b/pgcli/main.py @@ -64,6 +64,8 @@ get_casing_file, load_config, config_location, + state_location, + migrate_file, ensure_dir_exists, get_config, get_config_filename, @@ -560,7 +562,8 @@ def write_to_file(self, pattern, **_): def initialize_logging(self): log_file = self.config["main"]["log_file"] if log_file == "default": - log_file = config_location() + "log" + log_file = state_location() + "log" + migrate_file(config_location() + "log", log_file) ensure_dir_exists(log_file) log_level = self.config["main"]["log_level"] @@ -950,7 +953,8 @@ def run_cli(self): history_file = self.config["main"]["history_file"] if history_file == "default": - history_file = config_location() + "history" + history_file = state_location() + "history" + migrate_file(config_location() + "history", history_file) history = FileHistory(os.path.expanduser(history_file)) self.refresh_completions(history=history, persist_priorities="none") diff --git a/pgcli/pgclirc b/pgcli/pgclirc index 35ff41c5a..da53e2883 100644 --- a/pgcli/pgclirc +++ b/pgcli/pgclirc @@ -66,7 +66,7 @@ generate_aliases = False alias_map_file = # log_file location. -# In Unix/Linux: ~/.config/pgcli/log +# In Unix/Linux: ~/.local/state/pgcli/log (respects $XDG_STATE_HOME if set) # In Windows: %USERPROFILE%\AppData\Local\dbcli\pgcli\log # %USERPROFILE% is typically C:\Users\{username} log_file = default @@ -88,7 +88,7 @@ generate_casing_file = False case_column_headers = True # history_file location. -# In Unix/Linux: ~/.config/pgcli/history +# In Unix/Linux: ~/.local/state/pgcli/history (respects $XDG_STATE_HOME if set) # In Windows: %USERPROFILE%\AppData\Local\dbcli\pgcli\history # %USERPROFILE% is typically C:\Users\{username} history_file = default diff --git a/tests/test_config.py b/tests/test_config.py index 08fe74e65..b492767da 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -4,7 +4,7 @@ import pytest -from pgcli.config import ensure_dir_exists, skip_initial_comment +from pgcli.config import ensure_dir_exists, migrate_file, skip_initial_comment, state_location def test_ensure_file_parent(tmpdir): @@ -31,6 +31,74 @@ def test_ensure_other_create_error(tmpdir): ensure_dir_exists(str(rcfile)) +def test_state_location_default(monkeypatch): + monkeypatch.delenv("XDG_STATE_HOME", raising=False) + loc = state_location() + assert loc == os.path.expanduser("~/.local/state/pgcli/") + + +def test_state_location_xdg(monkeypatch, tmp_path): + monkeypatch.setenv("XDG_STATE_HOME", str(tmp_path)) + loc = state_location() + assert loc == str(tmp_path) + "/pgcli/" + + +def test_migrate_file_old_only(tmp_path): + old = tmp_path / "old" / "history" + old.parent.mkdir() + old.write_text("cmd1\ncmd2\n") + new = tmp_path / "new" / "history" + + migrate_file(str(old), str(new)) + + assert not old.exists() + assert new.read_text() == "cmd1\ncmd2\n" + + +def test_migrate_file_both_exist(tmp_path): + old = tmp_path / "old" / "history" + old.parent.mkdir() + old.write_text("old content\n") + new = tmp_path / "new" / "history" + new.parent.mkdir() + new.write_text("new content\n") + + migrate_file(str(old), str(new)) + + # neither file should be touched + assert old.read_text() == "old content\n" + assert new.read_text() == "new content\n" + + +def test_migrate_file_old_missing(tmp_path): + old = tmp_path / "old" / "history" + new = tmp_path / "new" / "history" + + migrate_file(str(old), str(new)) + + assert not new.exists() + + +def test_migrate_file_error_is_logged(tmp_path, caplog): + import logging + + old = tmp_path / "old" / "history" + old.parent.mkdir() + old.write_text("cmd1\n") + # make the destination parent directory read-only so the move fails + new_dir = tmp_path / "new" + new_dir.mkdir() + os.chmod(str(new_dir), stat.S_IREAD | stat.S_IEXEC) + + new = new_dir / "subdir" / "history" + + with caplog.at_level(logging.ERROR, logger="pgcli.config"): + migrate_file(str(old), str(new)) + + assert any("Failed to migrate" in r.message for r in caplog.records) + assert old.exists() # original untouched since move failed + + @pytest.mark.parametrize( "text, skipped_lines", (