diff --git a/doc/specs/2026-05-19-insert-operation-design.md b/doc/specs/2026-05-19-insert-operation-design.md new file mode 100644 index 0000000..4cf00c5 --- /dev/null +++ b/doc/specs/2026-05-19-insert-operation-design.md @@ -0,0 +1,203 @@ +# Insert Operation for Block Sequences + +**Date:** 2026-05-19 +**Issue:** #24 + +## Problem + +There is no way to insert an item at a specific position in a YAML sequence while preserving comments on existing items. The only options are `append` (end only) or `replace` on the entire list (loses internal comments). + +Use case: in usethis, pre-commit hooks have a defined ordering. Adding a new hook repo at a specific position requires inserting between existing repos. + +## Approach + +Direct string surgery in yamltrip's Rust layer (`document.rs`), following the same pattern as `apply_complex_replace`. The operation is intercepted in `apply_patches_impl` before reaching yamlpatch, since `yamlpatch::Op` has no `InsertAt` variant and we cannot upstream to yamlpatch. + +## Python API + +### Document (immutable) + +```python +doc = doc.insert('repos', index=1, value={'repo': 'https://...', 'hooks': [...]}) +doc = doc.insert('repos', index=-1, value='before_last') # negative index +doc = doc.insert('repos', index=0, value='prepend') # prepend +``` + +**Signature:** `insert(*keys: KeyPart, index: int, value: Any) -> Document` + +### Editor (mutable) + +```python +with Editor('file.yaml') as ed: + ed.insert('repos', index=1, value=new_repo) +``` + +**Signature:** `insert(*keys: KeyPart, index: int, value: Any) -> None` + +### Index Semantics + +Follows Python `list.insert()`: +- Non-negative indices insert before the item at that position. +- Negative indices count from the end: `-1` means before the last item, `-2` before the second-to-last, etc. +- Out-of-range indices clamp: `index >= len` acts like append, `index <= -len` acts like prepend. + +## Rust Layer + +### The Problem with `yamlpatch::Op` + +`yamlpatch::Op` is an external enum we cannot extend. We need a way to represent `InsertAt` in the patch pipeline. + +### Solution: Extend `PyOp` with a local variant + +Introduce an internal enum in `ops.rs` that wraps both yamlpatch ops and local-only ops: + +```rust +/// Operations that yamltrip handles locally (not in yamlpatch). +#[derive(Clone, Debug)] +pub enum LocalOp { + InsertAt { index: i64, value: serde_yaml::Value }, +} + +/// The inner representation of a PyOp. +#[derive(Clone, Debug)] +pub enum OpInner { + Yamlpatch(yamlpatch::Op<'static>), + Local(LocalOp), +} +``` + +`PyOp` changes from holding `yamlpatch::Op<'static>` directly to holding `OpInner`. The existing `#[staticmethod]` constructors (`replace`, `add`, `remove`, `append`, etc.) wrap in `OpInner::Yamlpatch(...)`. A new `insert_at` constructor wraps in `OpInner::Local(LocalOp::InsertAt { ... })`. + +### Patch Routing in `apply_patches_impl` + +The existing logic batches yamlpatch ops and flushes them together. `InsertAt` (and any future local ops) flush the batch first, then execute via dedicated string surgery — identical to the complex-replace pattern: + +``` +for each patch: + if patch is Local(InsertAt { index, value }): + flush yamlpatch batch + apply_insert_at(¤t_doc, &route, index, &value) + else: + add to yamlpatch batch +flush remaining batch +``` + +### `Op.insert_at` Python-facing constructor + +```python +op = Op.insert_at(index=1, value={'repo': '...'}) +``` + +### `kind` getter + +Returns `"insert_at"` for the new variant. + +## Algorithm: `apply_insert_at` + +Input: `yamlpath::Document`, `yamlpath::Route` (pointing to the sequence), `index: i64`, `serde_yaml::Value`. + +1. **Query the sequence feature** — `document.query_exact(&route)` → verify `FeatureKind::BlockSequence`. Error if not found, or if it's a flow sequence or non-sequence. + +2. **Count items** — Iterate `route + [0]`, `route + [1]`, ... until `query_exists` returns false. Let `len` = item count. + +3. **Resolve index (Python semantics):** + ``` + if index < 0: + resolved = max(0, len + index) + else: + resolved = min(index, len) + ``` + +4. **Delegate to append if `resolved == len`:** + Build a `yamlpatch::Op::Append { value }` patch and apply via yamlpatch. This reuses existing append logic including its indentation handling. + +5. **Locate insertion byte position:** + - Query `route + [resolved]` with `query_exact` to get the item feature. + - The insertion point is the byte offset of the item's `- ` dash (accounting for leading whitespace on its line). + - Walk backward from `feature.location.byte_span.0` to the start of the line to include the leading whitespace/indentation. + +6. **Determine indentation:** + - Extract the indentation string from the target item's line (characters from line start to `- `). + - This becomes the indentation for the new item. + +7. **Serialize new value as a sequence item:** + - Scalar: `- value\n` + - Mapping: `- key1: val1\n key2: val2\n...` (indented relative to the dash) + - Sequence: `- - item1\n - item2\n...` (nested sequence) + - Use `serde_yaml::to_string(&value)` then reformat as a sequence item with correct indentation. + +8. **String splice** — Insert the formatted item text at the computed byte position (before the target item's line start). + +9. **Re-parse** — `yamlpath::Document::new(patched_source)` to validate. + +### Serialization Detail + +The new item text must be a complete block sequence entry. Given base indent `I` (spaces before `-`): + +- For a scalar value `v`: `"{I}- {v}\n"` +- For a mapping `{k1: v1, k2: v2}`: + ``` + {I}- k1: v1 + {I} k2: v2 + ``` +- For a sequence `[a, b]`: + ``` + {I}- - a + {I} - b + ``` + +Use `serde_yaml::to_string` for the value portion, then prefix the first line with `{I}- ` and subsequent lines with `{I} ` (indent = base + 2). + +## Error Cases + +| Condition | Error | +|-----------|-------| +| Path doesn't exist | `PatchError("Path not found: ...")` | +| Target is not a sequence | `PatchError("Value at ... is not a sequence")` | +| Target is a flow sequence | `PatchError("insert requires a block sequence; flow sequences are not supported")` | + +No `IndexError` is raised — indices always clamp (matching `list.insert()` behavior). + +## Design Decisions + +**Leading comments on sequence items are not tracked by insertion.** + +When a sequence item has comment lines immediately above it (e.g. `# description` before `- value`), `query_exact` for that item returns only the value's span — the comment is not part of the feature location. The insertion point is computed as the start of the item's `- ` line, so a new item inserted before index N will appear *between* any leading comments and the item at index N. The comments visually become associated with the newly inserted item. + +This is by design because: +- A comment above a block sequence item may describe the entire block (the sequence as a whole, or a group of items), not just the item on the next line. There is no way to determine intent from syntax alone. +- yamlpath provides no mechanism to determine which comments "belong to" a given sequence item (comments are free-floating in YAML) +- The existing behavior is predictable: insertion always occurs at the `- ` line boundary + +Users who need to preserve comment–item associations should structure their insertions to avoid splitting comments from their items (e.g. insert *after* the commented item rather than before it). + +## Files Changed + +| File | Change | +|------|--------| +| `src/ops.rs` | Add `LocalOp` enum, `OpInner` wrapper, `insert_at` constructor, update `kind`/`__repr__` | +| `src/document.rs` | Add `apply_insert_at` function, update `apply_patches_impl` routing | +| `src/yamltrip/document.py` | Add `Document.insert()` method | +| `src/yamltrip/editor.py` | Add `Editor.insert()` method | +| `src/yamltrip/_core.pyi` | Add type stub for `Op.insert_at` | +| `tests/test_core_ops.py` | Tests for `Op.insert_at` construction | +| `tests/test_document.py` | Tests for `Document.insert()` | +| `tests/test_editor.py` | Tests for `Editor.insert()` | + +## Test Cases + +1. Insert at beginning (`index=0`) of multi-item list +2. Insert in middle (`index=1` of 3-item list) +3. Insert at end (`index=len`) — should produce same result as append +4. Negative index (`index=-1` inserts before last) +5. Negative out-of-range (`index=-100` prepends) +6. Positive out-of-range (`index=100` appends) +7. Complex value (dict with nested structure) +8. Comment preservation — comments remain in the document (note: leading comments above an item may visually shift to the inserted item; see Known Limitations) +9. Empty sequence (`index=0`) +10. Single-item sequence (insert before and after) +11. Value with special YAML characters (strings needing quoting) +12. Nested sequence insertion (list within list) +13. Target is not a sequence → error +14. Target is a flow sequence → error +15. Path doesn't exist → error diff --git a/src/document.rs b/src/document.rs index 91b46f5..ad23139 100644 --- a/src/document.rs +++ b/src/document.rs @@ -1,6 +1,6 @@ use pyo3::prelude::*; -use crate::ops::PyPatch; +use crate::ops::{LocalOp, OpInner, PyPatch}; use crate::types::{PyFeature, PyFeatureKind, PyLocation, PyRoute}; /// A parsed YAML document. @@ -161,19 +161,30 @@ pub(crate) fn apply_patches_impl( let mut batch: Vec = Vec::new(); for (idx, patch) in patches.iter().enumerate() { - let is_complex_replace = matches!( - &patch.operation.inner, - yamlpatch::Op::Replace(v) if matches!(v, serde_yaml::Value::Mapping(_) | serde_yaml::Value::Sequence(_)) - ); + let needs_direct_handling = match &patch.operation.inner { + OpInner::Yamlpatch(yamlpatch::Op::Replace(v)) => { + matches!( + v, + serde_yaml::Value::Mapping(_) | serde_yaml::Value::Sequence(_) + ) + } + OpInner::Local(_) => true, + _ => false, + }; - if is_complex_replace { + if needs_direct_handling { // Flush any pending yamlpatch batch first if !batch.is_empty() { let yaml_patches: Vec> = batch .iter() - .map(|&i| yamlpatch::Patch { - route: patches[i].route.to_yamlpath_route(), - operation: patches[i].operation.inner.clone(), + .filter_map(|&i| { + patches[i] + .operation + .as_yamlpatch_op() + .map(|op| yamlpatch::Patch { + route: patches[i].route.to_yamlpath_route(), + operation: op.clone(), + }) }) .collect(); current_doc = yamlpatch::apply_yaml_patches(¤t_doc, &yaml_patches) @@ -181,18 +192,16 @@ pub(crate) fn apply_patches_impl( batch.clear(); } - // Apply the complex replace directly. - // NOTE: This re-parses the document for each complex replace (O(N) parses). - // This is consistent with yamlpatch::apply_yaml_patches, which also - // re-parses per patch via apply_single_patch. A bottom-up single-pass - // approach could avoid this, but isn't warranted until profiling shows - // it matters. let route = patch.route.to_yamlpath_route(); - let value = match &patch.operation.inner { - yamlpatch::Op::Replace(v) => v, + match &patch.operation.inner { + OpInner::Yamlpatch(yamlpatch::Op::Replace(value)) => { + current_doc = apply_complex_replace(¤t_doc, &route, value)?; + } + OpInner::Local(LocalOp::InsertAt { index, value }) => { + current_doc = apply_insert_at(¤t_doc, &route, *index, value)?; + } _ => unreachable!(), - }; - current_doc = apply_complex_replace(¤t_doc, &route, value)?; + } } else { batch.push(idx); } @@ -202,9 +211,14 @@ pub(crate) fn apply_patches_impl( if !batch.is_empty() { let yaml_patches: Vec> = batch .iter() - .map(|&i| yamlpatch::Patch { - route: patches[i].route.to_yamlpath_route(), - operation: patches[i].operation.inner.clone(), + .filter_map(|&i| { + patches[i] + .operation + .as_yamlpatch_op() + .map(|op| yamlpatch::Patch { + route: patches[i].route.to_yamlpath_route(), + operation: op.clone(), + }) }) .collect(); current_doc = yamlpatch::apply_yaml_patches(¤t_doc, &yaml_patches) @@ -214,6 +228,101 @@ pub(crate) fn apply_patches_impl( Ok(current_doc) } +fn apply_insert_at( + doc: &yamlpath::Document, + route: &yamlpath::Route<'_>, + index: i64, + value: &serde_yaml::Value, +) -> Result { + let source = doc.source(); + + // 1. Query the sequence feature and verify it's a block sequence + let feature = doc + .query_exact(route) + .map_err(|e| format!("Query failed: {e}"))? + .ok_or_else(|| "insert_at: sequence not found at route".to_string())?; + + if feature.kind() != yamlpath::FeatureKind::BlockSequence { + return Err(format!( + "insert_at: expected BlockSequence, got {:?}", + feature.kind() + )); + } + + // 2. Count items in the sequence + let mut len: i64 = 0; + loop { + let item_route = route.with_key(yamlpath::Component::Index(len as usize)); + if !doc.query_exists(&item_route) { + break; + } + len += 1; + } + + // 3. Resolve index with Python list.insert semantics + let resolved = if index < 0 { + 0i64.max(len + index) + } else { + index.min(len) + } as usize; + + // 4. Delegate to append if inserting at the end + if resolved as i64 == len { + let patch = yamlpatch::Patch { + route: route.clone(), + operation: yamlpatch::Op::Append { + value: value.clone(), + }, + }; + return yamlpatch::apply_yaml_patches(doc, &[patch]).map_err(|e| e.to_string()); + } + + // 5. Locate insertion byte position + let item_route = route.with_key(yamlpath::Component::Index(resolved)); + let item_feature = doc + .query_exact(&item_route) + .map_err(|e| format!("Query failed: {e}"))? + .ok_or_else(|| format!("insert_at: item at index {resolved} not found"))?; + + let item_start = item_feature.location.byte_span.0; + let line_start = source[..item_start] + .rfind('\n') + .map(|nl| nl + 1) + .unwrap_or(0); + + // 6. Determine indentation from the existing item's line prefix + let prefix = &source[line_start..item_start]; + let dash_pos = prefix.find('-').unwrap_or(prefix.len()); + let base_indent = &prefix[..dash_pos]; + + // 7. Serialize new value as a sequence item + let serialized = + serde_yaml::to_string(value).map_err(|e| format!("Failed to serialize YAML: {e}"))?; + let trimmed = serialized.trim_end_matches('\n'); + + let mut item_text = String::new(); + for (i, line) in trimmed.lines().enumerate() { + if i == 0 { + item_text.push_str(base_indent); + item_text.push_str("- "); + item_text.push_str(line); + } else { + item_text.push('\n'); + item_text.push_str(base_indent); + item_text.push_str(" "); + item_text.push_str(line); + } + } + item_text.push('\n'); + + // 8. String splice — insert before the target item's line + let mut result = source.to_string(); + result.insert_str(line_start, &item_text); + + // 9. Re-parse to validate + yamlpath::Document::new(result).map_err(|e| format!("Failed to re-parse YAML: {e}")) +} + fn apply_complex_replace( doc: &yamlpath::Document, route: &yamlpath::Route<'_>, diff --git a/src/ops.rs b/src/ops.rs index 8499ed7..7701b44 100644 --- a/src/ops.rs +++ b/src/ops.rs @@ -3,11 +3,27 @@ use pyo3::prelude::*; use crate::convert::py_to_yaml_value; use crate::types::PyRoute; +/// Operations that yamltrip handles locally (not in yamlpatch). +#[derive(Clone, Debug)] +pub enum LocalOp { + InsertAt { + index: i64, + value: serde_yaml::Value, + }, +} + +/// The inner representation of a PyOp. +#[derive(Clone, Debug)] +pub enum OpInner { + Yamlpatch(yamlpatch::Op<'static>), + Local(LocalOp), +} + /// A YAML patch operation. #[pyclass(name = "Op", module = "yamltrip._core")] #[derive(Clone, Debug)] pub struct PyOp { - pub inner: yamlpatch::Op<'static>, + pub inner: OpInner, } #[pymethods] @@ -16,7 +32,7 @@ impl PyOp { fn replace(value: &Bound<'_, PyAny>) -> PyResult { let val = py_to_yaml_value(value)?; Ok(Self { - inner: yamlpatch::Op::Replace(val), + inner: OpInner::Yamlpatch(yamlpatch::Op::Replace(val)), }) } @@ -24,17 +40,17 @@ impl PyOp { fn add(key: &str, value: &Bound<'_, PyAny>) -> PyResult { let val = py_to_yaml_value(value)?; Ok(Self { - inner: yamlpatch::Op::Add { + inner: OpInner::Yamlpatch(yamlpatch::Op::Add { key: key.to_string(), value: val, - }, + }), }) } #[staticmethod] fn remove() -> Self { Self { - inner: yamlpatch::Op::Remove, + inner: OpInner::Yamlpatch(yamlpatch::Op::Remove), } } @@ -42,7 +58,15 @@ impl PyOp { fn append(value: &Bound<'_, PyAny>) -> PyResult { let val = py_to_yaml_value(value)?; Ok(Self { - inner: yamlpatch::Op::Append { value: val }, + inner: OpInner::Yamlpatch(yamlpatch::Op::Append { value: val }), + }) + } + + #[staticmethod] + fn insert_at(index: i64, value: &Bound<'_, PyAny>) -> PyResult { + let val = py_to_yaml_value(value)?; + Ok(Self { + inner: OpInner::Local(LocalOp::InsertAt { index, value: val }), }) } @@ -65,56 +89,78 @@ impl PyOp { map.insert(key_str, val); } Ok(Self { - inner: yamlpatch::Op::MergeInto { + inner: OpInner::Yamlpatch(yamlpatch::Op::MergeInto { key: key.to_string(), updates: map, - }, + }), }) } fn __repr__(&self) -> String { match &self.inner { - yamlpatch::Op::Replace(val) => { - format!("Op.replace({})", yaml_value_repr(val)) - } - yamlpatch::Op::Add { key, value } => { - format!("Op.add({}, {})", yaml_str_repr(key), yaml_value_repr(value)) - } - yamlpatch::Op::Remove => "Op.remove()".to_string(), - yamlpatch::Op::Append { value } => { - format!("Op.append({})", yaml_value_repr(value)) - } - yamlpatch::Op::MergeInto { key, .. } => { - format!("Op.merge_into({}, ...)", yaml_str_repr(key)) - } - yamlpatch::Op::RewriteFragment { from, to } => { - format!( - "Op.rewrite_fragment({}, {})", - yaml_str_repr(&format!("{from:?}")), - yaml_str_repr(to) - ) - } - yamlpatch::Op::ReplaceComment { new } => { - format!("Op.replace_comment({})", yaml_str_repr(new)) - } - yamlpatch::Op::EmplaceComment { new } => { - format!("Op.emplace_comment({})", yaml_str_repr(new)) - } + OpInner::Yamlpatch(op) => match op { + yamlpatch::Op::Replace(val) => { + format!("Op.replace({})", yaml_value_repr(val)) + } + yamlpatch::Op::Add { key, value } => { + format!("Op.add({}, {})", yaml_str_repr(key), yaml_value_repr(value)) + } + yamlpatch::Op::Remove => "Op.remove()".to_string(), + yamlpatch::Op::Append { value } => { + format!("Op.append({})", yaml_value_repr(value)) + } + yamlpatch::Op::MergeInto { key, .. } => { + format!("Op.merge_into({}, ...)", yaml_str_repr(key)) + } + yamlpatch::Op::RewriteFragment { from, to } => { + format!( + "Op.rewrite_fragment({}, {})", + yaml_str_repr(&format!("{from:?}")), + yaml_str_repr(to) + ) + } + yamlpatch::Op::ReplaceComment { new } => { + format!("Op.replace_comment({})", yaml_str_repr(new)) + } + yamlpatch::Op::EmplaceComment { new } => { + format!("Op.emplace_comment({})", yaml_str_repr(new)) + } + }, + OpInner::Local(local_op) => match local_op { + LocalOp::InsertAt { index, value } => { + format!("Op.insert_at({}, {})", index, yaml_value_repr(value)) + } + }, } } - /// The kind of operation: "replace", "add", "remove", "append", or "merge_into". + /// The kind of operation: "replace", "add", "remove", "append", "merge_into", or "insert_at". #[getter] fn kind(&self) -> &str { match &self.inner { - yamlpatch::Op::Replace(_) => "replace", - yamlpatch::Op::Add { .. } => "add", - yamlpatch::Op::Remove => "remove", - yamlpatch::Op::Append { .. } => "append", - yamlpatch::Op::MergeInto { .. } => "merge_into", - yamlpatch::Op::RewriteFragment { .. } => "rewrite_fragment", - yamlpatch::Op::ReplaceComment { .. } => "replace_comment", - yamlpatch::Op::EmplaceComment { .. } => "emplace_comment", + OpInner::Yamlpatch(op) => match op { + yamlpatch::Op::Replace(_) => "replace", + yamlpatch::Op::Add { .. } => "add", + yamlpatch::Op::Remove => "remove", + yamlpatch::Op::Append { .. } => "append", + yamlpatch::Op::MergeInto { .. } => "merge_into", + yamlpatch::Op::RewriteFragment { .. } => "rewrite_fragment", + yamlpatch::Op::ReplaceComment { .. } => "replace_comment", + yamlpatch::Op::EmplaceComment { .. } => "emplace_comment", + }, + OpInner::Local(local_op) => match local_op { + LocalOp::InsertAt { .. } => "insert_at", + }, + } + } +} + +impl PyOp { + /// Get the inner yamlpatch op, if this is a yamlpatch operation. + pub fn as_yamlpatch_op(&self) -> Option<&yamlpatch::Op<'static>> { + match &self.inner { + OpInner::Yamlpatch(op) => Some(op), + OpInner::Local(_) => None, } } } diff --git a/src/yamltrip/_core.pyi b/src/yamltrip/_core.pyi index 9680e1b..7730713 100644 --- a/src/yamltrip/_core.pyi +++ b/src/yamltrip/_core.pyi @@ -106,6 +106,8 @@ class Op: def append(value: Any) -> Op: ... @staticmethod def merge_into(key: str, updates: dict[str, Any]) -> Op: ... + @staticmethod + def insert_at(index: int, value: Any) -> Op: ... @property def kind(self) -> str: ... def __repr__(self) -> str: ... diff --git a/src/yamltrip/document.py b/src/yamltrip/document.py index 045dac6..33d6310 100644 --- a/src/yamltrip/document.py +++ b/src/yamltrip/document.py @@ -270,6 +270,17 @@ def append(self, *keys: KeyPart, value: Any) -> Document: patch = _core.Patch(route=route, operation=op) return self._apply_patches([patch]) + def insert(self, *keys: KeyPart, index: int, value: Any) -> Document: + """Insert an item at a specific position in the sequence at path. + + Uses Python list.insert() semantics for index resolution: + negative indices count from the end, out-of-range indices clamp. + """ + route = _make_route(keys) + op = _core.Op.insert_at(index=index, value=value) + patch = _core.Patch(route=route, operation=op) + return self._apply_patches([patch]) + def extend_list(self, *keys: KeyPart, values: Sequence[Any]) -> Document: """Append multiple items to the sequence at path.""" if not values: diff --git a/src/yamltrip/editor.py b/src/yamltrip/editor.py index 898755e..1f08dd0 100644 --- a/src/yamltrip/editor.py +++ b/src/yamltrip/editor.py @@ -125,6 +125,10 @@ def append(self, *keys: KeyPart, value: Any) -> None: """Append an item to the sequence at path.""" self._document = self.document.append(*keys, value=value) + def insert(self, *keys: KeyPart, index: int, value: Any) -> None: + """Insert an item at a specific position in the sequence at path.""" + self._document = self.document.insert(*keys, index=index, value=value) + def extend_list(self, *keys: KeyPart, values: Sequence[Any]) -> None: """Append multiple items to the sequence at path.""" self._document = self.document.extend_list(*keys, values=values) diff --git a/tests/test_core_ops.py b/tests/test_core_ops.py index 089abea..66d4c07 100644 --- a/tests/test_core_ops.py +++ b/tests/test_core_ops.py @@ -90,3 +90,109 @@ def test_preserves_indentation(self): result = apply_patches(source, patches) assert " b: 99" in result assert " c: 2" in result + + +class TestOpInsertAt: + def test_constructor(self): + op = Op.insert_at(index=1, value="item") + assert op is not None + assert op.kind == "insert_at" + + def test_repr(self): + op = Op.insert_at(index=2, value="hello") + assert "insert_at" in repr(op) + assert "2" in repr(op) + + def test_negative_index(self): + op = Op.insert_at(index=-1, value="item") + assert op.kind == "insert_at" + + +class TestApplyPatchesInsertAt: + def test_insert_at_middle(self): + source = "items:\n - a\n - b\n - c\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=1, value="x")) + ] + result = apply_patches(source, patches) + assert "- a" in result + assert "- x" in result + assert "- b" in result + assert "- c" in result + + def test_insert_at_beginning(self): + source = "items:\n - a\n - b\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=0, value="x")) + ] + result = apply_patches(source, patches) + assert "- x" in result + assert "- a" in result + + def test_insert_at_end_acts_as_append(self): + source = "items:\n - a\n - b\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=2, value="x")) + ] + result = apply_patches(source, patches) + assert "- x" in result + assert "- b" in result + + def test_insert_at_negative_index(self): + source = "items:\n - a\n - b\n - c\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=-1, value="x")) + ] + result = apply_patches(source, patches) + # -1 means before the last item (c), so order should be a, b, x, c + lines = result.strip().split("\n") + item_lines = [line.strip() for line in lines if line.strip().startswith("- ")] + assert item_lines == ["- a", "- b", "- x", "- c"] + + def test_insert_at_clamps_large_positive(self): + source = "items:\n - a\n - b\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=100, value="x")) + ] + result = apply_patches(source, patches) + # Should append at end + lines = result.strip().split("\n") + item_lines = [line.strip() for line in lines if line.strip().startswith("- ")] + assert item_lines[-1] == "- x" + + def test_insert_at_clamps_large_negative(self): + source = "items:\n - a\n - b\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=-100, value="x")) + ] + result = apply_patches(source, patches) + # Should prepend + lines = result.strip().split("\n") + item_lines = [line.strip() for line in lines if line.strip().startswith("- ")] + assert item_lines[0] == "- x" + + def test_insert_at_complex_value(self): + source = "repos:\n - repo: a\n - repo: c\n" + patches = [ + Patch( + route=Route(["repos"]), + operation=Op.insert_at( + index=1, value={"repo": "b", "hooks": [{"id": "check"}]} + ), + ) + ] + result = apply_patches(source, patches) + assert "repo: a" in result + assert "repo: b" in result + assert "repo: c" in result + assert "id: check" in result + + def test_insert_at_preserves_comments(self): + source = "items:\n # first item\n - a\n # third item\n - c\n" + patches = [ + Patch(route=Route(["items"]), operation=Op.insert_at(index=1, value="b")) + ] + result = apply_patches(source, patches) + assert "# first item" in result + assert "# third item" in result + assert "- b" in result diff --git a/tests/test_document.py b/tests/test_document.py index 5afa49b..e35afe3 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -5,6 +5,7 @@ from yamltrip.errors import ( KeyExistsError, KeyMissingError, + PatchError, QueryError, ) @@ -396,3 +397,76 @@ def test_remove_from_list(self): assert "b" not in result assert "a" in result assert "c" in result + + +class TestDocumentInsert: + def test_insert_middle(self): + doc = Document("items:\n - a\n - b\n - c\n") + doc2 = doc.insert("items", index=1, value="x") + assert doc2["items"] == ["a", "x", "b", "c"] + + def test_insert_beginning(self): + doc = Document("items:\n - a\n - b\n") + doc2 = doc.insert("items", index=0, value="x") + assert doc2["items"] == ["x", "a", "b"] + + def test_insert_end(self): + doc = Document("items:\n - a\n - b\n") + doc2 = doc.insert("items", index=2, value="x") + assert doc2["items"] == ["a", "b", "x"] + + def test_insert_negative_index(self): + doc = Document("items:\n - a\n - b\n - c\n") + doc2 = doc.insert("items", index=-1, value="x") + assert doc2["items"] == ["a", "b", "x", "c"] + + def test_insert_clamps_large_positive(self): + doc = Document("items:\n - a\n - b\n") + doc2 = doc.insert("items", index=100, value="x") + assert doc2["items"] == ["a", "b", "x"] + + def test_insert_clamps_large_negative(self): + doc = Document("items:\n - a\n - b\n") + doc2 = doc.insert("items", index=-100, value="x") + assert doc2["items"] == ["x", "a", "b"] + + def test_insert_complex_value(self): + doc = Document("repos:\n - repo: a\n - repo: c\n") + doc2 = doc.insert("repos", index=1, value={"repo": "b"}) + repos = doc2["repos"] + assert repos[0] == {"repo": "a"} + assert repos[1] == {"repo": "b"} + assert repos[2] == {"repo": "c"} + + def test_insert_preserves_comments(self): + source = "items:\n # first\n - a\n # third\n - c\n" + doc = Document(source) + doc2 = doc.insert("items", index=1, value="b") + assert "# first" in doc2.source + assert "# third" in doc2.source + + def test_insert_immutable(self): + doc = Document("items:\n - a\n - b\n") + doc2 = doc.insert("items", index=0, value="x") + assert doc["items"] == ["a", "b"] + assert doc2["items"] == ["x", "a", "b"] + + def test_insert_not_a_sequence(self): + doc = Document("name: foo\n") + with pytest.raises(PatchError): + doc.insert("name", index=0, value="x") + + def test_insert_path_not_found(self): + doc = Document("name: foo\n") + with pytest.raises(PatchError): + doc.insert("missing", index=0, value="x") + + def test_insert_flow_sequence_error(self): + doc = Document("items: [a, b, c]\n") + with pytest.raises(PatchError, match="BlockSequence"): + doc.insert("items", index=1, value="x") + + def test_insert_nested_path(self): + doc = Document("config:\n items:\n - a\n - b\n") + doc2 = doc.insert("config", "items", index=1, value="x") + assert doc2["config", "items"] == ["a", "x", "b"] diff --git a/tests/test_editor.py b/tests/test_editor.py index 5bd9e86..ad59e5d 100644 --- a/tests/test_editor.py +++ b/tests/test_editor.py @@ -123,6 +123,29 @@ def test_extract(self, yaml_file): assert "foo" in text +class TestEditorInsert: + def test_insert_middle(self, yaml_file): + with Editor(yaml_file) as editor: + editor.insert("items", index=1, value="x") + assert editor["items"] == ["a", "x", "b"] + + def test_insert_beginning(self, yaml_file): + with Editor(yaml_file) as editor: + editor.insert("items", index=0, value="x") + assert editor["items"] == ["x", "a", "b"] + + def test_insert_persists_to_file(self, yaml_file): + with Editor(yaml_file) as editor: + editor.insert("items", index=1, value="x") + content = yaml_file.read_text(encoding="utf-8") + assert "- x" in content + + def test_insert_negative(self, yaml_file): + with Editor(yaml_file) as editor: + editor.insert("items", index=-1, value="x") + assert editor["items"] == ["a", "x", "b"] + + class TestEditorGuards: def test_original_outside_context(self, yaml_file): editor = Editor(yaml_file) diff --git a/tests/test_integration.py b/tests/test_integration.py index 088f034..730fd71 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -81,3 +81,41 @@ def test_editor_multi_operation(self, tmp_path): assert reloaded["name"] == "Bob" assert reloaded["age"] == 25 assert reloaded["items"] == ["x", "y"] + + +class TestInsertPreCommitRepo: + """The primary use case: inserting a pre-commit repo at a specific position.""" + + def test_insert_repo_between_existing(self): + source = """\ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: trailing-whitespace + # ruff for linting + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.1.0 + hooks: + - id: ruff +""" + doc = yamltrip.Document(source) + new_repo = { + "repo": "https://github.com/psf/black", + "rev": "23.10.0", + "hooks": [{"id": "black"}], + } + doc2 = doc.insert("repos", index=1, value=new_repo) + + # Verify ordering + repos = doc2["repos"] + assert repos[0]["repo"] == "https://github.com/pre-commit/pre-commit-hooks" + assert repos[1]["repo"] == "https://github.com/psf/black" + assert repos[2]["repo"] == "https://github.com/astral-sh/ruff-pre-commit" + + # Verify comments preserved + assert "# ruff for linting" in doc2.source + + # Verify the new repo content + assert "id: black" in doc2.source + assert "rev: 23.10.0" in doc2.source