From 369828f51c5157813e73374df29ea8174af09c1a Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Fri, 15 Nov 2024 15:01:16 -0700 Subject: [PATCH] Require save confirmation and prevent autosave for deleted files (#20742) * `has_conflict` will now return true if the file has been deleted on disk. This is for treating multi-buffers as conflicted, and also blocks auto-save. * `has_deleted_file` is added so that the single-file buffer save can specifically mention the delete conflict. This does not yet handle discard (#20745). Closes #9101 Closes #9568 Closes #20462 Release Notes: - Improved handling of externally deleted files: auto-save will be disabled, multibuffers will treat this as a save conflict, and single buffers will ask for restore confirmation. Co-authored-by: Conrad --- crates/diagnostics/src/diagnostics.rs | 4 ++ crates/editor/src/items.rs | 4 ++ crates/language/src/buffer.rs | 15 +++-- crates/multi_buffer/src/multi_buffer.rs | 13 +++++ crates/workspace/src/item.rs | 8 +++ crates/workspace/src/pane.rs | 76 +++++++++++++++++-------- 6 files changed, 93 insertions(+), 27 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 933bed2711297c96e3a0c025d0713bc51ac30e39..6f20b91689cbc464df8756b32cf5945ce8a04490 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -727,6 +727,10 @@ impl Item for ProjectDiagnosticsEditor { self.excerpts.read(cx).is_dirty(cx) } + fn has_deleted_file(&self, cx: &AppContext) -> bool { + self.excerpts.read(cx).has_deleted_file(cx) + } + fn has_conflict(&self, cx: &AppContext) -> bool { self.excerpts.read(cx).has_conflict(cx) } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index bc8119c82964eaca23c71b79a285ee0335a8c858..8057daafa5d5286e6e91a27c81ad4717e0fa7c4e 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -708,6 +708,10 @@ impl Item for Editor { self.buffer().read(cx).read(cx).is_dirty() } + fn has_deleted_file(&self, cx: &AppContext) -> bool { + self.buffer().read(cx).read(cx).has_deleted_file() + } + fn has_conflict(&self, cx: &AppContext) -> bool { self.buffer().read(cx).read(cx).has_conflict() } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 690b2d15baa29ec55f505dff9cbc798a719f74a8..77fc53705fd3631c38260122ba7acbf8a55511cf 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -1749,13 +1749,20 @@ impl Buffer { .map_or(false, |file| file.is_deleted() || !file.is_created())) } + pub fn is_deleted(&self) -> bool { + self.file.as_ref().map_or(false, |file| file.is_deleted()) + } + /// Checks if the buffer and its file have both changed since the buffer /// was last saved or reloaded. pub fn has_conflict(&self) -> bool { - self.has_conflict - || self.file.as_ref().map_or(false, |file| { - file.mtime() > self.saved_mtime && self.has_unsaved_edits() - }) + if self.has_conflict { + return true; + } + let Some(file) = self.file.as_ref() else { + return false; + }; + file.is_deleted() || (file.mtime() > self.saved_mtime && self.has_unsaved_edits()) } /// Gets a [`Subscription`] that tracks all of the changes to the buffer's text. diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index bdeb57912300972fee9f25772a4a2d6d100bdae8..5d111d81cec4d06be2b1cecc4e39d4f7dd91a113 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -186,6 +186,7 @@ pub struct MultiBufferSnapshot { non_text_state_update_count: usize, edit_count: usize, is_dirty: bool, + has_deleted_file: bool, has_conflict: bool, show_headers: bool, } @@ -494,6 +495,10 @@ impl MultiBuffer { self.read(cx).is_dirty() } + pub fn has_deleted_file(&self, cx: &AppContext) -> bool { + self.read(cx).has_deleted_file() + } + pub fn has_conflict(&self, cx: &AppContext) -> bool { self.read(cx).has_conflict() } @@ -1419,6 +1424,7 @@ impl MultiBuffer { snapshot.excerpts = Default::default(); snapshot.trailing_excerpt_update_count += 1; snapshot.is_dirty = false; + snapshot.has_deleted_file = false; snapshot.has_conflict = false; self.subscriptions.publish_mut([Edit { @@ -2003,6 +2009,7 @@ impl MultiBuffer { let mut excerpts_to_edit = Vec::new(); let mut non_text_state_updated = false; let mut is_dirty = false; + let mut has_deleted_file = false; let mut has_conflict = false; let mut edited = false; let mut buffers = self.buffers.borrow_mut(); @@ -2028,6 +2035,7 @@ impl MultiBuffer { edited |= buffer_edited; non_text_state_updated |= buffer_non_text_state_updated; is_dirty |= buffer.is_dirty(); + has_deleted_file |= buffer.file().map_or(false, |file| file.is_deleted()); has_conflict |= buffer.has_conflict(); } if edited { @@ -2037,6 +2045,7 @@ impl MultiBuffer { snapshot.non_text_state_update_count += 1; } snapshot.is_dirty = is_dirty; + snapshot.has_deleted_file = has_deleted_file; snapshot.has_conflict = has_conflict; excerpts_to_edit.sort_unstable_by_key(|(locator, _, _)| *locator); @@ -3691,6 +3700,10 @@ impl MultiBufferSnapshot { self.is_dirty } + pub fn has_deleted_file(&self) -> bool { + self.has_deleted_file + } + pub fn has_conflict(&self) -> bool { self.has_conflict } diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 2f1c900ecf9c427600adda42f212b59a443262b8..5f14b9ba62738ce8301195ee8f5b40d1baba05dc 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -228,6 +228,9 @@ pub trait Item: FocusableView + EventEmitter { fn is_dirty(&self, _: &AppContext) -> bool { false } + fn has_deleted_file(&self, _: &AppContext) -> bool { + false + } fn has_conflict(&self, _: &AppContext) -> bool { false } @@ -405,6 +408,7 @@ pub trait ItemHandle: 'static + Send { fn item_id(&self) -> EntityId; fn to_any(&self) -> AnyView; fn is_dirty(&self, cx: &AppContext) -> bool; + fn has_deleted_file(&self, cx: &AppContext) -> bool; fn has_conflict(&self, cx: &AppContext) -> bool; fn can_save(&self, cx: &AppContext) -> bool; fn save( @@ -768,6 +772,10 @@ impl ItemHandle for View { self.read(cx).is_dirty(cx) } + fn has_deleted_file(&self, cx: &AppContext) -> bool { + self.read(cx).has_deleted_file(cx) + } + fn has_conflict(&self, cx: &AppContext) -> bool { self.read(cx).has_conflict(cx) } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index b21a531922a96187376127784c65f031a11bd7d4..24e48b53b08c7660d746e99ab4ddcaa6421a49f9 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1541,18 +1541,25 @@ impl Pane { const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; + const DELETED_MESSAGE: &str = + "This file has been deleted on disk since you started editing it. Do you want to recreate it?"; + if save_intent == SaveIntent::Skip { return Ok(true); } - let (mut has_conflict, mut is_dirty, mut can_save, can_save_as) = cx.update(|cx| { - ( - item.has_conflict(cx), - item.is_dirty(cx), - item.can_save(cx), - item.is_singleton(cx), - ) - })?; + let (mut has_conflict, mut is_dirty, mut can_save, is_singleton, has_deleted_file) = cx + .update(|cx| { + ( + item.has_conflict(cx), + item.is_dirty(cx), + item.can_save(cx), + item.is_singleton(cx), + item.has_deleted_file(cx), + ) + })?; + + let can_save_as = is_singleton; // when saving a single buffer, we ignore whether or not it's dirty. if save_intent == SaveIntent::Save || save_intent == SaveIntent::SaveWithoutFormat { @@ -1572,22 +1579,45 @@ impl Pane { let should_format = save_intent != SaveIntent::SaveWithoutFormat; if has_conflict && can_save { - let answer = pane.update(cx, |pane, cx| { - pane.activate_item(item_ix, true, true, cx); - cx.prompt( - PromptLevel::Warning, - CONFLICT_MESSAGE, - None, - &["Overwrite", "Discard", "Cancel"], - ) - })?; - match answer.await { - Ok(0) => { - pane.update(cx, |_, cx| item.save(should_format, project, cx))? - .await? + if has_deleted_file && is_singleton { + let answer = pane.update(cx, |pane, cx| { + pane.activate_item(item_ix, true, true, cx); + cx.prompt( + PromptLevel::Warning, + DELETED_MESSAGE, + None, + &["Overwrite", "Close", "Cancel"], + ) + })?; + match answer.await { + Ok(0) => { + pane.update(cx, |_, cx| item.save(should_format, project, cx))? + .await? + } + Ok(1) => { + pane.update(cx, |pane, cx| pane.remove_item(item_ix, false, false, cx))?; + } + _ => return Ok(false), + } + return Ok(true); + } else { + let answer = pane.update(cx, |pane, cx| { + pane.activate_item(item_ix, true, true, cx); + cx.prompt( + PromptLevel::Warning, + CONFLICT_MESSAGE, + None, + &["Overwrite", "Discard", "Cancel"], + ) + })?; + match answer.await { + Ok(0) => { + pane.update(cx, |_, cx| item.save(should_format, project, cx))? + .await? + } + Ok(1) => pane.update(cx, |_, cx| item.reload(project, cx))?.await?, + _ => return Ok(false), } - Ok(1) => pane.update(cx, |_, cx| item.reload(project, cx))?.await?, - _ => return Ok(false), } } else if is_dirty && (can_save || can_save_as) { if save_intent == SaveIntent::Close {