Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

vim: Fix :wq in multibuffer (#24603)

Browse source 
Supercedes #24561
Closes #21059

Before this change we would skip saving multibuffers regardless of the
save intent. Now we correctly save them.

Along the way:
* Prompt to save when closing the last singleton copy of an item (even
if it's still open in a multibuffer).
* Update our file name prompt to pull out dirty project items from
multibuffers instead of counting multibuffers as untitled files.
* Fix our prompt test helpers to require passing the button name instead
of the index. A few tests were passing invalid responses to save
prompts.
* Refactor the code a bit to hopefully clarify it for the next bug.

Release Notes:

- Fixed edge-cases when closing multiple items including multibuffers.
Previously no prompt was generated when closing an item that was open in
a multibuffer, now you will be prompted.
- vim: Fix :wq in a multibuffer
This commit is contained in:
Conrad Irwin 2025-02-13 10:13:43 -07:00 • committed by GitHub
parent 8c780ba287
commit 2f741c8686
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 318 additions and 290 deletions
Download patch file Download diff file Expand all files Collapse all files

216
crates/workspace/src/pane.rs
View file

@ -1453,6 +1453,40 @@ impl Pane {
.for_each(|&index| self._remove_item(index, false, false, None, window, cx));
}
// Usually when you close an item that has unsaved changes, we prompt you to
// save it. That said, if you still have the buffer open in a different pane
// we can close this one without fear of losing data.
pub fn skip_save_on_close(item: &dyn ItemHandle, workspace: &Workspace, cx: &App) -> bool {
let mut dirty_project_item_ids = Vec::new();
item.for_each_project_item(cx, &mut |project_item_id, project_item| {
if project_item.is_dirty() {
dirty_project_item_ids.push(project_item_id);
}
});
if dirty_project_item_ids.is_empty() {
if item.is_singleton(cx) && item.is_dirty(cx) {
return false;
}
return true;
}
for open_item in workspace.items(cx) {
if open_item.item_id() == item.item_id() {
continue;
}
if !open_item.is_singleton(cx) {
continue;
}
let other_project_item_ids = open_item.project_item_model_ids(cx);
dirty_project_item_ids.retain(|id| !other_project_item_ids.contains(id));
}
if dirty_project_item_ids.is_empty() {
return true;
}
false
}
pub(super) fn file_names_for_prompt(
items: &mut dyn Iterator<Item = &Box<dyn ItemHandle>>,
all_dirty_items: usize,
@ -1460,19 +1494,23 @@ impl Pane {
) -> (String, String) {
/// Quantity of item paths displayed in prompt prior to cutoff..
const FILE_NAMES_CUTOFF_POINT: usize = 10;
let mut file_names: Vec<_> = items
.filter_map(|item| {
item.project_path(cx).and_then(|project_path| {
project_path
.path
let mut file_names = Vec::new();
let mut should_display_followup_text = false;
for (ix, item) in items.enumerate() {
item.for_each_project_item(cx, &mut |_, project_item| {
let filename = project_item.project_path(cx).and_then(|path| {
path.path
.file_name()
.and_then(|name| name.to_str().map(ToOwned::to_owned))
})
})
.take(FILE_NAMES_CUTOFF_POINT)
.collect();
let should_display_followup_text =
all_dirty_items > FILE_NAMES_CUTOFF_POINT || file_names.len() != all_dirty_items;
});
file_names.push(filename.unwrap_or("untitled".to_string()));
});
if ix == FILE_NAMES_CUTOFF_POINT {
should_display_followup_text = true;
break;
}
}
if should_display_followup_text {
let not_shown_files = all_dirty_items - file_names.len();
if not_shown_files == 1 {
@ -1499,34 +1537,40 @@ impl Pane {
) -> Task<Result<()>> {
// Find the items to close.
let mut items_to_close = Vec::new();
let mut item_ids_to_close = HashSet::default();
let mut dirty_items = Vec::new();
for item in &self.items {
if should_close(item.item_id()) {
items_to_close.push(item.boxed_clone());
item_ids_to_close.insert(item.item_id());
if item.is_dirty(cx) {
dirty_items.push(item.boxed_clone());
}
}
}
let active_item_id = self.active_item().map(|item| item.item_id());
items_to_close.sort_by_key(|item| {
let path = item.project_path(cx);
// Put the currently active item at the end, because if the currently active item is not closed last
// closing the currently active item will cause the focus to switch to another item
// This will cause Zed to expand the content of the currently active item
active_item_id.filter(|&id| id == item.item_id()).is_some()
// If a buffer is open both in a singleton editor and in a multibuffer, make sure
// to focus the singleton buffer when prompting to save that buffer, as opposed
// to focusing the multibuffer, because this gives the user a more clear idea
// of what content they would be saving.
|| !item.is_singleton(cx)
//
// Beyond that sort in order of project path, with untitled files and multibuffers coming last.
(active_item_id == Some(item.item_id()), path.is_none(), path)
});
let workspace = self.workspace.clone();
let Some(project) = self.project.upgrade() else {
return Task::ready(Ok(()));
};
cx.spawn_in(window, |pane, mut cx| async move {
let dirty_items = workspace.update(&mut cx, |workspace, cx| {
items_to_close
.iter()
.filter(|item| {
item.is_dirty(cx)
&& !Self::skip_save_on_close(item.as_ref(), &workspace, cx)
})
.map(|item| item.boxed_clone())
.collect::<Vec<_>>()
})?;
if save_intent == SaveIntent::Close && dirty_items.len() > 1 {
let answer = pane.update_in(&mut cx, |_, window, cx| {
let (prompt, detail) =
@ -1542,68 +1586,33 @@ impl Pane {
match answer.await {
Ok(0) => save_intent = SaveIntent::SaveAll,
Ok(1) => save_intent = SaveIntent::Skip,
Ok(2) => return Ok(()),
_ => {}
}
}
let mut saved_project_items_ids = HashSet::default();
for item_to_close in items_to_close {
// Find the item's current index and its set of dirty project item models. Avoid
// storing these in advance, in case they have changed since this task
// was started.
let mut dirty_project_item_ids = Vec::new();
let Some(item_ix) = pane.update(&mut cx, |pane, cx| {
item_to_close.for_each_project_item(
cx,
&mut |project_item_id, project_item| {
if project_item.is_dirty() {
dirty_project_item_ids.push(project_item_id);
}
},
);
pane.index_for_item(&*item_to_close)
})?
else {
continue;
};
// Check if this view has any project items that are not open anywhere else
// in the workspace, AND that the user has not already been prompted to save.
// If there are any such project entries, prompt the user to save this item.
let project = workspace.update(&mut cx, |workspace, cx| {
for open_item in workspace.items(cx) {
let open_item_id = open_item.item_id();
if !item_ids_to_close.contains(&open_item_id) {
let other_project_item_ids = open_item.project_item_model_ids(cx);
dirty_project_item_ids
.retain(|id| !other_project_item_ids.contains(id));
let mut should_save = true;
if save_intent == SaveIntent::Close {
workspace.update(&mut cx, |workspace, cx| {
if Self::skip_save_on_close(item_to_close.as_ref(), &workspace, cx) {
should_save = false;
}
}
workspace.project().clone()
})?;
let should_save = dirty_project_item_ids
.iter()
.any(|id| saved_project_items_ids.insert(*id))
// Always propose to save singleton files without any project paths: those cannot be saved via multibuffer, as require a file path selection modal.
|| cx
.update(|_window, cx| {
item_to_close.can_save(cx) && item_to_close.is_dirty(cx)
&& item_to_close.is_singleton(cx)
&& item_to_close.project_path(cx).is_none()
})
.unwrap_or(false);
})?;
}
if should_save
&& !Self::save_item(
if should_save {
if !Self::save_item(
project.clone(),
&pane,
item_ix,
&*item_to_close,
save_intent,
&mut cx,
)
.await?
{
break;
{
break;
}
}
// Remove the item from the pane.
@ -1777,7 +1786,6 @@ impl Pane {
pub async fn save_item(
project: Entity<Project>,
pane: &WeakEntity<Pane>,
item_ix: usize,
item: &dyn ItemHandle,
save_intent: SaveIntent,
cx: &mut AsyncWindowContext,
@ -1791,6 +1799,13 @@ impl Pane {
if save_intent == SaveIntent::Skip {
return Ok(true);
}
let Some(item_ix) = pane
.update(cx, |pane, _| pane.index_for_item(item))
.ok()
.flatten()
else {
return Ok(true);
};
let (mut has_conflict, mut is_dirty, mut can_save, is_singleton, has_deleted_file) = cx
.update(|_window, cx| {
@ -1939,6 +1954,7 @@ impl Pane {
.await?;
} else if can_save_as {
let abs_path = pane.update_in(cx, |pane, window, cx| {
pane.activate_item(item_ix, true, true, window, cx);
pane.workspace.update(cx, |workspace, cx| {
workspace.prompt_for_new_path(window, cx)
})
@ -4382,15 +4398,15 @@ mod tests {
add_labeled_item(&pane, "A", true, cx).update(cx, |item, cx| {
item.project_items
.push(TestProjectItem::new(1, "A.txt", cx))
.push(TestProjectItem::new_dirty(1, "A.txt", cx))
});
add_labeled_item(&pane, "B", true, cx).update(cx, |item, cx| {
item.project_items
.push(TestProjectItem::new(2, "B.txt", cx))
.push(TestProjectItem::new_dirty(2, "B.txt", cx))
});
add_labeled_item(&pane, "C", true, cx).update(cx, |item, cx| {
item.project_items
.push(TestProjectItem::new(3, "C.txt", cx))
.push(TestProjectItem::new_dirty(3, "C.txt", cx))
});
assert_item_labels(&pane, ["A^", "B^", "C*^"], cx);
@ -4408,7 +4424,7 @@ mod tests {
.unwrap();
cx.executor().run_until_parked();
cx.simulate_prompt_answer(2);
cx.simulate_prompt_answer("Save all");
save.await.unwrap();
assert_item_labels(&pane, [], cx);
@ -4430,11 +4446,55 @@ mod tests {
.unwrap();
cx.executor().run_until_parked();
cx.simulate_prompt_answer(2);
cx.simulate_prompt_answer("Discard all");
save.await.unwrap();
assert_item_labels(&pane, [], cx);
}
#[gpui::test]
async fn test_close_with_save_intent(cx: &mut TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor());
let project = Project::test(fs, None, cx).await;
let (workspace, cx) =
cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
let pane = workspace.update(cx, |workspace, _| workspace.active_pane().clone());
let a = cx.update(|_, cx| TestProjectItem::new_dirty(1, "A.txt", cx));
let b = cx.update(|_, cx| TestProjectItem::new_dirty(1, "B.txt", cx));
let c = cx.update(|_, cx| TestProjectItem::new_dirty(1, "C.txt", cx));
add_labeled_item(&pane, "AB", true, cx).update(cx, |item, _| {
item.project_items.push(a.clone());
item.project_items.push(b.clone());
});
add_labeled_item(&pane, "C", true, cx)
.update(cx, |item, _| item.project_items.push(c.clone()));
assert_item_labels(&pane, ["AB^", "C*^"], cx);
pane.update_in(cx, |pane, window, cx| {
pane.close_all_items(
&CloseAllItems {
save_intent: Some(SaveIntent::Save),
close_pinned: false,
},
window,
cx,
)
})
.unwrap()
.await
.unwrap();
assert_item_labels(&pane, [], cx);
cx.update(|_, cx| {
assert!(!a.read(cx).is_dirty);
assert!(!b.read(cx).is_dirty);
assert!(!c.read(cx).is_dirty);
});
}
#[gpui::test]
async fn test_close_all_items_including_pinned(cx: &mut TestAppContext) {
init_test(cx);