Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ doc.append("items", value="c")
doc.insert("items", index=1, value="between") # positional insert
doc.extend_list("items", values=["d", "e"])
doc.remove_from_list("items", values=["a"])
doc.ensure_in_list("items", value="c") # no-op if already present
doc.ensure_in_list("repos", where={"name": "x"}, value={"name": "x", "version": "1.0"})
doc.sync("items", value=["a", "new", "b"]) # minimal diff-and-patch
doc.find_index("repos", where={"id": "x"}) # find in list-of-dicts; returns int | None

Expand Down
176 changes: 176 additions & 0 deletions doc/specs/2026-05-22-ensure-in-list-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
# Design: `ensure_in_list` — Idempotent List Addition

**Date:** 2026-05-22
**Status:** Approved

## Problem

"Add this value to the list if it's not already there" requires manual
check-then-append boilerplate:

```python
items = doc["items"]
if "new_hook" not in items:
doc = doc.append("items", value="new_hook")
```

This is the most common pattern when managing hook lists, dependency arrays,
and plugin registrations.

## API

### Document (immutable, returns new Document)

```python
def ensure_in_list(
self,
*keys: KeyPart,
value: Any,
where: dict[str, Any] | None = None,
) -> Document:
```

### Editor (mutable, returns None)

```python
def ensure_in_list(
self,
*keys: KeyPart,
value: Any,
where: dict[str, Any] | None = None,
) -> None:
```

## Semantics

### Scalar matching (`where=None`)

Check if `value` is already in the list via Python `==`. No-op if present,
append if not.

```python
doc = doc.ensure_in_list("hooks", value="pre-commit")
# If "pre-commit" already in doc["hooks"], returns self unchanged.
# Otherwise appends it.
```

### Dict matching (`where={...}`)

Check if any item in the list has all key/value pairs in `where` (AND
semantics, equality via Python `==`). No-op if a match exists, append `value`
if not.

The matched item is NOT updated — `ensure_in_list` guarantees presence, not
correctness. Users who need update-in-place should use `find_index` + `replace`
or `sync`.

```python
doc = doc.ensure_in_list(
"repos",
where={"repo": "https://github.com/pre-commit/mirrors-prettier"},
value={
"repo": "https://github.com/pre-commit/mirrors-prettier",
"rev": "v3.0.0",
"hooks": [{"id": "prettier"}],
},
)
```

### Path missing — create the list

If the path does not exist, create intermediate mappings (like `upsert`) and
set the value to `[value]`.

```python
doc = doc.ensure_in_list("new_list", value="first_item")
# Result: new_list:\n- first_item
```

### Path exists but is not a list

Raise `NodeTypeError`.

### Flow sequence fallback

Same pattern as `append`: catch the flow-sequence error from yamlpatch and fall
back to reading the current list, checking membership in Python, and replacing
with the new list if needed.

### Idempotence

Calling `ensure_in_list` twice with the same arguments always produces the same
result. This is guaranteed by the no-op-if-present semantics.

## What this does NOT include

- **No `values` plural parameter.** Caller loops for multiple items. This
avoids ambiguity about `where` + `values` interaction.
- **No update-in-place.** If `where` matches an existing item, returns self
unchanged regardless of whether `value` differs from the matched item.
- **No `upsert_in_list`.** Deferred until demand appears.

## Error conditions

| Condition | Behavior |
|-----------|----------|
| Path missing | Create list with `[value]` |
| Path is not a list | Raise `NodeTypeError` |
| `where` is empty dict | Raise `ValueError` (matches `find_index`) |
| `where` provided but list items aren't dicts | No match found → append |

## Implementation approach

Pure Python in `document.py`. No Rust changes needed — this composes existing
primitives (`__contains__`, `__getitem__`, `append`, `upsert`).

Rough logic:

```python
def ensure_in_list(self, *keys, value, where=None):
if where is not None:
if not where:
raise ValueError("where must be a non-empty dict")

# Check if path exists
route = _make_route(keys)
if not self._core_doc.query_exists(route):
# Path missing: create with [value]
return self.upsert(*keys, value=[value])

current = self[keys]
if not isinstance(current, list):
msg = f"Value at {keys} is not a list"
raise NodeTypeError(msg)

# Check membership
if where is None:
if value in current:
return self
else:
for item in current:
if isinstance(item, dict) and all(
k in item and item[k] == v for k, v in where.items()
):
return self

# Not present: append
return self.append(*keys, value=value)
```

## Documentation

Add an example to README.md showing both the scalar and `where`-based usage.

## Testing

- Scalar value already present → no-op (same source)
- Scalar value missing → appended
- Dict matching via `where` already present → no-op
- Dict matching via `where` missing → appended
- Path does not exist → list created
- Path is a scalar → `NodeTypeError`
- Path is a mapping → `NodeTypeError`
- Empty `where` dict → `ValueError`
- Flow sequence handling (inline `[a, b]` style)
- Idempotence: calling twice gives same result
- Editor wrapper delegates correctly
47 changes: 47 additions & 0 deletions src/yamltrip/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,53 @@ def remove_from_list(self, *keys: KeyPart, values: Sequence[Any]) -> Document:
]
return self._apply_patches(patches)

def ensure_in_list(
self, *keys: KeyPart, value: Any, where: dict[str, Any] | None = None
) -> Document:
"""Ensure a value is present in the sequence at path.

If the value is already present, returns self unchanged (no-op).
If the path does not exist, creates the list with [value].
Uses Python == for equality checks.

Args:
*keys: Path to the sequence within the document.
value: The value to ensure is in the list.
where: Optional dict of key/value pairs for matching dicts in
the list (AND semantics). If provided, checks whether any
list item matches all pairs; if so, returns self unchanged.

Raises:
NodeTypeError: If the value at path is not a list.
ValueError: If where is an empty dict.
PatchError: If keys is empty and the document is empty (root
sequence creation is not supported).
"""
if where is not None and not where:
msg = "where must be a non-empty dict"
raise ValueError(msg)

route = _make_route(keys)
if not self._core_doc.query_exists(route):
return self.upsert(*keys, value=[value])

Comment thread
nathanjmcdougall marked this conversation as resolved.
current = self[keys]
if not isinstance(current, list):
msg = f"Value at {keys} is not a list"
raise NodeTypeError(msg)

if where is None:
if value in current:
return self
else:
for item in current:
if isinstance(item, dict) and all(
k in item and item[k] == v for k, v in where.items()
):
return self

return self.append(*keys, value=value)

def sync(self, *keys: KeyPart, value: Any) -> Document:
"""Sync the value at path to match the desired value.

Expand Down
6 changes: 6 additions & 0 deletions src/yamltrip/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ def remove_from_list(self, *keys: KeyPart, values: Sequence[Any]) -> None:
"""Remove all occurrences of given values from the sequence at path."""
self._document = self.document.remove_from_list(*keys, values=values)

def ensure_in_list(
self, *keys: KeyPart, value: Any, where: dict[str, Any] | None = None
) -> None:
"""Ensure a value is present in the sequence at path."""
self._document = self.document.ensure_in_list(*keys, value=value, where=where)

def sync(self, *keys: KeyPart, value: Any) -> None:
"""Sync the value at path to match the desired value."""
self._document = self.document.sync(*keys, value=value)
Expand Down
103 changes: 103 additions & 0 deletions tests/test_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -745,3 +745,106 @@ def test_root_upsert_on_empty_raises_patch_error(self):
PatchError, match="Cannot replace root of an empty document"
):
doc.upsert(value=42)


class TestEnsureInList:
def test_scalar_already_present_noop(self):
doc = Document("items:\n - a\n - b\n")
result = doc.ensure_in_list("items", value="a")
assert result.source == doc.source

def test_scalar_missing_appends(self):
doc = Document("items:\n - a\n - b\n")
result = doc.ensure_in_list("items", value="c")
assert result["items"] == ["a", "b", "c"]

def test_integer_value(self):
doc = Document("ports:\n - 8080\n - 9090\n")
result = doc.ensure_in_list("ports", value=8080)
assert result.source == doc.source

def test_path_missing_creates_list(self):
doc = Document("name: foo\n")
result = doc.ensure_in_list("items", value="first")
assert result["items"] == ["first"]

def test_path_not_a_list_raises(self):
doc = Document("name: foo\n")
with pytest.raises(NodeTypeError):
doc.ensure_in_list("name", value="bar")

def test_path_is_mapping_raises(self):
doc = Document("config:\n host: localhost\n port: 8080\n")
with pytest.raises(NodeTypeError):
doc.ensure_in_list("config", value="x")

def test_idempotent(self):
doc = Document("items:\n - a\n")
result1 = doc.ensure_in_list("items", value="b")
result2 = result1.ensure_in_list("items", value="b")
assert result1.source == result2.source

def test_where_match_found_noop(self):
doc = Document(
"repos:\n - repo: https://a\n rev: v1\n - repo: https://b\n rev: v2\n"
)
result = doc.ensure_in_list(
"repos",
where={"repo": "https://a"},
value={"repo": "https://a", "rev": "v1"},
)
assert result.source == doc.source

def test_where_no_match_appends(self):
doc = Document("repos:\n - repo: https://a\n rev: v1\n")
result = doc.ensure_in_list(
"repos",
where={"repo": "https://b"},
value={"repo": "https://b", "rev": "v2"},
)
assert result["repos", 1] == {"repo": "https://b", "rev": "v2"}

def test_where_empty_raises(self):
doc = Document("items:\n - a\n")
with pytest.raises(ValueError, match="where must be a non-empty dict"):
doc.ensure_in_list("items", where={}, value="x")

def test_where_items_not_dicts_appends(self):
doc = Document("items:\n - a\n - b\n")
result = doc.ensure_in_list(
"items",
where={"name": "foo"},
value={"name": "foo"},
)
assert result["items"] == ["a", "b", {"name": "foo"}]

def test_where_multiple_keys_all_must_match(self):
doc = Document("items:\n - name: foo\n ver: 1\n - name: bar\n ver: 2\n")
result = doc.ensure_in_list(
"items",
where={"name": "foo", "ver": 2},
value={"name": "foo", "ver": 2},
)
# Neither item matches both (foo has ver=1, bar has ver=2)
assert len(result["items"]) == 3

def test_flow_sequence_appends(self):
doc = Document("items: [a, b]\n")
result = doc.ensure_in_list("items", value="c")
assert result["items"] == ["a", "b", "c"]

def test_flow_sequence_noop(self):
doc = Document("items: [a, b]\n")
result = doc.ensure_in_list("items", value="a")
assert result.source == doc.source

def test_nested_path_missing_creates(self):
doc = Document("config:\n name: foo\n")
result = doc.ensure_in_list("config", "hooks", value="pre-commit")
assert result["config", "hooks"] == ["pre-commit"]

def test_deeply_nested_path(self):
doc = Document("a:\n b: 1\n")
result = doc.ensure_in_list("a", "c", value="x")
assert result["a", "c"] == ["x"]
assert result["a", "b"] == 1
26 changes: 26 additions & 0 deletions tests/test_editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,32 @@ def test_non_list_raises_node_type_error(self, tmp_path):
ed.find_index("name", where={"k": "v"})


class TestEditorEnsureInList:
def test_scalar_missing_appends(self, yaml_file):
with Editor(yaml_file) as editor:
editor.ensure_in_list("items", value="c")
content = yaml_file.read_text(encoding="utf-8")
assert content == "name: foo\nage: 30\nitems:\n - a\n - b\n - c\n"

def test_scalar_present_noop(self, yaml_file):
with Editor(yaml_file) as editor:
editor.ensure_in_list("items", value="a")
content = yaml_file.read_text(encoding="utf-8")
assert content == "name: foo\nage: 30\nitems:\n - a\n - b\n"

def test_where_matching(self, tmp_path):
p = tmp_path / "test.yml"
p.write_text("repos:\n - name: foo\n ver: 1\n", encoding="utf-8")
with Editor(p) as editor:
editor.ensure_in_list(
"repos",
where={"name": "foo"},
value={"name": "foo", "ver": 1},
)
content = p.read_text(encoding="utf-8")
assert content == "repos:\n - name: foo\n ver: 1\n"


class TestEditorGet:
def test_get_existing_key(self, yaml_file):
with Editor(yaml_file) as editor:
Expand Down
Loading