From f83a594c916bd5b2efc35a5e6401d7dfef9a6823 Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Thu, 22 Oct 2026 19:45:46 -0600 Subject: [PATCH] fix(cfg): give the lines of each duplicate config key instead of crashing --- docs/guide/cli-reference.md | 2 +- tests/commands/test_cfg.py | 58 +++++++++++++++++++++++++++++++++++++ unshackle/commands/cfg.py | 36 ++++++++++++++++++++++- 3 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 tests/commands/test_cfg.py diff --git a/docs/guide/cli-reference.md b/docs/guide/cli-reference.md index a2d0e9a1..8b328085 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. + 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 use the later copy. !!! example ```shell diff --git a/tests/commands/test_cfg.py b/tests/commands/test_cfg.py new file mode 100644 index 00000000..1f370232 --- /dev/null +++ b/tests/commands/test_cfg.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from unshackle.commands import cfg as cfg_module + + +@pytest.fixture +def config_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + path = tmp_path / "unshackle.yaml" + monkeypatch.setattr(cfg_module, "get_config_path", lambda: path) + return path + + +def test_a_duplicate_key_names_its_lines_but_no_key_or_value(config_file: Path) -> 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 + 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 + + +def test_a_duplicate_key_blocks_a_write(config_file: Path) -> None: + original = "theme: rainbow\ncredentials:\n EXAMPLE: a\n EXAMPLE: b\n" + config_file.write_text(original, encoding="utf-8") + result = CliRunner().invoke(cfg_module.cfg, ["theme", "plain"]) + assert result.exit_code == 1 + assert config_file.read_text(encoding="utf-8") == original + + +def test_a_valid_config_still_reads_a_key(config_file: Path) -> None: + config_file.write_text("theme: rainbow\n", encoding="utf-8") + result = CliRunner().invoke(cfg_module.cfg, ["theme"]) + assert result.exit_code == 0, result.output + assert "rainbow" in result.output + + +def test_duplicate_key_lines_follows_aliases_without_looping() -> None: + from ruamel.yaml import YAML + + root = YAML().compose("base: &b\n x: 1\n x: 2\n self: *b\nother: *b\nlist:\n - y: 1\n y: 2\n") + assert cfg_module.duplicate_key_lines(root) == [(2, 3), (7, 8)] + + +def test_a_duplicate_only_ruamel_sees_still_gives_its_line(config_file: Path) -> 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()) diff --git a/unshackle/commands/cfg.py b/unshackle/commands/cfg.py index 15c4f77c..6a178374 100644 --- a/unshackle/commands/cfg.py +++ b/unshackle/commands/cfg.py @@ -4,11 +4,37 @@ import sys import click from ruamel.yaml import YAML +from ruamel.yaml.constructor import DuplicateKeyError +from ruamel.yaml.nodes import MappingNode, Node, ScalarNode, SequenceNode from unshackle.core.config import config, get_config_path from unshackle.core.constants import context_settings +def duplicate_key_lines(root: Node) -> list[tuple[int, int]]: + """Return (first line, repeat line), counted from 1, for each repeated scalar key in the node tree.""" + pairs, stack, visited = [], [root], set() + while stack: + node = stack.pop() + if node in visited: # an alias reuses its node, and can point back at itself + continue + visited.add(node) + if isinstance(node, MappingNode): + first: dict[tuple[str, str], int] = {} + for key_node, value_node in node.value: + if isinstance(key_node, ScalarNode): + key = (key_node.tag, key_node.value) + line = key_node.start_mark.line + 1 + if key in first: + pairs.append((first[key], line)) + else: + first[key] = line + stack += [key_node, value_node] + elif isinstance(node, SequenceNode): + stack += node.value + return sorted(pairs) + + @click.command( short_help="Manage configuration values for the program and its services.", context_settings=context_settings ) @@ -41,7 +67,15 @@ def cfg(ctx: click.Context, key: str, value: str, unset: bool, list_: bool) -> N config_path = get_config_path() or config.directories.user_configs / config.filenames.root_config if config_path.exists(): - data = yaml.load(config_path) + try: + data = yaml.load(config_path) + except DuplicateKeyError as e: + pairs = duplicate_key_lines(yaml.compose(config_path)) + where = ", ".join(f"lines {first} and {repeat}" for first, repeat in pairs) + where = where or f"line {e.problem_mark.line + 1}" + raise click.ClickException( + f"{config_path.name}: duplicate key on {where}. Unshackle uses the later one." + ) from None if not data: log.warning("No config file was found or it has no data, yet")