diff --git a/.importlinter b/.importlinter index b5704e4..6ccde1e 100644 --- a/.importlinter +++ b/.importlinter @@ -15,6 +15,7 @@ containers = exhaustive = True exhaustive_ignores = _core + _display _types ignore_imports = yamltrip.document -> yamltrip diff --git a/doc/specs/2026-05-22-error-message-tuple-repr-design.md b/doc/specs/2026-05-22-error-message-tuple-repr-design.md new file mode 100644 index 0000000..8984c36 --- /dev/null +++ b/doc/specs/2026-05-22-error-message-tuple-repr-design.md @@ -0,0 +1,72 @@ +# Design: Human-Readable Path Formatting in NodeTypeError Messages + +**Date:** 2026-05-22 +**Status:** Approved + +## Problem + +`NodeTypeError` messages interpolate raw Python tuples into error strings: + +```python +msg = f"Value at {keys} is not a list" +# Produces: "Value at ('repos',) is not a list" +``` + +The tuple syntax (`('repos',)`) is implementation noise that leaks internal representation to users. + +## Solution + +Introduce a `format_path` helper in a new `_display.py` module that converts key tuples to a human-readable path string using ` > ` as separator. + +### Output Examples + +| Input | Output | +|-------|--------| +| `('repos',)` | `repos` | +| `('repos', 0, 'steps')` | `repos > 0 > steps` | +| `()` | `` | + +### Separator Choice: ` > ` (ASCII arrow) + +- Unambiguous: no YAML key naturally contains ` > ` +- Works in all terminals and log viewers (no UTF-8 issues) +- Easy to grep and type +- Reads naturally as "drill into" + +## New Module: `src/yamltrip/_display.py` + +Single function: + +```python +def format_path(keys: tuple[str | int, ...]) -> str: + if not keys: + return "" + return " > ".join(str(k) for k in keys) +``` + +## Affected Call Sites (3) + +All in `src/yamltrip/document.py`, all with the same pattern: + +1. `remove_from_list` — type-checks retrieved value is a list +2. `ensure_in_list` — type-checks retrieved value is a list +3. `find_index` — type-checks retrieved value is a list + +Each changes from: +```python +msg = f"Value at {keys} is not a list" +``` +To: +```python +msg = f"Value at {format_path(keys)} is not a list" +``` + +## Out of Scope + +- Rust-originating error messages (lines that use `msg = str(e)`) — these come from the Rust layer and don't contain tuple repr. +- Reformatting other exception types. + +## Testing + +- Unit test for `format_path` covering: single key, multi-key, integer indices, empty tuple. +- Update any existing tests that assert on `NodeTypeError` message content. diff --git a/src/yamltrip/_display.py b/src/yamltrip/_display.py new file mode 100644 index 0000000..e5454f4 --- /dev/null +++ b/src/yamltrip/_display.py @@ -0,0 +1,28 @@ +"""Display helpers for human-readable formatting.""" + +from __future__ import annotations + + +def format_path(keys: tuple[str | int, ...]) -> str: + """Format a key tuple as a human-readable path string. + + Examples: + >>> format_path(("repos",)) + 'repos' + >>> format_path(("repos", 0, "steps")) + 'repos > 0 > steps' + >>> format_path(("a", "b>c")) + "a > 'b>c'" + >>> format_path(()) + '' + """ + if not keys: + return "" + + def _fmt(k: str | int) -> str: + s = str(k) + if ">" in s: + return f"'{s}'" + return s + + return " > ".join(_fmt(k) for k in keys) diff --git a/src/yamltrip/document.py b/src/yamltrip/document.py index 92f0dec..677a17d 100644 --- a/src/yamltrip/document.py +++ b/src/yamltrip/document.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Any, cast from yamltrip import _core +from yamltrip._display import format_path from yamltrip.errors import ( KeyExistsError, KeyMissingError, @@ -418,7 +419,7 @@ def remove_from_list(self, *keys: KeyPart, values: Sequence[Any]) -> Document: """Remove all occurrences of given values from the sequence at path.""" current_list = self[keys] if not isinstance(current_list, list): - msg = f"Value at {keys} is not a list" + msg = f"Value at {format_path(keys)} is not a list" raise NodeTypeError(msg) values_list = list(values) @@ -470,7 +471,7 @@ def ensure_in_list( current = self[keys] if not isinstance(current, list): - msg = f"Value at {keys} is not a list" + msg = f"Value at {format_path(keys)} is not a list" raise NodeTypeError(msg) if where is None: @@ -557,7 +558,7 @@ def find_index(self, *keys: KeyPart, where: dict[str, Any]) -> int | None: value = self[keys] if not isinstance(value, list): - msg = f"Value at {keys} is not a list" + msg = f"Value at {format_path(keys)} is not a list" raise NodeTypeError(msg) for i, item in enumerate(value): diff --git a/tests/test_display.py b/tests/test_display.py new file mode 100644 index 0000000..27dafef --- /dev/null +++ b/tests/test_display.py @@ -0,0 +1,21 @@ +from yamltrip._display import format_path + + +class TestFormatPath: + def test_single_string_key(self): + assert format_path(("repos",)) == "repos" + + def test_multi_key_path(self): + assert format_path(("repos", 0, "steps")) == "repos > 0 > steps" + + def test_integer_index(self): + assert format_path(("items", 2)) == "items > 2" + + def test_empty_tuple_returns_root(self): + assert format_path(()) == "" + + def test_key_containing_gt_is_quoted(self): + assert format_path(("a", "b>c")) == "a > 'b>c'" + + def test_bare_gt_key_is_quoted(self): + assert format_path((">",)) == "'>'" diff --git a/tests/test_document.py b/tests/test_document.py index 16149d1..a32b1f6 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -592,6 +592,26 @@ def test_extend_list_on_scalar_raises_node_type_error(self): with pytest.raises(NodeTypeError): doc.extend_list("name", values=["a", "b"]) + def test_remove_from_list_error_message_uses_path_format(self): + doc = Document("name: foo\n") + with pytest.raises(NodeTypeError, match="Value at name is not a list"): + doc.remove_from_list("name", values=["foo"]) + + def test_find_index_error_message_uses_path_format(self): + doc = Document("name: foo\n") + with pytest.raises(NodeTypeError, match="Value at name is not a list"): + doc.find_index("name", where={"k": "v"}) + + def test_ensure_in_list_error_message_uses_path_format(self): + doc = Document("name: foo\n") + with pytest.raises(NodeTypeError, match="Value at name is not a list"): + doc.ensure_in_list("name", value="bar") + + def test_error_message_multi_key_path(self): + doc = Document("a:\n b: scalar\n") + with pytest.raises(NodeTypeError, match=r"Value at a > b is not a list"): + doc.remove_from_list("a", "b", values=["x"]) + class TestDocumentFindIndex: def test_finds_first_match(self):