Return a task from Workspace::save_active_item

This required changing our approach to OS prompts and this commit greatly
simplifies that. We now avoid passing a callback and return a simple future
instead. This lets callers spawn tasks to handle those futures.
This commit is contained in:
Antonio Scandurra 2022-01-20 09:51:29 +01:00
parent e56c043693
commit 634340dd84
7 changed files with 134 additions and 187 deletions

View file

@ -11,7 +11,7 @@ use anyhow::{anyhow, Result};
use keymap::MatchResult;
use parking_lot::Mutex;
use platform::Event;
use postage::{mpsc, sink::Sink as _, stream::Stream as _};
use postage::{mpsc, oneshot, sink::Sink as _, stream::Stream as _};
use smol::prelude::*;
use std::{
any::{type_name, Any, TypeId},
@ -498,11 +498,11 @@ impl TestAppContext {
.as_any_mut()
.downcast_mut::<platform::test::Window>()
.unwrap();
let callback = test_window
let mut done_tx = test_window
.last_prompt
.take()
.expect("prompt was not called");
(callback)(answer);
let _ = done_tx.try_send(answer);
}
}
@ -922,61 +922,26 @@ impl MutableAppContext {
self.foreground_platform.set_menus(menus);
}
fn prompt<F>(
fn prompt(
&self,
window_id: usize,
level: PromptLevel,
msg: &str,
answers: &[&str],
done_fn: F,
) where
F: 'static + FnOnce(usize, &mut MutableAppContext),
{
let app = self.weak_self.as_ref().unwrap().upgrade().unwrap();
let foreground = self.foreground.clone();
) -> oneshot::Receiver<usize> {
let (_, window) = &self.presenters_and_platform_windows[&window_id];
window.prompt(
level,
msg,
answers,
Box::new(move |answer| {
foreground
.spawn(async move { (done_fn)(answer, &mut *app.borrow_mut()) })
.detach();
}),
);
window.prompt(level, msg, answers)
}
pub fn prompt_for_paths<F>(&self, options: PathPromptOptions, done_fn: F)
where
F: 'static + FnOnce(Option<Vec<PathBuf>>, &mut MutableAppContext),
{
let app = self.weak_self.as_ref().unwrap().upgrade().unwrap();
let foreground = self.foreground.clone();
self.foreground_platform.prompt_for_paths(
options,
Box::new(move |paths| {
foreground
.spawn(async move { (done_fn)(paths, &mut *app.borrow_mut()) })
.detach();
}),
);
pub fn prompt_for_paths(
&self,
options: PathPromptOptions,
) -> oneshot::Receiver<Option<Vec<PathBuf>>> {
self.foreground_platform.prompt_for_paths(options)
}
pub fn prompt_for_new_path<F>(&self, directory: &Path, done_fn: F)
where
F: 'static + FnOnce(Option<PathBuf>, &mut MutableAppContext),
{
let app = self.weak_self.as_ref().unwrap().upgrade().unwrap();
let foreground = self.foreground.clone();
self.foreground_platform.prompt_for_new_path(
directory,
Box::new(move |path| {
foreground
.spawn(async move { (done_fn)(path, &mut *app.borrow_mut()) })
.detach();
}),
);
pub fn prompt_for_new_path(&self, directory: &Path) -> oneshot::Receiver<Option<PathBuf>> {
self.foreground_platform.prompt_for_new_path(directory)
}
pub fn subscribe<E, H, F>(&mut self, handle: &H, mut callback: F) -> Subscription
@ -2234,26 +2199,24 @@ impl<'a, T: View> ViewContext<'a, T> {
self.app.platform()
}
pub fn prompt<F>(&self, level: PromptLevel, msg: &str, answers: &[&str], done_fn: F)
where
F: 'static + FnOnce(usize, &mut MutableAppContext),
{
self.app
.prompt(self.window_id, level, msg, answers, done_fn)
pub fn prompt(
&self,
level: PromptLevel,
msg: &str,
answers: &[&str],
) -> oneshot::Receiver<usize> {
self.app.prompt(self.window_id, level, msg, answers)
}
pub fn prompt_for_paths<F>(&self, options: PathPromptOptions, done_fn: F)
where
F: 'static + FnOnce(Option<Vec<PathBuf>>, &mut MutableAppContext),
{
self.app.prompt_for_paths(options, done_fn)
pub fn prompt_for_paths(
&self,
options: PathPromptOptions,
) -> oneshot::Receiver<Option<Vec<PathBuf>>> {
self.app.prompt_for_paths(options)
}
pub fn prompt_for_new_path<F>(&self, directory: &Path, done_fn: F)
where
F: 'static + FnOnce(Option<PathBuf>, &mut MutableAppContext),
{
self.app.prompt_for_new_path(directory, done_fn)
pub fn prompt_for_new_path(&self, directory: &Path) -> oneshot::Receiver<Option<PathBuf>> {
self.app.prompt_for_new_path(directory)
}
pub fn debug_elements(&self) -> crate::json::Value {

View file

@ -20,6 +20,7 @@ use crate::{
use anyhow::Result;
use async_task::Runnable;
pub use event::Event;
use postage::oneshot;
use std::{
any::Any,
path::{Path, PathBuf},
@ -70,13 +71,8 @@ pub(crate) trait ForegroundPlatform {
fn prompt_for_paths(
&self,
options: PathPromptOptions,
done_fn: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
);
fn prompt_for_new_path(
&self,
directory: &Path,
done_fn: Box<dyn FnOnce(Option<std::path::PathBuf>)>,
);
) -> oneshot::Receiver<Option<Vec<PathBuf>>>;
fn prompt_for_new_path(&self, directory: &Path) -> oneshot::Receiver<Option<PathBuf>>;
}
pub trait Dispatcher: Send + Sync {
@ -89,13 +85,7 @@ pub trait Window: WindowContext {
fn on_event(&mut self, callback: Box<dyn FnMut(Event)>);
fn on_resize(&mut self, callback: Box<dyn FnMut()>);
fn on_close(&mut self, callback: Box<dyn FnOnce()>);
fn prompt(
&self,
level: PromptLevel,
msg: &str,
answers: &[&str],
done_fn: Box<dyn FnOnce(usize)>,
);
fn prompt(&self, level: PromptLevel, msg: &str, answers: &[&str]) -> oneshot::Receiver<usize>;
}
pub trait WindowContext {

View file

@ -33,6 +33,7 @@ use objc::{
runtime::{Class, Object, Sel},
sel, sel_impl,
};
use postage::oneshot;
use ptr::null_mut;
use std::{
cell::{Cell, RefCell},
@ -248,15 +249,15 @@ impl platform::ForegroundPlatform for MacForegroundPlatform {
fn prompt_for_paths(
&self,
options: platform::PathPromptOptions,
done_fn: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
) {
) -> oneshot::Receiver<Option<Vec<PathBuf>>> {
unsafe {
let panel = NSOpenPanel::openPanel(nil);
panel.setCanChooseDirectories_(options.directories.to_objc());
panel.setCanChooseFiles_(options.files.to_objc());
panel.setAllowsMultipleSelection_(options.multiple.to_objc());
panel.setResolvesAliases_(false.to_objc());
let done_fn = Cell::new(Some(done_fn));
let (done_tx, done_rx) = oneshot::channel();
let done_tx = Cell::new(Some(done_tx));
let block = ConcreteBlock::new(move |response: NSModalResponse| {
let result = if response == NSModalResponse::NSModalResponseOk {
let mut result = Vec::new();
@ -275,27 +276,25 @@ impl platform::ForegroundPlatform for MacForegroundPlatform {
None
};
if let Some(done_fn) = done_fn.take() {
(done_fn)(result);
if let Some(mut done_tx) = done_tx.take() {
let _ = postage::sink::Sink::try_send(&mut done_tx, result);
}
});
let block = block.copy();
let _: () = msg_send![panel, beginWithCompletionHandler: block];
done_rx
}
}
fn prompt_for_new_path(
&self,
directory: &Path,
done_fn: Box<dyn FnOnce(Option<std::path::PathBuf>)>,
) {
fn prompt_for_new_path(&self, directory: &Path) -> oneshot::Receiver<Option<PathBuf>> {
unsafe {
let panel = NSSavePanel::savePanel(nil);
let path = ns_string(directory.to_string_lossy().as_ref());
let url = NSURL::fileURLWithPath_isDirectory_(nil, path, true.to_objc());
panel.setDirectoryURL(url);
let done_fn = Cell::new(Some(done_fn));
let (done_tx, done_rx) = oneshot::channel();
let done_tx = Cell::new(Some(done_tx));
let block = ConcreteBlock::new(move |response: NSModalResponse| {
let result = if response == NSModalResponse::NSModalResponseOk {
let url = panel.URL();
@ -311,12 +310,13 @@ impl platform::ForegroundPlatform for MacForegroundPlatform {
None
};
if let Some(done_fn) = done_fn.take() {
(done_fn)(result);
if let Some(mut done_tx) = done_tx.take() {
let _ = postage::sink::Sink::try_send(&mut done_tx, result);
}
});
let block = block.copy();
let _: () = msg_send![panel, beginWithCompletionHandler: block];
done_rx
}
}
}

View file

@ -28,6 +28,7 @@ use objc::{
runtime::{Class, Object, Protocol, Sel, BOOL, NO, YES},
sel, sel_impl,
};
use postage::oneshot;
use smol::Timer;
use std::{
any::Any,
@ -317,8 +318,7 @@ impl platform::Window for Window {
level: platform::PromptLevel,
msg: &str,
answers: &[&str],
done_fn: Box<dyn FnOnce(usize)>,
) {
) -> oneshot::Receiver<usize> {
unsafe {
let alert: id = msg_send![class!(NSAlert), alloc];
let alert: id = msg_send![alert, init];
@ -333,10 +333,11 @@ impl platform::Window for Window {
let button: id = msg_send![alert, addButtonWithTitle: ns_string(answer)];
let _: () = msg_send![button, setTag: ix as NSInteger];
}
let done_fn = Cell::new(Some(done_fn));
let (done_tx, done_rx) = oneshot::channel();
let done_tx = Cell::new(Some(done_tx));
let block = ConcreteBlock::new(move |answer: NSInteger| {
if let Some(done_fn) = done_fn.take() {
(done_fn)(answer.try_into().unwrap());
if let Some(mut done_tx) = done_tx.take() {
let _ = postage::sink::Sink::try_send(&mut done_tx, answer.try_into().unwrap());
}
});
let block = block.copy();
@ -345,6 +346,7 @@ impl platform::Window for Window {
beginSheetModalForWindow: self.0.borrow().native_window
completionHandler: block
];
done_rx
}
}
}

View file

@ -5,9 +5,10 @@ use crate::{
};
use anyhow::{anyhow, Result};
use parking_lot::Mutex;
use postage::oneshot;
use std::{
any::Any,
cell::RefCell,
cell::{Cell, RefCell},
path::{Path, PathBuf},
rc::Rc,
sync::Arc,
@ -23,7 +24,7 @@ pub struct Platform {
#[derive(Default)]
pub struct ForegroundPlatform {
last_prompt_for_new_path_args: RefCell<Option<(PathBuf, Box<dyn FnOnce(Option<PathBuf>)>)>>,
last_prompt_for_new_path_args: RefCell<Option<(PathBuf, oneshot::Sender<Option<PathBuf>>)>>,
}
struct Dispatcher;
@ -35,7 +36,7 @@ pub struct Window {
event_handlers: Vec<Box<dyn FnMut(super::Event)>>,
resize_handlers: Vec<Box<dyn FnMut()>>,
close_handlers: Vec<Box<dyn FnOnce()>>,
pub(crate) last_prompt: RefCell<Option<Box<dyn FnOnce(usize)>>>,
pub(crate) last_prompt: Cell<Option<oneshot::Sender<usize>>>,
}
impl ForegroundPlatform {
@ -43,11 +44,11 @@ impl ForegroundPlatform {
&self,
result: impl FnOnce(PathBuf) -> Option<PathBuf>,
) {
let (dir_path, callback) = self
let (dir_path, mut done_tx) = self
.last_prompt_for_new_path_args
.take()
.expect("prompt_for_new_path was not called");
callback(result(dir_path));
let _ = postage::sink::Sink::try_send(&mut done_tx, result(dir_path));
}
pub(crate) fn did_prompt_for_new_path(&self) -> bool {
@ -77,12 +78,15 @@ impl super::ForegroundPlatform for ForegroundPlatform {
fn prompt_for_paths(
&self,
_: super::PathPromptOptions,
_: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
) {
) -> oneshot::Receiver<Option<Vec<PathBuf>>> {
let (_done_tx, done_rx) = oneshot::channel();
done_rx
}
fn prompt_for_new_path(&self, path: &Path, f: Box<dyn FnOnce(Option<std::path::PathBuf>)>) {
*self.last_prompt_for_new_path_args.borrow_mut() = Some((path.to_path_buf(), f));
fn prompt_for_new_path(&self, path: &Path) -> oneshot::Receiver<Option<PathBuf>> {
let (done_tx, done_rx) = oneshot::channel();
*self.last_prompt_for_new_path_args.borrow_mut() = Some((path.to_path_buf(), done_tx));
done_rx
}
}
@ -170,7 +174,7 @@ impl Window {
close_handlers: Vec::new(),
scale_factor: 1.0,
current_scene: None,
last_prompt: RefCell::new(None),
last_prompt: Default::default(),
}
}
}
@ -220,8 +224,10 @@ impl super::Window for Window {
self.close_handlers.push(callback);
}
fn prompt(&self, _: crate::PromptLevel, _: &str, _: &[&str], f: Box<dyn FnOnce(usize)>) {
self.last_prompt.replace(Some(f));
fn prompt(&self, _: crate::PromptLevel, _: &str, _: &[&str]) -> oneshot::Receiver<usize> {
let (done_tx, done_rx) = oneshot::channel();
self.last_prompt.replace(Some(done_tx));
done_rx
}
}

View file

@ -66,7 +66,11 @@ pub fn init(cx: &mut MutableAppContext) {
});
cx.add_action(Workspace::toggle_share);
cx.add_action(Workspace::save_active_item);
cx.add_action(
|workspace: &mut Workspace, _: &Save, cx: &mut ViewContext<Workspace>| {
workspace.save_active_item(cx).detach_and_log_err(cx);
},
);
cx.add_action(Workspace::debug_elements);
cx.add_action(Workspace::toggle_sidebar_item);
cx.add_action(Workspace::toggle_sidebar_item_focus);
@ -224,7 +228,7 @@ pub trait ItemViewHandle {
project: ModelHandle<Project>,
abs_path: PathBuf,
cx: &mut MutableAppContext,
) -> Task<anyhow::Result<()>>;
) -> Task<Result<()>>;
}
pub trait WeakItemViewHandle {
@ -804,36 +808,29 @@ impl Workspace {
.and_then(|entry| self.project.read(cx).path_for_entry(entry, cx))
}
pub fn save_active_item(&mut self, _: &Save, cx: &mut ViewContext<Self>) {
pub fn save_active_item(&mut self, cx: &mut ViewContext<Self>) -> Task<Result<()>> {
if let Some(item) = self.active_item(cx) {
let handle = cx.handle();
if item.can_save(cx) {
if item.has_conflict(cx.as_ref()) {
const CONFLICT_MESSAGE: &'static str = "This file has changed on disk since you started editing it. Do you want to overwrite it?";
cx.prompt(
let mut answer = cx.prompt(
PromptLevel::Warning,
CONFLICT_MESSAGE,
&["Overwrite", "Cancel"],
move |answer, cx| {
if answer == 0 {
cx.spawn(|mut cx| async move {
if let Err(error) = cx.update(|cx| item.save(cx)).unwrap().await
{
error!("failed to save item: {:?}, ", error);
}
})
.detach();
}
},
);
cx.spawn(|_, mut cx| async move {
let answer = answer.recv().await;
if answer == Some(0) {
cx.update(|cx| item.save(cx))?.await?;
}
Ok(())
})
} else {
cx.spawn(|_, mut cx| async move {
if let Err(error) = cx.update(|cx| item.save(cx)).unwrap().await {
error!("failed to save item: {:?}, ", error);
}
cx.update(|cx| item.save(cx))?.await?;
Ok(())
})
.detach();
}
} else if item.can_save_as(cx) {
let worktree = self.worktrees(cx).first();
@ -841,13 +838,19 @@ impl Workspace {
.and_then(|w| w.read(cx).as_local())
.map_or(Path::new(""), |w| w.abs_path())
.to_path_buf();
cx.prompt_for_new_path(&start_abs_path, move |abs_path, cx| {
if let Some(abs_path) = abs_path {
let project = handle.read(cx).project().clone();
cx.update(|cx| item.save_as(project, abs_path, cx).detach_and_log_err(cx));
let mut abs_path = cx.prompt_for_new_path(&start_abs_path);
cx.spawn(|this, mut cx| async move {
if let Some(abs_path) = abs_path.recv().await.flatten() {
let project = this.read_with(&cx, |this, _| this.project().clone());
cx.update(|cx| item.save_as(project, abs_path, cx)).await?;
}
});
Ok(())
})
} else {
Task::ready(Ok(()))
}
} else {
Task::ready(Ok(()))
}
}
@ -1397,18 +1400,17 @@ impl std::fmt::Debug for OpenParams {
fn open(action: &Open, cx: &mut MutableAppContext) {
let app_state = action.0.clone();
cx.prompt_for_paths(
PathPromptOptions {
files: true,
directories: true,
multiple: true,
},
move |paths, cx| {
if let Some(paths) = paths {
cx.dispatch_global_action(OpenPaths(OpenParams { paths, app_state }));
}
},
);
let mut paths = cx.prompt_for_paths(PathPromptOptions {
files: true,
directories: true,
multiple: true,
});
cx.spawn(|mut cx| async move {
if let Some(paths) = paths.recv().await.flatten() {
cx.update(|cx| cx.dispatch_global_action(OpenPaths(OpenParams { paths, app_state })));
}
})
.detach();
}
pub fn open_paths(

View file

@ -214,18 +214,13 @@ mod tests {
assert!(editor.text(cx).is_empty());
});
workspace.update(&mut cx, |workspace, cx| {
workspace.save_active_item(&workspace::Save, cx)
});
let save_task = workspace.update(&mut cx, |workspace, cx| workspace.save_active_item(cx));
app_state.fs.as_fake().insert_dir("/root").await.unwrap();
cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name")));
editor
.condition(&cx, |editor, cx| editor.title(cx) == "the-new-name")
.await;
editor.update(&mut cx, |editor, cx| {
save_task.await.unwrap();
editor.read_with(&cx, |editor, cx| {
assert!(!editor.is_dirty(cx));
assert_eq!(editor.title(cx), "the-new-name");
});
}
@ -472,12 +467,13 @@ mod tests {
.await;
cx.read(|cx| assert!(editor.is_dirty(cx)));
cx.update(|cx| workspace.update(cx, |w, cx| w.save_active_item(&workspace::Save, cx)));
let save_task = workspace.update(&mut cx, |workspace, cx| workspace.save_active_item(cx));
cx.simulate_prompt_answer(window_id, 0);
editor
.condition(&cx, |editor, cx| !editor.is_dirty(cx))
.await;
cx.read(|cx| assert!(!editor.has_conflict(cx)));
save_task.await.unwrap();
editor.read_with(&cx, |editor, cx| {
assert!(!editor.is_dirty(cx));
assert!(!editor.has_conflict(cx));
});
}
#[gpui::test]
@ -525,9 +521,7 @@ mod tests {
});
// Save the buffer. This prompts for a filename.
workspace.update(&mut cx, |workspace, cx| {
workspace.save_active_item(&workspace::Save, cx)
});
let save_task = workspace.update(&mut cx, |workspace, cx| workspace.save_active_item(cx));
cx.simulate_new_path_selection(|parent_dir| {
assert_eq!(parent_dir, Path::new("/root"));
Some(parent_dir.join("the-new-name.rs"))
@ -537,17 +531,13 @@ mod tests {
assert_eq!(editor.title(cx), "untitled");
});
// When the save completes, the buffer's title is updated.
editor
.condition(&cx, |editor, cx| !editor.is_dirty(cx))
.await;
cx.read(|cx| {
// When the save completes, the buffer's title is updated and the language is assigned based
// on the path.
save_task.await.unwrap();
editor.read_with(&cx, |editor, cx| {
assert!(!editor.is_dirty(cx));
assert_eq!(editor.title(cx), "the-new-name.rs");
});
// The language is assigned based on the path
editor.read_with(&cx, |editor, cx| {
assert_eq!(editor.language(cx).unwrap().name(), "Rust")
assert_eq!(editor.language(cx).unwrap().name(), "Rust");
});
// Edit the file and save it again. This time, there is no filename prompt.
@ -555,14 +545,13 @@ mod tests {
editor.handle_input(&editor::Input(" there".into()), cx);
assert_eq!(editor.is_dirty(cx.as_ref()), true);
});
workspace.update(&mut cx, |workspace, cx| {
workspace.save_active_item(&workspace::Save, cx)
});
let save_task = workspace.update(&mut cx, |workspace, cx| workspace.save_active_item(cx));
save_task.await.unwrap();
assert!(!cx.did_prompt_for_new_path());
editor
.condition(&cx, |editor, cx| !editor.is_dirty(cx))
.await;
cx.read(|cx| assert_eq!(editor.title(cx), "the-new-name.rs"));
editor.read_with(&cx, |editor, cx| {
assert!(!editor.is_dirty(cx));
assert_eq!(editor.title(cx), "the-new-name.rs")
});
// Open the same newly-created file in another pane item. The new editor should reuse
// the same buffer.
@ -624,17 +613,12 @@ mod tests {
});
// Save the buffer. This prompts for a filename.
workspace.update(&mut cx, |workspace, cx| {
workspace.save_active_item(&workspace::Save, cx)
});
let save_task = workspace.update(&mut cx, |workspace, cx| workspace.save_active_item(cx));
cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name.rs")));
editor
.condition(&cx, |editor, cx| !editor.is_dirty(cx))
.await;
// The language is assigned based on the path
save_task.await.unwrap();
// The buffer is not dirty anymore and the language is assigned based on the path.
editor.read_with(&cx, |editor, cx| {
assert!(!editor.is_dirty(cx));
assert_eq!(editor.language(cx).unwrap().name(), "Rust")
});
}