feat(config): refactor id field and introduce deduplication on legacy loader

espanso-config/src/legacy/mod.rs

@@ -22,8 +22,8 @@ use regex::Regex;
 use std::{collections::HashMap, path::Path};
 
 use self::config::LegacyConfig;
-use crate::config::store::DefaultConfigStore;
 use crate::matches::group::loader::yaml::parse::{YAMLMatch, YAMLVariable};
+use crate::{config::store::DefaultConfigStore, counter::StructId};
 use crate::{
   config::Config,
   config::{AppProperties, ConfigStore},
@@ -43,13 +43,28 @@ pub fn load(
 ) -> Result<(Box<dyn ConfigStore>, Box<dyn MatchStore>)> {
   let config_set = config::LegacyConfigSet::load(base_dir, package_dir)?;
 
-  let (default_config, default_match_group) = split_config(config_set.default);
+  let mut match_deduplicate_map = HashMap::new();
+  let mut var_deduplicate_map = HashMap::new();
+
+  let (default_config, mut default_match_group) = split_config(config_set.default);
+  deduplicate_ids(
+    &mut default_match_group,
+    &mut match_deduplicate_map,
+    &mut var_deduplicate_map,
+  );
+
   let mut match_groups = HashMap::new();
   match_groups.insert("default".to_string(), default_match_group);
 
   let mut custom_configs: Vec<Box<dyn Config>> = Vec::new();
   for custom in config_set.specific {
-    let (custom_config, custom_match_group) = split_config(custom);
+    let (custom_config, mut custom_match_group) = split_config(custom);
+    deduplicate_ids(
+      &mut custom_match_group,
+      &mut match_deduplicate_map,
+      &mut var_deduplicate_map,
+    );
+
     match_groups.insert(custom_config.name.clone(), custom_match_group);
     custom_configs.push(Box::new(custom_config));
   }
@@ -90,6 +105,34 @@ fn split_config(config: LegacyConfig) -> (LegacyInteropConfig, LegacyMatchGroup)
   (config, match_group)
 }
 
+/// Due to the way the legacy configs are loaded (matches are copied multiple times in the various configs)
+/// we need to deduplicate the ids of those matches (and global vars).
+fn deduplicate_ids(
+  match_group: &mut LegacyMatchGroup,
+  match_map: &mut HashMap<Match, StructId>,
+  var_map: &mut HashMap<Variable, StructId>,
+) {
+  for m in match_group.matches.iter_mut() {
+    let mut m_without_id = m.clone();
+    m_without_id.id = 0;
+    if let Some(id) = match_map.get(&m_without_id) {
+      m.id = *id;
+    } else {
+      match_map.insert(m_without_id, m.id);
+    }
+  }
+
+  for v in match_group.global_vars.iter_mut() {
+    let mut v_without_id = v.clone();
+    v_without_id.id = 0;
+    if let Some(id) = var_map.get(&v_without_id) {
+      v.id = *id;
+    } else {
+      var_map.insert(v_without_id, v.id);
+    }
+  }
+}
+
 struct LegacyInteropConfig {
   pub name: String,
   match_paths: Vec<String>,
@@ -352,6 +395,75 @@ mod tests {
     });
   }
 
+  #[test]
+  fn load_legacy_deduplicates_ids_correctly() {
+    use_test_directory(|base, user, packages| {
+      std::fs::write(
+        base.join("default.yml"),
+        r#"
+      backend: Clipboard
+
+      global_vars:
+        - name: var1
+          type: test
+
+      matches:
+        - trigger: "hello"
+          replace: "world"
+      "#,
+      )
+      .unwrap();
+
+      std::fs::write(
+        user.join("specific.yml"),
+        r#"
+      name: specific
+      filter_title: "Google"
+      "#,
+      )
+      .unwrap();
+
+      let (config_store, match_store) = load(base, packages).unwrap();
+
+      let default_config = config_store.default();
+      let active_config = config_store.active(&AppProperties {
+        title: Some("Google"),
+        class: None,
+        exec: None,
+      });
+
+      assert_eq!(
+        match_store
+          .query(default_config.match_paths())
+          .matches
+          .first()
+          .unwrap()
+          .id,
+        match_store
+          .query(active_config.match_paths())
+          .matches
+          .first()
+          .unwrap()
+          .id,
+      );
+
+      assert_eq!(
+        match_store
+          .query(default_config.match_paths())
+          .global_vars
+          .first()
+          .unwrap()
+          .id,
+        match_store
+          .query(active_config.match_paths())
+          .global_vars
+          .first()
+          .unwrap()
+          .id,
+      );
+    });
+  }
+
   #[test]
   fn load_legacy_with_packages() {
     use_test_directory(|base, _, packages| {

espanso-config/src/matches/group/loader/yaml/mod.rs

@@ -17,10 +17,10 @@
  * along with espanso.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use crate::matches::{
+use crate::{counter::next_id, matches::{
   group::{path::resolve_imports, MatchGroup},
   Match, Variable,
-};
+}};
 use anyhow::Result;
 use log::warn;
 use parse::YAMLMatchGroup;
@@ -138,7 +138,7 @@ impl TryFrom<YAMLMatch> for Match {
       cause,
       effect,
       label: None,
-      ..Default::default()
+      id: next_id(),
     })
   }
 }
@@ -151,7 +151,7 @@ impl TryFrom<YAMLVariable> for Variable {
       name: yaml_var.name,
       var_type: yaml_var.var_type,
       params: yaml_var.params,
-      ..Default::default()
+      id: next_id(),
     })
   }
 }
@@ -165,7 +165,14 @@ mod tests {
 
   fn create_match(yaml: &str) -> Result<Match> {
     let yaml_match: YAMLMatch = serde_yaml::from_str(yaml)?;
-    let m: Match = yaml_match.try_into()?;
+    let mut m: Match = yaml_match.try_into()?;
+
+    // Reset the IDs to correctly compare them
+    m.id = 0;
+    if let MatchEffect::Text(e) = &mut m.effect {
+      e.vars.iter_mut().for_each(|v| v.id = 0);
+    }
+    
     Ok(m)
   }
 
@@ -429,7 +436,10 @@ mod tests {
       std::fs::write(&sub_file, "").unwrap();
 
       let importer = YAMLImporter::new();
-      let group = importer.load_group(&base_file).unwrap();
+      let mut group = importer.load_group(&base_file).unwrap();
+      // Reset the ids to compare them correctly
+      group.matches.iter_mut().for_each(|mut m| m.id = 0);
+      group.global_vars.iter_mut().for_each(|mut v| v.id = 0);
 
       let vars = vec![Variable {
         name: "var1".to_string(),

espanso-config/src/matches/mod.rs

@@ -19,21 +19,20 @@
 
 use serde_yaml::Mapping;
 
-use crate::counter::{next_id, StructId};
+use crate::counter::{StructId};
 
 pub(crate) mod group;
 pub mod store;
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Match {
-  cause: MatchCause,
+  pub id: StructId,
-  effect: MatchEffect,
+
+  pub cause: MatchCause,
+  pub effect: MatchEffect,
 
   // Metadata
-  label: Option<String>,
+  pub label: Option<String>,
-
-  // Internals
-  _id: StructId,
 }
 
 impl Default for Match {
@@ -42,20 +41,14 @@ impl Default for Match {
       cause: MatchCause::None,
       effect: MatchEffect::None,
       label: None,
-      _id: next_id(),
+      id: 0,
     }
   }
 }
 
-impl PartialEq for Match {
-  fn eq(&self, other: &Self) -> bool {
-    self.cause == other.cause && self.effect == other.effect && self.label == other.label
-  }
-}
-
 // Causes
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, Eq, Hash, PartialEq)]
 pub enum MatchCause {
   None,
   Trigger(TriggerCause),
@@ -63,7 +56,7 @@ pub enum MatchCause {
   // TODO: shortcut
 }
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct TriggerCause {
   pub triggers: Vec<String>,
 
@@ -86,7 +79,7 @@ impl Default for TriggerCause {
 
 // Effects
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum MatchEffect {
   None,
   Text(TextEffect),
@@ -94,7 +87,7 @@ pub enum MatchEffect {
   // TODO: rich text
 }
 
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct TextEffect {
   pub replace: String,
   pub vars: Vec<Variable>,
@@ -109,29 +102,21 @@ impl Default for TextEffect {
   }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Variable {
+  pub id: StructId,
   pub name: String,
   pub var_type: String,
   pub params: Mapping,
-
-  // Internals
-  _id: StructId,
 }
 
 impl Default for Variable {
   fn default() -> Self {
     Self {
+      id: 0,
       name: String::new(),
       var_type: String::new(),
       params: Mapping::new(),
-      _id: next_id(),
     }
   }
 }
-
-impl PartialEq for Variable {
-  fn eq(&self, other: &Self) -> bool {
-    self.name == other.name && self.var_type == other.var_type && self.params == other.params
-  }
-}

espanso-config/src/matches/store/default.rs

@@ -42,9 +42,7 @@ impl DefaultMatchStore {
     // top-level ones.
     load_match_groups_recursively(&mut groups, paths);
 
-    Self {
+    Self { groups }
-      groups,
-    }
   }
 }
 
@@ -117,16 +115,16 @@ fn query_matches_for_paths<'a>(
         );
 
         for m in group.matches.iter() {
-          if !visited_matches.contains(&m._id) {
+          if !visited_matches.contains(&m.id) {
             matches.push(m);
-            visited_matches.insert(m._id);
+            visited_matches.insert(m.id);
           }
         }
 
         for var in group.global_vars.iter() {
-          if !visited_global_vars.contains(&var._id) {
+          if !visited_global_vars.contains(&var.id) {
             global_vars.push(var);
-            visited_global_vars.insert(var._id);
+            visited_global_vars.insert(var.id);
           }
         }
       }
@@ -232,16 +230,27 @@ mod tests {
         .get(&base_file.to_string_lossy().to_string())
         .unwrap()
         .matches;
-      assert_eq!(base_group, &create_matches(&[("hello", "world")]));
+      let base_group: Vec<Match> = base_group.iter().map(|m| {
+        let mut copy = m.clone();
+        copy.id = 0;
+        copy
+      }).collect();
+
+      assert_eq!(base_group, create_matches(&[("hello", "world")]));
 
       let another_group = &match_store
         .groups
         .get(&another_file.to_string_lossy().to_string())
         .unwrap()
         .matches;
+      let another_group: Vec<Match> = another_group.iter().map(|m| {
+        let mut copy = m.clone();
+        copy.id = 0;
+        copy
+      }).collect();
       assert_eq!(
         another_group,
-        &create_matches(&[("hello", "world2"), ("foo", "bar")])
+        create_matches(&[("hello", "world2"), ("foo", "bar")])
       );
 
       let sub_group = &match_store
@@ -249,7 +258,12 @@ mod tests {
         .get(&sub_file.to_string_lossy().to_string())
         .unwrap()
         .matches;
-      assert_eq!(sub_group, &create_matches(&[("hello", "world3")]));
+      let sub_group: Vec<Match> = sub_group.iter().map(|m| {
+        let mut copy = m.clone();
+        copy.id = 0;
+        copy
+      }).collect();
+      assert_eq!(sub_group, create_matches(&[("hello", "world3")]));
     });
   }
 
@@ -373,6 +387,7 @@ mod tests {
           .matches
           .into_iter()
           .cloned()
+          .map(|mut m| { m.id = 0; m })
           .collect::<Vec<Match>>(),
         create_matches(&[
           ("hello", "world3"),
@@ -387,6 +402,7 @@ mod tests {
           .global_vars
           .into_iter()
           .cloned()
+          .map(|mut v| { v.id = 0; v })
           .collect::<Vec<Variable>>(),
         create_vars(&["var2", "var1"])
       );
@@ -460,6 +476,7 @@ mod tests {
           .matches
           .into_iter()
           .cloned()
+          .map(|mut m| { m.id = 0; m })
           .collect::<Vec<Match>>(),
         create_matches(&[
           ("hello", "world3"),
@@ -474,6 +491,7 @@ mod tests {
           .global_vars
           .into_iter()
           .cloned()
+          .map(|mut v| { v.id = 0; v})
           .collect::<Vec<Variable>>(),
         create_vars(&["var2", "var1"])
       );
@@ -532,7 +550,10 @@ mod tests {
       )
       .unwrap();
 
-      let match_store = DefaultMatchStore::new(&[base_file.to_string_lossy().to_string(), sub_file.to_string_lossy().to_string()]);
+      let match_store = DefaultMatchStore::new(&[
+        base_file.to_string_lossy().to_string(),
+        sub_file.to_string_lossy().to_string(),
+      ]);
 
       let match_set = match_store.query(&[
         base_file.to_string_lossy().to_string(),
@@ -544,6 +565,7 @@ mod tests {
           .matches
           .into_iter()
           .cloned()
+          .map(|mut m| { m.id = 0; m })
           .collect::<Vec<Match>>(),
         create_matches(&[
           ("hello", "world2"),
@@ -558,6 +580,7 @@ mod tests {
           .global_vars
           .into_iter()
           .cloned()
+          .map(|mut v| { v.id = 0; v })
           .collect::<Vec<Variable>>(),
         create_vars(&["var1", "var2"])
       );
@@ -631,6 +654,7 @@ mod tests {
           .matches
           .into_iter()
           .cloned()
+          .map(|mut m| { m.id = 0; m })
           .collect::<Vec<Match>>(),
         create_matches(&[
           ("hello", "world3"), // This appears only once, though it appears 2 times
@@ -645,6 +669,7 @@ mod tests {
           .global_vars
           .into_iter()
           .cloned()
+          .map(|mut v| { v.id = 0; v })
           .collect::<Vec<Variable>>(),
         create_vars(&["var2", "var1"])
       );