From c988bb934ce8a9d9a5836a2e5fc43581704cb71d Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Tue, 26 May 2020 18:25:00 +0200 Subject: [PATCH 01/12] Implemente typed selectors. --- druid/examples/blocking_function.rs | 25 ++-- druid/examples/ext_event.rs | 6 +- druid/examples/identity.rs | 15 +-- druid/examples/multiwin.rs | 16 +-- druid/examples/open_save.rs | 38 +++--- druid/src/app_delegate.rs | 4 +- druid/src/command.rs | 175 ++++++++++++++++++---------- druid/src/contexts.rs | 6 +- druid/src/ext_event.rs | 24 ++-- druid/src/menu.rs | 20 ++-- druid/src/tests/helpers.rs | 2 +- druid/src/tests/mod.rs | 8 +- druid/src/widget/textbox.rs | 16 ++- druid/src/win_handler.rs | 87 +++++++------- 14 files changed, 244 insertions(+), 198 deletions(-) diff --git a/druid/examples/blocking_function.rs b/druid/examples/blocking_function.rs index 0caa288dc5..2700de6fd4 100644 --- a/druid/examples/blocking_function.rs +++ b/druid/examples/blocking_function.rs @@ -23,9 +23,9 @@ use druid::{ use druid::widget::{Button, Either, Flex, Label}; -const START_SLOW_FUNCTION: Selector = Selector::new("start_slow_function"); +const START_SLOW_FUNCTION: Selector = Selector::new("start_slow_function"); -const FINISH_SLOW_FUNCTION: Selector = Selector::new("finish_slow_function"); +const FINISH_SLOW_FUNCTION: Selector = Selector::new("finish_slow_function"); struct Delegate { eventsink: ExtEventSink, @@ -61,20 +61,15 @@ impl AppDelegate for Delegate { data: &mut AppState, _env: &Env, ) -> bool { - match cmd.selector { - START_SLOW_FUNCTION => { - data.processing = true; - wrapped_slow_function(self.eventsink.clone(), data.value); - true - } - FINISH_SLOW_FUNCTION => { - data.processing = false; - let number = cmd.get_object::().expect("api violation"); - data.value = *number; - true - } - _ => true, + if cmd.is(START_SLOW_FUNCTION) { + data.processing = true; + wrapped_slow_function(self.eventsink.clone(), data.value); } + if let Some(number) = cmd.get(FINISH_SLOW_FUNCTION) { + data.processing = false; + data.value = *number; + } + true } } diff --git a/druid/examples/ext_event.rs b/druid/examples/ext_event.rs index 6ef2c9450a..6a0aaf4067 100644 --- a/druid/examples/ext_event.rs +++ b/druid/examples/ext_event.rs @@ -22,7 +22,7 @@ use druid::kurbo::RoundedRect; use druid::widget::prelude::*; use druid::{AppLauncher, Color, Data, LocalizedString, Rect, Selector, WidgetExt, WindowDesc}; -const SET_COLOR: Selector = Selector::new("event-example.set-color"); +const SET_COLOR: Selector = Selector::new("event-example.set-color"); /// A widget that displays a color. struct ColorWell; @@ -53,8 +53,8 @@ impl ColorWell { impl Widget for ColorWell { fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut MyColor, _env: &Env) { match event { - Event::Command(cmd) if cmd.selector == SET_COLOR => { - data.0 = cmd.get_object::().unwrap().clone(); + Event::Command(cmd) if cmd.is(SET_COLOR) => { + data.0 = cmd.get_unchecked(SET_COLOR).clone(); ctx.request_paint(); } _ => (), diff --git a/druid/examples/identity.rs b/druid/examples/identity.rs index 3c8c863eee..93a8ac21f2 100644 --- a/druid/examples/identity.rs +++ b/druid/examples/identity.rs @@ -40,7 +40,7 @@ use druid::{ const CYCLE_DURATION: Duration = Duration::from_millis(100); -const FREEZE_COLOR: Selector = Selector::new("identity-example.freeze-color"); +const FREEZE_COLOR: Selector = Selector::new("identity-example.freeze-color"); const UNFREEZE_COLOR: Selector = Selector::new("identity-example.unfreeze-color"); /// Honestly: it's just a color in fancy clothing. @@ -109,20 +109,13 @@ impl Widget for ColorWell { self.token = ctx.request_timer(CYCLE_DURATION); ctx.request_paint(); } - Event::WindowConnected if self.randomize => { self.token = ctx.request_timer(CYCLE_DURATION); } - - Event::Command(cmd) if cmd.selector == FREEZE_COLOR => { - self.frozen = cmd - .get_object::() - .ok() - .cloned() - .expect("payload is always a Color") - .into(); + Event::Command(cmd) if cmd.is(FREEZE_COLOR) => { + self.frozen = cmd.get(FREEZE_COLOR).cloned(); } - Event::Command(cmd) if cmd.selector == UNFREEZE_COLOR => self.frozen = None, + Event::Command(cmd) if cmd.is(UNFREEZE_COLOR) => self.frozen = None, _ => (), } } diff --git a/druid/examples/multiwin.rs b/druid/examples/multiwin.rs index acb51b4fdf..ceaac73ce0 100644 --- a/druid/examples/multiwin.rs +++ b/druid/examples/multiwin.rs @@ -25,7 +25,7 @@ use druid::{ }; use log::info; -const MENU_COUNT_ACTION: Selector = Selector::new("menu-count-action"); +const MENU_COUNT_ACTION: Selector = Selector::new("menu-count-action"); const MENU_INCREMENT_ACTION: Selector = Selector::new("menu-increment-action"); const MENU_DECREMENT_ACTION: Selector = Selector::new("menu-decrement-action"); const MENU_SWITCH_GLOW_ACTION: Selector = Selector::new("menu-switch-glow"); @@ -155,16 +155,16 @@ impl AppDelegate for Delegate { data: &mut State, _env: &Env, ) -> bool { - match cmd.selector { - sys_cmds::NEW_FILE => { + match cmd { + _ if cmd.is(sys_cmds::NEW_FILE) => { let new_win = WindowDesc::new(ui_builder) .menu(make_menu(data)) .window_size((data.selected as f64 * 100.0 + 300.0, 500.0)); ctx.new_window(new_win); false } - MENU_COUNT_ACTION => { - data.selected = *cmd.get_object().unwrap(); + _ if cmd.is(MENU_COUNT_ACTION) => { + data.selected = *cmd.get_unchecked(MENU_COUNT_ACTION); let menu = make_menu::(data); for id in &self.windows { ctx.set_menu(menu.clone(), *id); @@ -173,7 +173,7 @@ impl AppDelegate for Delegate { } // wouldn't it be nice if a menu (like a button) could just mutate state // directly if desired? - MENU_INCREMENT_ACTION => { + _ if cmd.is(MENU_INCREMENT_ACTION) => { data.menu_count += 1; let menu = make_menu::(data); for id in &self.windows { @@ -181,7 +181,7 @@ impl AppDelegate for Delegate { } false } - MENU_DECREMENT_ACTION => { + _ if cmd.is(MENU_DECREMENT_ACTION) => { data.menu_count = data.menu_count.saturating_sub(1); let menu = make_menu::(data); for id in &self.windows { @@ -189,7 +189,7 @@ impl AppDelegate for Delegate { } false } - MENU_SWITCH_GLOW_ACTION => { + _ if cmd.is(MENU_SWITCH_GLOW_ACTION) => { data.glow_hot = !data.glow_hot; false } diff --git a/druid/examples/open_save.rs b/druid/examples/open_save.rs index 571a3a1557..82b9a067e3 100644 --- a/druid/examples/open_save.rs +++ b/druid/examples/open_save.rs @@ -14,7 +14,7 @@ use druid::widget::{Align, Button, Flex, TextBox}; use druid::{ - AppDelegate, AppLauncher, Command, DelegateCtx, Env, FileDialogOptions, FileInfo, FileSpec, + commands, AppDelegate, AppLauncher, Command, DelegateCtx, Env, FileDialogOptions, FileSpec, LocalizedString, Target, Widget, WindowDesc, }; @@ -77,30 +77,24 @@ impl AppDelegate for Delegate { data: &mut String, _env: &Env, ) -> bool { - match cmd.selector { - druid::commands::SAVE_FILE => { - if let Ok(file_info) = cmd.get_object::() { - if let Err(e) = std::fs::write(file_info.path(), &data[..]) { - println!("Error writing file: {}", e); - } - } - true + if let Some(Some(file_info)) = cmd.get(commands::SAVE_FILE) { + if let Err(e) = std::fs::write(file_info.path(), &data[..]) { + println!("Error writing file: {}", e); } - druid::commands::OPEN_FILE => { - if let Ok(file_info) = cmd.get_object::() { - match std::fs::read_to_string(file_info.path()) { - Ok(s) => { - let first_line = s.lines().next().unwrap_or(""); - *data = first_line.to_owned(); - } - Err(e) => { - println!("Error opening file: {}", e); - } - } + return true; + } + if let Some(file_info) = cmd.get(commands::OPEN_FILE) { + match std::fs::read_to_string(file_info.path()) { + Ok(s) => { + let first_line = s.lines().next().unwrap_or(""); + *data = first_line.to_owned(); + } + Err(e) => { + println!("Error opening file: {}", e); } - true } - _ => false, + return true; } + false } } diff --git a/druid/src/app_delegate.rs b/druid/src/app_delegate.rs index 24833ccf4f..d29e4871f7 100644 --- a/druid/src/app_delegate.rs +++ b/druid/src/app_delegate.rs @@ -57,7 +57,7 @@ impl<'a> DelegateCtx<'a> { pub fn new_window(&mut self, desc: WindowDesc) { if self.app_data_type == TypeId::of::() { self.submit_command( - Command::new(commands::NEW_WINDOW, SingleUse::new(desc)), + Command::new(commands::NEW_WINDOW, SingleUse::new(Box::new(desc))), Target::Global, ); } else { @@ -77,7 +77,7 @@ impl<'a> DelegateCtx<'a> { pub fn set_menu(&mut self, menu: MenuDesc, window: WindowId) { if self.app_data_type == TypeId::of::() { self.submit_command( - Command::new(commands::SET_MENU, menu), + Command::new(commands::SET_MENU, Box::new(menu)), Target::Window(window), ); } else { diff --git a/druid/src/command.rs b/druid/src/command.rs index ce8bd349d1..4aa0d45e1f 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -14,25 +14,32 @@ //! Custom commands. -use std::any::Any; -use std::sync::{Arc, Mutex}; +use std::any::{self, Any}; +use std::{ + marker::PhantomData, + sync::{Arc, Mutex}, +}; use crate::{WidgetId, WindowId}; +pub(crate) type SelectorSymbol = &'static str; + /// An identifier for a particular command. /// +/// The type parameter `T` specifies the commands payload type. +/// /// This should be a unique string identifier. Certain `Selector`s are defined /// by druid, and have special meaning to the framework; these are listed in the /// [`druid::commands`] module. /// /// [`druid::commands`]: commands/index.html -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Selector(&'static str); +#[derive(Debug, PartialEq, Eq)] +pub struct Selector(SelectorSymbol, PhantomData); /// An arbitrary command. /// -/// A `Command` consists of a [`Selector`], that indicates what the command is, -/// and an optional argument, that can be used to pass arbitrary data. +/// A `Command` consists of a [`Selector`], that indicates what the command is +/// and what type of payload it carries, as well as the actual payload. /// /// If the payload can't or shouldn't be cloned, /// wrapping it with [`SingleUse`] allows you to `take` the object. @@ -45,7 +52,7 @@ pub struct Selector(&'static str); /// let rows = vec![1, 3, 10, 12]; /// let command = Command::new(selector, rows); /// -/// assert_eq!(command.get_object(), Ok(&vec![1, 3, 10, 12])); +/// assert_eq!(command.get(selector), Some(&vec![1, 3, 10, 12])); /// ``` /// /// [`Command::new`]: #method.new @@ -54,9 +61,8 @@ pub struct Selector(&'static str); /// [`Selector`]: struct.Selector.html #[derive(Debug, Clone)] pub struct Command { - /// The command's `Selector`. - pub selector: Selector, - object: Option>, + selector: SelectorSymbol, + object: Arc, } /// A wrapper type for [`Command`] arguments that should only be used once. @@ -74,7 +80,7 @@ pub struct Command { /// let num = CantClone(42); /// let command = Command::new(selector, SingleUse::new(num)); /// -/// let object: &SingleUse = command.get_object().unwrap(); +/// let object: &SingleUse = command.get_unchecked(selector); /// if let Some(num) = object.take() { /// // now you own the data /// assert_eq!(num.0, 42); @@ -87,15 +93,6 @@ pub struct Command { /// [`Command`]: struct.Command.html pub struct SingleUse(Mutex>); -/// 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 could not be downcast to the specified type. - IncorrectType, -} - /// The target of a command. #[derive(Clone, Copy, Debug, PartialEq)] pub enum Target { @@ -115,6 +112,8 @@ pub enum Target { /// [`Command`]: ../struct.Command.html pub mod sys { use super::Selector; + use crate::{FileDialogOptions, FileInfo, SingleUse}; + use std::any::Any; /// Quit the running application. This command is handled by the druid library. pub const QUIT_APP: Selector = Selector::new("druid-builtin.quit-app"); @@ -126,7 +125,8 @@ pub mod sys { pub const HIDE_OTHERS: Selector = Selector::new("druid-builtin.menu-hide-others"); /// The selector for a command to create a new window. - pub(crate) const NEW_WINDOW: Selector = Selector::new("druid-builtin.new-window"); + pub(crate) const NEW_WINDOW: Selector>> = + Selector::new("druid-builtin.new-window"); /// The selector for a command to close a window. /// @@ -149,13 +149,14 @@ pub mod sys { /// object to be displayed. /// /// [`ContextMenu`]: ../struct.ContextMenu.html - pub(crate) const SHOW_CONTEXT_MENU: Selector = Selector::new("druid-builtin.show-context-menu"); + pub(crate) const SHOW_CONTEXT_MENU: Selector> = + Selector::new("druid-builtin.show-context-menu"); /// The selector for a command to set the window's menu. The argument should /// be a [`MenuDesc`] object. /// /// [`MenuDesc`]: ../struct.MenuDesc.html - pub(crate) const SET_MENU: Selector = Selector::new("druid-builtin.set-menu"); + pub(crate) const SET_MENU: Selector> = Selector::new("druid-builtin.set-menu"); /// Show the application preferences. pub const SHOW_PREFERENCES: Selector = Selector::new("druid-builtin.menu-show-preferences"); @@ -172,18 +173,15 @@ pub mod sys { /// System command. A file picker dialog will be shown to the user, and an /// [`OPEN_FILE`] command will be sent if a file is chosen. /// - /// The argument should be a [`FileDialogOptions`] struct. - /// /// [`OPEN_FILE`]: constant.OPEN_FILE.html /// [`FileDialogOptions`]: ../struct.FileDialogOptions.html - pub const SHOW_OPEN_PANEL: Selector = Selector::new("druid-builtin.menu-file-open"); + pub const SHOW_OPEN_PANEL: Selector = + Selector::new("druid-builtin.menu-file-open"); - /// Open a file. - /// - /// The argument must be a [`FileInfo`] object for the file to be opened. + /// Open a file, must be handled by the application. /// /// [`FileInfo`]: ../struct.FileInfo.html - pub const OPEN_FILE: Selector = Selector::new("druid-builtin.open-file-path"); + pub const OPEN_FILE: Selector = Selector::new("druid-builtin.open-file-path"); /// Special command. When issued, the system will show the 'save as' panel, /// and if a path is selected the system will issue a [`SAVE_FILE`] command @@ -193,12 +191,15 @@ pub mod sys { /// /// [`SAVE_FILE`]: constant.SAVE_FILE.html /// [`FileDialogOptions`]: ../struct.FileDialogOptions.html - pub const SHOW_SAVE_PANEL: Selector = Selector::new("druid-builtin.menu-file-save-as"); + pub const SHOW_SAVE_PANEL: Selector = + Selector::new("druid-builtin.menu-file-save-as"); - /// Save the current file. + /// Save the current file, must be handled by the application. /// - /// The argument, if present, should be the path where the file should be saved. - pub const SAVE_FILE: Selector = Selector::new("druid-builtin.menu-file-save"); + /// How this should be handled depends on the payload: + /// `Some(handle)`: the app should save to that file and store the `handle` for future use. + /// `None`: the app should have received `Some` before and use the stored `FileInfo`. + pub const SAVE_FILE: Selector> = Selector::new("druid-builtin.menu-file-save"); /// Show the print-setup window. pub const PRINT_SETUP: Selector = Selector::new("druid-builtin.menu-file-print-setup"); @@ -228,37 +229,93 @@ pub mod sys { impl Selector { /// A selector that does nothing. pub const NOOP: Selector = Selector::new(""); +} +impl Selector { /// Create a new `Selector` with the given string. - pub const fn new(s: &'static str) -> Selector { - Selector(s) + pub const fn new(s: &'static str) -> Selector { + Selector(s, PhantomData) + } + + pub(crate) const fn symbol(&self) -> SelectorSymbol { + self.0 + } +} + +impl Selector { + pub fn carry(self, object: T) -> Command { + Command::new(self, object) } } impl Command { /// Create a new `Command` with an argument. If you do not need /// an argument, `Selector` implements `Into`. - pub fn new(selector: Selector, arg: impl Any) -> Self { + pub fn new(selector: Selector, object: T) -> Self { Command { - selector, - object: Some(Arc::new(arg)), + selector: selector.symbol(), + object: Arc::new(object), } } /// Used to create a command from the types sent via an `ExtEventSink`. - pub(crate) fn from_ext(selector: Selector, object: Option>) -> Self { - let object: Option> = object.map(|obj| obj as Box); - let object = object.map(|o| o.into()); + pub(crate) fn from_ext(selector: SelectorSymbol, object: Arc) -> Self { Command { selector, object } } - /// Return a reference to this `Command`'s object, if it has one. - pub fn get_object(&self) -> Result<&T, ArgumentError> { - match self.object.as_ref() { - Some(o) => o.downcast_ref().ok_or(ArgumentError::IncorrectType), - None => Err(ArgumentError::NoArgument), + /// Checks if this was created using `selector`. + pub fn is(&self, selector: Selector) -> bool { + self.selector == selector.symbol() + } + + /// Returns `Some(reference)` to this `Command`'s object, if the selector matches. + /// + /// Returns `None` when `self.is(selector) == false`. + /// + /// If the selector has already been checked, [`get_unchecked`] can be used safely. + /// + /// # Panics + /// + /// Panics when the payload has a different type, than what the selector is supposed to carry. + /// This can happen when two selectors with different types but the same key are used. + /// + /// [`get_unchecked`]: #method.get_unchecked + pub fn get(&self, selector: Selector) -> Option<&T> { + if self.selector == selector.symbol() { + Some(self.object.downcast_ref().unwrap_or_else(|| { + panic!( + "The selector \"{}\" exists twice with different types. See druid::Command::get_object for more information", + selector.symbol() + ) + })) + } else { + None } } + + /// Return a reference to this `Command`'s object. + /// + /// If the selector has already been checked, `get_unchecked` can be used safely. + /// Otherwise you should either use [`get`] instead, or check the selector using [`is`] first. + /// + /// # Panics + /// + /// Panics when `self.is(selector) == false`. + /// + /// Panics when the payload has a different type, than what the selector is supposed to carry. + /// This can happen when two selectors with different types but the same key are used. + /// + /// [`is`]: #method.is + /// [`get`]: #method.get + pub fn get_unchecked(&self, selector: Selector) -> &T { + self.get(selector).unwrap_or_else(|| { + panic!( + "Expected selector \"{}\" but the command was \"{}\".", + selector.symbol(), + self.selector + ) + }) + } } impl SingleUse { @@ -275,29 +332,27 @@ impl SingleUse { impl From for Command { fn from(selector: Selector) -> Command { Command { - selector, - object: None, + selector: selector.symbol(), + object: Arc::new(()), } } } -impl std::fmt::Display for Selector { +impl std::fmt::Display for Selector { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "Selector('{}')", self.0) + write!(f, "Selector(\"{}\", {})", self.0, any::type_name::()) } } -impl std::fmt::Display for ArgumentError { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - ArgumentError::NoArgument => write!(f, "Command has no argument"), - ArgumentError::IncorrectType => write!(f, "Downcast failed: wrong concrete type"), - } +// This has do be done explicitly, to avoid the Copy bound on `T`. +// See https://doc.rust-lang.org/std/marker/trait.Copy.html#how-can-i-implement-copy . +impl Copy for Selector {} +impl Clone for Selector { + fn clone(&self) -> Self { + *self } } -impl std::error::Error for ArgumentError {} - impl From for Target { fn from(id: WindowId) -> Target { Target::Window(id) @@ -330,6 +385,6 @@ mod tests { let sel = Selector::new("my-selector"); let objs = vec![0, 1, 2]; let command = Command::new(sel, objs); - assert_eq!(command.get_object(), Ok(&vec![0, 1, 2])); + assert_eq!(command.get(sel), Some(&vec![0, 1, 2])); } } diff --git a/druid/src/contexts.rs b/druid/src/contexts.rs index d2ef1f4f7e..45aed3552d 100644 --- a/druid/src/contexts.rs +++ b/druid/src/contexts.rs @@ -250,7 +250,7 @@ impl EventCtx<'_, '_> { pub fn new_window(&mut self, desc: WindowDesc) { if self.state.root_app_data_type == TypeId::of::() { self.submit_command( - Command::new(commands::NEW_WINDOW, SingleUse::new(desc)), + Command::new(commands::NEW_WINDOW, SingleUse::new(Box::new(desc))), Target::Global, ); } else { @@ -278,7 +278,7 @@ impl EventCtx<'_, '_> { pub fn show_context_menu(&mut self, menu: ContextMenu) { if self.state.root_app_data_type == TypeId::of::() { self.submit_command( - Command::new(commands::SHOW_CONTEXT_MENU, menu), + Command::new(commands::SHOW_CONTEXT_MENU, Box::new(menu)), Target::Window(self.state.window_id), ); } else { @@ -883,7 +883,7 @@ impl<'a> ContextState<'a> { fn set_menu(&mut self, menu: MenuDesc) { if self.root_app_data_type == TypeId::of::() { self.submit_command( - Command::new(commands::SET_MENU, menu), + Command::new(commands::SET_MENU, Box::new(menu)), Some(Target::Window(self.window_id)), ); } else { diff --git a/druid/src/ext_event.rs b/druid/src/ext_event.rs index 3d1361b121..e25e4ede69 100644 --- a/druid/src/ext_event.rs +++ b/druid/src/ext_event.rs @@ -20,9 +20,9 @@ use std::sync::{Arc, Mutex}; use crate::shell::IdleHandle; use crate::win_handler::EXT_EVENT_IDLE_TOKEN; -use crate::{Command, Selector, Target, WindowId}; +use crate::{command::SelectorSymbol, Command, Selector, Target, WindowId}; -pub(crate) type ExtCommand = (Selector, Option>, Option); +pub(crate) type ExtCommand = (SelectorSymbol, Arc, Option); /// A thing that can move into other threads and be used to submit commands back /// to the running application. @@ -90,8 +90,7 @@ impl ExtEventSink { /// instead you have to pass the [`Selector`] and the (optional) argument /// separately, and it will be turned into a `Command` when it is received. /// - /// The `obj` argument can be any type which implements `Any + Send`, or `None` - /// if this command has no argument. + /// The `object` argument must implement `Any + Send + Sync`. /// /// If no explicit `Target` is submitted, the `Command` will be sent to /// the application's first window; if that window is subsequently closed, @@ -101,21 +100,22 @@ impl ExtEventSink { /// /// [`Command`]: struct.Command.html /// [`Selector`]: struct.Selector.html - pub fn submit_command( + pub fn submit_command( &self, - sel: Selector, - obj: impl Into>, + selector: Selector, + object: T, target: impl Into>, ) -> Result<(), ExtEventError> { let target = target.into(); - let obj = obj.into().map(|o| Box::new(o) as Box); + let object = Arc::new(object); if let Some(handle) = self.handle.lock().unwrap().as_mut() { handle.schedule_idle(EXT_EVENT_IDLE_TOKEN); } - self.queue - .lock() - .map_err(|_| ExtEventError)? - .push_back((sel, obj, target)); + self.queue.lock().map_err(|_| ExtEventError)?.push_back(( + selector.symbol(), + object, + target, + )); Ok(()) } } diff --git a/druid/src/menu.rs b/druid/src/menu.rs index 31cac785d1..198445b8c6 100644 --- a/druid/src/menu.rs +++ b/druid/src/menu.rs @@ -268,7 +268,7 @@ impl MenuDesc { /// use druid::{Command, LocalizedString, MenuDesc, MenuItem, Selector}; /// /// let num_items: usize = 4; - /// const MENU_COUNT_ACTION: Selector = Selector::new("menu-count-action"); + /// const MENU_COUNT_ACTION: Selector = Selector::new("menu-count-action"); /// /// let my_menu: MenuDesc = MenuDesc::empty() /// .append_iter(|| (0..num_items).map(|i| { @@ -508,6 +508,7 @@ pub mod sys { /// [the win32 documentation]: https://docs.microsoft.com/en-us/windows/win32/uxguide/cmd-menus#standard-menus pub mod file { use super::*; + use crate::FileDialogOptions; /// A default file menu. /// @@ -541,7 +542,7 @@ pub mod sys { pub fn open() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-open"), - commands::SHOW_OPEN_PANEL, + commands::SHOW_OPEN_PANEL.carry(FileDialogOptions::default()), ) .hotkey(RawMods::Ctrl, "o") } @@ -558,7 +559,7 @@ pub mod sys { pub fn save() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save"), - commands::SAVE_FILE, + commands::SAVE_FILE.carry(None), ) .hotkey(RawMods::Ctrl, "s") } @@ -569,7 +570,7 @@ pub mod sys { pub fn save_ellipsis() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-ellipsis"), - commands::SHOW_SAVE_PANEL, + commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), ) .hotkey(RawMods::Ctrl, "s") } @@ -578,7 +579,7 @@ pub mod sys { pub fn save_as() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-as"), - commands::SHOW_SAVE_PANEL, + commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), ) .hotkey(RawMods::CtrlShift, "s") } @@ -705,6 +706,7 @@ pub mod sys { /// The file menu. pub mod file { use super::*; + use crate::FileDialogOptions; /// A default file menu. /// @@ -744,7 +746,7 @@ pub mod sys { pub fn open_file() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-open"), - commands::SHOW_OPEN_PANEL, + commands::SHOW_OPEN_PANEL.carry(FileDialogOptions::default()), ) .hotkey(RawMods::Meta, "o") } @@ -762,7 +764,7 @@ pub mod sys { pub fn save() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save"), - commands::SAVE_FILE, + commands::SAVE_FILE.carry(None), ) .hotkey(RawMods::Meta, "s") } @@ -773,7 +775,7 @@ pub mod sys { pub fn save_ellipsis() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-ellipsis"), - commands::SHOW_SAVE_PANEL, + commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), ) .hotkey(RawMods::Meta, "s") } @@ -782,7 +784,7 @@ pub mod sys { pub fn save_as() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-as"), - commands::SHOW_SAVE_PANEL, + commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), ) .hotkey(RawMods::MetaShift, "s") } diff --git a/druid/src/tests/helpers.rs b/druid/src/tests/helpers.rs index f28c3b3d13..f0d7be4b68 100644 --- a/druid/src/tests/helpers.rs +++ b/druid/src/tests/helpers.rs @@ -214,7 +214,7 @@ impl ReplaceChild { impl Widget for ReplaceChild { fn event(&mut self, ctx: &mut EventCtx, event: &Event, data: &mut T, env: &Env) { if let Event::Command(cmd) = event { - if cmd.selector == REPLACE_CHILD { + if cmd.is(REPLACE_CHILD) { self.inner = WidgetPod::new((self.replacer)()); ctx.children_changed(); return; diff --git a/druid/src/tests/mod.rs b/druid/src/tests/mod.rs index 79fb54a232..0cf4912b40 100644 --- a/druid/src/tests/mod.rs +++ b/druid/src/tests/mod.rs @@ -158,7 +158,7 @@ fn take_focus() { ModularWidget::new(inner) .event_fn(|_, ctx, event, _data, _env| { if let Event::Command(cmd) = event { - if cmd.selector == TAKE_FOCUS { + if cmd.is(TAKE_FOCUS) { ctx.request_focus(); } } @@ -225,12 +225,12 @@ fn focus_changed() { ModularWidget::new(children) .event_fn(|children, ctx, event, data, env| { if let Event::Command(cmd) = event { - if cmd.selector == TAKE_FOCUS { + if cmd.is(TAKE_FOCUS) { ctx.request_focus(); // Stop propagating this command so children // aren't requesting focus too. ctx.set_handled(); - } else if cmd.selector == ALL_TAKE_FOCUS_BEFORE { + } else if cmd.is(ALL_TAKE_FOCUS_BEFORE) { ctx.request_focus(); } } @@ -238,7 +238,7 @@ fn focus_changed() { .iter_mut() .for_each(|a| a.event(ctx, event, data, env)); if let Event::Command(cmd) = event { - if cmd.selector == ALL_TAKE_FOCUS_AFTER { + if cmd.is(ALL_TAKE_FOCUS_AFTER) { ctx.request_focus(); } } diff --git a/druid/src/widget/textbox.rs b/druid/src/widget/textbox.rs index 08e9962abe..379ad3e369 100644 --- a/druid/src/widget/textbox.rs +++ b/druid/src/widget/textbox.rs @@ -53,7 +53,8 @@ pub struct TextBox { impl TextBox { /// Perform an `EditAction`. The payload *must* be an `EditAction`. - pub const PERFORM_EDIT: Selector = Selector::new("druid-builtin.textbox.perform-edit"); + pub const PERFORM_EDIT: Selector = + Selector::new("druid-builtin.textbox.perform-edit"); /// Create a new TextBox widget pub fn new() -> TextBox { @@ -281,22 +282,19 @@ impl Widget for TextBox { } Event::Command(ref cmd) if ctx.is_focused() - && (cmd.selector == crate::commands::COPY - || cmd.selector == crate::commands::CUT) => + && (cmd.is(crate::commands::COPY) || cmd.is(crate::commands::CUT)) => { if let Some(text) = data.slice(self.selection.range()) { Application::global().clipboard().put_string(text); } - if !self.selection.is_caret() && cmd.selector == crate::commands::CUT { + if !self.selection.is_caret() && cmd.is(crate::commands::CUT) { edit_action = Some(EditAction::Delete); } ctx.set_handled(); } - Event::Command(cmd) if cmd.selector == RESET_BLINK => self.reset_cursor_blink(ctx), - Event::Command(cmd) if cmd.selector == TextBox::PERFORM_EDIT => { - let edit = cmd - .get_object::() - .expect("PERFORM_EDIT contained non-edit payload"); + Event::Command(cmd) if cmd.is(RESET_BLINK) => self.reset_cursor_blink(ctx), + Event::Command(cmd) if cmd.is(TextBox::PERFORM_EDIT) => { + let edit = cmd.get_unchecked(TextBox::PERFORM_EDIT); self.do_edit_action(edit.to_owned(), data); } Event::Paste(ref item) => { diff --git a/druid/src/win_handler.rs b/druid/src/win_handler.rs index 13b9b379cb..792335eb8a 100644 --- a/druid/src/win_handler.rs +++ b/druid/src/win_handler.rs @@ -21,9 +21,7 @@ use std::rc::Rc; use crate::kurbo::{Rect, Size}; use crate::piet::Piet; -use crate::shell::{ - Application, FileDialogOptions, IdleToken, MouseEvent, Scale, WinHandler, WindowHandle, -}; +use crate::shell::{Application, IdleToken, MouseEvent, Scale, WinHandler, WindowHandle}; use crate::app_delegate::{AppDelegate, DelegateCtx}; use crate::core::CommandQueue; @@ -31,8 +29,8 @@ use crate::ext_event::ExtEventHost; use crate::menu::ContextMenu; use crate::window::Window; use crate::{ - Command, Data, Env, Event, InternalEvent, KeyEvent, MenuDesc, SingleUse, Target, TimerToken, - WindowDesc, WindowId, + Command, Data, Env, Event, InternalEvent, KeyEvent, MenuDesc, Target, TimerToken, WindowDesc, + WindowId, }; use crate::command::sys as sys_cmd; @@ -316,10 +314,11 @@ impl Inner { match target { Target::Window(id) => { // first handle special window-level events - match cmd.selector { - sys_cmd::SET_MENU => return self.set_menu(id, &cmd), - sys_cmd::SHOW_CONTEXT_MENU => return self.show_context_menu(id, &cmd), - _ => (), + if cmd.is(sys_cmd::SET_MENU) { + return self.set_menu(id, &cmd); + } + if cmd.is(sys_cmd::SHOW_CONTEXT_MENU) { + return self.show_context_menu(id, &cmd); } if let Some(w) = self.windows.get_mut(id) { let event = Event::Command(cmd); @@ -371,20 +370,32 @@ impl Inner { fn set_menu(&mut self, window_id: WindowId, cmd: &Command) { if let Some(win) = self.windows.get_mut(window_id) { - match cmd.get_object::>() { - Ok(menu) => win.set_menu(menu.to_owned(), &self.data, &self.env), - Err(e) => log::warn!("set-menu object error: '{}'", e), + match cmd + .get_unchecked(sys_cmd::SET_MENU) + .downcast_ref::>() + { + Some(menu) => win.set_menu(menu.clone(), &self.data, &self.env), + None => panic!( + "{} command must carry a MenuDesc.", + sys_cmd::SET_MENU + ), } } } fn show_context_menu(&mut self, window_id: WindowId, cmd: &Command) { if let Some(win) = self.windows.get_mut(window_id) { - match cmd.get_object::>() { - Ok(ContextMenu { menu, location }) => { + match cmd + .get_unchecked(sys_cmd::SHOW_CONTEXT_MENU) + .downcast_ref::>() + { + Some(ContextMenu { menu, location }) => { win.show_context_menu(menu.to_owned(), *location, &self.data, &self.env) } - Err(e) => log::warn!("show-context-menu object error: '{}'", e), + None => panic!( + "{} command must carry a ContextMenu.", + sys_cmd::SHOW_CONTEXT_MENU + ), } } } @@ -526,35 +537,36 @@ impl AppState { /// windows) have their logic here; other commands are passed to the window. fn handle_cmd(&mut self, target: Target, cmd: Command) { use Target as T; - match (target, &cmd.selector) { + match target { // these are handled the same no matter where they come from - (_, &sys_cmd::QUIT_APP) => self.quit(), - (_, &sys_cmd::HIDE_APPLICATION) => self.hide_app(), - (_, &sys_cmd::HIDE_OTHERS) => self.hide_others(), - (_, &sys_cmd::NEW_WINDOW) => { + _ if cmd.is(sys_cmd::QUIT_APP) => self.quit(), + _ if cmd.is(sys_cmd::HIDE_APPLICATION) => self.hide_app(), + _ if cmd.is(sys_cmd::HIDE_OTHERS) => self.hide_others(), + _ if cmd.is(sys_cmd::NEW_WINDOW) => { if let Err(e) = self.new_window(cmd) { log::error!("failed to create window: '{}'", e); } } - (_, &sys_cmd::CLOSE_ALL_WINDOWS) => self.request_close_all_windows(), + _ if cmd.is(sys_cmd::CLOSE_ALL_WINDOWS) => self.request_close_all_windows(), // these should come from a window // FIXME: we need to be able to open a file without a window handle - (T::Window(id), &sys_cmd::SHOW_OPEN_PANEL) => self.show_open_panel(cmd, id), - (T::Window(id), &sys_cmd::SHOW_SAVE_PANEL) => self.show_save_panel(cmd, id), - (T::Window(id), &sys_cmd::CLOSE_WINDOW) => self.request_close_window(id), - (T::Window(id), &sys_cmd::SHOW_WINDOW) => self.show_window(id), - (T::Window(id), &sys_cmd::PASTE) => self.do_paste(id), - (_, &sys_cmd::CLOSE_WINDOW) => log::warn!("CLOSE_WINDOW command must target a window."), - (_, &sys_cmd::SHOW_WINDOW) => log::warn!("SHOW_WINDOW command must target a window."), + T::Window(id) if cmd.is(sys_cmd::SHOW_OPEN_PANEL) => self.show_open_panel(cmd, id), + T::Window(id) if cmd.is(sys_cmd::SHOW_SAVE_PANEL) => self.show_save_panel(cmd, id), + T::Window(id) if cmd.is(sys_cmd::CLOSE_WINDOW) => self.request_close_window(id), + T::Window(id) if cmd.is(sys_cmd::SHOW_WINDOW) => self.show_window(id), + T::Window(id) if cmd.is(sys_cmd::PASTE) => self.do_paste(id), + _ if cmd.is(sys_cmd::CLOSE_WINDOW) => { + log::warn!("CLOSE_WINDOW command must target a window.") + } + _ if cmd.is(sys_cmd::SHOW_WINDOW) => { + log::warn!("SHOW_WINDOW command must target a window.") + } _ => self.inner.borrow_mut().dispatch_cmd(target, cmd), } } fn show_open_panel(&mut self, cmd: Command, window_id: WindowId) { - let options = cmd - .get_object::() - .map(|opts| opts.to_owned()) - .unwrap_or_default(); + let options = cmd.get_unchecked(sys_cmd::SHOW_OPEN_PANEL).to_owned(); //FIXME: this is blocking; if we hold `borrow_mut` we are likely to cause //a crash. as a workaround we take a clone of the window handle. //it's less clear what the better solution would be. @@ -573,10 +585,7 @@ impl AppState { } fn show_save_panel(&mut self, cmd: Command, window_id: WindowId) { - let options = cmd - .get_object::() - .map(|opts| opts.to_owned()) - .unwrap_or_default(); + let options = cmd.get_unchecked(sys_cmd::SHOW_SAVE_PANEL).to_owned(); let handle = self .inner .borrow_mut() @@ -585,16 +594,16 @@ impl AppState { .map(|w| w.handle.clone()); let result = handle.and_then(|mut handle| handle.save_as_sync(options)); if let Some(info) = result { - let cmd = Command::new(sys_cmd::SAVE_FILE, info); + let cmd = Command::new(sys_cmd::SAVE_FILE, Some(info)); self.inner.borrow_mut().dispatch_cmd(window_id.into(), cmd); } } fn new_window(&mut self, cmd: Command) -> Result<(), Box> { - let desc = cmd.get_object::>>()?; + let desc = cmd.get_unchecked(sys_cmd::NEW_WINDOW); // 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 desc = desc.take().unwrap().downcast::>().unwrap(); let window = desc.build_native(self)?; window.show(); Ok(()) From e6924ba5da4a70a357cd5d0eb19688633b085c3d Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Tue, 26 May 2020 20:20:23 +0200 Subject: [PATCH 02/12] Avoid Sync bound for ExtCommand's using Box. --- druid/src/command.rs | 4 ++-- druid/src/ext_event.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 4aa0d45e1f..abd293d28b 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -259,8 +259,8 @@ impl Command { } /// Used to create a command from the types sent via an `ExtEventSink`. - pub(crate) fn from_ext(selector: SelectorSymbol, object: Arc) -> Self { - Command { selector, object } + pub(crate) fn from_ext(selector: SelectorSymbol, object: Box) -> Self { + Command { selector, object: object.into() } } /// Checks if this was created using `selector`. diff --git a/druid/src/ext_event.rs b/druid/src/ext_event.rs index e25e4ede69..0a3b3e51b0 100644 --- a/druid/src/ext_event.rs +++ b/druid/src/ext_event.rs @@ -22,7 +22,7 @@ use crate::shell::IdleHandle; use crate::win_handler::EXT_EVENT_IDLE_TOKEN; use crate::{command::SelectorSymbol, Command, Selector, Target, WindowId}; -pub(crate) type ExtCommand = (SelectorSymbol, Arc, Option); +pub(crate) type ExtCommand = (SelectorSymbol, Box, Option); /// A thing that can move into other threads and be used to submit commands back /// to the running application. @@ -103,11 +103,11 @@ impl ExtEventSink { pub fn submit_command( &self, selector: Selector, - object: T, + object: impl Into>, target: impl Into>, ) -> Result<(), ExtEventError> { let target = target.into(); - let object = Arc::new(object); + let object = object.into(); if let Some(handle) = self.handle.lock().unwrap().as_mut() { handle.schedule_idle(EXT_EVENT_IDLE_TOKEN); } From 7381374a95d58df2de083ee7c11b6cbb84742b0a Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Tue, 26 May 2020 20:32:38 +0200 Subject: [PATCH 03/12] Add changelog entry. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf4ef40d4c..83284b5b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i - 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]) - Renamed `WidgetPod` methods: `paint` to `paint_raw`, `paint_with_offset` to `paint`, `paint_with_offset_always` to `paint_always`. ([#980] by [@totsteps]) +- `Command` and `Selector` are now statically typed, similarly to `Env` and `Key`. ([#993] by [@finnerale]) ### Deprecated @@ -242,6 +243,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i [#984]: https://github.com/xi-editor/druid/pull/984 [#990]: https://github.com/xi-editor/druid/pull/990 [#991]: https://github.com/xi-editor/druid/pull/991 +[#993]: https://github.com/xi-editor/druid/pull/993 ## [0.5.0] - 2020-04-01 From c55f127c6ef89f57d49cce2d8aa77d992ebd4eb7 Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Tue, 26 May 2020 20:40:57 +0200 Subject: [PATCH 04/12] fmt. --- druid/src/command.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index abd293d28b..06a765d0a5 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -260,7 +260,10 @@ impl Command { /// Used to create a command from the types sent via an `ExtEventSink`. pub(crate) fn from_ext(selector: SelectorSymbol, object: Box) -> Self { - Command { selector, object: object.into() } + Command { + selector, + object: object.into(), + } } /// Checks if this was created using `selector`. From 025f841cd69d310183212b89a7c27b77cd229807 Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Tue, 26 May 2020 21:00:15 +0200 Subject: [PATCH 05/12] clippy. --- druid/src/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 06a765d0a5..874b0ac0b8 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -237,7 +237,7 @@ impl Selector { Selector(s, PhantomData) } - pub(crate) const fn symbol(&self) -> SelectorSymbol { + pub(crate) const fn symbol(self) -> SelectorSymbol { self.0 } } From af696499b5aed114fc3769abf8933f9f8a0a3efb Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 15:10:33 +0200 Subject: [PATCH 06/12] Apply suggestions from review. --- druid/src/command.rs | 83 +++++++++++++++++++++++------------------- druid/src/ext_event.rs | 10 ++--- 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 874b0ac0b8..a989f25277 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -22,11 +22,14 @@ use std::{ use crate::{WidgetId, WindowId}; +/// A [`Selector`]s identity. +/// +/// [`Selector`]: struct.Selector.html pub(crate) type SelectorSymbol = &'static str; /// An identifier for a particular command. /// -/// The type parameter `T` specifies the commands payload type. +/// The type parameter `T` specifies the command's payload type. /// /// This should be a unique string identifier. Certain `Selector`s are defined /// by druid, and have special meaning to the framework; these are listed in the @@ -34,7 +37,7 @@ pub(crate) type SelectorSymbol = &'static str; /// /// [`druid::commands`]: commands/index.html #[derive(Debug, PartialEq, Eq)] -pub struct Selector(SelectorSymbol, PhantomData); +pub struct Selector(SelectorSymbol, PhantomData<*const T>); /// An arbitrary command. /// @@ -42,7 +45,7 @@ pub struct Selector(SelectorSymbol, PhantomData); /// and what type of payload it carries, as well as the actual payload. /// /// If the payload can't or shouldn't be cloned, -/// wrapping it with [`SingleUse`] allows you to `take` the object. +/// wrapping it with [`SingleUse`] allows you to `take` the payload. /// /// # Examples /// ``` @@ -55,17 +58,15 @@ pub struct Selector(SelectorSymbol, PhantomData); /// assert_eq!(command.get(selector), Some(&vec![1, 3, 10, 12])); /// ``` /// -/// [`Command::new`]: #method.new -/// [`Command::get_object`]: #method.get_object /// [`SingleUse`]: struct.SingleUse.html /// [`Selector`]: struct.Selector.html #[derive(Debug, Clone)] pub struct Command { - selector: SelectorSymbol, - object: Arc, + symbol: SelectorSymbol, + payload: Arc, } -/// A wrapper type for [`Command`] arguments that should only be used once. +/// A wrapper type for [`Command`] payloads 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. @@ -111,9 +112,10 @@ pub enum Target { /// /// [`Command`]: ../struct.Command.html pub mod sys { + use std::any::Any; + use super::Selector; use crate::{FileDialogOptions, FileInfo, SingleUse}; - use std::any::Any; /// Quit the running application. This command is handled by the druid library. pub const QUIT_APP: Selector = Selector::new("druid-builtin.quit-app"); @@ -145,14 +147,14 @@ pub mod sys { /// will automatically target the window containing the widget. pub const SHOW_WINDOW: Selector = Selector::new("druid-builtin.show-window"); - /// Display a context (right-click) menu. The argument must be the [`ContextMenu`]. + /// Display a context (right-click) menu. The payload must be the [`ContextMenu`] /// object to be displayed. /// /// [`ContextMenu`]: ../struct.ContextMenu.html pub(crate) const SHOW_CONTEXT_MENU: Selector> = Selector::new("druid-builtin.show-context-menu"); - /// The selector for a command to set the window's menu. The argument should + /// The selector for a command to set the window's menu. The payload should /// be a [`MenuDesc`] object. /// /// [`MenuDesc`]: ../struct.MenuDesc.html @@ -185,9 +187,7 @@ pub mod sys { /// Special command. When issued, the system will show the 'save as' panel, /// and if a path is selected the system will issue a [`SAVE_FILE`] command - /// with the selected path as the argument. - /// - /// The argument should be a [`FileDialogOptions`] object. + /// with the selected path as the payload. /// /// [`SAVE_FILE`]: constant.SAVE_FILE.html /// [`FileDialogOptions`]: ../struct.FileDialogOptions.html @@ -226,7 +226,7 @@ pub mod sys { pub const REDO: Selector = Selector::new("druid-builtin.menu-redo"); } -impl Selector { +impl Selector<()> { /// A selector that does nothing. pub const NOOP: Selector = Selector::new(""); } @@ -237,45 +237,54 @@ impl Selector { Selector(s, PhantomData) } + /// Returns the `SelectorSymbol` identifying this `Selector`. pub(crate) const fn symbol(self) -> SelectorSymbol { self.0 } } impl Selector { - pub fn carry(self, object: T) -> Command { - Command::new(self, object) + /// Convenience method for [`Command::new`] with this selector. + /// + /// [`Command::new`]: struct.Command.html#method.new + pub fn carry(self, payload: T) -> Command { + Command::new(self, payload) } } impl Command { - /// Create a new `Command` with an argument. If you do not need - /// an argument, `Selector` implements `Into`. - pub fn new(selector: Selector, object: T) -> Self { + /// Create a new `Command` with payload. + /// + /// [`Selector::carry`] can be used to create `Command`s more conveniently. + /// + /// If you do not need an payload, `Selector` implements `Into`. + /// + /// [`Selector::carry`]: struct.Selector.html#method.carry + pub fn new(selector: Selector, payload: T) -> Self { Command { - selector: selector.symbol(), - object: Arc::new(object), + symbol: selector.symbol(), + payload: Arc::new(payload), } } /// Used to create a command from the types sent via an `ExtEventSink`. - pub(crate) fn from_ext(selector: SelectorSymbol, object: Box) -> Self { + pub(crate) fn from_ext(symbol: SelectorSymbol, payload: Box) -> Self { Command { - selector, - object: object.into(), + symbol, + payload: payload.into(), } } /// Checks if this was created using `selector`. pub fn is(&self, selector: Selector) -> bool { - self.selector == selector.symbol() + self.symbol == selector.symbol() } - /// Returns `Some(reference)` to this `Command`'s object, if the selector matches. + /// Returns `Some(reference)` to this `Command`'s payload, if the selector matches. /// /// Returns `None` when `self.is(selector) == false`. /// - /// If the selector has already been checked, [`get_unchecked`] can be used safely. + /// Alternatively you can check the selector with [`is`] and then use [`get_unchecked`]. /// /// # Panics /// @@ -284,10 +293,10 @@ impl Command { /// /// [`get_unchecked`]: #method.get_unchecked pub fn get(&self, selector: Selector) -> Option<&T> { - if self.selector == selector.symbol() { - Some(self.object.downcast_ref().unwrap_or_else(|| { + if self.symbol == selector.symbol() { + Some(self.payload.downcast_ref().unwrap_or_else(|| { panic!( - "The selector \"{}\" exists twice with different types. See druid::Command::get_object for more information", + "The selector \"{}\" exists twice with different types. See druid::Command::get for more information", selector.symbol() ) })) @@ -296,10 +305,10 @@ impl Command { } } - /// Return a reference to this `Command`'s object. + /// Returns a reference to this `Command`'s payload. /// - /// If the selector has already been checked, `get_unchecked` can be used safely. - /// Otherwise you should either use [`get`] instead, or check the selector using [`is`] first. + /// If the selector has already been checked with [`is`], then `get_unchecked` can be used safely. + /// Otherwise you should use [`get`] instead. /// /// # Panics /// @@ -315,7 +324,7 @@ impl Command { panic!( "Expected selector \"{}\" but the command was \"{}\".", selector.symbol(), - self.selector + self.symbol ) }) } @@ -335,8 +344,8 @@ impl SingleUse { impl From for Command { fn from(selector: Selector) -> Command { Command { - selector: selector.symbol(), - object: Arc::new(()), + symbol: selector.symbol(), + payload: Arc::new(()), } } } diff --git a/druid/src/ext_event.rs b/druid/src/ext_event.rs index 0a3b3e51b0..00bc67835e 100644 --- a/druid/src/ext_event.rs +++ b/druid/src/ext_event.rs @@ -87,10 +87,10 @@ impl ExtEventSink { /// Submit a [`Command`] to the running application. /// /// [`Command`] is not thread safe, so you cannot submit it directly; - /// instead you have to pass the [`Selector`] and the (optional) argument + /// instead you have to pass the [`Selector`] and the payload /// separately, and it will be turned into a `Command` when it is received. /// - /// The `object` argument must implement `Any + Send + Sync`. + /// The `payload` must implement `Any + Send + Sync`. /// /// If no explicit `Target` is submitted, the `Command` will be sent to /// the application's first window; if that window is subsequently closed, @@ -103,17 +103,17 @@ impl ExtEventSink { pub fn submit_command( &self, selector: Selector, - object: impl Into>, + payload: impl Into>, target: impl Into>, ) -> Result<(), ExtEventError> { let target = target.into(); - let object = object.into(); + let payload = payload.into(); if let Some(handle) = self.handle.lock().unwrap().as_mut() { handle.schedule_idle(EXT_EVENT_IDLE_TOKEN); } self.queue.lock().map_err(|_| ExtEventError)?.push_back(( selector.symbol(), - object, + payload, target, )); Ok(()) From 0b37f1cf3576fef2635ba402c7142c85b9e3594f Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 15:13:06 +0200 Subject: [PATCH 07/12] Clarify _PANEL commands documentation. --- druid/src/command.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index a989f25277..ff748e96b8 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -172,11 +172,10 @@ pub mod sys { /// Show the new file dialog. pub const NEW_FILE: Selector = Selector::new("druid-builtin.menu-file-new"); - /// System command. A file picker dialog will be shown to the user, and an - /// [`OPEN_FILE`] command will be sent if a file is chosen. + /// When submitted by the application, a file picker dialog will be shown to the user, + /// and an [`OPEN_FILE`] command will be sent if a file is chosen. /// /// [`OPEN_FILE`]: constant.OPEN_FILE.html - /// [`FileDialogOptions`]: ../struct.FileDialogOptions.html pub const SHOW_OPEN_PANEL: Selector = Selector::new("druid-builtin.menu-file-open"); @@ -185,12 +184,11 @@ pub mod sys { /// [`FileInfo`]: ../struct.FileInfo.html pub const OPEN_FILE: Selector = Selector::new("druid-builtin.open-file-path"); - /// Special command. When issued, the system will show the 'save as' panel, + /// When submitted by the application, the system will show the 'save as' panel, /// and if a path is selected the system will issue a [`SAVE_FILE`] command /// with the selected path as the payload. /// /// [`SAVE_FILE`]: constant.SAVE_FILE.html - /// [`FileDialogOptions`]: ../struct.FileDialogOptions.html pub const SHOW_SAVE_PANEL: Selector = Selector::new("druid-builtin.menu-file-save-as"); From 83e8c9fc427e5d65509ac7e50b57a49da5651993 Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 15:18:32 +0200 Subject: [PATCH 08/12] Replace two more uses of object with payload. --- druid/src/command.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index ff748e96b8..766ac256ca 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -81,14 +81,14 @@ pub struct Command { /// let num = CantClone(42); /// let command = Command::new(selector, SingleUse::new(num)); /// -/// let object: &SingleUse = command.get_unchecked(selector); -/// if let Some(num) = object.take() { +/// let payload: &SingleUse = command.get_unchecked(selector); +/// if let Some(num) = payload.take() { /// // now you own the data /// assert_eq!(num.0, 42); /// } /// /// // subsequent calls will return `None` -/// assert!(object.take().is_none()); +/// assert!(payload.take().is_none()); /// ``` /// /// [`Command`]: struct.Command.html @@ -390,11 +390,12 @@ impl Into> for WidgetId { #[cfg(test)] mod tests { use super::*; + #[test] - fn get_object() { + fn get_payload() { let sel = Selector::new("my-selector"); - let objs = vec![0, 1, 2]; - let command = Command::new(sel, objs); + let payload = vec![0, 1, 2]; + let command = Command::new(sel, payload); assert_eq!(command.get(sel), Some(&vec![0, 1, 2])); } } From e0d24f7a04847c65d614e0283ba387dd654881e6 Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 16:26:09 +0200 Subject: [PATCH 09/12] More doc cleanups. --- druid/src/command.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 766ac256ca..3f45e34579 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -22,7 +22,7 @@ use std::{ use crate::{WidgetId, WindowId}; -/// A [`Selector`]s identity. +/// The identity of a [`Selector`]. /// /// [`Selector`]: struct.Selector.html pub(crate) type SelectorSymbol = &'static str; @@ -251,12 +251,13 @@ impl Selector { } impl Command { - /// Create a new `Command` with payload. + /// Create a new `Command` with a payload. /// /// [`Selector::carry`] can be used to create `Command`s more conveniently. /// - /// If you do not need an payload, `Selector` implements `Into`. + /// If you do not need a payload, [`Selector`] implements `Into`. /// + /// [`Selector`]: struct.Selector.html /// [`Selector::carry`]: struct.Selector.html#method.carry pub fn new(selector: Selector, payload: T) -> Self { Command { From 657f5041959d2f685e4b89299403ec092b25a91e Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 16:27:14 +0200 Subject: [PATCH 10/12] Update changelog entry. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83284b5b71..ec5c782189 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,7 +72,7 @@ This means that druid no longer requires cairo on macOS and uses Core Graphics i - 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]) - Renamed `WidgetPod` methods: `paint` to `paint_raw`, `paint_with_offset` to `paint`, `paint_with_offset_always` to `paint_always`. ([#980] by [@totsteps]) -- `Command` and `Selector` are now statically typed, similarly to `Env` and `Key`. ([#993] by [@finnerale]) +- `Command` and `Selector` have been reworked and are now statically typed, similarly to `Env` and `Key`. ([#993] by [@finnerale]) ### Deprecated From f369f9130ba48c6845e8fa4fac42bba4b7457933 Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 18:34:01 +0200 Subject: [PATCH 11/12] Rename carry to with and improve its docs. --- druid/src/command.rs | 9 ++++++--- druid/src/menu.rs | 16 ++++++++-------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 3f45e34579..44863da268 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -244,8 +244,11 @@ impl Selector { impl Selector { /// Convenience method for [`Command::new`] with this selector. /// + /// If the payload is `()` there is no need to call this, + /// as `Selector<()>` implements `Into`. + /// /// [`Command::new`]: struct.Command.html#method.new - pub fn carry(self, payload: T) -> Command { + pub fn with(self, payload: T) -> Command { Command::new(self, payload) } } @@ -253,12 +256,12 @@ impl Selector { impl Command { /// Create a new `Command` with a payload. /// - /// [`Selector::carry`] can be used to create `Command`s more conveniently. + /// [`Selector::with`] can be used to create `Command`s more conveniently. /// /// If you do not need a payload, [`Selector`] implements `Into`. /// /// [`Selector`]: struct.Selector.html - /// [`Selector::carry`]: struct.Selector.html#method.carry + /// [`Selector::with`]: struct.Selector.html#method.with pub fn new(selector: Selector, payload: T) -> Self { Command { symbol: selector.symbol(), diff --git a/druid/src/menu.rs b/druid/src/menu.rs index 198445b8c6..41611c89d9 100644 --- a/druid/src/menu.rs +++ b/druid/src/menu.rs @@ -542,7 +542,7 @@ pub mod sys { pub fn open() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-open"), - commands::SHOW_OPEN_PANEL.carry(FileDialogOptions::default()), + commands::SHOW_OPEN_PANEL.with(FileDialogOptions::default()), ) .hotkey(RawMods::Ctrl, "o") } @@ -559,7 +559,7 @@ pub mod sys { pub fn save() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save"), - commands::SAVE_FILE.carry(None), + commands::SAVE_FILE.with(None), ) .hotkey(RawMods::Ctrl, "s") } @@ -570,7 +570,7 @@ pub mod sys { pub fn save_ellipsis() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-ellipsis"), - commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), + commands::SHOW_SAVE_PANEL.with(FileDialogOptions::default()), ) .hotkey(RawMods::Ctrl, "s") } @@ -579,7 +579,7 @@ pub mod sys { pub fn save_as() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-as"), - commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), + commands::SHOW_SAVE_PANEL.with(FileDialogOptions::default()), ) .hotkey(RawMods::CtrlShift, "s") } @@ -746,7 +746,7 @@ pub mod sys { pub fn open_file() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-open"), - commands::SHOW_OPEN_PANEL.carry(FileDialogOptions::default()), + commands::SHOW_OPEN_PANEL.with(FileDialogOptions::default()), ) .hotkey(RawMods::Meta, "o") } @@ -764,7 +764,7 @@ pub mod sys { pub fn save() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save"), - commands::SAVE_FILE.carry(None), + commands::SAVE_FILE.with(None), ) .hotkey(RawMods::Meta, "s") } @@ -775,7 +775,7 @@ pub mod sys { pub fn save_ellipsis() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-ellipsis"), - commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), + commands::SHOW_SAVE_PANEL.with(FileDialogOptions::default()), ) .hotkey(RawMods::Meta, "s") } @@ -784,7 +784,7 @@ pub mod sys { pub fn save_as() -> MenuItem { MenuItem::new( LocalizedString::new("common-menu-file-save-as"), - commands::SHOW_SAVE_PANEL.carry(FileDialogOptions::default()), + commands::SHOW_SAVE_PANEL.with(FileDialogOptions::default()), ) .hotkey(RawMods::MetaShift, "s") } From 8b528b4ba43027225192aee4cd36b1e5d81e16c9 Mon Sep 17 00:00:00 2001 From: Leopold Luley Date: Thu, 28 May 2020 18:35:52 +0200 Subject: [PATCH 12/12] Apply doc changes from review. --- druid/src/command.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/druid/src/command.rs b/druid/src/command.rs index 44863da268..2036564c06 100644 --- a/druid/src/command.rs +++ b/druid/src/command.rs @@ -277,12 +277,12 @@ impl Command { } } - /// Checks if this was created using `selector`. + /// Returns `true` if `self` matches this `selector`. pub fn is(&self, selector: Selector) -> bool { self.symbol == selector.symbol() } - /// Returns `Some(reference)` to this `Command`'s payload, if the selector matches. + /// Returns `Some(&T)` (this `Command`'s payload) if the selector matches. /// /// Returns `None` when `self.is(selector) == false`. ///