feat: add leaf-only tree-editor removal#2654
Merged
Merged
Conversation
Sebastian Thiel (Byron)
force-pushed
the
tree-editor-improvement
branch
from
June 19, 2026 04:30
829533b to
a46e812
Compare
Sebastian Thiel (Byron)
marked this pull request as ready for review
June 19, 2026 04:30
Motivation comes from gitbutlerapp/gitbutler#14312, where callers had to split tree-entry deletions from additions to avoid accidentally deleting a newly-added subtree after adding A/one and then deleting A. Add Editor::remove_leaf() to gix-object and expose it through gix object tree editors and cursors. The new API keeps remove() behavior unchanged, remains tolerant of absent paths, but returns an error when the target entry exists and is a tree. Co-authored-by: Sebastian Thiel <[email protected]>
Sebastian Thiel (Byron)
force-pushed
the
tree-editor-improvement
branch
from
June 19, 2026 04:32
a46e812 to
cd610db
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a “leaf-only” tree-editor removal API to prevent accidental deletion of entire subtrees when a caller intends to remove only a leaf entry. It adds remove_leaf() in the plumbing gix-object editor and wires it through higher-level wrappers, with new regression tests covering the intended semantics.
Changes:
- Add
remove_leaf()togix_object::tree::Editorand itsCursor, returning an error when the target is a tree. - Expose
remove_leaf()through thegixtree editor and cursor wrappers. - Add tests ensuring leaf-only removal rejects trees, tolerates absent paths, and handles empty/unreachable path segments as intended.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
gix-object/src/tree/editor.rs |
Adds remove_leaf() and refactors edit operations to support leaf-only vs any removal modes. |
gix-object/tests/object/tree/editor.rs |
Adds regression tests for leaf-only removal behavior and unreachable empty path components. |
gix/src/object/tree/editor.rs |
Exposes remove_leaf() on the public gix tree editor and cursor wrappers. |
gix/tests/gix/repository/object.rs |
Adds an integration test asserting tree entries are rejected by leaf-only removal. |
src/object/tree/editor.rs |
Introduces a tree editor wrapper module (currently appears misplaced/orphaned) and adds remove_leaf() on its cursor/editor API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Reported issue
Byron asked Codex to take a look at https://github.com/gitbutlerapp/gitbutler/pull/14312/commits and see if this commit-message comment could be resolved by adding such an API:
Motivation: gitbutlerapp/gitbutler#14312
Summary
remove_leaf()togix_object::tree::Editorand cursor, preserving existingremove()semantics.remove_leaf()through the publicgixtree editor and cursor wrappers.Validation
cargo fmt --allcargo test -p gix-object --test object from_empty_remove_leaf_rejects_tree_entriescargo test -p gix-object --test object tree::editorcargo test -p gix --test gix repository::object::edit_treecargo check -p gix-object -p gix --features tree-editorcargo check -p gix --no-default-features --features tree-editor,sha1cargo check -p gix-object -p gix --no-default-features --features tree-editor,sha1git diff --checkcodex review --commit 829533b5f52ee35e578984a82e092f566b947292