From 32be0c58841252db4f07c273c894a01f1fa414ff Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Wed, 9 Aug 2023 02:28:14 +0300 Subject: [PATCH] feat: add `MenuId::new` (#94) --- .changes/clone.md | 5 +++ .changes/menu-id-new.md | 5 +++ src/items/mod.rs | 2 +- src/lib.rs | 52 ++------------------------- src/menu_id.rs | 62 ++++++++++++++++++++++++++++++++ src/platform_impl/macos/mod.rs | 8 ++--- src/platform_impl/windows/mod.rs | 43 +++++++++++----------- 7 files changed, 102 insertions(+), 75 deletions(-) create mode 100644 .changes/clone.md create mode 100644 .changes/menu-id-new.md create mode 100644 src/menu_id.rs diff --git a/.changes/clone.md b/.changes/clone.md new file mode 100644 index 0000000..9ed58ca --- /dev/null +++ b/.changes/clone.md @@ -0,0 +1,5 @@ +--- +"muda": "patch" +--- + +On Windows, reduce some unneccassry string cloning. \ No newline at end of file diff --git a/.changes/menu-id-new.md b/.changes/menu-id-new.md new file mode 100644 index 0000000..0219d12 --- /dev/null +++ b/.changes/menu-id-new.md @@ -0,0 +1,5 @@ +--- +"muda": "patch" +--- + +Add `MenuId::new` convenience method. \ No newline at end of file diff --git a/src/items/mod.rs b/src/items/mod.rs index 43b9a3b..a79a77e 100644 --- a/src/items/mod.rs +++ b/src/items/mod.rs @@ -20,7 +20,7 @@ mod test { #[test] fn it_returns_same_id() { - let id = MenuId("1".into()); + let id = MenuId::new("1"); assert_eq!(id, MenuItem::with_id(id.clone(), "", true, None).id()); assert_eq!(id, Submenu::with_id(id.clone(), "", true).id()); assert_eq!( diff --git a/src/lib.rs b/src/lib.rs index 25f9b0a..45fd17f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,8 +126,6 @@ //! } //! ``` -use std::{convert::Infallible, str::FromStr}; - use crossbeam_channel::{unbounded, Receiver, Sender}; use once_cell::sync::{Lazy, OnceCell}; @@ -139,6 +137,7 @@ mod error; mod icon; mod items; mod menu; +mod menu_id; mod platform_impl; mod util; @@ -153,54 +152,7 @@ pub use error::*; pub use icon::{BadIcon, Icon, NativeIcon}; pub use items::*; pub use menu::Menu; - -/// An unique id that is associated with a menu item. -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Default, Hash)] -pub struct MenuId(pub String); - -impl AsRef for MenuId { - fn as_ref(&self) -> &str { - self.0.as_ref() - } -} - -impl From for MenuId { - fn from(value: String) -> Self { - Self(value) - } -} - -impl From<&str> for MenuId { - fn from(value: &str) -> Self { - Self(value.to_string()) - } -} - -impl PartialEq<&str> for MenuId { - fn eq(&self, other: &&str) -> bool { - other == &self.0 - } -} - -impl PartialEq for MenuId { - fn eq(&self, other: &String) -> bool { - other == &self.0 - } -} - -impl PartialEq<&MenuId> for MenuId { - fn eq(&self, other: &&MenuId) -> bool { - other.0 == self.0 - } -} - -impl FromStr for MenuId { - type Err = Infallible; - - fn from_str(s: &str) -> std::result::Result { - Ok(Self(s.to_string())) - } -} +pub use menu_id::MenuId; /// An enumeration of all available menu types, useful to match against /// the items returned from [`Menu::items`] or [`Submenu::items`] diff --git a/src/menu_id.rs b/src/menu_id.rs new file mode 100644 index 0000000..08288e6 --- /dev/null +++ b/src/menu_id.rs @@ -0,0 +1,62 @@ +use std::{convert::Infallible, str::FromStr}; + +/// An unique id that is associated with a menu item. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Default, Hash)] +pub struct MenuId(pub String); + +impl MenuId { + /// Create a new menu id. + pub fn new>(id: S) -> Self { + Self(id.as_ref().to_string()) + } +} + +impl AsRef for MenuId { + fn as_ref(&self) -> &str { + self.0.as_ref() + } +} + +impl From for MenuId { + fn from(value: String) -> Self { + Self::new(value) + } +} + +impl From<&str> for MenuId { + fn from(value: &str) -> Self { + Self::new(value) + } +} + +impl FromStr for MenuId { + type Err = Infallible; + + fn from_str(s: &str) -> std::result::Result { + Ok(Self::new(s)) + } +} + +impl PartialEq<&str> for MenuId { + fn eq(&self, other: &&str) -> bool { + other == &self.0 + } +} + +impl PartialEq for MenuId { + fn eq(&self, other: &String) -> bool { + other == &self.0 + } +} + +impl PartialEq<&String> for MenuId { + fn eq(&self, other: &&String) -> bool { + other == &&self.0 + } +} + +impl PartialEq<&MenuId> for MenuId { + fn eq(&self, other: &&MenuId) -> bool { + other.0 == self.0 + } +} diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index c3fbcb8..32c5970 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -723,7 +723,7 @@ impl MenuChild { } self.ns_menu_items - .entry(menu_id.clone()) + .entry(menu_id) .or_insert_with(Vec::new) .push(ns_menu_item); @@ -762,7 +762,7 @@ impl MenuChild { } self.ns_menu_items - .entry(menu_id.clone()) + .entry(menu_id) .or_insert_with(Vec::new) .push(ns_menu_item); @@ -792,7 +792,7 @@ impl MenuChild { } self.ns_menu_items - .entry(menu_id.clone()) + .entry(menu_id) .or_insert_with(Vec::new) .push(ns_menu_item); @@ -825,7 +825,7 @@ impl MenuChild { } self.ns_menu_items - .entry(menu_id.clone()) + .entry(menu_id) .or_insert_with(Vec::new) .push(ns_menu_item); diff --git a/src/platform_impl/windows/mod.rs b/src/platform_impl/windows/mod.rs index e26dd8e..4437bbe 100644 --- a/src/platform_impl/windows/mod.rs +++ b/src/platform_impl/windows/mod.rs @@ -43,23 +43,21 @@ use windows_sys::Win32::{ static COUNTER: Counter = Counter::new_with_start(1000); -type AccelWrapper = (HACCEL, HashMap); - macro_rules! inner_menu_child_and_flags { ($item:ident) => {{ let mut flags = 0; let child = match $item.kind() { MenuItemKind::Submenu(i) => { flags |= MF_POPUP; - i.inner.clone() + i.inner } MenuItemKind::MenuItem(i) => { flags |= MF_STRING; - i.inner.clone() + i.inner } MenuItemKind::Predefined(i) => { - let child = i.inner.clone(); + let child = i.inner; let child_ = child.borrow(); match child_.predefined_item_type.as_ref().unwrap() { PredefinedMenuItemType::None => return Ok(()), @@ -74,7 +72,7 @@ macro_rules! inner_menu_child_and_flags { child } MenuItemKind::Check(i) => { - let child = i.inner.clone(); + let child = i.inner; flags |= MF_STRING; if child.borrow().checked { flags |= MF_CHECKED; @@ -83,7 +81,7 @@ macro_rules! inner_menu_child_and_flags { } MenuItemKind::Icon(i) => { flags |= MF_STRING; - i.inner.clone() + i.inner } }; @@ -91,9 +89,12 @@ macro_rules! inner_menu_child_and_flags { }}; } +type AccelWrapper = (HACCEL, HashMap); + #[derive(Debug)] pub(crate) struct Menu { id: MenuId, + internal_id: u32, hmenu: HMENU, hpopupmenu: HMENU, hwnds: Vec, @@ -107,17 +108,17 @@ impl Drop for Menu { let _ = self.remove_for_hwnd(hwnd); } - fn remove_from_children_stores(id: &MenuId, children: &Vec>>) { + fn remove_from_children_stores(internal_id: u32, children: &Vec>>) { for child in children { let mut child_ = child.borrow_mut(); - child_.root_menu_haccel_stores.remove(id); + child_.root_menu_haccel_stores.remove(&internal_id); if child_.item_type == MenuItemType::Submenu { - remove_from_children_stores(id, child_.children.as_ref().unwrap()); + remove_from_children_stores(internal_id, child_.children.as_ref().unwrap()); } } } - remove_from_children_stores(&self.id, &self.children); + remove_from_children_stores(self.internal_id, &self.children); for child in &self.children { let child_ = child.borrow(); @@ -141,8 +142,10 @@ impl Drop for Menu { impl Menu { pub fn new(id: Option) -> Self { + let internal_id = COUNTER.next(); Self { - id: id.unwrap_or_else(|| MenuId(COUNTER.next().to_string())), + id: id.unwrap_or_else(|| MenuId::new(internal_id.to_string())), + internal_id, hmenu: unsafe { CreateMenu() }, hpopupmenu: unsafe { CreatePopupMenu() }, haccel_store: Rc::new(RefCell::new((0, HashMap::new()))), @@ -162,7 +165,7 @@ impl Menu { child .borrow_mut() .root_menu_haccel_stores - .insert(self.id.clone(), self.haccel_store.clone()); + .insert(self.internal_id, self.haccel_store.clone()); } { @@ -407,7 +410,7 @@ pub(crate) struct MenuChild { text: String, enabled: bool, parents_hemnu: Vec, - root_menu_haccel_stores: HashMap>>, + root_menu_haccel_stores: HashMap>>, // menu item fields internal_id: u32, @@ -461,7 +464,7 @@ impl MenuChild { enabled, parents_hemnu: Vec::new(), internal_id, - id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), + id: id.unwrap_or_else(|| MenuId::new(internal_id.to_string())), accelerator, root_menu_haccel_stores: HashMap::new(), predefined_item_type: None, @@ -483,7 +486,7 @@ impl MenuChild { children: Some(Vec::new()), hmenu: unsafe { CreateMenu() }, internal_id, - id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), + id: id.unwrap_or_else(|| MenuId::new(internal_id.to_string())), hpopupmenu: unsafe { CreatePopupMenu() }, root_menu_haccel_stores: HashMap::new(), predefined_item_type: None, @@ -501,7 +504,7 @@ impl MenuChild { enabled: true, parents_hemnu: Vec::new(), internal_id, - id: MenuId(internal_id.to_string()), + id: MenuId::new(internal_id.to_string()), accelerator: item_type.accelerator(), predefined_item_type: Some(item_type), root_menu_haccel_stores: HashMap::new(), @@ -527,7 +530,7 @@ impl MenuChild { enabled, parents_hemnu: Vec::new(), internal_id, - id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), + id: id.unwrap_or_else(|| MenuId::new(internal_id.to_string())), accelerator, checked, root_menu_haccel_stores: HashMap::new(), @@ -553,7 +556,7 @@ impl MenuChild { enabled, parents_hemnu: Vec::new(), internal_id, - id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), + id: id.unwrap_or_else(|| MenuId::new(internal_id.to_string())), accelerator, icon, root_menu_haccel_stores: HashMap::new(), @@ -579,7 +582,7 @@ impl MenuChild { enabled, parents_hemnu: Vec::new(), internal_id, - id: id.unwrap_or_else(|| MenuId(internal_id.to_string())), + id: id.unwrap_or_else(|| MenuId::new(internal_id.to_string())), accelerator, root_menu_haccel_stores: HashMap::new(), predefined_item_type: None,