Fix staging and unstaging of added and deleted files (#25631)

Max Brunsfeld created 10 months ago

* When staging in a buffer whose file has been deleted, do not save the
file
* Fix logic for writing to index when file is deleted

Release Notes:

- N/A

Change summary

crates/buffer_diff/src/buffer_diff.rs         | 38 ++++++++---
crates/editor/src/editor.rs                   | 64 +++++++++-----------
crates/editor/src/editor_tests.rs             |  1 
crates/editor/src/test/editor_test_context.rs |  9 ++
crates/language/src/buffer.rs                 |  8 ++
crates/project/src/git.rs                     | 16 ++++
6 files changed, 88 insertions(+), 48 deletions(-)

Detailed changes

crates/buffer_diff/src/buffer_diff.rs

@@ -184,17 +184,31 @@ impl BufferDiffSnapshot {
         cx: &mut App,
     ) -> Option<Rope> {
         let secondary_diff = self.secondary_diff()?;
-        let index_base = if let Some(index_base) = secondary_diff.base_text() {
-            index_base.text.as_rope().clone()
-        } else if stage {
-            Rope::from("")
-        } else {
-            return None;
+        let head_text = self.base_text().map(|text| text.as_rope().clone());
+        let index_text = secondary_diff
+            .base_text()
+            .map(|text| text.as_rope().clone());
+        let (index_text, head_text) = match (index_text, head_text) {
+            (Some(index_text), Some(head_text)) => (index_text, head_text),
+            // file is deleted in both index and head
+            (None, None) => return None,
+            // file is deleted in index
+            (None, Some(head_text)) => {
+                return if stage {
+                    Some(buffer.as_rope().clone())
+                } else {
+                    Some(head_text)
+                }
+            }
+            // file exists in the index, but is deleted in head
+            (Some(_), None) => {
+                return if stage {
+                    Some(buffer.as_rope().clone())
+                } else {
+                    None
+                }
+            }
         };
-        let head_base = self.base_text().map_or_else(
-            || Rope::from(""),
-            |snapshot| snapshot.text.as_rope().clone(),
-        );
 
         let mut secondary_cursor = secondary_diff.inner.hunks.cursor::<DiffHunkSummary>(buffer);
         secondary_cursor.next(buffer);
@@ -251,7 +265,7 @@ impl BufferDiffSnapshot {
                     .collect::<String>()
             } else {
                 log::debug!("unstaging");
-                head_base
+                head_text
                     .chunks_in_range(diff_base_byte_range.clone())
                     .collect::<String>()
             };
@@ -259,7 +273,7 @@ impl BufferDiffSnapshot {
         }
 
         let buffer = cx.new(|cx| {
-            language::Buffer::local_normalized(index_base, text::LineEnding::default(), cx)
+            language::Buffer::local_normalized(index_text, text::LineEnding::default(), cx)
         });
         let new_text = buffer.update(cx, |buffer, cx| {
             buffer.edit(edits, None, cx);

crates/editor/src/editor.rs

@@ -102,9 +102,9 @@ use language::{
         self, all_language_settings, language_settings, InlayHintSettings, RewrapBehavior,
     },
     point_from_lsp, text_diff_with_options, AutoindentMode, BracketMatch, BracketPair, Buffer,
-    Capability, CharKind, CodeLabel, CursorShape, Diagnostic, DiffOptions, DiskState,
-    EditPredictionsMode, EditPreview, HighlightedText, IndentKind, IndentSize, Language,
-    OffsetRangeExt, Point, Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions,
+    Capability, CharKind, CodeLabel, CursorShape, Diagnostic, DiffOptions, EditPredictionsMode,
+    EditPreview, HighlightedText, IndentKind, IndentSize, Language, OffsetRangeExt, Point,
+    Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions,
 };
 use language::{point_to_lsp, BufferRow, CharClassifier, Runnable, RunnableRange};
 use linked_editing_ranges::refresh_linked_ranges;
@@ -13483,13 +13483,16 @@ impl Editor {
         buffer_id: BufferId,
         hunks: impl Iterator<Item = MultiBufferDiffHunk>,
         snapshot: &MultiBufferSnapshot,
-        cx: &mut Context<Self>,
+        cx: &mut App,
     ) {
         let Some(buffer) = project.read(cx).buffer_for_id(buffer_id, cx) else {
             log::debug!("no buffer for id");
             return;
         };
         let buffer_snapshot = buffer.read(cx).snapshot();
+        let file_exists = buffer_snapshot
+            .file()
+            .is_some_and(|file| file.disk_state().exists());
         let Some((repo, path)) = project
             .read(cx)
             .repository_and_path_for_buffer_id(buffer_id, cx)
@@ -13502,40 +13505,33 @@ impl Editor {
             return;
         };
 
-        let Some(new_index_text) = diff.new_secondary_text_for_stage_or_unstage(
-            stage,
-            hunks.filter_map(|hunk| {
-                if stage && hunk.secondary_status == DiffHunkSecondaryStatus::None {
-                    return None;
-                } else if !stage
-                    && hunk.secondary_status == DiffHunkSecondaryStatus::HasSecondaryHunk
-                {
-                    return None;
-                }
-                Some((hunk.buffer_range.clone(), hunk.diff_base_byte_range.clone()))
-            }),
-            &buffer_snapshot,
-            cx,
-        ) else {
-            log::debug!("missing secondary diff or index text");
-            return;
-        };
-        let new_index_text = if new_index_text.is_empty()
-            && !stage
-            && (diff.is_single_insertion
-                || buffer_snapshot
-                    .file()
-                    .map_or(false, |file| file.disk_state() == DiskState::New))
-        {
+        let new_index_text = if !stage && diff.is_single_insertion || stage && !file_exists {
             log::debug!("removing from index");
             None
         } else {
-            Some(new_index_text)
+            diff.new_secondary_text_for_stage_or_unstage(
+                stage,
+                hunks.filter_map(|hunk| {
+                    if stage && hunk.secondary_status == DiffHunkSecondaryStatus::None {
+                        return None;
+                    } else if !stage
+                        && hunk.secondary_status == DiffHunkSecondaryStatus::HasSecondaryHunk
+                    {
+                        return None;
+                    }
+                    Some((hunk.buffer_range.clone(), hunk.diff_base_byte_range.clone()))
+                }),
+                &buffer_snapshot,
+                cx,
+            )
         };
-        let buffer_store = project.read(cx).buffer_store().clone();
-        buffer_store
-            .update(cx, |buffer_store, cx| buffer_store.save_buffer(buffer, cx))
-            .detach_and_log_err(cx);
+
+        if file_exists {
+            let buffer_store = project.read(cx).buffer_store().clone();
+            buffer_store
+                .update(cx, |buffer_store, cx| buffer_store.save_buffer(buffer, cx))
+                .detach_and_log_err(cx);
+        }
 
         cx.background_spawn(
             repo.read(cx)

crates/editor/src/editor_tests.rs

@@ -16510,6 +16510,7 @@ fn assert_hunk_revert(
 ) {
     cx.set_state(not_reverted_text_with_selections);
     cx.set_head_text(base_text);
+    cx.clear_index_text();
     cx.executor().run_until_parked();
 
     let actual_hunk_statuses_before = cx.update_editor(|editor, window, cx| {

crates/editor/src/test/editor_test_context.rs

@@ -298,6 +298,15 @@ impl EditorTestContext {
         self.cx.run_until_parked();
     }
 
+    pub fn clear_index_text(&mut self) {
+        self.cx.run_until_parked();
+        let fs = self.update_editor(|editor, _, cx| {
+            editor.project.as_ref().unwrap().read(cx).fs().as_fake()
+        });
+        fs.set_index_for_repo(&Self::root_path().join(".git"), &[]);
+        self.cx.run_until_parked();
+    }
+
     pub fn set_index_text(&mut self, diff_base: &str) {
         self.cx.run_until_parked();
         let fs = self.update_editor(|editor, _, cx| {

crates/language/src/buffer.rs

@@ -368,6 +368,14 @@ impl DiskState {
             DiskState::Deleted => None,
         }
     }
+
+    pub fn exists(&self) -> bool {
+        match self {
+            DiskState::New => false,
+            DiskState::Present { .. } => true,
+            DiskState::Deleted => false,
+        }
+    }
 }
 
 /// The file associated with a buffer, in the case where the file is on the local disk.

crates/project/src/git.rs

@@ -1071,7 +1071,13 @@ impl Repository {
                     };
                     let project_path = (self.worktree_id, path).into();
                     if let Some(buffer) = buffer_store.get_by_path(&project_path, cx) {
-                        save_futures.push(buffer_store.save_buffer(buffer, cx));
+                        if buffer
+                            .read(cx)
+                            .file()
+                            .map_or(false, |file| file.disk_state().exists())
+                        {
+                            save_futures.push(buffer_store.save_buffer(buffer, cx));
+                        }
                     }
                 }
             })
@@ -1110,7 +1116,13 @@ impl Repository {
                     };
                     let project_path = (self.worktree_id, path).into();
                     if let Some(buffer) = buffer_store.get_by_path(&project_path, cx) {
-                        save_futures.push(buffer_store.save_buffer(buffer, cx));
+                        if buffer
+                            .read(cx)
+                            .file()
+                            .map_or(false, |file| file.disk_state().exists())
+                        {
+                            save_futures.push(buffer_store.save_buffer(buffer, cx));
+                        }
                     }
                 }
             })