Use previous cluster context

Agus Zubiaga , Ben Kunkle , and Max Brunsfeld created

Co-authored-by: Ben Kunkle <ben.kunkle@gmail.com>
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Change summary

crates/zeta_cli/src/example.rs | 246 ++++++++++++++++++++++++++++-------
1 file changed, 195 insertions(+), 51 deletions(-)

Detailed changes

crates/zeta_cli/src/example.rs 🔗

@@ -5,6 +5,7 @@ use std::{
     fs,
     io::Write,
     mem,
+    ops::Range,
     path::{Path, PathBuf},
 };
 
@@ -428,27 +429,32 @@ pub async fn apply_diff(
     use std::fmt::Write;
 
     #[derive(Debug, Default)]
-    struct Edit {
+    struct HunkState {
         context: String,
-        deletion_start: Option<usize>,
-        addition: String,
+        edits: Vec<Edit>,
+    }
+
+    // #[derive(Debug, Default)]
+    // struct Edit {
+    //     deletion_start: Option<usize>,
+    //     addition: String,
+    // }
+
+    #[derive(Debug)]
+    struct Edit {
+        range: Range<usize>,
+        text: String,
     }
 
     let mut old_path = None;
     let mut new_path = None;
-    let mut pending = Edit::default();
+    let mut hunk = HunkState::default();
     let mut diff_lines = diff.lines().map(DiffLine::parse).peekable();
     let mut open_buffers = HashSet::default();
 
     while let Some(diff_line) = diff_lines.next() {
         match diff_line {
-            DiffLine::OldPath { path } => {
-                mem::take(&mut pending);
-                old_path = Some(path)
-            }
-            DiffLine::HunkHeader(_) => {
-                mem::take(&mut pending);
-            }
+            DiffLine::OldPath { path } => old_path = Some(path),
             DiffLine::NewPath { path } => {
                 if old_path.is_none() {
                     anyhow::bail!(
@@ -458,36 +464,45 @@ pub async fn apply_diff(
                 new_path = Some(path)
             }
             DiffLine::Context(ctx) => {
-                writeln!(&mut pending.context, "{ctx}")?;
+                writeln!(&mut hunk.context, "{ctx}")?;
             }
             DiffLine::Deletion(del) => {
-                pending.deletion_start.get_or_insert(pending.context.len());
-                writeln!(&mut pending.context, "{del}")?;
+                let range = hunk.context.len()..hunk.context.len() + del.len() + '\n'.len_utf8();
+                if let Some(last_edit) = hunk.edits.last_mut()
+                    && last_edit.range.end == range.start
+                {
+                    last_edit.range.end = range.end;
+                } else {
+                    hunk.edits.push(Edit {
+                        range,
+                        text: String::new(),
+                    });
+                }
+                writeln!(&mut hunk.context, "{del}")?;
             }
             DiffLine::Addition(add) => {
-                writeln!(&mut pending.addition, "{add}")?;
+                let range = hunk.context.len()..hunk.context.len();
+                if let Some(last_edit) = hunk.edits.last_mut()
+                    && last_edit.range.end == range.start
+                {
+                    writeln!(&mut last_edit.text, "{add}").unwrap();
+                } else {
+                    hunk.edits.push(Edit {
+                        range,
+                        text: format!("{add}\n"),
+                    });
+                }
             }
-            DiffLine::Garbage => {}
+            DiffLine::HunkHeader(_) | DiffLine::Garbage => {}
         }
 
-        let commit_pending = match diff_lines.peek() {
-            Some(DiffLine::OldPath { .. })
-            | Some(DiffLine::HunkHeader(_))
-            | Some(DiffLine::Context(_))
-            | None => {
-                // commit pending edit cluster
-                !pending.addition.is_empty() || pending.deletion_start.is_some()
-            }
-            Some(DiffLine::Deletion(_)) => {
-                // start a new cluster if we have any additions specifically
-                // if we only have deletions, we continue to aggregate them
-                !pending.addition.is_empty()
-            }
+        let at_hunk_end = match diff_lines.peek() {
+            Some(DiffLine::OldPath { .. }) | Some(DiffLine::HunkHeader(_)) | None => true,
             _ => false,
         };
 
-        if commit_pending {
-            let edit = mem::take(&mut pending);
+        if at_hunk_end {
+            let hunk = mem::take(&mut hunk);
 
             let Some(old_path) = old_path.as_deref() else {
                 anyhow::bail!("Missing old path (`---`) header")
@@ -523,27 +538,36 @@ pub async fn apply_diff(
 
             // TODO is it worth using project search?
             buffer.update(cx, |buffer, cx| {
-                if edit.context.is_empty() {
-                    buffer.edit([(0..0, edit.addition)], None, cx);
-                    return anyhow::Ok(());
-                }
-
-                let text = buffer.text();
-                // todo! check there's only one
-                if let Some(context_offset) = text.find(&edit.context) {
-                    let end = context_offset + edit.context.len();
-                    let start = if let Some(deletion_start) = edit.deletion_start {
-                        context_offset + deletion_start
+                let context_offset = if hunk.context.is_empty() {
+                    0
+                } else {
+                    let text = buffer.text();
+                    if let Some(offset) = text.find(&hunk.context) {
+                        if text[offset + 1..].find(&hunk.context).is_some() {
+                            anyhow::bail!("Context is not unique enough:\n{}", hunk.context);
+                        }
+                        offset
                     } else {
-                        end
-                    };
-
-                    buffer.edit([(start..end, edit.addition)], None, cx);
+                        anyhow::bail!(
+                            "Failed to match context:\n{}\n\nBuffer:\n{}",
+                            hunk.context,
+                            text
+                        );
+                    }
+                };
+
+                buffer.edit(
+                    hunk.edits.into_iter().map(|edit| {
+                        (
+                            context_offset + edit.range.start..context_offset + edit.range.end,
+                            edit.text,
+                        )
+                    }),
+                    None,
+                    cx,
+                );
 
-                    anyhow::Ok(())
-                } else {
-                    anyhow::bail!("Failed to match context:\n{}", edit.context);
-                }
+                anyhow::Ok(())
             })??;
         }
     }
@@ -563,7 +587,7 @@ mod tests {
     use settings::SettingsStore;
 
     #[gpui::test]
-    async fn test_apply_diff(cx: &mut TestAppContext) {
+    async fn test_apply_diff_successful(cx: &mut TestAppContext) {
         let buffer_1_text = indoc! {r#"
             one
             two
@@ -679,4 +703,124 @@ mod tests {
             assert_eq!(buffer.text(), buffer_2_text_final);
         });
     }
+
+    #[gpui::test]
+    async fn test_apply_diff_non_unique(cx: &mut TestAppContext) {
+        let buffer_1_text = indoc! {r#"
+            one
+            two
+            three
+            four
+            five
+            one
+            two
+            three
+            four
+            five
+        "# };
+
+        cx.update(|cx| {
+            let settings_store = SettingsStore::test(cx);
+            cx.set_global(settings_store);
+            Project::init_settings(cx);
+            language::init(cx);
+        });
+
+        let fs = FakeFs::new(cx.background_executor().clone());
+        fs.insert_tree(
+            "/root",
+            json!({
+                "file1": buffer_1_text,
+            }),
+        )
+        .await;
+
+        let project = Project::test(fs, ["/root".as_ref()], cx).await;
+
+        let diff = indoc! {r#"
+            --- a/root/file1
+            +++ b/root/file1
+             one
+             two
+            -three
+            +3
+             four
+             five
+        "#};
+
+        apply_diff(diff, &project, &mut cx.to_async())
+            .await
+            .expect_err("Non-unique edits should fail");
+    }
+
+    #[gpui::test]
+    async fn test_apply_diff_unique_via_previous_context(cx: &mut TestAppContext) {
+        let start = indoc! {r#"
+            one
+            two
+            three
+            four
+            five
+
+            four
+            five
+        "# };
+
+        let end = indoc! {r#"
+            one
+            two
+            3
+            four
+            5
+
+            four
+            five
+        "# };
+
+        cx.update(|cx| {
+            let settings_store = SettingsStore::test(cx);
+            cx.set_global(settings_store);
+            Project::init_settings(cx);
+            language::init(cx);
+        });
+
+        let fs = FakeFs::new(cx.background_executor().clone());
+        fs.insert_tree(
+            "/root",
+            json!({
+                "file1": start,
+            }),
+        )
+        .await;
+
+        let project = Project::test(fs, ["/root".as_ref()], cx).await;
+
+        let diff = indoc! {r#"
+            --- a/root/file1
+            +++ b/root/file1
+             one
+             two
+            -three
+            +3
+             four
+            -five
+            +5
+        "#};
+
+        let _buffers = apply_diff(diff, &project, &mut cx.to_async())
+            .await
+            .unwrap();
+
+        let buffer_1 = project
+            .update(cx, |project, cx| {
+                let project_path = project.find_project_path("/root/file1", cx).unwrap();
+                project.open_buffer(project_path, cx)
+            })
+            .await
+            .unwrap();
+
+        buffer_1.read_with(cx, |buffer, _cx| {
+            assert_eq!(buffer.text(), end);
+        });
+    }
 }