Remove one shot commands. (#959)

Author: luleyleo

CHANGELOG.md

@@ -64,6 +64,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i
 - `AppDelegate::command` now receives a `Target` instead of a `&Target`. ([#909] by [@xStrom])
 - `SHOW_WINDOW` and `CLOSE_WINDOW` commands now only use `Target` to determine the affected window. ([#928] by [@finnerale])
 - Replaced `NEW_WINDOW`, `SET_MENU` and `SHOW_CONTEXT_MENU` commands with methods on `EventCtx` and `DelegateCtx`. ([#931] by [@finnerale])
+- Replaced `Command::one_shot` and `::take_object` with a `SingleUse` payload wrapper type. ([#959] by [@finnerale])
 
 ### Deprecated
 
@@ -198,6 +199,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i
 [#951]: https://github.com/xi-editor/druid/pull/951
 [#953]: https://github.com/xi-editor/druid/pull/953
 [#954]: https://github.com/xi-editor/druid/pull/954
+[#959]: https://github.com/xi-editor/druid/pull/959
 
 ## [0.5.0] - 2020-04-01
 

druid/src/app_delegate.rs

@@ -19,7 +19,9 @@ use std::{
     collections::VecDeque,
 };
 
-use crate::{commands, Command, Data, Env, Event, MenuDesc, Target, WindowDesc, WindowId};
+use crate::{
+    commands, Command, Data, Env, Event, MenuDesc, SingleUse, Target, WindowDesc, WindowId,
+};
 
 /// A context passed in to [`AppDelegate`] functions.
 ///
@@ -55,7 +57,7 @@ impl<'a> DelegateCtx<'a> {
     pub fn new_window<T: Any>(&mut self, desc: WindowDesc<T>) {
         if self.app_data_type == TypeId::of::<T>() {
             self.submit_command(
-                Command::one_shot(commands::NEW_WINDOW, desc),
+                Command::new(commands::NEW_WINDOW, SingleUse::new(desc)),
                 Target::Global,
             );
         } else {

druid/src/command.rs

@@ -34,17 +34,8 @@ pub struct Selector(&'static str);
 /// A `Command` consists of a [`Selector`], that indicates what the command is,
 /// and an optional argument, that can be used to pass arbitrary data.
 ///
-///
-/// # One-shot and reusable `Commands`
-///
-/// Commands come in two varieties, 'reusable' and 'one-shot'.
-///
-/// Regular commands are created with [`Command::new`], and their argument
-/// objects may be accessed repeatedly, via [`Command::get_object`].
-///
-/// One-shot commands are intended for cases where an object should only be
-/// used once; an example would be if you have some resource that cannot be
-/// cloned, and you wish to send it to another widget.
+/// If the payload can't or shouldn't be cloned,
+/// wrapping it with [`SingleUse`] allows you to `take` the object.
 ///
 /// # Examples
 /// ```
@@ -59,31 +50,50 @@ pub struct Selector(&'static str);
 ///
 /// [`Command::new`]: #method.new
 /// [`Command::get_object`]: #method.get_object
+/// [`SingleUse`]: struct.SingleUse.html
 /// [`Selector`]: struct.Selector.html
 #[derive(Debug, Clone)]
 pub struct Command {
     /// The command's `Selector`.
     pub selector: Selector,
-    object: Option<Arg>,
+    object: Option<Arc<dyn Any>>,
 }
 
-#[derive(Debug, Clone)]
-enum Arg {
-    Reusable(Arc<dyn Any>),
-    OneShot(Arc<Mutex<Option<Box<dyn Any>>>>),
-}
+/// A wrapper type for [`Command`] arguments that should only be used once.
+///
+/// This is useful if you have some resource that cannot be
+/// cloned, and you wish to send it to another widget.
+///
+/// # Examples
+/// ```
+/// use druid::{Command, Selector, SingleUse};
+///
+/// struct CantClone(u8);
+///
+/// let selector = Selector::new("use-once");
+/// let num = CantClone(42);
+/// let command = Command::new(selector, SingleUse::new(num));
+///
+/// let object: &SingleUse<CantClone> = command.get_object().unwrap();
+/// if let Some(num) = object.take() {
+///     // now you own the data
+///     assert_eq!(num.0, 42);
+/// }
+///
+/// // subsequent calls will return `None`
+/// assert!(object.take().is_none());
+/// ```
+///
+/// [`Command`]: struct.Command.html
+pub struct SingleUse<T>(Mutex<Option<T>>);
 
 /// Errors that can occur when attempting to retrieve the a command's argument.
 #[derive(Debug, Clone, PartialEq)]
 pub enum ArgumentError {
     /// The command did not have an argument.
     NoArgument,
-    /// The argument was expected to be reusable and wasn't, or vice-versa.
-    WrongVariant,
     /// The argument could not be downcast to the specified type.
     IncorrectType,
-    /// The one-shot argument has already been taken.
-    Consumed,
 }
 
 /// The target of a command.
@@ -231,67 +241,34 @@ impl Command {
     pub fn new(selector: Selector, arg: impl Any) -> Self {
         Command {
             selector,
-            object: Some(Arg::Reusable(Arc::new(arg))),
-        }
-    }
-
-    /// Create a new 'one-shot' `Command`.
-    ///
-    /// Unlike those created with `Command::new`, one-shot commands cannot
-    /// be reused; their argument is consumed when it is accessed, via
-    /// [`take_object`].
-    ///
-    /// [`take_object`]: #method.take_object
-    pub fn one_shot(selector: Selector, arg: impl Any) -> Self {
-        Command {
-            selector,
-            object: Some(Arg::OneShot(Arc::new(Mutex::new(Some(Box::new(arg)))))),
+            object: Some(Arc::new(arg)),
         }
     }
 
     /// Used to create a command from the types sent via an `ExtEventSink`.
     pub(crate) fn from_ext(selector: Selector, object: Option<Box<dyn Any + Send>>) -> Self {
         let object: Option<Box<dyn Any>> = object.map(|obj| obj as Box<dyn Any>);
-        let object = object.map(|o| Arg::Reusable(o.into()));
+        let object = object.map(|o| o.into());
         Command { selector, object }
     }
 
     /// Return a reference to this `Command`'s object, if it has one.
-    ///
-    /// This only works for 'reusable' commands; it does not work for commands
-    /// created with [`one_shot`].
-    ///
-    /// [`one_shot`]: #method.one_shot
     pub fn get_object<T: Any>(&self) -> Result<&T, ArgumentError> {
         match self.object.as_ref() {
-            Some(Arg::Reusable(o)) => o.downcast_ref().ok_or(ArgumentError::IncorrectType),
-            Some(Arg::OneShot(_)) => Err(ArgumentError::WrongVariant),
+            Some(o) => o.downcast_ref().ok_or(ArgumentError::IncorrectType),
             None => Err(ArgumentError::NoArgument),
         }
     }
+}
 
-    /// Attempt to take the object of a [`one-shot`] command.
-    ///
-    /// [`one-shot`]: #method.one_shot
-    pub fn take_object<T: Any>(&self) -> Result<Box<T>, ArgumentError> {
-        match self.object.as_ref() {
-            Some(Arg::Reusable(_)) => Err(ArgumentError::WrongVariant),
-            Some(Arg::OneShot(inner)) => {
-                let obj = inner
-                    .lock()
-                    .unwrap()
-                    .take()
-                    .ok_or(ArgumentError::Consumed)?;
-                match obj.downcast::<T>() {
-                    Ok(obj) => Ok(obj),
-                    Err(obj) => {
-                        inner.lock().unwrap().replace(obj);
-                        Err(ArgumentError::IncorrectType)
-                    }
-                }
-            }
-            None => Err(ArgumentError::NoArgument),
-        }
+impl<T: Any> SingleUse<T> {
+    pub fn new(data: T) -> Self {
+        SingleUse(Mutex::new(Some(data)))
+    }
+
+    /// Takes the value, leaving a None in its place.
+    pub fn take(&self) -> Option<T> {
+        self.0.lock().unwrap().take()
     }
 }
 
@@ -315,12 +292,6 @@ impl std::fmt::Display for ArgumentError {
         match self {
             ArgumentError::NoArgument => write!(f, "Command has no argument"),
             ArgumentError::IncorrectType => write!(f, "Downcast failed: wrong concrete type"),
-            ArgumentError::Consumed => write!(f, "One-shot command arguemnt already consumed"),
-            ArgumentError::WrongVariant => write!(
-                f,
-                "Incorrect access method for argument type; \
-                 check Command::one_shot docs for more detail."
-            ),
         }
     }
 }

druid/src/contexts.rs

@@ -24,8 +24,8 @@ use crate::core::{BaseState, CommandQueue, FocusChange};
 use crate::piet::Piet;
 use crate::piet::RenderContext;
 use crate::{
-    commands, Affine, Command, ContextMenu, Cursor, Insets, MenuDesc, Point, Rect, Size, Target,
-    Text, TimerToken, Vec2, WidgetId, WindowDesc, WindowHandle, WindowId,
+    commands, Affine, Command, ContextMenu, Cursor, Insets, MenuDesc, Point, Rect, SingleUse, Size,
+    Target, Text, TimerToken, Vec2, WidgetId, WindowDesc, WindowHandle, WindowId,
 };
 
 /// A mutable context provided to event handling methods of widgets.
@@ -255,7 +255,7 @@ impl<'a> EventCtx<'a> {
     pub fn new_window<T: Any>(&mut self, desc: WindowDesc<T>) {
         if self.app_data_type == TypeId::of::<T>() {
             self.submit_command(
-                Command::one_shot(commands::NEW_WINDOW, desc),
+                Command::new(commands::NEW_WINDOW, SingleUse::new(desc)),
                 Target::Global,
             );
         } else {

druid/src/lib.rs

@@ -153,7 +153,7 @@ pub use crate::core::WidgetPod;
 pub use app::{AppLauncher, WindowDesc};
 pub use app_delegate::{AppDelegate, DelegateCtx};
 pub use box_constraints::BoxConstraints;
-pub use command::{sys as commands, Command, Selector, Target};
+pub use command::{sys as commands, Command, Selector, SingleUse, Target};
 pub use contexts::{EventCtx, LayoutCtx, LifeCycleCtx, PaintCtx, Region, UpdateCtx};
 pub use data::Data;
 pub use env::{Env, Key, KeyOrValue, Value, ValueType};

druid/src/win_handler.rs

@@ -31,8 +31,8 @@ use crate::ext_event::ExtEventHost;
 use crate::menu::ContextMenu;
 use crate::window::Window;
 use crate::{
-    Command, Data, Env, Event, InternalEvent, KeyEvent, MenuDesc, Target, TimerToken, WindowDesc,
-    WindowId,
+    Command, Data, Env, Event, InternalEvent, KeyEvent, MenuDesc, SingleUse, Target, TimerToken,
+    WindowDesc, WindowId,
 };
 
 use crate::command::sys as sys_cmd;
@@ -591,7 +591,10 @@ impl<T: Data> AppState<T> {
     }
 
     fn new_window(&mut self, cmd: Command) -> Result<(), Box<dyn std::error::Error>> {
-        let desc = cmd.take_object::<WindowDesc<T>>()?;
+        let desc = cmd.get_object::<SingleUse<WindowDesc<T>>>()?;
+        // The NEW_WINDOW command is private and only druid can receive it by normal means,
+        // thus unwrapping can be considered safe and deserves a panic.
+        let desc = desc.take().unwrap();
         let window = desc.build_native(self)?;
         window.show();
         Ok(())