From 71f293161eec2318b2747e0606ddab43ed16c03b Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Thu, 22 Oct 2026 20:43:40 -0600 Subject: [PATCH] fix(cfg): show only the duplicate-key warning, once --- docs/guide/cli-reference.md | 2 +- tests/commands/test_cfg.py | 29 +++++++++++++++++++---------- unshackle/commands/cfg.py | 13 ++++++++----- unshackle/core/config.py | 15 ++++++--------- 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index b17eafda..74788de2 100644 --- a/docs/guide/cli-reference.md +++ b/docs/guide/cli-reference.md @@ -339,7 +339,7 @@ unshackle cfg [KEY] [VALUE] [OPTIONS] | `--list` | List all set configuration values. | !!! warning "Comments are stripped" - Writing a value through `cfg` rewrites the configuration file and **removes all comments** from it. If your config relies on comments, edit it by hand instead. Setting a value and using `--unset` together is an error. If a config key is set twice in the same block, `cfg` stops and gives the line numbers of the two copies. Other commands warn with the same line numbers and use the later copy. + Writing a value through `cfg` rewrites the configuration file and **removes all comments** from it. If your config relies on comments, edit it by hand instead. Setting a value and using `--unset` together is an error. If a config key is set twice in the same block, every command warns with the line numbers of the two copies and uses the later copy. `cfg` stops without a change. !!! example ```shell diff --git a/tests/commands/test_cfg.py b/tests/commands/test_cfg.py index 899d2ca8..5327e1c0 100644 --- a/tests/commands/test_cfg.py +++ b/tests/commands/test_cfg.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging from pathlib import Path import pytest @@ -15,19 +16,25 @@ def config_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: return path -def test_a_duplicate_key_names_its_lines_but_no_key_or_value(config_file: Path) -> None: +def test_a_duplicate_key_logs_its_lines_once_and_names_no_key_or_value( + config_file: Path, caplog: pytest.LogCaptureFixture +) -> None: config_file.write_text( "theme: rainbow\ncredentials:\n EXAMPLE: first-secret\n OTHER: other\n EXAMPLE: second-secret\n" "serve:\n users:\n key-secret: {tier: a}\n key-secret: {tier: b}\n", encoding="utf-8", ) - result = CliRunner().invoke(cfg_module.cfg, ["theme"]) - assert isinstance(result.exception, SystemExit), result.exception # a ClickException, not a crash + with caplog.at_level(logging.WARNING, logger="config"): + result = CliRunner().invoke(cfg_module.cfg, ["theme"]) + CliRunner().invoke(cfg_module.cfg, ["theme"]) + assert isinstance(result.exception, SystemExit), result.exception assert result.exit_code == 1 - text = " ".join(result.output.replace("│", " ").split()) # the error panel wraps its lines - assert "unshackle.yaml: duplicate key on lines 3 and 5, lines 8 and 9. Unshackle uses the later one." in text - assert "secret" not in result.output - assert "EXAMPLE" not in result.output + assert caplog.messages == [ + f"{config_file}: duplicate key on lines 3 and 5, lines 8 and 9. Unshackle uses the later one." + ] + assert "Error" not in result.output and "lines" not in result.output + for output in (result.output, caplog.text): + assert "secret" not in output and "EXAMPLE" not in output def test_a_duplicate_key_blocks_a_write(config_file: Path) -> None: @@ -45,7 +52,9 @@ def test_a_valid_config_still_reads_a_key(config_file: Path) -> None: assert "rainbow" in result.output -def test_a_duplicate_only_ruamel_sees_still_gives_its_line(config_file: Path) -> None: +def test_a_duplicate_only_ruamel_sees_still_gives_its_line(config_file: Path, caplog: pytest.LogCaptureFixture) -> None: config_file.write_text("a: 1\n1: x\n0x1: y\n", encoding="utf-8") - result = CliRunner().invoke(cfg_module.cfg, ["a"]) - assert "unshackle.yaml: duplicate key on line 3." in " ".join(result.output.replace("│", " ").split()) + with caplog.at_level(logging.WARNING): + result = CliRunner().invoke(cfg_module.cfg, ["a"]) + assert result.exit_code == 1 + assert caplog.messages == [f"{config_file}: duplicate key on line 3. Unshackle uses the later one."] diff --git a/unshackle/commands/cfg.py b/unshackle/commands/cfg.py index 526f1f34..2cdc9983 100644 --- a/unshackle/commands/cfg.py +++ b/unshackle/commands/cfg.py @@ -6,7 +6,7 @@ import click from ruamel.yaml import YAML from ruamel.yaml.constructor import DuplicateKeyError -from unshackle.core.config import config, duplicate_key_lines, duplicate_key_message, get_config_path +from unshackle.core.config import config, duplicate_key_lines, get_config_path, load_yaml from unshackle.core.constants import context_settings @@ -45,10 +45,13 @@ def cfg(ctx: click.Context, key: str, value: str, unset: bool, list_: bool) -> N try: data = yaml.load(config_path) except DuplicateKeyError as e: - pairs = duplicate_key_lines(yaml.compose(config_path)) - raise click.ClickException( - duplicate_key_message(config_path.name, pairs, e.problem_mark.line + 1) - ) from None + if duplicate_key_lines(yaml.compose(config_path)): + load_yaml(config_path) + else: + log.warning( + f"{config_path}: duplicate key on line {e.problem_mark.line + 1}. Unshackle uses the later one." + ) + sys.exit(1) if not data: log.warning("No config file was found or it has no data, yet") diff --git a/unshackle/core/config.py b/unshackle/core/config.py index 1a36e8ad..f704c233 100644 --- a/unshackle/core/config.py +++ b/unshackle/core/config.py @@ -406,12 +406,6 @@ def duplicate_key_lines(root: Any) -> list[tuple[int, int]]: return sorted(pairs) -def duplicate_key_message(name: str, pairs: list[tuple[int, int]], line: int = 0) -> str: - """Say where a file repeats a key: each (first, repeat) pair, or `line` when no pair is known.""" - where = ", ".join(f"lines {first} and {repeat}" for first, repeat in pairs) or f"line {line}" - return f"{name}: duplicate key on {where}. Unshackle uses the later one." - - _warned: set[str] = set() @@ -422,9 +416,12 @@ def load_yaml(path: Path) -> Any: node = loader.get_single_node() if node is None: return None - if (pairs := duplicate_key_lines(node)) and (message := duplicate_key_message(str(path), pairs)) not in _warned: - _warned.add(message) - logging.getLogger("config").warning(message) + if pairs := duplicate_key_lines(node): + where = ", ".join(f"lines {first} and {repeat}" for first, repeat in pairs) + message = f"{path}: duplicate key on {where}. Unshackle uses the later one." + if message not in _warned: + _warned.add(message) + logging.getLogger("config").warning(message) return loader.construct_document(node) finally: loader.dispose()