Skip to content

Support complex values (dicts/lists) in replace and upsert#19

Merged
nathanjmcdougall merged 12 commits into
mainfrom
18-support-complex-values-dictslists-in-upsert-and-replace
May 17, 2026
Merged

Support complex values (dicts/lists) in replace and upsert#19
nathanjmcdougall merged 12 commits into
mainfrom
18-support-complex-values-dictslists-in-upsert-and-replace

Conversation

@nathanjmcdougall
Copy link
Copy Markdown
Collaborator

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

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
…or 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"
## 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
@nathanjmcdougall nathanjmcdougall linked an issue May 15, 2026 that may be closed by this pull request
- 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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for complex (Mapping/Sequence) values in Document.replace() and Document.upsert(), which previously raised PatchError because yamlpatch serializes Replace values inline after the key colon. The fix intercepts complex Replace operations in the Rust layer and performs direct string surgery on the document source, while scalar Replaces and other operations still flow through yamlpatch unchanged.

Changes:

  • New apply_complex_replace plus find_key_colon / extract_inline_comment / indent_block helpers in src/document.rs to handle multi-line block-style replacement with comment relocation and indentation.
  • Extracted shared apply_patches_impl so both Document.apply_patches and _core.apply_patches route complex replaces through the new path; the old duplicated batch logic in src/ops.rs is removed.
  • New tests (19 in tests/test_document.py, 3 in tests/test_core_ops.py) covering dict/list replaces, nesting, comment relocation, root-level replace, indentation depths, and quoted-key edge cases; README and a design doc are added.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/document.rs Implements complex-value replace via string surgery and shared apply_patches_impl.
src/ops.rs Routes apply_patches through the shared impl instead of calling yamlpatch directly.
tests/test_document.py Adds TestDocumentReplaceComplex and TestDocumentUpsertComplex covering dict/list cases.
tests/test_core_ops.py Adds _core.apply_patches tests for dict, list, and mixed batches.
README.md Documents that dicts/lists are now accepted by replace/upsert.
doc/specs/2026-05-15-complex-value-replace-design.md Design rationale, algorithm, scope, and known limitations.
Comments suppressed due to low confidence (1)

src/document.rs:377

  • The single-quote handling treats every ' as a state toggle, which is correct for strict open/close but mishandles YAML's escaped single quote ('' inside a single-quoted string represents a literal '). For a string like 'it''s # ok', the toggling will close on the second quote, treat s # ok as outside the string, and incorrectly extract # ok as an inline comment. Consider treating '' while in_single_quote as an escaped quote (advance i by 2 and stay inside the string).
            b'\'' if !in_double_quote => {
                in_single_quote = !in_single_quote;
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/document.rs
Comment thread src/document.rs
Comment thread src/document.rs Outdated
Comment thread src/document.rs
…amlpatch 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.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@nathanjmcdougall nathanjmcdougall merged commit 73a6976 into main May 17, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support complex values (dicts/lists) in upsert and replace

2 participants