feat(ui): refactor context menu structure

espanso-ui/src/mac/AppDelegate.mm

@@ -100,7 +100,7 @@ void addSeparatorMenu(NSMenu * parent)
 void addSingleMenu(NSMenu * parent, id item)
 {
   id label = [item objectForKey:@"label"];
-  id raw_id = [item objectForKey:@"raw_id"];
+  id raw_id = [item objectForKey:@"id"];
   if (label == nil || raw_id == nil)
   {
     return;

espanso-ui/src/menu.rs

@@ -1,75 +1,34 @@
-use std::collections::{HashMap, HashSet};
+/*
+ * This file is part of espanso.
+ *
+ * Copyright (C) 2019-2021 Federico Terzi
+ *
+ * espanso is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * espanso is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with espanso.  If not, see <https://www.gnu.org/licenses/>.
+ */
 
 use anyhow::Result;
 use serde::{Deserialize, Serialize};
-use thiserror::Error;
 
 #[derive(Debug)]
 pub struct Menu {
-  items: Vec<MenuItem>,
+  pub items: Vec<MenuItem>,
-
-  // Mapping between the raw numeric ids and string ids
-  raw_ids: HashMap<u32, String>,
 }
 
 impl Menu {
-  pub fn from(mut items: Vec<MenuItem>) -> Result<Self> {
-    // Generate the raw ids, also checking for duplicate ids
-    let mut raw_ids: HashMap<u32, String> = HashMap::new();
-    let mut ids: HashSet<String> = HashSet::new();
-    let mut current = 0;
-    for item in items.iter_mut() {
-      Self::generate_raw_id(&mut raw_ids, &mut ids, &mut current, item)?;
-    }
-
-    Ok(Self { items, raw_ids })
-  }
-
   pub fn to_json(&self) -> Result<String> {
     Ok(serde_json::to_string(&self.items)?)
   }
-
-  pub fn get_item_id(&self, raw_id: u32) -> Option<String> {
-    self.raw_ids.get(&raw_id).cloned()
-  }
-
-  fn generate_raw_id(
-    raw_ids: &mut HashMap<u32, String>,
-    ids: &mut HashSet<String>,
-    current: &mut u32,
-    item: &mut MenuItem,
-  ) -> Result<()> {
-    match item {
-      MenuItem::Simple(simple_item) => {
-        if ids.contains::<str>(&simple_item.id) {
-          // Duplicate id, throw error
-          Err(MenuError::DuplicateMenuId(simple_item.id.to_string()).into())
-        } else {
-          ids.insert(simple_item.id.to_string());
-          raw_ids.insert(*current, simple_item.id.to_string());
-          simple_item.raw_id = Some(*current);
-          *current += 1;
-          Ok(())
-        }
-      }
-      MenuItem::Sub(SubMenuItem { items, .. }) => {
-        for sub_item in items.iter_mut() {
-          Self::generate_raw_id(raw_ids, ids, current, sub_item)?
-        }
-        Ok(())
-      }
-      MenuItem::Separator => Ok(()),
-    }
-  }
-}
-
-#[derive(Error, Debug)]
-pub enum MenuError {
-  #[error("json serialization error")]
-  Serialization(#[from] serde_json::Error),
-
-  #[error("two or more menu items share the same id")]
-  DuplicateMenuId(String),
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -83,34 +42,14 @@ pub enum MenuItem {
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct SimpleMenuItem {
-  id: String,
+  pub id: u32,
-  label: String,
+  pub label: String,
-  raw_id: Option<u32>,
-}
-
-impl SimpleMenuItem {
-  pub fn new(id: &str, label: &str) -> Self {
-    Self {
-      id: id.to_string(),
-      label: label.to_string(),
-      raw_id: None,
-    }
-  }
 }
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct SubMenuItem {
-  label: String,
+  pub label: String,
-  items: Vec<MenuItem>,
+  pub items: Vec<MenuItem>,
-}
-
-impl SubMenuItem {
-  pub fn new(label: &str, items: Vec<MenuItem>) -> Self {
-    Self {
-      label: label.to_string(),
-      items,
-    }
-  }
 }
 
 #[cfg(test)]
@@ -119,59 +58,30 @@ mod tests {
 
   #[test]
   fn test_context_menu_serializes_correctly() {
-    let menu = Menu::from(vec![
+    let menu = Menu { items: vec![
-      MenuItem::Simple(SimpleMenuItem::new("open", "Open")),
+      MenuItem::Simple(SimpleMenuItem {
+        id: 0,
+        label: "Open".to_string()
+      }),
       MenuItem::Separator,
-      MenuItem::Sub(SubMenuItem::new(
+      MenuItem::Sub(SubMenuItem {
-        "Sub",
+        label: "Sub".to_string(),
-        vec![
+        items: vec![
-          MenuItem::Simple(SimpleMenuItem::new("sub1", "Sub 1")),
+          MenuItem::Simple(SimpleMenuItem {
-          MenuItem::Simple(SimpleMenuItem::new("sub2", "Sub 2")),
+            label: "Sub 1".to_string(),
+            id: 1,
+          }),
+          MenuItem::Simple(SimpleMenuItem {
+            label: "Sub 2".to_string(),
+            id: 2,
+          }),
         ],
-      )),
+      }),
-    ])
+    ]};
-    .unwrap();
+
     assert_eq!(
       menu.to_json().unwrap(),
-      r#"[{"type":"simple","id":"open","label":"Open","raw_id":0},{"type":"separator"},{"type":"sub","label":"Sub","items":[{"type":"simple","id":"sub1","label":"Sub 1","raw_id":1},{"type":"simple","id":"sub2","label":"Sub 2","raw_id":2}]}]"#
+      r#"[{"type":"simple","id":0,"label":"Open"},{"type":"separator"},{"type":"sub","label":"Sub","items":[{"type":"simple","id":1,"label":"Sub 1"},{"type":"simple","id":2,"label":"Sub 2"}]}]"#
-    );
-  }
-
-  #[test]
-  fn test_context_menu_raw_ids_work_correctly() {
-    let menu = Menu::from(vec![
-      MenuItem::Simple(SimpleMenuItem::new("open", "Open")),
-      MenuItem::Separator,
-      MenuItem::Sub(SubMenuItem::new(
-        "Sub",
-        vec![
-          MenuItem::Simple(SimpleMenuItem::new("sub1", "Sub 1")),
-          MenuItem::Simple(SimpleMenuItem::new("sub2", "Sub 2")),
-        ],
-      )),
-    ])
-    .unwrap();
-    assert_eq!(menu.get_item_id(0).unwrap(), "open");
-    assert_eq!(menu.get_item_id(1).unwrap(), "sub1");
-    assert_eq!(menu.get_item_id(2).unwrap(), "sub2");
-  }
-
-  #[test]
-  fn test_context_menu_detects_duplicate_ids() {
-    let menu = Menu::from(vec![
-      MenuItem::Simple(SimpleMenuItem::new("open", "Open")),
-      MenuItem::Separator,
-      MenuItem::Sub(SubMenuItem::new(
-        "Sub",
-        vec![
-          MenuItem::Simple(SimpleMenuItem::new("open", "Sub 1")),
-          MenuItem::Simple(SimpleMenuItem::new("sub2", "Sub 2")),
-        ],
-      )),
-    ]);
-    assert!(
-      matches!(menu.unwrap_err().downcast::<MenuError>().unwrap(), 
-            MenuError::DuplicateMenuId(id) if id == "open")
     );
   }
 }

espanso-ui/src/win32/native.cpp

@@ -358,12 +358,12 @@ void _insert_separator_menu(HMENU parent)
 
 void _insert_single_menu(HMENU parent, json item)
 {
-  if (!item["label"].is_string() || !item["raw_id"].is_number())
+  if (!item["label"].is_string() || !item["id"].is_number())
   {
     return;
   }
   std::string label = item["label"];
-  uint32_t raw_id = item["raw_id"];
+  uint32_t raw_id = item["id"];
 
   // Convert to wide chars
   std::wstring wide_label(label.length(), L'#');