Poking around

7 files changed, 177 insertions, 138 deletions

diff --git a/src/cli.rs b/src/cli.rs
index 67a92196..d6d3057e 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -25,7 +25,7 @@ pub struct CliArgs {
     pub layout: Option<PathBuf>,
 
     /// Path to the configuration yaml file
-    #[structopt(short, long)]
+    #[structopt(long)]
     pub config: Option<PathBuf>,
 
     #[structopt(short, long)]
diff --git a/src/common/config.rs b/src/common/config.rs
index bf8a9a9f..4a66acb0 100644
--- a/src/common/config.rs
+++ b/src/common/config.rs
@@ -1,12 +1,11 @@
 //! Deserializes configuration options.
-use std;
 use std::error;
 use std::fmt::{self, Display};
 use std::fs::File;
 use std::io::{self, Read};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
-use super::input::keybinds::{Keybinds, MultipleKeybinds};
+use super::input::keybinds::{Keybinds, KeybindsFromYaml};
 use crate::utils::logging::*;
 
 use directories_next::ProjectDirs;
@@ -15,7 +14,7 @@ use serde::Deserialize;
 /// Intermediate deserialisation config struct
 #[derive(Debug, Deserialize)]
 pub struct ConfigFromYaml {
-    pub keybinds: Option<MultipleKeybinds>,
+    pub keybinds: Option<KeybindsFromYaml>,
 }
 
 /// Main configuration.
@@ -53,7 +52,7 @@ impl Config {
     /// The allow is here, because rust assumes there is no
     /// error handling when logging the error to the log file.
     #[allow(unused_must_use)]
-    pub fn new(path: &PathBuf) -> Result<Config, ConfigError> {
+    pub fn new(path: &Path) -> Result<Config, ConfigError> {
         match File::open(path) {
             Ok(mut file) => {
                 let mut yaml_config = String::new();
@@ -81,7 +80,7 @@ impl Config {
             Ok(Config::new(&config_path)?)
         } else {
             let project_dirs = ProjectDirs::from("org", "Zellij Contributors", "Zellij").unwrap();
-            let mut config_path: PathBuf = project_dirs.config_dir().to_owned().into();
+            let mut config_path: PathBuf = project_dirs.config_dir().to_owned();
             config_path.push("config.yaml");
             Ok(Config::new(&config_path)?)
         }
diff --git a/src/common/input/handler.rs b/src/common/input/handler.rs
index 9b08491b..3438bba8 100644
--- a/src/common/input/handler.rs
+++ b/src/common/input/handler.rs
@@ -60,49 +60,43 @@ impl InputHandler {
         self.send_pty_instructions.update(err_ctx);
         self.send_app_instructions.update(err_ctx);
         self.send_screen_instructions.update(err_ctx);
-        if let Ok(_keybinds) = Keybinds::get_default_keybinds() {
-            let keybinds = self.config.keybinds.clone();
-            'input_loop: loop {
-                //@@@ I think this should actually just iterate over stdin directly
-                let stdin_buffer = self.os_input.read_from_stdin();
-                drop(
-                    self.send_plugin_instructions
-                        .send(PluginInstruction::GlobalInput(stdin_buffer.clone())),
-                );
-                for key_result in stdin_buffer.events_and_raw() {
-                    match key_result {
-                        Ok((event, raw_bytes)) => match event {
-                            termion::event::Event::Key(key) => {
-                                // FIXME this explicit break is needed because the current test
-                                // framework relies on it to not create dead threads that loop
-                                // and eat up CPUs. Do not remove until the test framework has
-                                // been revised. Sorry about this (@categorille)
-                                if {
-                                    let mut should_break = false;
-                                    // Hacked on way to have a means of testing Macros, needs to
-                                    // get properly integrated
-                                    for action in Keybinds::key_to_actions(
-                                        &key, raw_bytes, &self.mode, &keybinds,
-                                    ) {
-                                        should_break |= self.dispatch_action(action);
-                                    }
-                                    should_break
-                                } {
-                                    break 'input_loop;
+        let keybinds = self.config.keybinds.clone();
+        'input_loop: loop {
+            //@@@ I think this should actually just iterate over stdin directly
+            let stdin_buffer = self.os_input.read_from_stdin();
+            drop(
+                self.send_plugin_instructions
+                    .send(PluginInstruction::GlobalInput(stdin_buffer.clone())),
+            );
+            for key_result in stdin_buffer.events_and_raw() {
+                match key_result {
+                    Ok((event, raw_bytes)) => match event {
+                        termion::event::Event::Key(key) => {
+                            // FIXME this explicit break is needed because the current test
+                            // framework relies on it to not create dead threads that loop
+                            // and eat up CPUs. Do not remove until the test framework has
+                            // been revised. Sorry about this (@categorille)
+                            if {
+                                let mut should_break = false;
+                                // Hacked on way to have a means of testing Macros, needs to
+                                // get properly integrated
+                                for action in
+                                    Keybinds::key_to_actions(&key, raw_bytes, &self.mode, &keybinds)
+                                {
+                                    should_break |= self.dispatch_action(action);
                                 }
+                                should_break
+                            } {
+                                break 'input_loop;
                             }
-                            termion::event::Event::Mouse(_)
-                            | termion::event::Event::Unsupported(_) => {
-                                unimplemented!("Mouse and unsupported events aren't supported!");
-                            }
-                        },
-                        Err(err) => panic!("Encountered read error: {:?}", err),
-                    }
+                        }
+                        termion::event::Event::Mouse(_) | termion::event::Event::Unsupported(_) => {
+                            unimplemented!("Mouse and unsupported events aren't supported!");
+                        }
+                    },
+                    Err(err) => panic!("Encountered read error: {:?}", err),
                 }
             }
-        } else {
-            //@@@ Error handling?
-            self.exit();
         }
     }
 
diff --git a/src/common/input/keybinds.rs b/src/common/input/keybinds.rs
index 8b089ecf..2c395d37 100644
--- a/src/common/input/keybinds.rs
+++ b/src/common/input/keybinds.rs
@@ -8,50 +8,41 @@ use serde::Deserialize;
 use strum::IntoEnumIterator;
 use termion::event::Key;
 
-#[derive(Clone, Debug, PartialEq, Deserialize)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct Keybinds(HashMap<InputMode, ModeKeybinds>);
-#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
+#[derive(Clone, Debug, Default, PartialEq)]
 pub struct ModeKeybinds(HashMap<Key, Vec<Action>>);
 
-/// Intermediate struct for configuration.
-#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
-pub struct MultipleKeybinds(HashMap<InputMode, Vec<ModeKeybind>>);
-/// Intermediate struct for configuration.
-#[derive(Clone, Debug, Default, PartialEq, Deserialize)]
-struct MultipleModeKeybinds(HashMap<Vec<Key>, Vec<Action>>);
+/// Intermediate struct used for deserialisation
+#[derive(Clone, Debug, PartialEq, Deserialize)]
+pub struct KeybindsFromYaml(HashMap<InputMode, Vec<KeyActionFromYaml>>);
 
-/// Intermediate enum for configuration.
-#[derive(Debug, Clone, PartialEq, Deserialize)]
-#[serde(untagged)]
-enum ModeKeybind {
-    ModeKeybinds(ModeKeybinds),
-    MultipleModeKeybinds(MultipleModeKeybinds),
+/// Intermediate struct used for deserialisation
+#[derive(Clone, Debug, PartialEq, Deserialize)]
+pub struct KeyActionFromYaml {
+    action: Vec<Action>,
+    key: Vec<Key>,
 }
 
 impl Default for Keybinds {
     fn default() -> Keybinds {
-        Keybinds::get_default_keybinds().expect("Something is wrong with the default Keybinds")
-    }
-}
-
-impl Keybinds {
-    pub fn new() -> Keybinds {
-        Keybinds::from(HashMap::<InputMode, ModeKeybinds>::new())
-    }
-
-    pub fn get_default_keybinds() -> Result<Keybinds, String> {
         let mut defaults = Keybinds::new();
 
         for mode in InputMode::iter() {
             defaults
                 .0
-                .insert(mode, Keybinds::get_defaults_for_mode(&mode)?);
+                .insert(mode, Keybinds::get_defaults_for_mode(&mode));
         }
+        defaults
+    }
+}
 
-        Ok(defaults)
+impl Keybinds {
+    pub fn new() -> Keybinds {
+        Keybinds(HashMap::<InputMode, ModeKeybinds>::new())
     }
 
-    pub fn get_default_keybinds_with_config(keybinds: Option<MultipleKeybinds>) -> Keybinds {
+    pub fn get_default_keybinds_with_config(keybinds: Option<KeybindsFromYaml>) -> Keybinds {
         let default_keybinds = Keybinds::default();
         if let Some(keybinds) = keybinds {
             default_keybinds.merge_keybinds(Keybinds::from(keybinds))
@@ -80,7 +71,7 @@ impl Keybinds {
     }
 
     /// Returns the default keybinds for a given [`InputMode`].
-    fn get_defaults_for_mode(mode: &InputMode) -> Result<ModeKeybinds, String> {
+    fn get_defaults_for_mode(mode: &InputMode) -> ModeKeybinds {
         let mut defaults = HashMap::new();
 
         match *mode {
@@ -212,8 +203,7 @@ impl Keybinds {
                 defaults.insert(Key::Esc, vec![Action::SwitchToMode(InputMode::Command)]);
             }
         }
-
-        Ok(ModeKeybinds::from(defaults))
+        ModeKeybinds(defaults)
     }
 
     /// Converts a [`Key`] terminal event to a sequence of [`Action`]s according to the current
@@ -242,39 +232,27 @@ impl Keybinds {
 }
 
 impl ModeKeybinds {
-    pub fn new() -> ModeKeybinds {
-        ModeKeybinds::from(HashMap::<Key, Vec<Action>>::new())
+    fn new() -> ModeKeybinds {
+        ModeKeybinds(HashMap::<Key, Vec<Action>>::new())
     }
 
     /// Merges `self` with `other`, if keys are the same, `other` overwrites.
-    pub fn merge(self, other: ModeKeybinds) -> ModeKeybinds {
+    fn merge(self, other: ModeKeybinds) -> ModeKeybinds {
         let mut merged = self;
-        merged.0.extend(other.0.clone());
+        merged.0.extend(other.0);
         merged
     }
 }
 
-/// Converts from the `MultipleKeybinds` struct to a `Keybinds` struct.
-/// TODO @a-kenji Error on conflicting keybinds?
-/// Atm the Lower Keybind will take precedence and will overwrite the
-/// one further up.
-impl From<MultipleKeybinds> for Keybinds {
-    fn from(multiple: MultipleKeybinds) -> Keybinds {
+impl From<KeybindsFromYaml> for Keybinds {
+    fn from(keybinds_from_yaml: KeybindsFromYaml) -> Keybinds {
         let mut keybinds = Keybinds::new();
 
         for mode in InputMode::iter() {
             let mut mode_keybinds = ModeKeybinds::new();
-            for mode_keybind in multiple.0.get(&mode).iter() {
-                for keybind in mode_keybind.iter() {
-                    match keybind {
-                        ModeKeybind::ModeKeybinds(k) => {
-                            mode_keybinds = mode_keybinds.clone().merge(k.clone());
-                        }
-                        ModeKeybind::MultipleModeKeybinds(k) => {
-                            mode_keybinds =
-                                mode_keybinds.clone().merge(ModeKeybinds::from(k.clone()));
-                        }
-                    }
+            for key_action in keybinds_from_yaml.0.get(&mode).iter() {
+                for keybind in key_action.iter() {
+                    mode_keybinds = mode_keybinds.merge(ModeKeybinds::from(keybind.clone()));
                 }
             }
             keybinds.0.insert(mode, mode_keybinds);
@@ -283,25 +261,18 @@ impl From<MultipleKeybinds> for Keybinds {
     }
 }
 
-impl From<HashMap<InputMode, ModeKeybinds>> for Keybinds {
-    fn from(map: HashMap<InputMode, ModeKeybinds>) -> Keybinds {
-        Keybinds(map)
-    }
-}
-impl From<HashMap<Key, Vec<Action>>> for ModeKeybinds {
-    fn from(map: HashMap<Key, Vec<Action>>) -> ModeKeybinds {
-        ModeKeybinds(map)
-    }
-}
+/// For each `Key` assigned to `Action`s,
+/// map the `Action`s to the key
+impl From<KeyActionFromYaml> for ModeKeybinds {
+    fn from(key_action: KeyActionFromYaml) -> ModeKeybinds {
+        let keys = key_action.key;
+        let actions = key_action.action;
 
-impl From<MultipleModeKeybinds> for ModeKeybinds {
-    fn from(multiple: MultipleModeKeybinds) -> ModeKeybinds {
-        let single: HashMap<Key, Vec<Action>> = multiple
-            .0
-            .into_iter()
-            .flat_map(|(k, v)| (k.into_iter().map(move |k| (k, v.clone()))))
-            .collect();
-        ModeKeybinds::from(single)
+        ModeKeybinds(
+            keys.into_iter()
+                .map(|k| (k, actions.clone()))
+                .collect::<HashMap<Key, Vec<Action>>>(),
+        )
     }
 }
 
diff --git a/src/common/input/ut/config_test.rs b/src/common/input/ut/config_test.rs
index 001aab15..17649d14 100644
--- a/src/common/input/ut/config_test.rs
+++ b/src/common/input/ut/config_test.rs
@@ -1,13 +1,12 @@
 //! For Configuration Options
 
 use super::super::config::*;
-use crate::common::input::keybinds::*;
 use crate::common::input::actions::*;
+use crate::common::input::keybinds::*;
 use std::path::PathBuf;
 
 use termion::event::Key;
 
-
 #[test]
 fn no_config_file_equals_default_config() {
     let no_file = PathBuf::from(r"../fixtures/config/config.yamlll");
@@ -23,29 +22,29 @@ fn no_config_option_file_equals_default_config() {
     assert_eq!(config, default);
 }
 
-#[test]
-fn multiple_keys_mapped_to_one_action() {
-    let options = r"
-    ---
-keybindings:
-    Normal:
-    - ? - F: 6
-        - F: 7
-        - F: 8
-      : - {GoToTab: 5}
-      ";
+//#[test]
+//fn multiple_keys_mapped_to_one_action() {
+//let options = r"
+//---
+//keybindings:
+//Normal:
+//- ? - F: 6
+//- F: 7
+//- F: 8
+//: - {GoToTab: 5}
+//";
 
-    let config_options = Config::from_yaml(&options).unwrap();
+//let config_options = Config::from_yaml(&options).unwrap();
 
-    assert_eq!(config_options, config_options)
-}
+//assert_eq!(config_options, config_options)
+//}
 
 //#[test]
 //fn merge_keybinds_merges(){
-    //let mut self_keybinds = Keybinds::new();
-    //let mut self_mode_keybinds = ModeKeybinds::new();
-    //self_mode_keybinds.0.insert(Key::F(1), vec![Action::GoToTab(5)]);
-    //let mut other_keybinds = Keybinds::new();
-    //let mut self_mode_keybinds = ModeKeybinds::new();
-    //let mut expected = Keybinds::new();
+//let mut self_keybinds = Keybinds::new();
+//let mut self_mode_keybinds = ModeKeybinds::new();
+//self_mode_keybinds.0.insert(Key::F(1), vec![Action::GoToTab(5)]);
+//let mut other_keybinds = Keybinds::new();
+//let mut self_mode_keybinds = ModeKeybinds::new();
+//let mut expected = Keybinds::new();
 //}
diff --git a/src/common/input/ut/keybinds_test.rs b/src/common/input/ut/keybinds_test.rs
index e69de29b..44472985 100644
--- a/src/common/input/ut/keybinds_test.rs
+++ b/src/common/input/ut/keybinds_test.rs
@@ -0,0 +1,76 @@
+use super::super::actions::*;
+use super::super::keybinds::*;
+use termion::event::Key;
+
+#[test]
+fn merge_keybinds_merges_different_keys() {
+    let mut mode_keybinds_self = ModeKeybinds::new();
+    mode_keybinds_self.0.insert(Key::F(1), vec![Action::NoOp]);
+    let mut mode_keybinds_other = ModeKeybinds::new();
+    mode_keybinds_other
+        .0
+        .insert(Key::Backspace, vec![Action::NoOp]);
+
+    let mut mode_keybinds_expected = ModeKeybinds::new();
+    mode_keybinds_expected
+        .0
+        .insert(Key::F(1), vec![Action::NoOp]);
+    mode_keybinds_expected
+        .0
+        .insert(Key::Backspace, vec![Action::NoOp]);
+
+    let mode_keybinds_merged = mode_keybinds_self.merge(mode_keybinds_other);
+
+    assert_eq!(mode_keybinds_expected, mode_keybinds_merged);
+}
+
+#[test]
+fn merge_mode_keybinds_overwrites_same_keys() {
+    let mut mode_keybinds_self = ModeKeybinds::new();
+    mode_keybinds_self.0.insert(Key::F(1), vec![Action::NoOp]);
+    let mut mode_keybinds_other = ModeKeybinds::new();
+    mode_keybinds_other
+        .0
+        .insert(Key::F(1), vec![Action::GoToTab(1)]);
+
+    let mut mode_keybinds_expected = ModeKeybinds::new();
+    mode_keybinds_expected
+        .0
+        .insert(Key::F(1), vec![Action::GoToTab(1)]);
+
+    let mode_keybinds_merged = mode_keybinds_self.merge(mode_keybinds_other);
+
+    assert_eq!(mode_keybinds_expected, mode_keybinds_merged);
+}
+
+#[test]
+fn merge_keybinds_merges() {
+    let mut mode_keybinds_self = ModeKeybinds::new();
+    mode_keybinds_self.0.insert(Key::F(1), vec![Action::NoOp]);
+    let mut mode_keybinds_other = ModeKeybinds::new();
+    mode_keybinds_other
+        .0
+        .insert(Key::Backspace, vec![Action::NoOp]);
+    let mut keybinds_self = Keybinds::new();
+    keybinds_self
+        .0
+        .insert(InputMode::Normal, mode_keybinds_self.clone());
+    let mut keybinds_other = Keybinds::new();
+    keybinds_other
+        .0
+        .insert(InputMode::Resize, mode_keybinds_other.clone());
+    let mut keybinds_expected = Keybinds::new();
+    keybinds_expected.0.insert(
+        InputMode::Normal,
+        mode_keybinds_self
+    );
+    keybinds_expected.0.insert(
+        InputMode::Resize,
+        mode_keybinds_other
+    );
+
+    assert_eq!(
+        keybinds_expected,
+        keybinds_self.merge_keybinds(keybinds_other)
+    )
+}
diff --git a/src/common/mod.rs b/src/common/mod.rs
index fdd66649..3e2f370a 100644
--- a/src/common/mod.rs
+++ b/src/common/mod.rs
@@ -656,7 +656,7 @@ pub fn start(mut os_input: Box<dyn OsApi>, opts: CliArgs) {
             let send_pty_instructions = send_pty_instructions.clone();
             let send_plugin_instructions = send_plugin_instructions.clone();
             let os_input = os_input.clone();
-            let config = config.clone();
+            let config = config;
             move || {
                 input_loop(
                     os_input,