Skip to content

Commit 55991b9

Browse files
jdc-pubjdc-pub
committed
fix: handle arbitrary-depth nested fields in ledger tree operations
Replace inline splitn(2) logic with recursive insert_nested() and remove_nested() helpers that properly build/modify/delete entries at any depth (a, a/b, a/b/c, …). Assisted-by: Zed (Claude Opus 4.6)
1 parent d08ddee commit 55991b9

1 file changed

Lines changed: 67 additions & 59 deletions

File tree

‎crates/git-ledger/src/lib.rs‎

Lines changed: 67 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -69,31 +69,73 @@ pub trait Ledger {
6969
// Helpers
7070
// ---------------------------------------------------------------------------
7171

72+
/// Recursively insert a blob at an arbitrary depth inside a tree builder.
73+
fn insert_nested(
74+
repo: &Repository,
75+
builder: &mut git2::TreeBuilder<'_>,
76+
components: &[&str],
77+
blob_oid: Oid,
78+
) -> Result<(), Error> {
79+
match components {
80+
[leaf] => {
81+
builder.insert(leaf, blob_oid, 0o100644)?;
82+
}
83+
[head, rest @ ..] => {
84+
let mut sub_builder = if let Some(existing) = builder.get(head)? {
85+
let existing_tree = repo.find_tree(existing.id())?;
86+
repo.treebuilder(Some(&existing_tree))?
87+
} else {
88+
repo.treebuilder(None)?
89+
};
90+
insert_nested(repo, &mut sub_builder, rest, blob_oid)?;
91+
let sub_tree = sub_builder.write()?;
92+
builder.insert(head, sub_tree, 0o040000)?;
93+
}
94+
[] => {}
95+
}
96+
Ok(())
97+
}
98+
99+
/// Recursively remove a blob at an arbitrary depth inside a tree builder.
100+
/// Returns `true` if the subtree at this level is now empty and should be pruned.
101+
fn remove_nested(
102+
repo: &Repository,
103+
builder: &mut git2::TreeBuilder<'_>,
104+
components: &[&str],
105+
) -> Result<bool, Error> {
106+
match components {
107+
[leaf] => {
108+
let _ = builder.remove(leaf);
109+
}
110+
[head, rest @ ..] => {
111+
let existing_tree_id = builder
112+
.get(head)?
113+
.filter(|e| e.kind() == Some(git2::ObjectType::Tree))
114+
.map(|e| e.id());
115+
if let Some(tree_id) = existing_tree_id {
116+
let et = repo.find_tree(tree_id)?;
117+
let mut sub_builder = repo.treebuilder(Some(&et))?;
118+
let empty = remove_nested(repo, &mut sub_builder, rest)?;
119+
if empty {
120+
let _ = builder.remove(head);
121+
} else {
122+
let sub_tree = sub_builder.write()?;
123+
builder.insert(head, sub_tree, 0o040000)?;
124+
}
125+
}
126+
}
127+
[] => {}
128+
}
129+
Ok(builder.is_empty())
130+
}
131+
72132
/// Build a tree from a list of field name/value pairs.
73133
fn build_fields_tree(repo: &Repository, fields: &[(&str, &[u8])]) -> Result<Oid, Error> {
74134
let mut builder = repo.treebuilder(None)?;
75135
for (name, value) in fields {
76136
let blob_oid = repo.blob(value)?;
77-
// Support nested fields: if name contains '/', create subtrees
78-
if name.contains('/') {
79-
// For simplicity, handle single-level nesting
80-
let parts: Vec<&str> = name.splitn(2, '/').collect();
81-
let sub_blob = repo.blob(value)?;
82-
// Build or update subtree
83-
let sub_tree = if let Some(existing) = builder.get(parts[0])? {
84-
let existing_tree = repo.find_tree(existing.id())?;
85-
let mut sub_builder = repo.treebuilder(Some(&existing_tree))?;
86-
sub_builder.insert(parts[1], sub_blob, 0o100644)?;
87-
sub_builder.write()?
88-
} else {
89-
let mut sub_builder = repo.treebuilder(None)?;
90-
sub_builder.insert(parts[1], sub_blob, 0o100644)?;
91-
sub_builder.write()?
92-
};
93-
builder.insert(parts[0], sub_tree, 0o040000)?;
94-
} else {
95-
builder.insert(name, blob_oid, 0o100644)?;
96-
}
137+
let components: Vec<&str> = name.split('/').collect();
138+
insert_nested(repo, &mut builder, &components, blob_oid)?;
97139
}
98140
builder.write()
99141
}
@@ -255,47 +297,13 @@ impl Ledger for Repository {
255297
for mutation in mutations {
256298
match mutation {
257299
Mutation::Set(name, value) => {
258-
if name.contains('/') {
259-
let parts: Vec<&str> = name.splitn(2, '/').collect();
260-
let sub_blob = self.blob(value)?;
261-
let sub_tree = if let Some(existing) = builder.get(parts[0])? {
262-
let et = self.find_tree(existing.id())?;
263-
let mut sub_builder = self.treebuilder(Some(&et))?;
264-
sub_builder.insert(parts[1], sub_blob, 0o100644)?;
265-
sub_builder.write()?
266-
} else {
267-
let mut sub_builder = self.treebuilder(None)?;
268-
sub_builder.insert(parts[1], sub_blob, 0o100644)?;
269-
sub_builder.write()?
270-
};
271-
builder.insert(parts[0], sub_tree, 0o040000)?;
272-
} else {
273-
let blob_oid = self.blob(value)?;
274-
builder.insert(name, blob_oid, 0o100644)?;
275-
}
300+
let blob_oid = self.blob(value)?;
301+
let components: Vec<&str> = name.split('/').collect();
302+
insert_nested(self, &mut builder, &components, blob_oid)?;
276303
}
277304
Mutation::Delete(name) => {
278-
if name.contains('/') {
279-
let parts: Vec<&str> = name.splitn(2, '/').collect();
280-
let existing_tree_id = builder
281-
.get(parts[0])?
282-
.filter(|e| e.kind() == Some(git2::ObjectType::Tree))
283-
.map(|e| e.id());
284-
if let Some(tree_id) = existing_tree_id {
285-
let et = self.find_tree(tree_id)?;
286-
let mut sub_builder = self.treebuilder(Some(&et))?;
287-
let _ = sub_builder.remove(parts[1]);
288-
if sub_builder.is_empty() {
289-
let _ = builder.remove(parts[0]);
290-
} else {
291-
let sub_tree = sub_builder.write()?;
292-
builder.insert(parts[0], sub_tree, 0o040000)?;
293-
}
294-
}
295-
} else {
296-
// Ignore error if the field doesn't exist
297-
let _ = builder.remove(name);
298-
}
305+
let components: Vec<&str> = name.split('/').collect();
306+
remove_nested(self, &mut builder, &components)?;
299307
}
300308
}
301309
}

0 commit comments

Comments
 (0)