disable ctrl+shift+v shortcut on windows (#91)

Author: houqp

crates/kiorg/src/config/mod.rs

@@ -152,11 +152,9 @@ pub fn load_config_with_override(
         Err(e) => return Err(ConfigError::TomlError(e, config_path)),
     };
 
-    // Validate user shortcuts for conflicts
-    if let Some(ref user_shortcuts) = user_config.shortcuts
-        && let Err(conflict_error) = validate_user_shortcuts(user_shortcuts)
-    {
-        return Err(ConfigError::ShortcutConflict(conflict_error, config_path));
+    // Validate user shortcuts
+    if let Some(ref user_shortcuts) = user_config.shortcuts {
+        validate_user_shortcuts(user_shortcuts, &config_path)?;
     }
 
     if let Some(layout) = &user_config.layout
@@ -219,11 +217,13 @@ pub fn get_kiorg_config_dir(override_path: Option<&PathBuf>) -> PathBuf {
     }
 }
 
-/// Validate user shortcuts for conflicts
+/// Validate user shortcuts for conflicts and reserved keys
 /// Returns an error if any shortcut is assigned to multiple different actions
+/// or if a reserved shortcut is used.
 fn validate_user_shortcuts(
     user_shortcuts: &shortcuts::Shortcuts,
-) -> Result<(), ShortcutConflictError> {
+    config_path: &std::path::Path,
+) -> Result<(), ConfigError> {
     use std::collections::HashMap;
 
     // Create a map from shortcut to actions that use it
@@ -234,6 +234,14 @@ fn validate_user_shortcuts(
 
     for (action, shortcuts_list) in user_shortcuts {
         for shortcut in shortcuts_list {
+            if let Ok(keys) = shortcut.to_shortcut_keys() {
+                for skey in keys {
+                    if let Err(e) = shortcuts::check_blacklisted_shortcut(&skey) {
+                        return Err(ConfigError::ValueError(e, config_path.to_path_buf()));
+                    }
+                }
+            }
+
             // Only add unique actions (don't count duplicates of the same action)
             let actions_for_shortcut = shortcut_to_actions.entry(shortcut.clone()).or_default();
 
@@ -247,11 +255,14 @@ fn validate_user_shortcuts(
     for (shortcut, actions) in shortcut_to_actions {
         if actions.len() > 1 {
             // Found a conflict - return error with the first two conflicting actions
-            return Err(ShortcutConflictError {
-                shortcut,
-                action1: actions[0],
-                action2: actions[1],
-            });
+            return Err(ConfigError::ShortcutConflict(
+                ShortcutConflictError {
+                    shortcut,
+                    action1: actions[0],
+                    action2: actions[1],
+                },
+                config_path.to_path_buf(),
+            ));
         }
     }
 

crates/kiorg/src/config/shortcuts.rs

@@ -18,6 +18,18 @@ pub struct ShortcutKey {
     pub modifiers: Modifiers,
 }
 
+#[inline]
+pub fn check_blacklisted_shortcut(_key: &ShortcutKey) -> Result<(), String> {
+    #[cfg(target_os = "windows")]
+    if _key.key == Key::V && _key.modifiers.ctrl && _key.modifiers.shift {
+        // see: https://github.com/houqp/kiorg/issues/3#issuecomment-3514588299
+        return Err(
+            "Shortcut Ctrl+Shift+V is reserved on Windows and cannot be registered".to_string(),
+        );
+    }
+    Ok(())
+}
+
 // Result of traversing a key buffer through the shortcut tree
 #[derive(Debug, Clone, PartialEq)]
 pub enum TraverseResult {
@@ -344,6 +356,8 @@ impl Shortcuts {
         let mut current_node = &mut self.shortcut_tree;
 
         for (i, key) in keys.iter().enumerate() {
+            check_blacklisted_shortcut(key)?;
+
             if i == keys.len() - 1 {
                 // Last key in sequence - insert the action
                 match current_node {

crates/kiorg/tests/ui_preview_test.rs

@@ -352,7 +352,7 @@ fn test_pdf_page_count_in_preview_content() {
                 Some(PreviewContent::Pdf(_))
             )
         },
-        std::time::Duration::from_secs(2),
+        std::time::Duration::from_secs(3),
     );
 
     match &harness.state().preview_content {

crates/kiorg/tests/ui_shortcuts_test.rs

@@ -404,3 +404,66 @@ key = "d"
         panic!("Expected ShortcutConflictError, got: {result:?}");
     }
 }
+#[test]
+#[cfg(target_os = "windows")]
+fn test_disallow_ctrl_shift_v_config() {
+    // Create a temporary directory for the config
+    let config_temp_dir = tempdir().unwrap();
+    let config_dir = config_temp_dir.path().to_path_buf();
+
+    // Create a TOML config file with Ctrl+Shift+V
+    let toml_content = r#"
+# Config with reserved Windows shortcut
+[[shortcuts.MoveDown]]
+key = "v"
+ctrl = true
+shift = true
+"#;
+
+    // Write the config file
+    create_config_file(&config_dir, toml_content);
+
+    // Try to load the config
+    let result = kiorg::config::load_config_with_override(Some(&config_dir));
+
+    // Verify that we got an error and it's a value error mentioning the reservation
+    assert!(
+        result.is_err(),
+        "Should return an error for reserved Windows shortcut"
+    );
+
+    let error_msg = result.unwrap_err().to_string();
+    assert!(
+        error_msg.contains("reserved on Windows"),
+        "Error message should mention it's reserved on Windows, got: {error_msg}"
+    );
+}
+
+#[test]
+#[cfg(not(target_os = "windows"))]
+fn test_allow_ctrl_shift_v_config_non_windows() {
+    // Create a temporary directory for the config
+    let config_temp_dir = tempdir().unwrap();
+    let config_dir = config_temp_dir.path().to_path_buf();
+
+    // Create a TOML config file with Ctrl+Shift+V
+    let toml_content = r#"
+# Config with Ctrl+Shift+V which is allowed on non-Windows
+[[shortcuts.MoveDown]]
+key = "v"
+ctrl = true
+shift = true
+"#;
+
+    // Write the config file
+    create_config_file(&config_dir, toml_content);
+
+    // Try to load the config
+    let result = kiorg::config::load_config_with_override(Some(&config_dir));
+
+    // Verify that it works on non-Windows
+    assert!(
+        result.is_ok(),
+        "Should NOT return an error for Ctrl+Shift+V on non-Windows platforms"
+    );
+}