Skip to content

Commit 73a6976

Browse files
Support complex values (dicts/lists) in replace and upsert (#19)
* test: add failing tests for replace/upsert with complex values (dicts/lists) * feat: support complex values (dicts/lists) in Replace operations Intercept Replace operations with Mapping/Sequence values in apply_patches before they reach yamlpatch. Perform direct string surgery using yamlpath for feature location and serde_yaml for block-style serialization. Handles: key-colon splitting, inline comment detection and relocation, indentation computation, batch flushing for mixed patch sequences. Fixes #18 * refactor: extract shared apply_patches_impl, fix ops::apply_patches for complex values\n\n- Extract batch-flush logic into pub(crate) apply_patches_impl in document.rs\n- ops::apply_patches now delegates to shared impl (fixes complex Replace via _core API)\n- Add tests: root-level replace, indentation depths 0/2, _core.apply_patches with dict/list\n- 183 tests pass" * Support complex values (dicts/lists) in replace and upsert ## Summary - Intercept `Replace` operations with Mapping/Sequence values in the Rust layer before they reach yamlpatch (which can't handle multi-line serialized values) - Perform direct string surgery: find key-colon boundary, extract inline comments, compute indentation, serialize via `serde_yaml::to_string()`, and reassemble - Extract shared `apply_patches_impl` so both `Document.apply_patches()` and `_core.apply_patches()` handle complex values consistently ## Changes - **src/document.rs**: Add `apply_complex_replace` + 3 helpers (`find_key_colon`, `extract_inline_comment`, `indent_block`), extract `apply_patches_impl` as shared batch-flush logic - **src/ops.rs**: Delegate to `apply_patches_impl` instead of passing everything to yamlpatch directly - **tests/test_document.py**: 17 new tests (replace with dict/list, nested, comment relocation, root-level, indentation depths) - **tests/test_core_ops.py**: 2 new tests for `_core.apply_patches` with complex values Fixes #18 * Harden complex-replace string surgery and add edge-case tests - find_key_colon: add bounds guards on post-quote index increments to prevent overshoot on malformed input; add precondition doc comment - apply_patches_impl: add doc comment explaining route validity across batch flushes (symbolic paths, not byte offsets) - apply_complex_replace: add safety comment on ws_len subtraction - Tests: empty dict/list, block/folded scalars, flow mappings, quoted keys with colons, hash-in-value, mixed scalar/complex batch * fix: collapse nested if into match guard to satisfy clippy * docs: document 2-space indent assumption as yamlpatch-wide limitation * docs: clarify indent_block preserves blank lines without trailing whitespace * docs: explain why single-quote handling is correct for '' escapes * docs: note O(N) re-parse is consistent with yamlpatch behavior * refactor: remove find_key_colon and extract_inline_comment to match yamlpatch behavior Simplify find_key_colon to naive str::find(':'), consistent with yamlpatch's own Replace implementation. Remove extract_inline_comment entirely since yamlpatch doesn't preserve inline comments during Replace either. Removes ~90 lines of quote-aware parsing logic. When yamlpatch fixes these limitations upstream, yamltrip will inherit the fixes uniformly across both scalar and complex replaces. * docs: update spec to reflect removal of comment preservation and simplified colon finding
1 parent 58f1fca commit 73a6976

6 files changed

Lines changed: 462 additions & 19 deletions

File tree

README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ doc2 = doc.replace("age", value=31)
3131
doc3 = doc2.add(key="city", value="Portland")
3232
print(doc3.dumps())
3333

34+
# Complex values (dicts/lists) work too
35+
doc4 = doc3.replace("age", value={"years": 31, "months": 4})
36+
doc5 = doc4.upsert("hobbies", value=["reading", "hiking"])
37+
3438
# File-based editing with a context manager
3539
with yamltrip.edit("config.yml") as editor:
3640
editor.replace("version", value="2.0")
@@ -62,8 +66,10 @@ doc["items", 0] # "a"
6266
("items", 0) in doc # True
6367

6468
doc.replace("items", 0, value="x")
69+
doc.replace("items", value=["x", "y"]) # dicts and lists accepted
6570
doc.add("items", key="c", value=3)
6671
doc.upsert("new", "nested", value=True)
72+
doc.upsert("config", value={"debug": True}) # dicts and lists accepted
6773
doc.remove("items", 0)
6874
doc.prune_remove("a", "b", "c") # remove + prune empty parents
6975
doc.append("items", value="c")
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# Complex Value Support for Replace and Upsert
2+
3+
**Date:** 2026-05-15
4+
**Issue:** #18
5+
6+
## Problem
7+
8+
`Document.replace()` and `Document.upsert()` only accept scalar values. Passing a dict or list raises `PatchError: Patch failed: input is not valid YAML` because yamlpatch's `apply_value_replacement` serializes the value inline after the key colon, producing invalid YAML when the serialized value is multi-line.
9+
10+
`Op.add` and `Op.append` already handle complex values correctly. Only `Op.replace` is broken.
11+
12+
## Approach
13+
14+
Intercept `Replace` operations with complex values (Mapping or Sequence) in yamltrip's Rust layer before they reach yamlpatch. Perform direct string surgery on the document source. Scalar `Replace` and all other operation types pass through to yamlpatch unchanged.
15+
16+
## Change Location
17+
18+
All Rust changes are in `src/document.rs`. No Python API changes. No yamlpatch changes. No new files.
19+
20+
A new function `apply_complex_replace` handles the complex case, called from `PyDocument::apply_patches`.
21+
22+
## Algorithm: `apply_complex_replace`
23+
24+
Input: a `yamlpath::Document`, a `yamlpath::Route`, and a `serde_yaml::Value` (Mapping or Sequence).
25+
26+
1. **Locate the feature**`document.query_pretty(&route)` to get the byte span with surrounding key context.
27+
28+
2. **Extract content**`extract_with_leading_whitespace(feature)` to get the full `key: value` text. Split at the first `:` (naive `find(':')`, consistent with yamlpatch's own Replace implementation) into `key_part` (through the colon) and the rest.
29+
30+
3. **Compute indentation** — Derive base indent from the number of leading spaces on the feature's line. Value content indentation = base indent + 2 spaces.
31+
32+
4. **Serialize the new value**`serde_yaml::to_string(&value)`, strip trailing newline, re-indent each line to the computed value indentation.
33+
34+
5. **Assemble replacement text**`key:\n indented_value`. Inline comments on the value line are not preserved, consistent with yamlpatch's own Replace behavior for scalar values.
35+
36+
6. **String surgery** — Replace the byte range from the feature's span (adjusted for leading whitespace) in the document source.
37+
38+
7. **Re-parse**`yamlpath::Document::new(patched_source)` to validate.
39+
40+
### Root-level replace
41+
42+
If the route is empty, skip key extraction. Replace the entire document content with the serialized value. This matches yamlpatch's existing root-replace behavior.
43+
44+
## Patch Routing in `apply_patches`
45+
46+
In `PyDocument::apply_patches`, before building the `Vec<yamlpatch::Patch>`:
47+
48+
```
49+
for each patch:
50+
if patch.operation is Replace(value) AND value is Mapping or Sequence:
51+
apply_complex_replace(document, route, value)
52+
re-parse document for subsequent patches
53+
else:
54+
collect into yamlpatch batch
55+
```
56+
57+
Patches are applied sequentially. If a complex replace is encountered mid-batch, the preceding yamlpatch batch is flushed first, then the complex replace is applied, then the next batch starts from the updated document.
58+
59+
## How Upsert Benefits
60+
61+
`Document.upsert()` delegates to `replace()` when the path exists, and to `_create_at()` (which uses `Op.add`/`Op.merge_into`) when it doesn't. Since `Op.add` and `Op.merge_into` already handle complex values, the only broken path is the `replace` delegation — fixed by this change. No changes to `document.py`.
62+
63+
## Serialization Style
64+
65+
No changes to `Op.add` behavior. Complex values in `Replace` use `serde_yaml::to_string()` which produces block style by default (block mappings, block sequences). This matches the pre-commit config use case where block style is expected.
66+
67+
## Testing
68+
69+
New tests in `tests/test_document.py`:
70+
71+
- `replace()` with a dict value
72+
- `replace()` with a list value
73+
- `replace()` with nested dict-in-list-in-dict
74+
- `upsert()` with a dict value (path exists → goes through replace)
75+
- `upsert()` with a dict value (path doesn't exist → goes through add, already works)
76+
- Replacing a complex value with another complex value
77+
- Root-level replace with complex value
78+
- Indentation correctness at various nesting depths (0, 2, 4 spaces)
79+
80+
## Known Limitations
81+
82+
- **Quoted keys with colons**`find_key_colon` uses a naive `str::find(':')`, which will misparse keys like `"host:port": value`. This is consistent with yamlpatch's own Replace implementation. When yamlpatch fixes this upstream, yamltrip should inherit the fix.
83+
- **Inline comments** — Not preserved during complex replace, consistent with yamlpatch's scalar Replace behavior.
84+
85+
## Scope Boundaries
86+
87+
**In scope:**
88+
- `Replace` with Mapping/Sequence values
89+
- Indentation handling
90+
91+
**Out of scope:**
92+
- Changing `Add`/`Append` serialization style
93+
- Multi-document YAML support
94+
- Tagged values
95+
- Flow-style output preference (uses serde_yaml default block style)

src/document.rs

Lines changed: 196 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,22 +137,209 @@ impl PyDocument {
137137
}
138138

139139
/// Apply patches to this document and return a new document.
140-
/// NOTE: Similar patch-application logic exists in ops::apply_patches (returns String).
141140
fn apply_patches(&self, patches: Vec<PyPatch>) -> PyResult<Self> {
142-
let yaml_patches: Vec<yamlpatch::Patch<'_>> = patches
141+
let doc = apply_patches_impl(&self.inner, &patches).map_err(|e| {
142+
PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!("Patch failed: {e}"))
143+
})?;
144+
Ok(Self { inner: doc })
145+
}
146+
}
147+
148+
/// Shared patch-application logic used by both PyDocument::apply_patches and ops::apply_patches.
149+
///
150+
/// Patches are applied in order. Complex replaces (Mapping/Sequence values) are handled via
151+
/// direct string surgery; all other operations are batched and passed to yamlpatch.
152+
///
153+
/// Routes are symbolic paths (key names / sequence indices), not byte offsets, so they remain
154+
/// valid across complex replaces that restructure sibling values. This is the same sequential
155+
/// semantics yamlpatch uses internally when applying multiple patches.
156+
pub(crate) fn apply_patches_impl(
157+
doc: &yamlpath::Document,
158+
patches: &[PyPatch],
159+
) -> Result<yamlpath::Document, String> {
160+
let mut current_doc = doc.clone();
161+
let mut batch: Vec<usize> = Vec::new();
162+
163+
for (idx, patch) in patches.iter().enumerate() {
164+
let is_complex_replace = matches!(
165+
&patch.operation.inner,
166+
yamlpatch::Op::Replace(v) if matches!(v, serde_yaml::Value::Mapping(_) | serde_yaml::Value::Sequence(_))
167+
);
168+
169+
if is_complex_replace {
170+
// Flush any pending yamlpatch batch first
171+
if !batch.is_empty() {
172+
let yaml_patches: Vec<yamlpatch::Patch<'_>> = batch
173+
.iter()
174+
.map(|&i| yamlpatch::Patch {
175+
route: patches[i].route.to_yamlpath_route(),
176+
operation: patches[i].operation.inner.clone(),
177+
})
178+
.collect();
179+
current_doc = yamlpatch::apply_yaml_patches(&current_doc, &yaml_patches)
180+
.map_err(|e| e.to_string())?;
181+
batch.clear();
182+
}
183+
184+
// Apply the complex replace directly.
185+
// NOTE: This re-parses the document for each complex replace (O(N) parses).
186+
// This is consistent with yamlpatch::apply_yaml_patches, which also
187+
// re-parses per patch via apply_single_patch. A bottom-up single-pass
188+
// approach could avoid this, but isn't warranted until profiling shows
189+
// it matters.
190+
let route = patch.route.to_yamlpath_route();
191+
let value = match &patch.operation.inner {
192+
yamlpatch::Op::Replace(v) => v,
193+
_ => unreachable!(),
194+
};
195+
current_doc = apply_complex_replace(&current_doc, &route, value)?;
196+
} else {
197+
batch.push(idx);
198+
}
199+
}
200+
201+
// Flush remaining batch
202+
if !batch.is_empty() {
203+
let yaml_patches: Vec<yamlpatch::Patch<'_>> = batch
143204
.iter()
144-
.map(|p| yamlpatch::Patch {
145-
route: p.route.to_yamlpath_route(),
146-
operation: p.operation.inner.clone(),
205+
.map(|&i| yamlpatch::Patch {
206+
route: patches[i].route.to_yamlpath_route(),
207+
operation: patches[i].operation.inner.clone(),
147208
})
148209
.collect();
210+
current_doc = yamlpatch::apply_yaml_patches(&current_doc, &yaml_patches)
211+
.map_err(|e| e.to_string())?;
212+
}
149213

150-
let result = yamlpatch::apply_yaml_patches(&self.inner, &yaml_patches).map_err(|e| {
151-
PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!("Patch failed: {e}"))
152-
})?;
214+
Ok(current_doc)
215+
}
216+
217+
fn apply_complex_replace(
218+
doc: &yamlpath::Document,
219+
route: &yamlpath::Route<'_>,
220+
value: &serde_yaml::Value,
221+
) -> Result<yamlpath::Document, String> {
222+
let source = doc.source();
223+
224+
// Root-level replace: just serialize the entire value
225+
if route.is_empty() {
226+
let serialized =
227+
serde_yaml::to_string(value).map_err(|e| format!("Failed to serialize YAML: {e}"))?;
228+
return yamlpath::Document::new(serialized)
229+
.map_err(|e| format!("Failed to re-parse YAML: {e}"));
230+
}
231+
232+
// Locate the feature (with key context)
233+
let feature = doc
234+
.query_pretty(route)
235+
.map_err(|e| format!("Query failed: {e}"))?;
236+
237+
let content_with_ws = doc.extract_with_leading_whitespace(&feature);
238+
let content = doc.extract(&feature);
153239

154-
Ok(Self { inner: result })
240+
// Calculate the start byte including leading whitespace.
241+
// Safety: ws_len <= byte_span.0 because extract_with_leading_whitespace only
242+
// extends backward to the last newline (or start of document), never beyond byte_span.0.
243+
let ws_len = content_with_ws.len() - content.len();
244+
let start_byte = feature.location.byte_span.0 - ws_len;
245+
let end_byte = feature.location.byte_span.1;
246+
247+
// Find the colon separating key from value
248+
let colon_pos = find_key_colon(content_with_ws);
249+
250+
let key_part = match colon_pos {
251+
Some(pos) => {
252+
let key = &content_with_ws[..pos + 1]; // through the colon
253+
key.to_string()
254+
}
255+
None => {
256+
// No colon found — bare value (e.g. sequence item)
257+
let serialized = serde_yaml::to_string(value)
258+
.map_err(|e| format!("Failed to serialize YAML: {e}"))?;
259+
let trimmed = serialized.trim_end_matches('\n');
260+
261+
let line_start = source[..feature.location.byte_span.0]
262+
.rfind('\n')
263+
.map(|nl| nl + 1)
264+
.unwrap_or(0);
265+
let base_indent = feature.location.byte_span.0 - line_start;
266+
let indent_str = " ".repeat(base_indent);
267+
268+
let indented = indent_block(trimmed, &indent_str);
269+
270+
let mut result = source.to_string();
271+
result.replace_range(
272+
feature.location.byte_span.0..feature.location.byte_span.1,
273+
&indented,
274+
);
275+
if !result.ends_with('\n') {
276+
result.push('\n');
277+
}
278+
return yamlpath::Document::new(result)
279+
.map_err(|e| format!("Failed to re-parse YAML: {e}"));
280+
}
281+
};
282+
283+
// Compute base indentation from the feature's actual position
284+
let feat_start = feature.location.byte_span.0;
285+
let line_start = source[..feat_start]
286+
.rfind('\n')
287+
.map(|nl| nl + 1)
288+
.unwrap_or(0);
289+
let base_indent = feat_start - line_start;
290+
// NOTE: The +2 assumes 2-space indentation. This is consistent with yamlpatch,
291+
// which also hardcodes 2-space indent in Add, Append, MergeInto, and Replace ops.
292+
let value_indent = " ".repeat(base_indent + 2);
293+
294+
// Serialize the new value in block style
295+
let serialized =
296+
serde_yaml::to_string(value).map_err(|e| format!("Failed to serialize YAML: {e}"))?;
297+
let trimmed = serialized.trim_end_matches('\n');
298+
299+
// Re-indent each line of the serialized value
300+
let indented_value = indent_block(trimmed, &value_indent);
301+
302+
// Assemble: key:\n indented_value
303+
// NOTE: Inline comments on the value line are not preserved, consistent
304+
// with yamlpatch's own Replace behavior for scalar values.
305+
let replacement = format!("{}\n{}", key_part, indented_value);
306+
307+
// Replace in source
308+
let mut result = source.to_string();
309+
result.replace_range(start_byte..end_byte, &replacement);
310+
311+
if !result.ends_with('\n') {
312+
result.push('\n');
313+
}
314+
315+
yamlpath::Document::new(result).map_err(|e| format!("Failed to re-parse YAML: {e}"))
316+
}
317+
318+
/// Find the first colon (key-value separator) in a YAML fragment.
319+
///
320+
/// Uses a naive `find(':')`, consistent with yamlpatch's own Replace
321+
/// implementation. This means colons inside quoted keys will be
322+
/// misidentified — a known yamlpatch limitation that will be fixed
323+
/// uniformly when yamlpatch addresses it.
324+
fn find_key_colon(content: &str) -> Option<usize> {
325+
content.find(':')
326+
}
327+
328+
fn indent_block(content: &str, indent: &str) -> String {
329+
let mut result = String::new();
330+
for (i, line) in content.lines().enumerate() {
331+
if i > 0 {
332+
result.push('\n');
333+
}
334+
// Blank lines are preserved (the \n above) but not indented,
335+
// avoiding trailing whitespace. In practice serde_yaml::to_string()
336+
// never emits blank lines for Mapping/Sequence values.
337+
if !line.trim().is_empty() {
338+
result.push_str(indent);
339+
result.push_str(line);
340+
}
155341
}
342+
result
156343
}
157344

158345
fn convert_feature(feature: &yamlpath::Feature<'_>) -> PyFeature {

src/ops.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -157,22 +157,13 @@ impl PyPatch {
157157
}
158158

159159
/// Apply a list of patches to a YAML source string.
160-
/// NOTE: Similar patch-application logic exists in PyDocument::apply_patches (returns Document).
161160
#[pyfunction]
162161
pub fn apply_patches(source: &str, patches: Vec<PyPatch>) -> PyResult<String> {
163162
let document = yamlpath::Document::new(source).map_err(|e| {
164163
PyErr::new::<pyo3::exceptions::PyValueError, _>(format!("Invalid YAML: {e}"))
165164
})?;
166165

167-
let yaml_patches: Vec<yamlpatch::Patch<'_>> = patches
168-
.iter()
169-
.map(|p| yamlpatch::Patch {
170-
route: p.route.to_yamlpath_route(),
171-
operation: p.operation.inner.clone(),
172-
})
173-
.collect();
174-
175-
let result = yamlpatch::apply_yaml_patches(&document, &yaml_patches).map_err(|e| {
166+
let result = crate::document::apply_patches_impl(&document, &patches).map_err(|e| {
176167
PyErr::new::<pyo3::exceptions::PyRuntimeError, _>(format!("Patch failed: {e}"))
177168
})?;
178169

tests/test_core_ops.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,33 @@ def test_append_to_sequence(self):
4949
assert "- c" in result
5050
assert "- a" in result
5151

52+
def test_replace_with_dict(self):
53+
source = "config:\n key: value\n"
54+
patches = [Patch(route=Route(["config"]), operation=Op.replace({"a": 1}))]
55+
result = apply_patches(source, patches)
56+
assert "a: 1" in result
57+
assert "key: value" not in result
58+
59+
def test_replace_with_list(self):
60+
source = "repos: []\n"
61+
patches = [
62+
Patch(route=Route(["repos"]), operation=Op.replace([{"repo": "local"}]))
63+
]
64+
result = apply_patches(source, patches)
65+
assert "repo: local" in result
66+
67+
def test_batch_scalar_then_complex_then_scalar(self):
68+
source = "name: foo\nconfig: old\nversion: 1\n"
69+
patches = [
70+
Patch(route=Route(["name"]), operation=Op.replace("bar")),
71+
Patch(route=Route(["config"]), operation=Op.replace({"a": 1})),
72+
Patch(route=Route(["version"]), operation=Op.replace(2)),
73+
]
74+
result = apply_patches(source, patches)
75+
assert "name: bar" in result
76+
assert "a: 1" in result
77+
assert "version: 2" in result
78+
5279
def test_preserves_comments(self):
5380
source = "# top comment\nname: foo # inline"
5481
patches = [Patch(route=Route(["name"]), operation=Op.replace("bar"))]

0 commit comments

Comments
 (0)