feat: libra add revert subcommand.#1266
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
731bf9d to
a93ee07
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements the libra revert subcommand to create new commits that undo changes from existing commits, following standard Git revert behavior. It provides a safe way to undo changes without altering project history.
Key changes include:
- Core revert implementation with support for regular commits and root commits
--no-commitflag to stage changes without committing- Comprehensive test coverage for various revert scenarios
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| libra/src/command/revert.rs | Main implementation of revert functionality with commit resolution, tree manipulation, and conflict detection |
| libra/src/command/mod.rs | Module registration and typo fix in save_object parameter name |
| libra/src/cli.rs | CLI integration for the new revert command |
| libra/tests/command/revert_test.rs | Comprehensive test suite covering basic revert, no-commit flag, root commits, and error cases |
| libra/tests/command/mod.rs | Test module registration |
| aria/contents/docs/libra/command/revert/index.mdx | Documentation for the revert command usage and examples |
Comments suppressed due to low confidence (1)
libra/tests/command/revert_test.rs:302
- The error test doesn't verify that the operation actually fails or check the error message. It should assert that the revert operation returns an error and verify the working directory remains unchanged.
// Test reverting non-existent commit should fail gracefully
| if current_files.get(path) != Some(&reverted_hash) && current_files.contains_key(path) { | ||
| return Err(format!( | ||
| "conflict: file '{}' was modified in a later commit", | ||
| path.display() | ||
| )); |
There was a problem hiding this comment.
The conflict detection logic is flawed. This condition will trigger false conflicts when the current file state legitimately differs from the reverted commit due to other changes. The check should compare if the current state conflicts with applying the revert, not just if it differs from the original commit.
| if current_files.get(path) != Some(&reverted_hash) && current_files.contains_key(path) { | |
| return Err(format!( | |
| "conflict: file '{}' was modified in a later commit", | |
| path.display() | |
| )); | |
| if let Some(current_hash) = current_files.get(path) { | |
| if let Some(parent_hash) = parent_hash { | |
| // Check if applying the revert would overwrite legitimate changes | |
| if current_hash != parent_hash { | |
| return Err(format!( | |
| "conflict: file '{}' was modified in a later commit", | |
| path.display() | |
| )); | |
| } | |
| } else { | |
| // File was newly added in the reverted commit, check if it still exists | |
| return Err(format!( | |
| "conflict: file '{}' was added in a later commit", | |
| path.display() | |
| )); | |
| } |
| if let Some(entry) = temp_index.get(path_str, 0) { | ||
| tree_items.push(mercury::internal::object::tree::TreeItem { | ||
| mode: mercury::internal::object::tree::TreeItemMode::Blob, | ||
| name: path_buf.file_name().unwrap().to_str().unwrap().to_string(), |
There was a problem hiding this comment.
Using only file_name() will cause issues with files in subdirectories as they will lose their directory structure. Files like 'dir/file.txt' will be stored as just 'file.txt' at the root level.
| name: path_buf.file_name().unwrap().to_str().unwrap().to_string(), | |
| name: path_buf.to_str().unwrap().to_string(), // Use full relative path |
| // Simplified implementation: create a temporary index, then create tree based on this index | ||
| let mut temp_index = Index::new(); | ||
| for (path, hash) in files { | ||
| let blob = mercury::internal::object::blob::Blob::load(&hash); |
There was a problem hiding this comment.
Loading blob objects just to get their size is inefficient. The size information should be available from the index entry or object metadata without loading the entire blob content.
a93ee07 to
a2c954b
Compare
Part of #1239
实现libra revert子命令的基本功能