Merge pull request #2109 from luca020400/luca/keybind

Duplicate key bind validation

Author: andymandias

CHANGELOG.md

@@ -7,6 +7,7 @@ Added:
 - Config option (`preview.card.description_decode_html`) to decode html strings in preview card descriptions
 - Support displaying a larger version of the card images in-app
 - IRCv3 `no-implicit-names` support
+- Keyboard shortcuts validation (e.g no duplicate key binds)
 
 Fixed:
 

data/src/config.rs

@@ -40,6 +40,7 @@ use crate::serde::{
     deserialize_f32_positive_float_maybe, deserialize_u8_positive_integer_maybe,
 };
 use crate::server::{ConfigMap as ServerMap, ServerName};
+use crate::shortcut::{Commands, KeyBind};
 use crate::{Theme, environment};
 
 pub mod actions;
@@ -561,6 +562,8 @@ impl Config {
         })
         .map_err(|e| Error::Parse(e.to_string()))?;
 
+        keyboard.validate()?;
+
         let servers = ServerMap::new(
             servers,
             sidebar.order_channels_by,
@@ -815,6 +818,8 @@ pub enum Error {
         "Exactly one of sasl.plain.password, sasl.plain.password_file or sasl.plain.password_command must be set."
     )]
     DuplicateSaslPassword,
+    #[error("Keybind \"{}\" is assigned to multiple actions: {}", keybind.as_config_string(), actions.as_config_string())]
+    KeyBindConflict { keybind: KeyBind, actions: Commands },
     #[error("Config does not exist")]
     ConfigMissing,
 }

data/src/config/keys.rs

@@ -1,6 +1,11 @@
+use std::collections::HashMap;
+
 use serde::Deserialize;
 
-use crate::shortcut::{KeyBind, KeyBinds, Shortcut, shortcut};
+use crate::config::Error;
+use crate::shortcut::{
+    Command, Commands, KeyBind, KeyBinds, Shortcut, shortcut,
+};
 
 #[derive(Debug, Clone, Deserialize)]
 #[serde(default)]
@@ -81,55 +86,76 @@ impl Default for Keyboard {
 }
 
 impl Keyboard {
-    pub fn shortcuts(&self) -> Vec<Shortcut> {
-        use crate::shortcut::Command::*;
+    fn keybind_pairs(&self) -> Vec<(&KeyBinds, Command)> {
+        use Command::*;
+        vec![
+            (&self.move_up, MoveUp),
+            (&self.move_down, MoveDown),
+            (&self.move_left, MoveLeft),
+            (&self.move_right, MoveRight),
+            (&self.new_horizontal_buffer, NewHorizontalBuffer),
+            (&self.new_vertical_buffer, NewVerticalBuffer),
+            (&self.close_buffer, CloseBuffer),
+            (&self.maximize_buffer, MaximizeBuffer),
+            (&self.restore_buffer, RestoreBuffer),
+            (&self.cycle_next_buffer, CycleNextBuffer),
+            (&self.cycle_previous_buffer, CyclePreviousBuffer),
+            (&self.leave_buffer, LeaveBuffer),
+            (&self.toggle_nick_list, ToggleNicklist),
+            (&self.toggle_topic, ToggleTopic),
+            (&self.toggle_sidebar, ToggleSidebar),
+            (&self.toggle_fullscreen, ToggleFullscreen),
+            (&self.command_bar, CommandBar),
+            (&self.reload_configuration, ReloadConfiguration),
+            (&self.file_transfers, FileTransfers),
+            (&self.logs, Logs),
+            (&self.theme_editor, ThemeEditor),
+            (&self.scroll_up_page, ScrollUpPage),
+            (&self.scroll_down_page, ScrollDownPage),
+            (&self.scroll_to_top, ScrollToTop),
+            (&self.scroll_to_bottom, ScrollToBottom),
+            (&self.highlights, Highlights),
+            (&self.cycle_next_unread_buffer, CycleNextUnreadBuffer),
+            (
+                &self.cycle_previous_unread_buffer,
+                CyclePreviousUnreadBuffer,
+            ),
+            (&self.mark_as_read, MarkAsRead),
+            (&self.quit_application, QuitApplication),
+            (&self.open_config_file, OpenConfigFile),
+        ]
+    }
 
-        let mut shortcuts = vec![];
+    pub fn validate(&self) -> Result<(), Error> {
+        let mut map: HashMap<KeyBind, Vec<Command>> = HashMap::new();
 
-        let mut push = |key_binds: &KeyBinds, command| {
-            shortcuts.extend(
-                key_binds
-                    .iter()
-                    .cloned()
-                    .map(|key_bind| shortcut(key_bind, command)),
-            );
-        };
+        for (keybinds, command) in self.keybind_pairs() {
+            for key in keybinds.iter() {
+                map.entry(key.clone()).or_default().push(command);
+            }
+        }
 
-        push(&self.move_up, MoveUp);
-        push(&self.move_down, MoveDown);
-        push(&self.move_left, MoveLeft);
-        push(&self.move_right, MoveRight);
-        push(&self.new_horizontal_buffer, NewHorizontalBuffer);
-        push(&self.new_vertical_buffer, NewVerticalBuffer);
-        push(&self.close_buffer, CloseBuffer);
-        push(&self.maximize_buffer, MaximizeBuffer);
-        push(&self.restore_buffer, RestoreBuffer);
-        push(&self.cycle_next_buffer, CycleNextBuffer);
-        push(&self.cycle_previous_buffer, CyclePreviousBuffer);
-        push(&self.leave_buffer, LeaveBuffer);
-        push(&self.toggle_nick_list, ToggleNicklist);
-        push(&self.toggle_topic, ToggleTopic);
-        push(&self.toggle_sidebar, ToggleSidebar);
-        push(&self.toggle_fullscreen, ToggleFullscreen);
-        push(&self.command_bar, CommandBar);
-        push(&self.reload_configuration, ReloadConfiguration);
-        push(&self.file_transfers, FileTransfers);
-        push(&self.logs, Logs);
-        push(&self.theme_editor, ThemeEditor);
-        push(&self.scroll_up_page, ScrollUpPage);
-        push(&self.scroll_down_page, ScrollDownPage);
-        push(&self.scroll_to_top, ScrollToTop);
-        push(&self.scroll_to_bottom, ScrollToBottom);
-        push(&self.highlights, Highlights);
-        push(&self.cycle_next_unread_buffer, CycleNextUnreadBuffer);
-        push(
-            &self.cycle_previous_unread_buffer,
-            CyclePreviousUnreadBuffer,
-        );
-        push(&self.mark_as_read, MarkAsRead);
-        push(&self.quit_application, QuitApplication);
-        push(&self.open_config_file, OpenConfigFile);
+        for (key, commands) in map {
+            if commands.len() > 1 {
+                return Err(Error::KeyBindConflict {
+                    keybind: key,
+                    actions: Commands::from(commands),
+                });
+            }
+        }
+
+        Ok(())
+    }
 
-        shortcuts
+    pub fn shortcuts(&self) -> Vec<Shortcut> {
+        self.keybind_pairs()
+            .into_iter()
+            .flat_map(|(keybinds, command)| {
+                keybinds
+                    .iter()
+                    .cloned()
+                    .map(move |key_bind| shortcut(key_bind, command))
+            })
+            .collect()
     }
 }

data/src/shortcut.rs

@@ -75,7 +75,8 @@ impl<'de> Deserialize<'de> for KeyBinds {
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, strum::Display)]
+#[strum(serialize_all = "snake_case")]
 pub enum Command {
     MoveUp,
     MoveDown,
@@ -148,6 +149,46 @@ macro_rules! default {
     };
 }
 
+#[derive(Debug, Clone, Default)]
+pub struct Commands(Vec<Command>);
+
+impl From<Vec<Command>> for Commands {
+    fn from(commands: Vec<Command>) -> Self {
+        Self(commands)
+    }
+}
+
+impl Commands {
+    pub fn as_config_string(&self) -> String {
+        if let Some((last, rest)) = self.0.split_last() {
+            if self.0.len() == 1 {
+                format!("{last}")
+            } else {
+                let mut config_string = String::new();
+
+                if self.0.len() == 2 {
+                    if let Some(command) = rest.first() {
+                        config_string
+                            .push_str(format!("{command} and ").as_str());
+                    }
+
+                    config_string.push_str(format!("{last}").as_str());
+                } else {
+                    for command in rest {
+                        config_string.push_str(format!("{command}, ").as_str());
+                    }
+
+                    config_string.push_str(format!("and {last}").as_str());
+                }
+
+                config_string
+            }
+        } else {
+            String::new()
+        }
+    }
+}
+
 #[derive(Debug, Clone, Eq, Ord, PartialOrd)]
 pub enum KeyBind {
     Bind {
@@ -169,6 +210,22 @@ impl fmt::Display for KeyBind {
     }
 }
 
+impl KeyBind {
+    pub fn as_config_string(&self) -> String {
+        match self {
+            KeyBind::Bind {
+                key_code,
+                modifiers,
+            } => format!(
+                "{}+{}",
+                modifiers.as_config_string(),
+                key_code.to_string().to_lowercase()
+            ),
+            KeyBind::Unbind => String::new(),
+        }
+    }
+}
+
 impl PartialEq for KeyBind {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
@@ -210,8 +267,12 @@ impl Hash for KeyBind {
                 key_code,
                 modifiers,
             } => {
-                key_code.hash(state);
                 modifiers.hash(state);
+                if let keyboard::Key::Character(c) = &key_code.0 {
+                    keyboard::Key::Character(c.to_lowercase()).hash(state);
+                } else {
+                    key_code.hash(state);
+                }
             }
             KeyBind::Unbind => {
                 std::mem::discriminant(self).hash(state);
@@ -420,6 +481,37 @@ impl fmt::Display for Modifiers {
     }
 }
 
+impl Modifiers {
+    fn as_config_string(&self) -> String {
+        let mut mods = vec![];
+        let inner = self.0;
+
+        if inner.contains(keyboard::Modifiers::SHIFT) {
+            mods.push("shift");
+        }
+        if inner.contains(keyboard::Modifiers::CTRL) {
+            mods.push("ctrl");
+        }
+        if inner.contains(keyboard::Modifiers::ALT) {
+            if cfg!(target_os = "macos") {
+                mods.push("opt");
+            } else {
+                mods.push("alt");
+            }
+        }
+        if inner.contains(keyboard::Modifiers::LOGO) {
+            if cfg!(target_os = "macos") {
+            } else if cfg!(target_os = "windows") {
+                mods.push("win");
+            } else {
+                mods.push("super");
+            }
+        }
+
+        mods.join("+")
+    }
+}
+
 impl fmt::Display for KeyCode {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let key = match self.0.clone() {