mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
action_log: Accept deleted hunks after commit (#56892)
Sometimes the action log would not auto-accept deleted hunks, and I
finally found a repro for this, here's an explanation of what went wrong
```rust
// Before
use crate::{Alpha, Beta};
fn keep() {
work();
}
fn remove() {
work();
}
fn after() {
work();
}
```
```rust
// After commit
use crate::{Alpha};
fn keep() {
work();
}
fn after() {
work();
}
```
The action log may track the deletion as:
```diff
fn keep() {
work();
}
-fn remove() {
- work();
-}
-
fn after() {
work();
}
```
But the commit diff may choose different boundaries because nearby lines
repeat:
```diff
fn keep() {
work();
-}
-
-fn remove() {
- work();
}
fn after() {
work();
}
```
Both diffs produce the same final file, but their row ranges differ. The
previous logic only accepted committed edits when those row ranges
matched exactly, so already-committed edits could remain marked as
unaccepted.
Now we have a fast path for checking if the base_text matches exactly,
which works fine in this case.
Release Notes:
- Fixed an issue where agent edits would sometimes not get auto-accepted
when they were commited
This commit is contained in:
parent
3a742b5e0d
commit
1fb920775b
3 changed files with 90 additions and 3 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -81,6 +81,7 @@ dependencies = [
|
|||
"futures 0.3.32",
|
||||
"git",
|
||||
"gpui",
|
||||
"indoc",
|
||||
"language",
|
||||
"log",
|
||||
"pretty_assertions",
|
||||
|
|
|
|||
|
|
@ -33,12 +33,12 @@ watch.workspace = true
|
|||
|
||||
[dev-dependencies]
|
||||
buffer_diff = { workspace = true, features = ["test-support"] }
|
||||
git.workspace = true
|
||||
collections = { workspace = true, features = ["test-support"] }
|
||||
clock = { workspace = true, features = ["test-support"] }
|
||||
collections = { workspace = true, features = ["test-support"] }
|
||||
ctor.workspace = true
|
||||
git.workspace = true
|
||||
gpui = { workspace = true, features = ["test-support"] }
|
||||
|
||||
indoc.workspace = true
|
||||
language = { workspace = true, features = ["test-support"] }
|
||||
log.workspace = true
|
||||
pretty_assertions.workspace = true
|
||||
|
|
|
|||
|
|
@ -387,6 +387,11 @@ impl ActionLog {
|
|||
let git_diff_base = git_diff.read(cx).base_text(cx).as_rope().clone();
|
||||
let buffer_text = tracked_buffer.snapshot.as_rope().clone();
|
||||
anyhow::Ok(cx.background_spawn(async move {
|
||||
if buffer_text.len() == git_diff_base.len()
|
||||
&& buffer_text.chars_at(0).eq(git_diff_base.chars_at(0))
|
||||
{
|
||||
return (Arc::<str>::from(git_diff_base.to_string()), git_diff_base);
|
||||
}
|
||||
let mut old_unreviewed_edits = old_unreviewed_edits.into_iter().peekable();
|
||||
let committed_edits = language::line_diff(
|
||||
&agent_diff_base.to_string(),
|
||||
|
|
@ -1320,6 +1325,7 @@ mod tests {
|
|||
use super::*;
|
||||
use buffer_diff::DiffHunkStatusKind;
|
||||
use gpui::TestAppContext;
|
||||
use indoc::indoc;
|
||||
use language::Point;
|
||||
use project::{FakeFs, Fs, Project, RemoveOptions};
|
||||
use rand::prelude::*;
|
||||
|
|
@ -2703,6 +2709,86 @@ mod tests {
|
|||
assert_eq!(unreviewed_hunks(&action_log, cx), vec![]);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_keep_edits_on_commit_with_shifted_diff_boundaries(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let initial_text = indoc! {"
|
||||
use crate::{Alpha, Beta};
|
||||
|
||||
fn keep() {
|
||||
work();
|
||||
}
|
||||
|
||||
fn remove() {
|
||||
work();
|
||||
}
|
||||
|
||||
fn after() {
|
||||
work();
|
||||
}
|
||||
"};
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/project"),
|
||||
json!({
|
||||
".git": {},
|
||||
"file.rs": initial_text,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
fs.set_head_for_repo(
|
||||
path!("/project/.git").as_ref(),
|
||||
&[("file.rs", initial_text.into())],
|
||||
"0000000",
|
||||
);
|
||||
cx.run_until_parked();
|
||||
|
||||
let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
|
||||
let action_log = cx.new(|_| ActionLog::new(project.clone()));
|
||||
|
||||
let file_path = project
|
||||
.read_with(cx, |project, cx| {
|
||||
project.find_project_path(path!("/project/file.rs"), cx)
|
||||
})
|
||||
.unwrap();
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| project.open_buffer(file_path, cx))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let final_text = indoc! {"
|
||||
use crate::{Alpha};
|
||||
|
||||
fn keep() {
|
||||
work();
|
||||
}
|
||||
|
||||
fn after() {
|
||||
work();
|
||||
}
|
||||
"};
|
||||
|
||||
cx.update(|cx| {
|
||||
action_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx));
|
||||
buffer.update(cx, |buffer, cx| {
|
||||
buffer.set_text(final_text, cx);
|
||||
});
|
||||
action_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx));
|
||||
});
|
||||
cx.run_until_parked();
|
||||
assert!(!unreviewed_hunks(&action_log, cx).is_empty());
|
||||
|
||||
fs.set_head_for_repo(
|
||||
path!("/project/.git").as_ref(),
|
||||
&[("file.rs", final_text.into())],
|
||||
"0000001",
|
||||
);
|
||||
cx.run_until_parked();
|
||||
|
||||
assert_eq!(unreviewed_hunks(&action_log, cx), vec![]);
|
||||
}
|
||||
|
||||
/// Regression test: when head_commit updates before the BufferDiff's base
|
||||
/// text does, an intermediate DiffChanged (e.g. from a buffer-edit diff
|
||||
/// recalculation) must NOT consume the commit signal. The subscription
|
||||
|
|
|
|||
Loading…
Reference in a new issue