Merge pull request #145 from k0kubun/modifier-lookup-v2

Make modifier lookup more efficient and flexible

src/config/application.rs

@@ -5,7 +5,7 @@ use regex::Regex;
 use serde::{Deserialize, Deserializer};
 
 // TODO: Use trait to allow only either `only` or `not`
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct Application {
     #[serde(default, deserialize_with = "deserialize_matchers")]
@@ -14,7 +14,7 @@ pub struct Application {
     pub not: Option<Vec<ApplicationMatcher>>,
 }
 
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub enum ApplicationMatcher {
     // class.name
     Literal(String),

src/config/key.rs

@@ -35,9 +35,13 @@ pub fn parse_key(input: &str) -> Result<Key, Box<dyn Error>> {
         "CONTROL_L" => Key::KEY_LEFTCTRL,
         "CTRL_R" => Key::KEY_RIGHTCTRL,
         "CTRL_L" => Key::KEY_LEFTCTRL,
+        "C_R" => Key::KEY_RIGHTCTRL,
+        "C_L" => Key::KEY_LEFTCTRL,
         // Alt
         "ALT_R" => Key::KEY_RIGHTALT,
         "ALT_L" => Key::KEY_LEFTALT,
+        "M_R" => Key::KEY_RIGHTALT,
+        "M_L" => Key::KEY_LEFTALT,
         // Windows
         "SUPER_R" => Key::KEY_RIGHTMETA,
         "SUPER_L" => Key::KEY_LEFTMETA,

src/config/key_press.rs

@@ -1,30 +1,23 @@
 use crate::config::key::parse_key;
 use evdev::Key;
 use serde::{Deserialize, Deserializer};
-use std::error;
+use std::error::{self, Error};
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct KeyPress {
     pub key: Key,
-    pub shift: ModifierState,
+    pub modifiers: Vec<Modifier>,
-    pub control: ModifierState,
-    pub alt: ModifierState,
-    pub windows: ModifierState,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
-pub enum ModifierState {
-    Left,
-    Right,
-    Either,
-    None,
-}
-
 pub enum Modifier {
+    // Matches left, right, or both
     Shift,
     Control,
     Alt,
     Windows,
+    // Matches exactly this key
+    Key(Key),
 }
 
 impl<'de> Deserialize<'de> for KeyPress {
@@ -39,69 +32,41 @@ impl<'de> Deserialize<'de> for KeyPress {
 
 fn parse_key_press(input: &str) -> Result<KeyPress, Box<dyn error::Error>> {
     let keys: Vec<&str> = input.split('-').collect();
-    if let Some((key, modifiers)) = keys.split_last() {
+    if let Some((key, modifier_keys)) = keys.split_last() {
-        let mut shift = ModifierState::None;
+        let mut modifiers = vec![];
-        let mut control = ModifierState::None;
+        for modifier_key in modifier_keys.iter() {
-        let mut alt = ModifierState::None;
+            match parse_modifier(modifier_key) {
-        let mut windows = ModifierState::None;
+                Ok(modifier) => modifiers.push(modifier),
-
+                Err(e) => return Err(e),
-        for modifier in modifiers.iter() {
-            match parse_modifier(modifier) {
-                Some((Modifier::Shift, state)) => shift = state,
-                Some((Modifier::Control, state)) => control = state,
-                Some((Modifier::Alt, state)) => alt = state,
-                Some((Modifier::Windows, state)) => windows = state,
-                None => return Err(format!("unknown modifier: {}", modifier).into()),
             }
         }
 
-        // TODO: invalidate modifier keys in `key`?
         Ok(KeyPress {
             key: parse_key(key)?,
-            shift,
+            modifiers,
-            control,
-            alt,
-            windows,
         })
     } else {
         Err(format!("empty key_press: {}", input).into())
     }
 }
 
-fn parse_modifier(modifier: &str) -> Option<(Modifier, ModifierState)> {
+fn parse_modifier(modifier: &str) -> Result<Modifier, Box<dyn Error>> {
     // Everything is case-insensitive
-    let mut modifier = &modifier.to_uppercase()[..];
+    match &modifier.to_uppercase()[..] {
-    let mut modifier_state = ModifierState::Either;
-    if modifier.ends_with("_L") {
-        modifier = remove_suffix(modifier);
-        modifier_state = ModifierState::Left;
-    } else if modifier.ends_with("_R") {
-        modifier = remove_suffix(modifier);
-        modifier_state = ModifierState::Right;
-    }
-
-    match modifier {
         // Shift
-        "SHIFT" => Some((Modifier::Shift, modifier_state)),
+        "SHIFT" => Ok(Modifier::Shift),
         // Control
-        "C" => Some((Modifier::Control, modifier_state)),
+        "C" => Ok(Modifier::Control),
-        "CTRL" => Some((Modifier::Control, modifier_state)),
+        "CTRL" => Ok(Modifier::Control),
-        "CONTROL" => Some((Modifier::Control, modifier_state)),
+        "CONTROL" => Ok(Modifier::Control),
         // Alt
-        "M" => Some((Modifier::Alt, modifier_state)),
+        "M" => Ok(Modifier::Alt),
-        "ALT" => Some((Modifier::Alt, modifier_state)),
+        "ALT" => Ok(Modifier::Alt),
         // Windows
-        "SUPER" => Some((Modifier::Windows, modifier_state)),
+        "SUPER" => Ok(Modifier::Windows),
-        "WIN" => Some((Modifier::Windows, modifier_state)),
+        "WIN" => Ok(Modifier::Windows),
-        "WINDOWS" => Some((Modifier::Windows, modifier_state)),
+        "WINDOWS" => Ok(Modifier::Windows),
         // else
-        _ => None,
+        key => parse_key(key).map(|key| Modifier::Key(key)),
     }
 }
-
-fn remove_suffix(string: &str) -> &str {
-    let mut chars = string.chars();
-    chars.next_back();
-    chars.next_back();
-    chars.as_str()
-}

src/config/keymap.rs

@@ -1,10 +1,14 @@
 use crate::config::action::{Action, Actions};
 use crate::config::application::deserialize_string_or_vec;
 use crate::config::application::Application;
-use crate::config::key_press::{KeyPress, Modifier, ModifierState};
+use crate::config::key_press::KeyPress;
+use evdev::Key;
 use serde::{Deserialize, Deserializer};
 use std::collections::HashMap;
 
+use super::key_press::Modifier;
+
+// Config interface
 #[derive(Debug, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct Keymap {
@@ -24,59 +28,65 @@ where
     let remap = HashMap::<KeyPress, Actions>::deserialize(deserializer)?;
     Ok(remap
         .into_iter()
-        .flat_map(|(key_press, actions)| {
+        .map(|(key_press, actions)| (key_press, actions.into_vec()))
-            expand_modifiers(key_press)
-                .into_iter()
-                .map(|k| (k, actions.clone().into_vec()))
-                .collect::<Vec<(KeyPress, Vec<Action>)>>()
-        })
         .collect())
 }
 
-// Expand ModifierState::Either to Left and Right. Not leaving Either to save some space and computation.
+// Internals for efficient keymap lookup
-// Note that we currently don't have `Both`. Does anybody need it?
+#[derive(Clone, Debug)]
-pub fn expand_modifiers(key_press: KeyPress) -> Vec<KeyPress> {
+pub struct KeymapEntry {
-    if key_press.shift == ModifierState::Either {
+    pub actions: Vec<Action>,
-        expand_modifier(key_press, &Modifier::Shift)
+    pub modifiers: Vec<Modifier>,
-    } else if key_press.control == ModifierState::Either {
+    pub application: Option<Application>,
-        expand_modifier(key_press, &Modifier::Control)
+    pub mode: Option<Vec<String>>,
-    } else if key_press.alt == ModifierState::Either {
-        expand_modifier(key_press, &Modifier::Alt)
-    } else if key_press.windows == ModifierState::Either {
-        expand_modifier(key_press, &Modifier::Windows)
-    } else {
-        vec![key_press]
-    }
 }
 
-fn expand_modifier(key_press: KeyPress, modifier: &Modifier) -> Vec<KeyPress> {
+// Convert an array of keymaps to a single hashmap whose key is a triggering key.
-    vec![
+//
-        change_modifier(key_press.clone(), modifier, ModifierState::Left),
+// For each key, Vec<KeymapEntry> is scanned once, matching the exact modifiers,
-        change_modifier(key_press, modifier, ModifierState::Right),
+// and then it's scanned again, allowing extra modifiers.
-    ]
+//
-    .into_iter()
+// First matching KeymapEntry wins at each iteration.
-    .flat_map(expand_modifiers)
+pub fn build_keymap_table(keymaps: &Vec<Keymap>) -> HashMap<Key, Vec<KeymapEntry>> {
-    .collect()
+    let mut table: HashMap<Key, Vec<KeymapEntry>> = HashMap::new();
+    for keymap in keymaps {
+        for (key_press, actions) in keymap.remap.iter() {
+            let mut entries: Vec<KeymapEntry> = match table.get(&key_press.key) {
+                Some(entries) => entries.to_vec(),
+                None => vec![],
+            };
+            entries.push(KeymapEntry {
+                actions: actions.to_vec(),
+                modifiers: key_press.modifiers.clone(),
+                application: keymap.application.clone(),
+                mode: keymap.mode.clone(),
+            });
+            table.insert(key_press.key, entries);
+        }
+    }
+    return table;
 }
 
-fn change_modifier(key_press: KeyPress, modifier: &Modifier, state: ModifierState) -> KeyPress {
+// Subset of KeymapEntry for override_remap
-    let mut shift = key_press.shift.clone();
+#[derive(Clone)]
-    let mut control = key_press.control.clone();
+pub struct OverrideEntry {
-    let mut alt = key_press.alt.clone();
+    pub actions: Vec<Action>,
-    let mut windows = key_press.windows.clone();
+    pub modifiers: Vec<Modifier>,
-
+}
-    match modifier {
-        Modifier::Shift => shift = state,
-        Modifier::Control => control = state,
-        Modifier::Alt => alt = state,
-        Modifier::Windows => windows = state,
-    }
 
-    KeyPress {
+// This is executed on runtime unlike build_keymap_table, but hopefully not called so often.
-        key: key_press.key,
+pub fn build_override_table(remap: &HashMap<KeyPress, Vec<Action>>) -> HashMap<Key, Vec<OverrideEntry>> {
-        shift,
+    let mut table: HashMap<Key, Vec<OverrideEntry>> = HashMap::new();
-        control,
+    for (key_press, actions) in remap.iter() {
-        alt,
+        let mut entries: Vec<OverrideEntry> = match table.get(&key_press.key) {
-        windows,
+            Some(entries) => entries.to_vec(),
+            None => vec![],
+        };
+        entries.push(OverrideEntry {
+            actions: actions.to_vec(),
+            modifiers: key_press.modifiers.clone(),
+        });
+        table.insert(key_press.key, entries);
     }
+    return table;
 }

src/config/mod.rs

@@ -12,29 +12,43 @@ mod tests;
 
 extern crate serde_yaml;
 
+use evdev::Key;
 use keymap::Keymap;
 use modmap::Modmap;
 use nix::sys::inotify::{AddWatchFlags, InitFlags, Inotify};
 use serde::Deserialize;
-use std::{error, fs, path::Path, time::SystemTime};
+use std::{collections::HashMap, error, fs, path::Path, time::SystemTime};
+
+use self::keymap::{build_keymap_table, KeymapEntry};
 
 #[derive(Debug, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct Config {
+    // Config interface
     #[serde(default = "Vec::new")]
     pub modmap: Vec<Modmap>,
     #[serde(default = "Vec::new")]
     pub keymap: Vec<Keymap>,
     #[serde(default = "default_mode")]
     pub default_mode: String,
+
+    // Internals
     #[serde(skip)]
     pub modify_time: Option<SystemTime>,
+    #[serde(skip)]
+    pub keymap_table: HashMap<Key, Vec<KeymapEntry>>,
 }
 
 pub fn load_config(filename: &Path) -> Result<Config, Box<dyn error::Error>> {
     let yaml = fs::read_to_string(&filename)?;
     let mut config: Config = serde_yaml::from_str(&yaml)?;
+
+    // Timestamp for --watch=config
     config.modify_time = filename.metadata()?.modified().ok();
+
+    // Convert keymap for efficient keymap lookup
+    config.keymap_table = build_keymap_table(&config.keymap);
+
     Ok(config)
 }
 

src/event_handler.rs

@@ -2,8 +2,8 @@ use crate::client::{build_client, WMClient};
 use crate::config::action::Action;
 use crate::config::application::Application;
 use crate::config::key_action::{KeyAction, MultiPurposeKey, PressReleaseKey};
-use crate::config::key_press::{KeyPress, Modifier, ModifierState};
+use crate::config::key_press::{KeyPress, Modifier};
-use crate::config::keymap::expand_modifiers;
+use crate::config::keymap::{build_override_table, OverrideEntry};
 use crate::config::remap::Remap;
 use crate::Config;
 use evdev::uinput::VirtualDevice;
@@ -14,18 +14,16 @@ use nix::sys::signal;
 use nix::sys::signal::{sigaction, SaFlags, SigAction, SigHandler, SigSet};
 use nix::sys::time::TimeSpec;
 use nix::sys::timerfd::{Expiration, TimerFd, TimerSetTimeFlags};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::error::Error;
 use std::process::{Command, Stdio};
 use std::time::Instant;
 
 pub struct EventHandler {
+    // Device to emit events
     device: VirtualDevice,
-    // Recognize modifier key combinations
+    // Currently pressed modifier keys
-    shift: PressState,
+    modifiers: HashSet<Key>,
-    control: PressState,
-    alt: PressState,
-    windows: PressState,
     // Make sure the original event is released even if remapping changes while holding the key
     pressed_keys: HashMap<Key, Key>,
     // Check the currently active application
@@ -34,7 +32,7 @@ pub struct EventHandler {
     // State machine for multi-purpose keys
     multi_purpose_keys: HashMap<Key, MultiPurposeKeyState>,
     // Current nested remaps
-    override_remap: Option<HashMap<KeyPress, Vec<Action>>>,
+    override_remap: Option<HashMap<Key, Vec<OverrideEntry>>>,
     // Key triggered on a timeout of nested remaps
     override_timeout_key: Option<Key>,
     // Trigger a timeout of nested remaps through select(2)
@@ -53,10 +51,7 @@ impl EventHandler {
     pub fn new(device: VirtualDevice, timer: TimerFd, mode: &str) -> EventHandler {
         EventHandler {
             device,
-            shift: PressState::new(),
+            modifiers: HashSet::new(),
-            control: PressState::new(),
-            alt: PressState::new(),
-            windows: PressState::new(),
             pressed_keys: HashMap::new(),
             application_client: build_client(),
             application_cache: None,
@@ -91,7 +86,7 @@ impl EventHandler {
         // Apply keymap
         for (key, value) in key_values.into_iter() {
             if MODIFIER_KEYS.contains(&key.code()) {
-                self.update_modifier(key.code(), value);
+                self.update_modifier(key, value);
             } else if is_pressed(value) {
                 if self.escape_next_key {
                     self.escape_next_key = false
@@ -127,6 +122,13 @@ impl EventHandler {
         Ok(())
     }
 
+    fn send_keys(&mut self, keys: &Vec<Key>, value: i32) -> std::io::Result<()> {
+        for key in keys {
+            self.send_key(key, value)?;
+        }
+        Ok(())
+    }
+
     fn send_key(&mut self, key: &Key, value: i32) -> std::io::Result<()> {
         let event = InputEvent::new(EventType::KEY, key.code(), value);
         self.send_event(event)
@@ -239,34 +241,36 @@ impl EventHandler {
     }
 
     fn find_keymap(&mut self, config: &Config, key: &Key) -> Result<Option<Vec<Action>>, Box<dyn Error>> {
-        let key_press = KeyPress {
-            key: *key,
-            shift: self.shift.to_modifier_state(),
-            control: self.control.to_modifier_state(),
-            alt: self.alt.to_modifier_state(),
-            windows: self.windows.to_modifier_state(),
-        };
         if let Some(override_remap) = &self.override_remap {
-            if let Some(actions) = override_remap.clone().get(&key_press) {
+            if let Some(entries) = override_remap.get(key) {
-                self.remove_override()?;
+                for exact_match in [true, false] {
-                return Ok(Some(actions.to_vec()));
+                    for entry in entries {
-            } else {
+                        if !self.match_modifiers(&entry.modifiers, exact_match) {
-                self.timeout_override()?;
+                            continue;
+                        }
+                        return Ok(Some(entry.actions.to_vec()));
+                    }
+                }
             }
         }
-        for keymap in &config.keymap {
+        if let Some(entries) = config.keymap_table.get(key) {
-            if let Some(actions) = keymap.remap.get(&key_press) {
+            for exact_match in [true, false] {
-                if let Some(application_matcher) = &keymap.application {
+                for entry in entries {
-                    if !self.match_application(application_matcher) {
+                    if !self.match_modifiers(&entry.modifiers, exact_match) {
                         continue;
                     }
-                }
+                    if let Some(application_matcher) = &entry.application {
-                if let Some(modes) = &keymap.mode {
+                        if !self.match_application(application_matcher) {
-                    if !modes.contains(&self.mode) {
+                            continue;
-                        continue;
+                        }
+                    }
+                    if let Some(modes) = &entry.mode {
+                        if !modes.contains(&self.mode) {
+                            continue;
+                        }
                     }
+                    return Ok(Some(entry.actions.clone()));
                 }
-                return Ok(Some(actions.to_vec()));
             }
         }
         Ok(None)
@@ -287,13 +291,7 @@ impl EventHandler {
                 timeout,
                 timeout_key,
             }) => {
-                let mut override_remap: HashMap<KeyPress, Vec<Action>> = HashMap::new();
+                self.override_remap = Some(build_override_table(remap));
-                for (key_press, actions) in remap.iter() {
-                    for key_press in expand_modifiers(key_press.clone()) {
-                        override_remap.insert(key_press, actions.to_vec());
-                    }
-                }
-                self.override_remap = Some(override_remap);
                 if let Some(timeout) = timeout {
                     let expiration = Expiration::OneShot(TimeSpec::from_duration(*timeout));
                     self.override_timer.unset()?;
@@ -314,121 +312,61 @@ impl EventHandler {
     }
 
     fn send_key_press(&mut self, key_press: &KeyPress) -> Result<(), Box<dyn Error>> {
-        let next_shift = self.build_state(Modifier::Shift, key_press.shift.clone());
+        // Build missing or extra modifiers
-        let next_control = self.build_state(Modifier::Control, key_press.control.clone());
+        let extra_modifiers: Vec<Key> = self
-        let next_alt = self.build_state(Modifier::Alt, key_press.alt.clone());
+            .modifiers
-        let next_windows = self.build_state(Modifier::Windows, key_press.windows.clone());
+            .iter()
+            .filter_map(|modifier| {
+                if self.contains_modifier(&key_press.modifiers, modifier) {
+                    None
+                } else {
+                    Some(modifier.clone())
+                }
+            })
+            .collect();
+        let missing_modifiers: Vec<Key> = key_press
+            .modifiers
+            .iter()
+            .filter_map(|modifier| {
+                if self.match_modifier(modifier) {
+                    None
+                } else {
+                    match modifier {
+                        Modifier::Shift => Some(Key::KEY_LEFTSHIFT),
+                        Modifier::Control => Some(Key::KEY_LEFTCTRL),
+                        Modifier::Alt => Some(Key::KEY_LEFTALT),
+                        Modifier::Windows => Some(Key::KEY_LEFTMETA),
+                        Modifier::Key(key) => Some(key.clone()),
+                    }
+                }
+            })
+            .collect();
 
-        let prev_shift = self.send_modifier(Modifier::Shift, &next_shift)?;
+        // Emulate the modifiers of KeyPress
-        let prev_control = self.send_modifier(Modifier::Control, &next_control)?;
+        self.send_keys(&extra_modifiers, RELEASE)?;
-        let prev_alt = self.send_modifier(Modifier::Alt, &next_alt)?;
+        self.send_keys(&missing_modifiers, PRESS)?;
-        let prev_windows = self.send_modifier(Modifier::Windows, &next_windows)?;
 
+        // Press the main key
         self.send_key(&key_press.key, PRESS)?;
         self.send_key(&key_press.key, RELEASE)?;
 
-        self.send_modifier(Modifier::Windows, &prev_windows)?;
+        // Resurrect the original modifiers
-        self.send_modifier(Modifier::Alt, &prev_alt)?;
+        self.send_keys(&missing_modifiers, RELEASE)?;
-        self.send_modifier(Modifier::Control, &prev_control)?;
+        self.send_keys(&extra_modifiers, PRESS)?;
-        self.send_modifier(Modifier::Shift, &prev_shift)?;
-        Ok(())
-    }
-
-    fn send_modifier(&mut self, modifier: Modifier, desired: &PressState) -> Result<PressState, Box<dyn Error>> {
-        let mut current = match modifier {
-            Modifier::Shift => &self.shift,
-            Modifier::Control => &self.control,
-            Modifier::Alt => &self.alt,
-            Modifier::Windows => &self.windows,
-        }
-        .clone();
-        let original = current.clone();
-        let left_key = match modifier {
-            Modifier::Shift => &SHIFT_KEYS[0],
-            Modifier::Control => &CONTROL_KEYS[0],
-            Modifier::Alt => &ALT_KEYS[0],
-            Modifier::Windows => &WINDOWS_KEYS[0],
-        };
-        let right_key = match modifier {
-            Modifier::Shift => &SHIFT_KEYS[1],
-            Modifier::Control => &CONTROL_KEYS[1],
-            Modifier::Alt => &ALT_KEYS[1],
-            Modifier::Windows => &WINDOWS_KEYS[1],
-        };
-
-        if !current.left && desired.left {
-            self.send_key(left_key, PRESS)?;
-            current.left = true;
-        } else if current.left && !desired.left {
-            self.send_key(left_key, RELEASE)?;
-            current.left = false;
-        }
-
-        if !current.right && desired.right {
-            self.send_key(right_key, PRESS)?;
-            current.right = true;
-        } else if current.right && !desired.right {
-            self.send_key(right_key, RELEASE)?;
-            current.right = false;
-        }
-
-        match modifier {
-            Modifier::Shift => self.shift = current,
-            Modifier::Control => self.control = current,
-            Modifier::Alt => self.alt = current,
-            Modifier::Windows => self.windows = current,
-        };
-        Ok(original)
-    }
 
-    fn build_state(&self, modifier: Modifier, modifier_state: ModifierState) -> PressState {
+        Ok(())
-        let press_state = match modifier {
-            Modifier::Shift => &self.shift,
-            Modifier::Control => &self.control,
-            Modifier::Alt => &self.alt,
-            Modifier::Windows => &self.windows,
-        };
-
-        match modifier_state {
-            ModifierState::Either => {
-                // Choose a PressState closest to the current PressState
-                if press_state.left || press_state.right {
-                    press_state.clone() // no change is necessary
-                } else {
-                    // Just press left
-                    PressState {
-                        left: true,
-                        right: false,
-                    }
-                }
-            }
-            ModifierState::Left => PressState {
-                left: true,
-                right: false,
-            },
-            ModifierState::Right => PressState {
-                left: false,
-                right: true,
-            },
-            ModifierState::None => PressState {
-                left: false,
-                right: false,
-            },
-        }
     }
 
     fn with_mark(&self, key_press: &KeyPress) -> KeyPress {
-        let mut shift = key_press.shift.clone();
+        if self.mark_set && !self.match_modifier(&Modifier::Shift) {
-        if self.mark_set && shift == ModifierState::None {
+            let mut modifiers = key_press.modifiers.clone();
-            // We don't leave Either in expand_modifiers(), so just using Left here.
+            modifiers.push(Modifier::Shift);
-            shift = ModifierState::Left;
+            KeyPress {
-        }
+                key: key_press.key,
-        KeyPress {
+                modifiers,
-            key: key_press.key,
+            }
-            shift,
+        } else {
-            control: key_press.control.clone(),
+            key_press.clone()
-            alt: key_press.alt.clone(),
-            windows: key_press.windows.clone(),
         }
     }
 
@@ -455,6 +393,49 @@ impl EventHandler {
         }
     }
 
+    fn contains_modifier(&self, modifiers: &Vec<Modifier>, key: &Key) -> bool {
+        for modifier in modifiers {
+            if match modifier {
+                Modifier::Shift => key == &Key::KEY_LEFTSHIFT || key == &Key::KEY_RIGHTSHIFT,
+                Modifier::Control => key == &Key::KEY_LEFTCTRL || key == &Key::KEY_RIGHTCTRL,
+                Modifier::Alt => key == &Key::KEY_LEFTALT || key == &Key::KEY_RIGHTALT,
+                Modifier::Windows => key == &Key::KEY_LEFTMETA || key == &Key::KEY_RIGHTMETA,
+                Modifier::Key(modifier_key) => key == modifier_key,
+            } {
+                return true;
+            }
+        }
+        false
+    }
+
+    fn match_modifiers(&self, modifiers: &Vec<Modifier>, exact_match: bool) -> bool {
+        if exact_match && self.modifiers.len() > modifiers.len() {
+            return false;
+        }
+        for modifier in modifiers {
+            if !self.match_modifier(modifier) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    fn match_modifier(&self, modifier: &Modifier) -> bool {
+        match modifier {
+            Modifier::Shift => {
+                self.modifiers.contains(&Key::KEY_LEFTSHIFT) || self.modifiers.contains(&Key::KEY_RIGHTSHIFT)
+            }
+            Modifier::Control => {
+                self.modifiers.contains(&Key::KEY_LEFTCTRL) || self.modifiers.contains(&Key::KEY_RIGHTCTRL)
+            }
+            Modifier::Alt => self.modifiers.contains(&Key::KEY_LEFTALT) || self.modifiers.contains(&Key::KEY_RIGHTALT),
+            Modifier::Windows => {
+                self.modifiers.contains(&Key::KEY_LEFTMETA) || self.modifiers.contains(&Key::KEY_RIGHTMETA)
+            }
+            Modifier::Key(key) => self.modifiers.contains(key),
+        }
+    }
+
     fn match_application(&mut self, application_matcher: &Application) -> bool {
         // Lazily fill the wm_class cache
         if self.application_cache.is_none() {
@@ -475,25 +456,11 @@ impl EventHandler {
         false
     }
 
-    fn update_modifier(&mut self, code: u16, value: i32) {
+    fn update_modifier(&mut self, key: Key, value: i32) {
-        if code == Key::KEY_LEFTSHIFT.code() {
+        if value == PRESS {
-            self.shift.left = is_pressed(value)
+            self.modifiers.insert(key);
-        } else if code == Key::KEY_RIGHTSHIFT.code() {
+        } else if value == RELEASE {
-            self.shift.right = is_pressed(value)
+            self.modifiers.remove(&key);
-        } else if code == Key::KEY_LEFTCTRL.code() {
-            self.control.left = is_pressed(value)
-        } else if code == Key::KEY_RIGHTCTRL.code() {
-            self.control.right = is_pressed(value)
-        } else if code == Key::KEY_LEFTALT.code() {
-            self.alt.left = is_pressed(value)
-        } else if code == Key::KEY_RIGHTALT.code() {
-            self.alt.right = is_pressed(value)
-        } else if code == Key::KEY_LEFTMETA.code() {
-            self.windows.left = is_pressed(value)
-        } else if code == Key::KEY_RIGHTMETA.code() {
-            self.windows.right = is_pressed(value)
-        } else {
-            panic!("unexpected key {:?} at update_modifier", Key::new(code));
         }
     }
 }
@@ -534,31 +501,6 @@ lazy_static! {
 
 //---
 
-#[derive(Clone)]
-struct PressState {
-    left: bool,
-    right: bool,
-}
-
-impl PressState {
-    fn new() -> PressState {
-        PressState {
-            left: false,
-            right: false,
-        }
-    }
-
-    fn to_modifier_state(&self) -> ModifierState {
-        if self.left {
-            ModifierState::Left
-        } else if self.right {
-            ModifierState::Right
-        } else {
-            ModifierState::None
-        }
-    }
-}
-
 fn is_pressed(value: i32) -> bool {
     value == PRESS || value == REPEAT
 }