From 4c51ab8a255910cb30efb2cd819b4bfec13623e0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 4 Jul 2023 20:58:33 +0300 Subject: [PATCH] Accept `null` as a valid action, to disable a keystroke co-authored-by: Mikayla Maki --- crates/gpui/src/app/window.rs | 8 +- crates/gpui/src/gpui.rs | 2 + crates/settings/src/keymap_file.rs | 47 +++++---- crates/zed/src/zed.rs | 161 +++++++++++++++++++++++++++++ 4 files changed, 197 insertions(+), 21 deletions(-) diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 23fbb33fe1..743f99ee62 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -14,8 +14,8 @@ use crate::{ text_layout::TextLayoutCache, util::post_inc, Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Effect, - Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, SceneBuilder, Subscription, - View, ViewContext, ViewHandle, WindowInvalidation, + Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, NoAction, SceneBuilder, + Subscription, View, ViewContext, ViewHandle, WindowInvalidation, }; use anyhow::{anyhow, bail, Result}; use collections::{HashMap, HashSet}; @@ -434,7 +434,11 @@ impl<'a> WindowContext<'a> { MatchResult::None => false, MatchResult::Pending => true, MatchResult::Matches(matches) => { + let no_action_id = (NoAction {}).id(); for (view_id, action) in matches { + if action.id() == no_action_id { + return false; + } if self.dispatch_action(Some(*view_id), action.as_ref()) { self.keystroke_matcher.clear_pending(); handled_by = Some(action.boxed_clone()); diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 25d022d8ed..3442934b3a 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -31,3 +31,5 @@ pub use window::{Axis, SizeConstraint, Vector2FExt, WindowContext}; pub use anyhow; pub use serde_json; + +actions!(zed, [NoAction]); diff --git a/crates/settings/src/keymap_file.rs b/crates/settings/src/keymap_file.rs index d2e656ebe3..93cb2ab3d7 100644 --- a/crates/settings/src/keymap_file.rs +++ b/crates/settings/src/keymap_file.rs @@ -1,7 +1,7 @@ use crate::{settings_store::parse_json_with_comments, SettingsAssets}; use anyhow::{anyhow, Context, Result}; use collections::BTreeMap; -use gpui::{keymap_matcher::Binding, AppContext}; +use gpui::{keymap_matcher::Binding, AppContext, NoAction}; use schemars::{ gen::{SchemaGenerator, SchemaSettings}, schema::{InstanceType, Schema, SchemaObject, SingleOrVec, SubschemaValidation}, @@ -11,18 +11,18 @@ use serde::Deserialize; use serde_json::Value; use util::{asset_str, ResultExt}; -#[derive(Deserialize, Default, Clone, JsonSchema)] +#[derive(Debug, Deserialize, Default, Clone, JsonSchema)] #[serde(transparent)] pub struct KeymapFile(Vec); -#[derive(Deserialize, Default, Clone, JsonSchema)] +#[derive(Debug, Deserialize, Default, Clone, JsonSchema)] pub struct KeymapBlock { #[serde(default)] context: Option, bindings: BTreeMap, } -#[derive(Deserialize, Default, Clone)] +#[derive(Debug, Deserialize, Default, Clone)] #[serde(transparent)] pub struct KeymapAction(Value); @@ -61,21 +61,22 @@ impl KeymapFile { // We want to deserialize the action data as a `RawValue` so that we can // deserialize the action itself dynamically directly from the JSON // string. But `RawValue` currently does not work inside of an untagged enum. - if let Value::Array(items) = action { - let Ok([name, data]): Result<[serde_json::Value; 2], _> = items.try_into() else { - return Some(Err(anyhow!("Expected array of length 2"))); - }; - let serde_json::Value::String(name) = name else { - return Some(Err(anyhow!("Expected first item in array to be a string."))) - }; - cx.deserialize_action( - &name, - Some(data), - ) - } else if let Value::String(name) = action { - cx.deserialize_action(&name, None) - } else { - return Some(Err(anyhow!("Expected two-element array, got {:?}", action))); + match action { + Value::Array(items) => { + let Ok([name, data]): Result<[serde_json::Value; 2], _> = items.try_into() else { + return Some(Err(anyhow!("Expected array of length 2"))); + }; + let serde_json::Value::String(name) = name else { + return Some(Err(anyhow!("Expected first item in array to be a string."))) + }; + cx.deserialize_action( + &name, + Some(data), + ) + }, + Value::String(name) => cx.deserialize_action(&name, None), + Value::Null => Ok(no_action()), + _ => return Some(Err(anyhow!("Expected two-element array, got {action:?}"))), } .with_context(|| { format!( @@ -115,6 +116,10 @@ impl KeymapFile { instance_type: Some(SingleOrVec::Single(Box::new(InstanceType::Array))), ..Default::default() }), + Schema::Object(SchemaObject { + instance_type: Some(SingleOrVec::Single(Box::new(InstanceType::Null))), + ..Default::default() + }), ]), ..Default::default() })), @@ -129,6 +134,10 @@ impl KeymapFile { } } +fn no_action() -> Box { + Box::new(NoAction {}) +} + #[cfg(test)] mod tests { use crate::KeymapFile; diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 874fea6500..0df16f4bab 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2074,6 +2074,167 @@ mod tests { line!(), ); + #[track_caller] + fn assert_key_bindings_for<'a>( + window_id: usize, + cx: &TestAppContext, + actions: Vec<(&'static str, &'a dyn Action)>, + line: u32, + ) { + for (key, action) in actions { + // assert that... + assert!( + cx.available_actions(window_id, 0) + .into_iter() + .any(|(_, bound_action, b)| { + // action names match... + bound_action.name() == action.name() + && bound_action.namespace() == action.namespace() + // and key strokes contain the given key + && b.iter() + .any(|binding| binding.keystrokes().iter().any(|k| k.key == key)) + }), + "On {} Failed to find {} with key binding {}", + line, + action.name(), + key + ); + } + } + } + + #[gpui::test] + async fn test_disabled_keymap_binding(cx: &mut gpui::TestAppContext) { + struct TestView; + + impl Entity for TestView { + type Event = (); + } + + impl View for TestView { + fn ui_name() -> &'static str { + "TestView" + } + + fn render(&mut self, _: &mut ViewContext) -> AnyElement { + Empty::new().into_any() + } + } + + let executor = cx.background(); + let fs = FakeFs::new(executor.clone()); + + actions!(test, [A, B]); + // From the Atom keymap + actions!(workspace, [ActivatePreviousPane]); + // From the JetBrains keymap + actions!(pane, [ActivatePrevItem]); + + fs.save( + "/settings.json".as_ref(), + &r#" + { + "base_keymap": "Atom" + } + "# + .into(), + Default::default(), + ) + .await + .unwrap(); + + fs.save( + "/keymap.json".as_ref(), + &r#" + [ + { + "bindings": { + "backspace": "test::A" + } + } + ] + "# + .into(), + Default::default(), + ) + .await + .unwrap(); + + cx.update(|cx| { + cx.set_global(SettingsStore::test(cx)); + theme::init(Assets, cx); + welcome::init(cx); + + cx.add_global_action(|_: &A, _cx| {}); + cx.add_global_action(|_: &B, _cx| {}); + cx.add_global_action(|_: &ActivatePreviousPane, _cx| {}); + cx.add_global_action(|_: &ActivatePrevItem, _cx| {}); + + let settings_rx = watch_config_file( + executor.clone(), + fs.clone(), + PathBuf::from("/settings.json"), + ); + let keymap_rx = + watch_config_file(executor.clone(), fs.clone(), PathBuf::from("/keymap.json")); + + handle_keymap_file_changes(keymap_rx, cx); + handle_settings_file_changes(settings_rx, cx); + }); + + cx.foreground().run_until_parked(); + + let (window_id, _view) = cx.add_window(|_| TestView); + + // Test loading the keymap base at all + assert_key_bindings_for( + window_id, + cx, + vec![("backspace", &A), ("k", &ActivatePreviousPane)], + line!(), + ); + + // Test disabling the key binding for the base keymap + fs.save( + "/keymap.json".as_ref(), + &r#" + [ + { + "bindings": { + "backspace": null + } + } + ] + "# + .into(), + Default::default(), + ) + .await + .unwrap(); + + cx.foreground().run_until_parked(); + + assert_key_bindings_for(window_id, cx, vec![("k", &ActivatePreviousPane)], line!()); + + // Test modifying the base, while retaining the users keymap + fs.save( + "/settings.json".as_ref(), + &r#" + { + "base_keymap": "JetBrains" + } + "# + .into(), + Default::default(), + ) + .await + .unwrap(); + + cx.foreground().run_until_parked(); + + assert_key_bindings_for(window_id, cx, vec![("[", &ActivatePrevItem)], line!()); + + #[track_caller] fn assert_key_bindings_for<'a>( window_id: usize, cx: &TestAppContext,