From eeb5b03d6344baa91e3f24cb6ef2f84bdef7edbe Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 22 Dec 2022 03:24:21 -0500 Subject: [PATCH 01/33] add command to copy system information to the clipboard --- crates/workspace/src/workspace.rs | 52 ++++++++++++++++++++++++++++--- crates/zed/src/menus.rs | 13 +++++--- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index a53f20aeda..c9c8120eb7 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -34,15 +34,16 @@ use gpui::{ elements::*, impl_actions, impl_internal_actions, platform::{CursorStyle, WindowOptions}, - AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, - MouseButton, MutableAppContext, PathPromptOptions, PromptLevel, RenderContext, Task, View, - ViewContext, ViewHandle, WeakViewHandle, + AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, ClipboardItem, Entity, + ModelContext, ModelHandle, MouseButton, MutableAppContext, PathPromptOptions, PromptLevel, + RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ProjectItem}; use language::LanguageRegistry; use std::{ any::TypeId, borrow::Cow, + env, future::Future, path::{Path, PathBuf}, sync::Arc, @@ -72,7 +73,7 @@ use status_bar::StatusBar; pub use status_bar::StatusItemView; use theme::{Theme, ThemeRegistry}; pub use toolbar::{ToolbarItemLocation, ToolbarItemView}; -use util::ResultExt; +use util::{channel::ReleaseChannel, ResultExt}; #[derive(Clone, PartialEq)] pub struct RemoveWorktreeFromProject(pub WorktreeId); @@ -96,6 +97,7 @@ actions!( ToggleRightSidebar, NewTerminal, NewSearch, + CopySystemDetailsIntoClipboard ] ); @@ -299,6 +301,12 @@ pub fn init(app_state: Arc, cx: &mut MutableAppContext) { }, ); + cx.add_action( + |workspace: &mut Workspace, _: &CopySystemDetailsIntoClipboard, cx| { + workspace.copy_system_details_into_clipboard(cx); + }, + ); + let client = &app_state.client; client.add_view_request_handler(Workspace::handle_follow); client.add_view_message_handler(Workspace::handle_unfollow); @@ -980,6 +988,42 @@ impl Workspace { }) } + fn copy_system_details_into_clipboard(&mut self, cx: &mut ViewContext) { + let platform = cx.platform(); + + let os_name: String = platform.os_name().into(); + let os_version = platform.os_version().ok(); + let app_version = env!("CARGO_PKG_VERSION"); + let release_channel = cx.global::().dev_name(); + let architecture = env::var("CARGO_CFG_TARGET_ARCH"); + + let os_information = match os_version { + Some(os_version) => format!("OS: {os_name} {os_version}"), + None => format!("OS: {os_name}"), + }; + let system_information = vec![ + Some(os_information), + Some(format!("Zed: {app_version} ({release_channel})")), + architecture + .map(|architecture| format!("Architecture: {architecture}")) + .ok(), + ]; + + let system_information = system_information + .into_iter() + .flatten() + .collect::>() + .join("\n"); + + let item = ClipboardItem::new(system_information.clone()); + cx.prompt( + gpui::PromptLevel::Info, + &format!("Copied into clipboard:\n\n{system_information}"), + &["OK"], + ); + cx.write_to_clipboard(item.clone()); + } + #[allow(clippy::type_complexity)] pub fn open_paths( &mut self, diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index 18e7d6fe9b..b8b0f69bd4 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -339,10 +339,8 @@ pub fn menus() -> Vec> { }, MenuItem::Separator, MenuItem::Action { - name: "Documentation", - action: Box::new(crate::OpenBrowser { - url: "https://zed.dev/docs".into(), - }), + name: "Copy System Details Into Clipboard", + action: Box::new(workspace::CopySystemDetailsIntoClipboard), }, MenuItem::Action { name: "Give Feedback", @@ -350,6 +348,13 @@ pub fn menus() -> Vec> { url: super::feedback::NEW_ISSUE_URL.into(), }), }, + MenuItem::Separator, + MenuItem::Action { + name: "Documentation", + action: Box::new(crate::OpenBrowser { + url: "https://zed.dev/docs".into(), + }), + }, MenuItem::Action { name: "Zed Twitter", action: Box::new(crate::OpenBrowser { From ea16082a42d4a5fad475410e2620da045bdda0b2 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 22 Dec 2022 14:27:32 -0500 Subject: [PATCH 02/33] Factored data into a SystemSpecs struct Co-Authored-By: Mikayla Maki --- crates/workspace/src/workspace.rs | 54 +++---------------------------- crates/zed/src/menus.rs | 4 +-- crates/zed/src/system_specs.rs | 46 ++++++++++++++++++++++++++ crates/zed/src/zed.rs | 18 ++++++++++- 4 files changed, 70 insertions(+), 52 deletions(-) create mode 100644 crates/zed/src/system_specs.rs diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index c9c8120eb7..06e19bbf88 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -34,16 +34,15 @@ use gpui::{ elements::*, impl_actions, impl_internal_actions, platform::{CursorStyle, WindowOptions}, - AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, ClipboardItem, Entity, - ModelContext, ModelHandle, MouseButton, MutableAppContext, PathPromptOptions, PromptLevel, - RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle, + AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, + MouseButton, MutableAppContext, PathPromptOptions, PromptLevel, RenderContext, Task, View, + ViewContext, ViewHandle, WeakViewHandle, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ProjectItem}; use language::LanguageRegistry; use std::{ any::TypeId, borrow::Cow, - env, future::Future, path::{Path, PathBuf}, sync::Arc, @@ -73,7 +72,7 @@ use status_bar::StatusBar; pub use status_bar::StatusItemView; use theme::{Theme, ThemeRegistry}; pub use toolbar::{ToolbarItemLocation, ToolbarItemView}; -use util::{channel::ReleaseChannel, ResultExt}; +use util::ResultExt; #[derive(Clone, PartialEq)] pub struct RemoveWorktreeFromProject(pub WorktreeId); @@ -96,8 +95,7 @@ actions!( ToggleLeftSidebar, ToggleRightSidebar, NewTerminal, - NewSearch, - CopySystemDetailsIntoClipboard + NewSearch ] ); @@ -301,12 +299,6 @@ pub fn init(app_state: Arc, cx: &mut MutableAppContext) { }, ); - cx.add_action( - |workspace: &mut Workspace, _: &CopySystemDetailsIntoClipboard, cx| { - workspace.copy_system_details_into_clipboard(cx); - }, - ); - let client = &app_state.client; client.add_view_request_handler(Workspace::handle_follow); client.add_view_message_handler(Workspace::handle_unfollow); @@ -988,42 +980,6 @@ impl Workspace { }) } - fn copy_system_details_into_clipboard(&mut self, cx: &mut ViewContext) { - let platform = cx.platform(); - - let os_name: String = platform.os_name().into(); - let os_version = platform.os_version().ok(); - let app_version = env!("CARGO_PKG_VERSION"); - let release_channel = cx.global::().dev_name(); - let architecture = env::var("CARGO_CFG_TARGET_ARCH"); - - let os_information = match os_version { - Some(os_version) => format!("OS: {os_name} {os_version}"), - None => format!("OS: {os_name}"), - }; - let system_information = vec![ - Some(os_information), - Some(format!("Zed: {app_version} ({release_channel})")), - architecture - .map(|architecture| format!("Architecture: {architecture}")) - .ok(), - ]; - - let system_information = system_information - .into_iter() - .flatten() - .collect::>() - .join("\n"); - - let item = ClipboardItem::new(system_information.clone()); - cx.prompt( - gpui::PromptLevel::Info, - &format!("Copied into clipboard:\n\n{system_information}"), - &["OK"], - ); - cx.write_to_clipboard(item.clone()); - } - #[allow(clippy::type_complexity)] pub fn open_paths( &mut self, diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index b8b0f69bd4..3865389e99 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -339,8 +339,8 @@ pub fn menus() -> Vec> { }, MenuItem::Separator, MenuItem::Action { - name: "Copy System Details Into Clipboard", - action: Box::new(workspace::CopySystemDetailsIntoClipboard), + name: "Copy System Specs Into Clipboard", + action: Box::new(crate::CopySystemSpecsIntoClipboard), }, MenuItem::Action { name: "Give Feedback", diff --git a/crates/zed/src/system_specs.rs b/crates/zed/src/system_specs.rs new file mode 100644 index 0000000000..56eeeb8167 --- /dev/null +++ b/crates/zed/src/system_specs.rs @@ -0,0 +1,46 @@ +use std::{env, fmt::Display}; + +use gpui::AppContext; +use util::channel::ReleaseChannel; + +pub struct SystemSpecs { + os_name: &'static str, + os_version: Option, + app_version: &'static str, + release_channel: &'static str, + architecture: &'static str, +} + +impl SystemSpecs { + pub fn new(cx: &AppContext) -> Self { + let platform = cx.platform(); + + SystemSpecs { + os_name: platform.os_name(), + os_version: platform + .os_version() + .ok() + .map(|os_version| os_version.to_string()), + app_version: env!("CARGO_PKG_VERSION"), + release_channel: cx.global::().dev_name(), + architecture: env::consts::ARCH, + } + } +} + +impl Display for SystemSpecs { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let os_information = match &self.os_version { + Some(os_version) => format!("OS: {} {}", self.os_name, os_version), + None => format!("OS: {}", self.os_name), + }; + let system_specs = [ + os_information, + format!("Zed: {} ({})", self.app_version, self.release_channel), + format!("Architecture: {}", self.architecture), + ] + .join("\n"); + + write!(f, "{system_specs}") + } +} diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 099fb4eea9..0936e317f4 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -1,6 +1,7 @@ mod feedback; pub mod languages; pub mod menus; +pub mod system_specs; #[cfg(any(test, feature = "test-support"))] pub mod test; @@ -21,7 +22,7 @@ use gpui::{ }, impl_actions, platform::{WindowBounds, WindowOptions}, - AssetSource, AsyncAppContext, TitlebarOptions, ViewContext, WindowKind, + AssetSource, AsyncAppContext, ClipboardItem, TitlebarOptions, ViewContext, WindowKind, }; use language::Rope; use lazy_static::lazy_static; @@ -33,6 +34,7 @@ use serde::Deserialize; use serde_json::to_string_pretty; use settings::{keymap_file_json_schema, settings_file_json_schema, Settings}; use std::{env, path::Path, str, sync::Arc}; +use system_specs::SystemSpecs; use util::{channel::ReleaseChannel, paths, ResultExt}; pub use workspace; use workspace::{sidebar::SidebarSide, AppState, Workspace}; @@ -67,6 +69,7 @@ actions!( ResetBufferFontSize, InstallCommandLineInterface, ResetDatabase, + CopySystemSpecsIntoClipboard ] ); @@ -245,6 +248,19 @@ pub fn init(app_state: &Arc, cx: &mut gpui::MutableAppContext) { }, ); + cx.add_action( + |_: &mut Workspace, _: &CopySystemSpecsIntoClipboard, cx: &mut ViewContext| { + let system_specs = SystemSpecs::new(cx).to_string(); + let item = ClipboardItem::new(system_specs.clone()); + cx.prompt( + gpui::PromptLevel::Info, + &format!("Copied into clipboard:\n\n{system_specs}"), + &["OK"], + ); + cx.write_to_clipboard(item); + }, + ); + activity_indicator::init(cx); call::init(app_state.client.clone(), app_state.user_store.clone(), cx); settings::KeymapFileContent::load_defaults(cx); From f65fda2fa4b5575bc6bfcaccb6b7b1a2f62e36bd Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 22 Dec 2022 18:10:49 -0500 Subject: [PATCH 03/33] Add memory to system specs --- Cargo.lock | 34 +++++++++++++++++++++++++++++++++- crates/zed/Cargo.toml | 2 ++ crates/zed/src/system_specs.rs | 16 +++++++++++----- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 57a7dde194..8bad1349cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2757,6 +2757,12 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4a1e36c821dbe04574f602848a19f742f4fb3c98d40449f11bcad18d6b17421" +[[package]] +name = "human_bytes" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "39b528196c838e8b3da8b665e08c30958a6f2ede91d79f2ffcd0d4664b9c64eb" + [[package]] name = "humantime" version = "2.1.0" @@ -3755,6 +3761,15 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "ntapi" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc51db7b362b205941f71232e56c625156eb9a929f8cf74a428fd5bc094a4afc" +dependencies = [ + "winapi 0.3.9", +] + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -4424,7 +4439,7 @@ source = "git+https://github.com/zed-industries/wezterm?rev=5cd757e5f2eb039ed0c6 dependencies = [ "libc", "log", - "ntapi", + "ntapi 0.3.7", "winapi 0.3.9", ] @@ -6219,6 +6234,21 @@ dependencies = [ "libc", ] +[[package]] +name = "sysinfo" +version = "0.27.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ccb297c0afb439440834b4bcf02c5c9da8ec2e808e70f36b0d8e815ff403bd24" +dependencies = [ + "cfg-if 1.0.0", + "core-foundation-sys", + "libc", + "ntapi 0.4.0", + "once_cell", + "rayon", + "winapi 0.3.9", +] + [[package]] name = "system-interface" version = "0.20.0" @@ -8180,6 +8210,7 @@ dependencies = [ "fuzzy", "go_to_line", "gpui", + "human_bytes", "ignore", "image", "indexmap", @@ -8213,6 +8244,7 @@ dependencies = [ "smallvec", "smol", "sum_tree", + "sysinfo", "tempdir", "terminal_view", "text", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 574111848c..1e3167d2b8 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -30,6 +30,7 @@ clock = { path = "../clock" } diagnostics = { path = "../diagnostics" } editor = { path = "../editor" } file_finder = { path = "../file_finder" } +human_bytes = "0.4.1" search = { path = "../search" } fs = { path = "../fs" } fsevent = { path = "../fsevent" } @@ -48,6 +49,7 @@ recent_projects = { path = "../recent_projects" } rpc = { path = "../rpc" } settings = { path = "../settings" } sum_tree = { path = "../sum_tree" } +sysinfo = "0.27.1" text = { path = "../text" } terminal_view = { path = "../terminal_view" } theme = { path = "../theme" } diff --git a/crates/zed/src/system_specs.rs b/crates/zed/src/system_specs.rs index 56eeeb8167..b6c2c0fcba 100644 --- a/crates/zed/src/system_specs.rs +++ b/crates/zed/src/system_specs.rs @@ -1,28 +1,33 @@ use std::{env, fmt::Display}; use gpui::AppContext; +use human_bytes::human_bytes; +use sysinfo::{System, SystemExt}; use util::channel::ReleaseChannel; pub struct SystemSpecs { - os_name: &'static str, - os_version: Option, app_version: &'static str, release_channel: &'static str, + os_name: &'static str, + os_version: Option, + memory: u64, architecture: &'static str, } impl SystemSpecs { pub fn new(cx: &AppContext) -> Self { let platform = cx.platform(); + let system = System::new_all(); SystemSpecs { + app_version: env!("CARGO_PKG_VERSION"), + release_channel: cx.global::().dev_name(), os_name: platform.os_name(), os_version: platform .os_version() .ok() .map(|os_version| os_version.to_string()), - app_version: env!("CARGO_PKG_VERSION"), - release_channel: cx.global::().dev_name(), + memory: system.total_memory(), architecture: env::consts::ARCH, } } @@ -35,8 +40,9 @@ impl Display for SystemSpecs { None => format!("OS: {}", self.os_name), }; let system_specs = [ - os_information, format!("Zed: {} ({})", self.app_version, self.release_channel), + os_information, + format!("Memory: {}", human_bytes(self.memory as f64)), format!("Architecture: {}", self.architecture), ] .join("\n"); From 41bff3947c5ec60efe6e9711dd20ddd5ae96bef8 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 22 Dec 2022 23:04:33 -0500 Subject: [PATCH 04/33] Add actions for requesting features and filing bug reports --- Cargo.lock | 7 +++++++ crates/zed/Cargo.toml | 1 + crates/zed/src/menus.rs | 10 ++++++---- crates/zed/src/zed.rs | 26 +++++++++++++++++++++++++- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8bad1349cc..6ee7abcb70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7231,6 +7231,12 @@ dependencies = [ "serde", ] +[[package]] +name = "urlencoding" +version = "2.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8db7427f936968176eaa7cdf81b7f98b980b18495ec28f1b5791ac3bfe3eea9" + [[package]] name = "usvg" version = "0.14.1" @@ -8273,6 +8279,7 @@ dependencies = [ "tree-sitter-typescript", "unindent", "url", + "urlencoding", "util", "vim", "workspace", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 1e3167d2b8..fc7a175355 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -110,6 +110,7 @@ tree-sitter-html = "0.19.0" tree-sitter-scheme = { git = "https://github.com/6cdh/tree-sitter-scheme", rev = "af0fd1fa452cb2562dc7b5c8a8c55551c39273b9"} tree-sitter-racket = { git = "https://github.com/zed-industries/tree-sitter-racket", rev = "eb010cf2c674c6fd9a6316a84e28ef90190fe51a"} url = "2.2" +urlencoding = "2.1.2" [dev-dependencies] call = { path = "../call", features = ["test-support"] } diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index 3865389e99..b46e8ad703 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -343,10 +343,12 @@ pub fn menus() -> Vec> { action: Box::new(crate::CopySystemSpecsIntoClipboard), }, MenuItem::Action { - name: "Give Feedback", - action: Box::new(crate::OpenBrowser { - url: super::feedback::NEW_ISSUE_URL.into(), - }), + name: "File Bug Report", + action: Box::new(crate::FileBugReport), + }, + MenuItem::Action { + name: "Request Feature", + action: Box::new(crate::RequestFeature), }, MenuItem::Separator, MenuItem::Action { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 0936e317f4..e6d50f43af 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -69,7 +69,9 @@ actions!( ResetBufferFontSize, InstallCommandLineInterface, ResetDatabase, - CopySystemSpecsIntoClipboard + CopySystemSpecsIntoClipboard, + RequestFeature, + FileBugReport ] ); @@ -261,6 +263,28 @@ pub fn init(app_state: &Arc, cx: &mut gpui::MutableAppContext) { }, ); + cx.add_action( + |_: &mut Workspace, _: &RequestFeature, cx: &mut ViewContext| { + let url = "https://github.com/zed-industries/feedback/issues/new?assignees=&labels=enhancement%2Ctriage&template=0_feature_request.yml"; + cx.dispatch_action(OpenBrowser { + url: url.into(), + }); + }, + ); + + cx.add_action( + |_: &mut Workspace, _: &FileBugReport, cx: &mut ViewContext| { + let system_specs_text = SystemSpecs::new(cx).to_string(); + let url = format!( + "https://github.com/zed-industries/feedback/issues/new?assignees=&labels=defect%2Ctriage&template=2_bug_report.yml&environment={}", + urlencoding::encode(&system_specs_text) + ); + cx.dispatch_action(OpenBrowser { + url: url.into(), + }); + }, + ); + activity_indicator::init(cx); call::init(app_state.client.clone(), app_state.user_store.clone(), cx); settings::KeymapFileContent::load_defaults(cx); From 21a0df406f78e7c8437ec79a8e64b175a4b7afce Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Mon, 26 Dec 2022 00:24:26 -0500 Subject: [PATCH 05/33] Add home and end key support --- assets/keymaps/default.json | 14 ++++++++++++++ crates/gpui/src/platform/mac/event.rs | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 783d90f8e1..7af7426f19 100644 --- a/assets/keymaps/default.json +++ b/assets/keymaps/default.json @@ -67,9 +67,11 @@ "up": "editor::MoveUp", "pageup": "editor::PageUp", "shift-pageup": "editor::MovePageUp", + "home": "editor::MoveToBeginningOfLine", "down": "editor::MoveDown", "pagedown": "editor::PageDown", "shift-pagedown": "editor::MovePageDown", + "end": "editor::MoveToEndOfLine", "left": "editor::MoveLeft", "right": "editor::MoveRight", "ctrl-p": "editor::MoveUp", @@ -110,6 +112,12 @@ "stop_at_soft_wraps": true } ], + "shift-home": [ + "editor::SelectToBeginningOfLine", + { + "stop_at_soft_wraps": true + } + ], "ctrl-shift-a": [ "editor::SelectToBeginningOfLine", { @@ -122,6 +130,12 @@ "stop_at_soft_wraps": true } ], + "shift-end": [ + "editor::SelectToEndOfLine", + { + "stop_at_soft_wraps": true + } + ], "ctrl-shift-e": [ "editor::SelectToEndOfLine", { diff --git a/crates/gpui/src/platform/mac/event.rs b/crates/gpui/src/platform/mac/event.rs index 36dab73149..0464840e86 100644 --- a/crates/gpui/src/platform/mac/event.rs +++ b/crates/gpui/src/platform/mac/event.rs @@ -47,6 +47,8 @@ pub fn key_to_native(key: &str) -> Cow { "right" => NSRightArrowFunctionKey, "pageup" => NSPageUpFunctionKey, "pagedown" => NSPageDownFunctionKey, + "home" => NSHomeFunctionKey, + "end" => NSEndFunctionKey, "delete" => NSDeleteFunctionKey, "f1" => NSF1FunctionKey, "f2" => NSF2FunctionKey, @@ -258,6 +260,8 @@ unsafe fn parse_keystroke(native_event: id) -> Keystroke { Some(NSRightArrowFunctionKey) => "right".to_string(), Some(NSPageUpFunctionKey) => "pageup".to_string(), Some(NSPageDownFunctionKey) => "pagedown".to_string(), + Some(NSHomeFunctionKey) => "home".to_string(), + Some(NSEndFunctionKey) => "end".to_string(), Some(NSDeleteFunctionKey) => "delete".to_string(), Some(NSF1FunctionKey) => "f1".to_string(), Some(NSF2FunctionKey) => "f2".to_string(), From ca3c4566dd328bdbc390fd590a274965bffd09f3 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 29 Dec 2022 01:43:49 -0500 Subject: [PATCH 06/33] Add close all items command --- assets/keymaps/default.json | 3 ++- crates/workspace/src/pane.rs | 51 +++++++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 16 deletions(-) diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 7af7426f19..5402686ecf 100644 --- a/assets/keymaps/default.json +++ b/assets/keymaps/default.json @@ -20,8 +20,9 @@ "alt-cmd-left": "pane::ActivatePrevItem", "alt-cmd-right": "pane::ActivateNextItem", "cmd-w": "pane::CloseActiveItem", - "cmd-shift-w": "workspace::CloseWindow", "alt-cmd-t": "pane::CloseInactiveItems", + "cmd-k cmd-w": "pane::CloseAllItems", + "cmd-shift-w": "workspace::CloseWindow", "cmd-s": "workspace::Save", "cmd-shift-s": "workspace::SaveAs", "cmd-=": "zed::IncreaseBufferFontSize", diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 428865ec3b..4b787af20d 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -44,6 +44,7 @@ actions!( ActivateLastItem, CloseActiveItem, CloseInactiveItems, + CloseAllItems, ReopenClosedItem, SplitLeft, SplitUp, @@ -122,6 +123,7 @@ pub fn init(cx: &mut MutableAppContext) { }); cx.add_async_action(Pane::close_active_item); cx.add_async_action(Pane::close_inactive_items); + cx.add_async_action(Pane::close_all_items); cx.add_async_action(|workspace: &mut Workspace, action: &CloseItem, cx| { let pane = action.pane.upgrade(cx)?; let task = Pane::close_item(workspace, pane, action.item_id, cx); @@ -258,6 +260,12 @@ pub enum ReorderBehavior { MoveToIndex(usize), } +enum ItemType { + Active, + Inactive, + All, +} + impl Pane { pub fn new(docked: Option, cx: &mut ViewContext) -> Self { let handle = cx.weak_handle(); @@ -696,26 +704,29 @@ impl Pane { _: &CloseActiveItem, cx: &mut ViewContext, ) -> Option>> { - let pane_handle = workspace.active_pane().clone(); - let pane = pane_handle.read(cx); - if pane.items.is_empty() { - None - } else { - let item_id_to_close = pane.items[pane.active_item_index].id(); - let task = Self::close_items(workspace, pane_handle, cx, move |item_id| { - item_id == item_id_to_close - }); - Some(cx.foreground().spawn(async move { - task.await?; - Ok(()) - })) - } + Self::close_main(workspace, ItemType::Active, cx) } pub fn close_inactive_items( workspace: &mut Workspace, _: &CloseInactiveItems, cx: &mut ViewContext, + ) -> Option>> { + Self::close_main(workspace, ItemType::Inactive, cx) + } + + pub fn close_all_items( + workspace: &mut Workspace, + _: &CloseAllItems, + cx: &mut ViewContext, + ) -> Option>> { + Self::close_main(workspace, ItemType::All, cx) + } + + fn close_main( + workspace: &mut Workspace, + close_item_type: ItemType, + cx: &mut ViewContext, ) -> Option>> { let pane_handle = workspace.active_pane().clone(); let pane = pane_handle.read(cx); @@ -724,7 +735,17 @@ impl Pane { } else { let active_item_id = pane.items[pane.active_item_index].id(); let task = - Self::close_items(workspace, pane_handle, cx, move |id| id != active_item_id); + Self::close_items( + workspace, + pane_handle, + cx, + move |item_id| match close_item_type { + ItemType::Active => item_id == active_item_id, + ItemType::Inactive => item_id != active_item_id, + ItemType::All => true, + }, + ); + Some(cx.foreground().spawn(async move { task.await?; Ok(()) From 60f29410caafabdcd64a8380aceb411c955042d1 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 29 Dec 2022 13:28:52 -0500 Subject: [PATCH 07/33] Add close clean items command --- assets/keymaps/default.json | 1 + crates/workspace/src/pane.rs | 55 +++++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 5402686ecf..fbbb517c93 100644 --- a/assets/keymaps/default.json +++ b/assets/keymaps/default.json @@ -21,6 +21,7 @@ "alt-cmd-right": "pane::ActivateNextItem", "cmd-w": "pane::CloseActiveItem", "alt-cmd-t": "pane::CloseInactiveItems", + "cmd-k u": "pane::CloseCleanItems", "cmd-k cmd-w": "pane::CloseAllItems", "cmd-shift-w": "workspace::CloseWindow", "cmd-s": "workspace::Save", diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 4b787af20d..0bd4befd77 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -44,6 +44,7 @@ actions!( ActivateLastItem, CloseActiveItem, CloseInactiveItems, + CloseCleanItems, CloseAllItems, ReopenClosedItem, SplitLeft, @@ -123,6 +124,7 @@ pub fn init(cx: &mut MutableAppContext) { }); cx.add_async_action(Pane::close_active_item); cx.add_async_action(Pane::close_inactive_items); + cx.add_async_action(Pane::close_clean_items); cx.add_async_action(Pane::close_all_items); cx.add_async_action(|workspace: &mut Workspace, action: &CloseItem, cx| { let pane = action.pane.upgrade(cx)?; @@ -263,6 +265,7 @@ pub enum ReorderBehavior { enum ItemType { Active, Inactive, + Clean, All, } @@ -723,6 +726,14 @@ impl Pane { Self::close_main(workspace, ItemType::All, cx) } + pub fn close_clean_items( + workspace: &mut Workspace, + _: &CloseCleanItems, + cx: &mut ViewContext, + ) -> Option>> { + Self::close_main(workspace, ItemType::Clean, cx) + } + fn close_main( workspace: &mut Workspace, close_item_type: ItemType, @@ -731,26 +742,32 @@ impl Pane { let pane_handle = workspace.active_pane().clone(); let pane = pane_handle.read(cx); if pane.items.is_empty() { - None - } else { - let active_item_id = pane.items[pane.active_item_index].id(); - let task = - Self::close_items( - workspace, - pane_handle, - cx, - move |item_id| match close_item_type { - ItemType::Active => item_id == active_item_id, - ItemType::Inactive => item_id != active_item_id, - ItemType::All => true, - }, - ); - - Some(cx.foreground().spawn(async move { - task.await?; - Ok(()) - })) + return None; } + + let active_item_id = pane.items[pane.active_item_index].id(); + let clean_buffers: Vec<_> = pane + .items() + .filter(|item| !item.is_dirty(cx)) + .map(|item| item.id()) + .collect(); + let task = + Self::close_items( + workspace, + pane_handle, + cx, + move |item_id| match close_item_type { + ItemType::Active => item_id == active_item_id, + ItemType::Inactive => item_id != active_item_id, + ItemType::Clean => clean_buffers.contains(&item_id), + ItemType::All => true, + }, + ); + + Some(cx.foreground().spawn(async move { + task.await?; + Ok(()) + })) } pub fn close_item( From 2bc36600d421eb1ad4faba8c67fa31f270436551 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Thu, 29 Dec 2022 13:43:56 -0500 Subject: [PATCH 08/33] Rename variable --- crates/workspace/src/pane.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 0bd4befd77..618c650e02 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -746,7 +746,7 @@ impl Pane { } let active_item_id = pane.items[pane.active_item_index].id(); - let clean_buffers: Vec<_> = pane + let clean_item_ids: Vec<_> = pane .items() .filter(|item| !item.is_dirty(cx)) .map(|item| item.id()) @@ -759,7 +759,7 @@ impl Pane { move |item_id| match close_item_type { ItemType::Active => item_id == active_item_id, ItemType::Inactive => item_id != active_item_id, - ItemType::Clean => clean_buffers.contains(&item_id), + ItemType::Clean => clean_item_ids.contains(&item_id), ItemType::All => true, }, ); From a5bccecd48fb32c5553ae1d861ea59d9848dacdb Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Fri, 30 Dec 2022 18:18:02 -0800 Subject: [PATCH 09/33] Add other line seperators to regex normalization --- crates/fs/src/fs.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index d44aebce0f..d2f690e0e0 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -35,7 +35,7 @@ use repository::FakeGitRepositoryState; use std::sync::Weak; lazy_static! { - static ref CARRIAGE_RETURNS_REGEX: Regex = Regex::new("\r\n|\r").unwrap(); + static ref LINE_SEPERATORS_REGEX: Regex = Regex::new("\r\n|\r|\u{2028}|\u{2029}").unwrap(); } #[derive(Clone, Copy, Debug, PartialEq)] @@ -80,13 +80,13 @@ impl LineEnding { } pub fn normalize(text: &mut String) { - if let Cow::Owned(replaced) = CARRIAGE_RETURNS_REGEX.replace_all(text, "\n") { + if let Cow::Owned(replaced) = LINE_SEPERATORS_REGEX.replace_all(text, "\n") { *text = replaced; } } pub fn normalize_arc(text: Arc) -> Arc { - if let Cow::Owned(replaced) = CARRIAGE_RETURNS_REGEX.replace_all(&text, "\n") { + if let Cow::Owned(replaced) = LINE_SEPERATORS_REGEX.replace_all(&text, "\n") { replaced.into() } else { text From 233b28a1b9ae6bfc681f39a202092220d4c0af08 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Sun, 1 Jan 2023 23:50:45 -0500 Subject: [PATCH 10/33] Appease clippy --- crates/gpui/build.rs | 4 ++-- crates/gpui/src/platform/mac/platform.rs | 2 +- crates/gpui_macros/src/gpui_macros.rs | 8 +++----- crates/live_kit_client/build.rs | 10 +++++----- crates/media/build.rs | 2 +- crates/media/src/media.rs | 6 +++--- crates/rpc/src/auth.rs | 2 +- crates/snippet/src/snippet.rs | 2 +- crates/sqlez/src/connection.rs | 6 +++--- crates/sqlez/src/statement.rs | 8 ++++---- crates/sqlez/src/thread_safe_connection.rs | 13 ++++++------- crates/sqlez/src/typed_statements.rs | 12 ++++++------ crates/sqlez_macros/src/sqlez_macros.rs | 6 +++--- crates/sum_tree/src/tree_map.rs | 2 +- crates/util/src/channel.rs | 2 +- crates/util/src/lib.rs | 2 +- crates/zed/src/languages/ruby.rs | 4 ++-- crates/zed/src/main.rs | 4 ++-- crates/zed/src/zed.rs | 6 +++--- 19 files changed, 49 insertions(+), 52 deletions(-) diff --git a/crates/gpui/build.rs b/crates/gpui/build.rs index 3fb0119782..23f6f04992 100644 --- a/crates/gpui/build.rs +++ b/crates/gpui/build.rs @@ -52,7 +52,7 @@ fn compile_metal_shaders() { println!("cargo:rerun-if-changed={}", shader_path); let output = Command::new("xcrun") - .args(&[ + .args([ "-sdk", "macosx", "metal", @@ -76,7 +76,7 @@ fn compile_metal_shaders() { } let output = Command::new("xcrun") - .args(&["-sdk", "macosx", "metallib"]) + .args(["-sdk", "macosx", "metallib"]) .arg(air_output_path) .arg("-o") .arg(metallib_output_path) diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index f703d863be..3e2ef73634 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -647,7 +647,7 @@ impl platform::Platform for MacPlatform { attrs.set(kSecReturnAttributes as *const _, cf_true); attrs.set(kSecReturnData as *const _, cf_true); - let mut result = CFTypeRef::from(ptr::null_mut()); + let mut result = CFTypeRef::from(ptr::null()); let status = SecItemCopyMatching(attrs.as_concrete_TypeRef(), &mut result); match status { security::errSecSuccess => {} diff --git a/crates/gpui_macros/src/gpui_macros.rs b/crates/gpui_macros/src/gpui_macros.rs index e28d1711d2..cabae1ac0a 100644 --- a/crates/gpui_macros/src/gpui_macros.rs +++ b/crates/gpui_macros/src/gpui_macros.rs @@ -162,11 +162,9 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream { if let FnArg::Typed(arg) = arg { if let Type::Path(ty) = &*arg.ty { let last_segment = ty.path.segments.last(); - match last_segment.map(|s| s.ident.to_string()).as_deref() { - Some("StdRng") => { - inner_fn_args.extend(quote!(rand::SeedableRng::seed_from_u64(seed),)); - } - _ => {} + + if let Some("StdRng") = last_segment.map(|s| s.ident.to_string()).as_deref() { + inner_fn_args.extend(quote!(rand::SeedableRng::seed_from_u64(seed),)); } } else { inner_fn_args.extend(quote!(cx,)); diff --git a/crates/live_kit_client/build.rs b/crates/live_kit_client/build.rs index bceb4fb927..bcd3f76dca 100644 --- a/crates/live_kit_client/build.rs +++ b/crates/live_kit_client/build.rs @@ -5,7 +5,7 @@ use std::{ process::Command, }; -const SWIFT_PACKAGE_NAME: &'static str = "LiveKitBridge"; +const SWIFT_PACKAGE_NAME: &str = "LiveKitBridge"; #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -61,8 +61,8 @@ fn build_bridge(swift_target: &SwiftTarget) { let swift_package_root = swift_package_root(); if !Command::new("swift") .arg("build") - .args(&["--configuration", &env::var("PROFILE").unwrap()]) - .args(&["--triple", &swift_target.target.triple]) + .args(["--configuration", &env::var("PROFILE").unwrap()]) + .args(["--triple", &swift_target.target.triple]) .current_dir(&swift_package_root) .status() .unwrap() @@ -116,7 +116,7 @@ fn get_swift_target() -> SwiftTarget { let target = format!("{}-apple-macosx{}", arch, MACOS_TARGET_VERSION); let swift_target_info_str = Command::new("swift") - .args(&["-target", &target, "-print-target-info"]) + .args(["-target", &target, "-print-target-info"]) .output() .unwrap() .stdout; @@ -143,7 +143,7 @@ fn copy_dir(source: &Path, destination: &Path) { assert!( Command::new("cp") .arg("-R") - .args(&[source, destination]) + .args([source, destination]) .status() .unwrap() .success(), diff --git a/crates/media/build.rs b/crates/media/build.rs index 1ee8f23e41..7e66efd39d 100644 --- a/crates/media/build.rs +++ b/crates/media/build.rs @@ -3,7 +3,7 @@ use std::{env, path::PathBuf, process::Command}; fn main() { let sdk_path = String::from_utf8( Command::new("xcrun") - .args(&["--sdk", "macosx", "--show-sdk-path"]) + .args(["--sdk", "macosx", "--show-sdk-path"]) .output() .unwrap() .stdout, diff --git a/crates/media/src/media.rs b/crates/media/src/media.rs index fe69e684e7..1c76e6befb 100644 --- a/crates/media/src/media.rs +++ b/crates/media/src/media.rs @@ -113,9 +113,9 @@ pub mod core_video { let mut this = ptr::null(); let result = CVMetalTextureCacheCreate( kCFAllocatorDefault, - ptr::null_mut(), + ptr::null(), metal_device, - ptr::null_mut(), + ptr::null(), &mut this, ); if result == kCVReturnSuccess { @@ -192,7 +192,7 @@ pub mod core_video { pub fn as_texture_ref(&self) -> &metal::TextureRef { unsafe { let texture = CVMetalTextureGetTexture(self.as_concrete_TypeRef()); - &metal::TextureRef::from_ptr(texture as *mut _) + metal::TextureRef::from_ptr(texture as *mut _) } } } diff --git a/crates/rpc/src/auth.rs b/crates/rpc/src/auth.rs index 5254ef9ca4..39c8c39a44 100644 --- a/crates/rpc/src/auth.rs +++ b/crates/rpc/src/auth.rs @@ -23,7 +23,7 @@ pub fn random_token() -> String { for byte in token_bytes.iter_mut() { *byte = rng.gen(); } - base64::encode_config(&token_bytes, base64::URL_SAFE) + base64::encode_config(token_bytes, base64::URL_SAFE) } impl PublicKey { diff --git a/crates/snippet/src/snippet.rs b/crates/snippet/src/snippet.rs index 808701569a..2957f415da 100644 --- a/crates/snippet/src/snippet.rs +++ b/crates/snippet/src/snippet.rs @@ -62,7 +62,7 @@ fn parse_snippet<'a>( } } Some(_) => { - let chunk_end = source.find(&['}', '$', '\\']).unwrap_or(source.len()); + let chunk_end = source.find(['}', '$', '\\']).unwrap_or(source.len()); let (chunk, rest) = source.split_at(chunk_end); text.push_str(chunk); source = rest; diff --git a/crates/sqlez/src/connection.rs b/crates/sqlez/src/connection.rs index 3342845d14..3c6cb31196 100644 --- a/crates/sqlez/src/connection.rs +++ b/crates/sqlez/src/connection.rs @@ -20,7 +20,7 @@ unsafe impl Send for Connection {} impl Connection { pub(crate) fn open(uri: &str, persistent: bool) -> Result { let mut connection = Self { - sqlite3: 0 as *mut _, + sqlite3: ptr::null_mut(), persistent, write: RefCell::new(true), _sqlite: PhantomData, @@ -32,7 +32,7 @@ impl Connection { CString::new(uri)?.as_ptr(), &mut connection.sqlite3, flags, - 0 as *const _, + ptr::null(), ); // Turn on extended error codes @@ -97,7 +97,7 @@ impl Connection { let remaining_sql_str = remaining_sql.to_str().unwrap().trim(); remaining_sql_str != ";" && !remaining_sql_str.is_empty() } { - let mut raw_statement = 0 as *mut sqlite3_stmt; + let mut raw_statement = ptr::null_mut::(); let mut remaining_sql_ptr = ptr::null(); sqlite3_prepare_v2( self.sqlite3, diff --git a/crates/sqlez/src/statement.rs b/crates/sqlez/src/statement.rs index 86035f5d0a..f3ec6d4854 100644 --- a/crates/sqlez/src/statement.rs +++ b/crates/sqlez/src/statement.rs @@ -48,7 +48,7 @@ impl<'a> Statement<'a> { .trim(); remaining_sql_str != ";" && !remaining_sql_str.is_empty() } { - let mut raw_statement = 0 as *mut sqlite3_stmt; + let mut raw_statement = ptr::null_mut::(); let mut remaining_sql_ptr = ptr::null(); sqlite3_prepare_v2( connection.sqlite3, @@ -101,7 +101,7 @@ impl<'a> Statement<'a> { } } - fn bind_index_with(&self, index: i32, bind: impl Fn(&*mut sqlite3_stmt) -> ()) -> Result<()> { + fn bind_index_with(&self, index: i32, bind: impl Fn(&*mut sqlite3_stmt)) -> Result<()> { let mut any_succeed = false; unsafe { for raw_statement in self.raw_statements.iter() { @@ -133,7 +133,7 @@ impl<'a> Statement<'a> { }) } - pub fn column_blob<'b>(&'b mut self, index: i32) -> Result<&'b [u8]> { + pub fn column_blob(&mut self, index: i32) -> Result<&[u8]> { let index = index as c_int; let pointer = unsafe { sqlite3_column_blob(self.current_statement(), index) }; @@ -217,7 +217,7 @@ impl<'a> Statement<'a> { }) } - pub fn column_text<'b>(&'b mut self, index: i32) -> Result<&'b str> { + pub fn column_text(&mut self, index: i32) -> Result<&str> { let index = index as c_int; let pointer = unsafe { sqlite3_column_text(self.current_statement(), index) }; diff --git a/crates/sqlez/src/thread_safe_connection.rs b/crates/sqlez/src/thread_safe_connection.rs index 2c51b776ed..c3fe4657ee 100644 --- a/crates/sqlez/src/thread_safe_connection.rs +++ b/crates/sqlez/src/thread_safe_connection.rs @@ -114,12 +114,12 @@ impl ThreadSafeConnection { let mut queues = QUEUES.write(); if !queues.contains_key(&self.uri) { let mut write_queue_constructor = - write_queue_constructor.unwrap_or(background_thread_queue()); + write_queue_constructor.unwrap_or_else(background_thread_queue); queues.insert(self.uri.clone(), write_queue_constructor()); return true; } } - return false; + false } pub fn builder(uri: &str, persistent: bool) -> ThreadSafeConnectionBuilder { @@ -187,10 +187,9 @@ impl ThreadSafeConnection { *connection.write.get_mut() = false; if let Some(initialize_query) = connection_initialize_query { - connection.exec(initialize_query).expect(&format!( - "Initialize query failed to execute: {}", - initialize_query - ))() + connection.exec(initialize_query).unwrap_or_else(|_| { + panic!("Initialize query failed to execute: {}", initialize_query) + })() .unwrap() } @@ -225,7 +224,7 @@ impl Clone for ThreadSafeConnection { Self { uri: self.uri.clone(), persistent: self.persistent, - connection_initialize_query: self.connection_initialize_query.clone(), + connection_initialize_query: self.connection_initialize_query, connections: self.connections.clone(), _migrator: PhantomData, } diff --git a/crates/sqlez/src/typed_statements.rs b/crates/sqlez/src/typed_statements.rs index c7d8b20aa5..df4a2987b5 100644 --- a/crates/sqlez/src/typed_statements.rs +++ b/crates/sqlez/src/typed_statements.rs @@ -8,7 +8,7 @@ use crate::{ impl Connection { pub fn exec<'a>(&'a self, query: &str) -> Result Result<()>> { - let mut statement = Statement::prepare(&self, query)?; + let mut statement = Statement::prepare(self, query)?; Ok(move || statement.exec()) } @@ -16,7 +16,7 @@ impl Connection { &'a self, query: &str, ) -> Result Result<()>> { - let mut statement = Statement::prepare(&self, query)?; + let mut statement = Statement::prepare(self, query)?; Ok(move |bindings| statement.with_bindings(bindings)?.exec()) } @@ -24,7 +24,7 @@ impl Connection { &'a self, query: &str, ) -> Result Result>> { - let mut statement = Statement::prepare(&self, query)?; + let mut statement = Statement::prepare(self, query)?; Ok(move || statement.rows::()) } @@ -32,7 +32,7 @@ impl Connection { &'a self, query: &str, ) -> Result Result>> { - let mut statement = Statement::prepare(&self, query)?; + let mut statement = Statement::prepare(self, query)?; Ok(move |bindings| statement.with_bindings(bindings)?.rows::()) } @@ -40,7 +40,7 @@ impl Connection { &'a self, query: &str, ) -> Result Result>> { - let mut statement = Statement::prepare(&self, query)?; + let mut statement = Statement::prepare(self, query)?; Ok(move || statement.maybe_row::()) } @@ -48,7 +48,7 @@ impl Connection { &'a self, query: &str, ) -> Result Result>> { - let mut statement = Statement::prepare(&self, query)?; + let mut statement = Statement::prepare(self, query)?; Ok(move |bindings| { statement .with_bindings(bindings) diff --git a/crates/sqlez_macros/src/sqlez_macros.rs b/crates/sqlez_macros/src/sqlez_macros.rs index 429f45db7e..675576c997 100644 --- a/crates/sqlez_macros/src/sqlez_macros.rs +++ b/crates/sqlez_macros/src/sqlez_macros.rs @@ -33,14 +33,14 @@ fn create_error( .skip_while(|(offset, _)| offset <= &error_offset) .map(|(_, span)| span) .next() - .unwrap_or(Span::call_site()); + .unwrap_or_else(Span::call_site); let error_text = format!("Sql Error: {}\nFor Query: {}", error, formatted_sql); TokenStream::from(Error::new(error_span.into(), error_text).into_compile_error()) } fn make_sql(tokens: TokenStream) -> (Vec<(usize, Span)>, String) { let mut sql_tokens = vec![]; - flatten_stream(tokens.clone(), &mut sql_tokens); + flatten_stream(tokens, &mut sql_tokens); // Lookup of spans by offset at the end of the token let mut spans: Vec<(usize, Span)> = Vec::new(); let mut sql = String::new(); @@ -67,7 +67,7 @@ fn flatten_stream(tokens: TokenStream, result: &mut Vec<(String, Span)>) { result.push((close_delimiter(group.delimiter()), group.span())); } TokenTree::Ident(ident) => { - result.push((format!("{} ", ident.to_string()), ident.span())); + result.push((format!("{} ", ident), ident.span())); } leaf_tree => result.push((leaf_tree.to_string(), leaf_tree.span())), } diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index 8672ca398f..112366cdf5 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/crates/sum_tree/src/tree_map.rs @@ -58,7 +58,7 @@ impl TreeMap { self.0.insert_or_replace(MapEntry { key, value }, &()); } - pub fn remove<'a>(&mut self, key: &'a K) -> Option { + pub fn remove(&mut self, key: &K) -> Option { let mut removed = None; let mut cursor = self.0.cursor::>(); let key = MapKeyRef(Some(key)); diff --git a/crates/util/src/channel.rs b/crates/util/src/channel.rs index 3edf26dc95..03b0f04271 100644 --- a/crates/util/src/channel.rs +++ b/crates/util/src/channel.rs @@ -4,7 +4,7 @@ use lazy_static::lazy_static; lazy_static! { pub static ref RELEASE_CHANNEL_NAME: String = env::var("ZED_RELEASE_CHANNEL") - .unwrap_or(include_str!("../../zed/RELEASE_CHANNEL").to_string()); + .unwrap_or_else(|_| include_str!("../../zed/RELEASE_CHANNEL").to_string()); pub static ref RELEASE_CHANNEL: ReleaseChannel = match RELEASE_CHANNEL_NAME.as_str() { "dev" => ReleaseChannel::Dev, "preview" => ReleaseChannel::Preview, diff --git a/crates/util/src/lib.rs b/crates/util/src/lib.rs index d9015ca6c0..e79cc269c9 100644 --- a/crates/util/src/lib.rs +++ b/crates/util/src/lib.rs @@ -36,7 +36,7 @@ pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { debug_assert!(max_chars >= 5); if s.len() > max_chars { - format!("{}…", truncate(&s, max_chars.saturating_sub(3))) + format!("{}…", truncate(s, max_chars.saturating_sub(3))) } else { s.to_string() } diff --git a/crates/zed/src/languages/ruby.rs b/crates/zed/src/languages/ruby.rs index 6aad293d34..cbfb5e35a7 100644 --- a/crates/zed/src/languages/ruby.rs +++ b/crates/zed/src/languages/ruby.rs @@ -50,14 +50,14 @@ impl LspAdapter for RubyLanguageServer { grammar.highlight_id_for_name("type")? } lsp::CompletionItemKind::KEYWORD => { - if label.starts_with(":") { + if label.starts_with(':') { grammar.highlight_id_for_name("string.special.symbol")? } else { grammar.highlight_id_for_name("keyword")? } } lsp::CompletionItemKind::VARIABLE => { - if label.starts_with("@") { + if label.starts_with('@') { grammar.highlight_id_for_name("property")? } else { return None; diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 5ac7cb36b6..f4fa919c48 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -101,7 +101,7 @@ fn main() { //Setup settings global before binding actions cx.set_global(SettingsFile::new( - &*paths::SETTINGS, + &paths::SETTINGS, settings_file_content.clone(), fs.clone(), )); @@ -586,7 +586,7 @@ async fn handle_cli_connection( responses .send(CliResponse::Exit { - status: if errored { 1 } else { 0 }, + status: i32::from(errored), }) .log_err(); } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index e6d50f43af..c3af91306d 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -338,11 +338,11 @@ pub fn initialize_workspace( }, "schemas": [ { - "fileMatch": [schema_file_match(&*paths::SETTINGS)], + "fileMatch": [schema_file_match(&paths::SETTINGS)], "schema": settings_file_json_schema(theme_names, language_names), }, { - "fileMatch": [schema_file_match(&*paths::KEYMAP)], + "fileMatch": [schema_file_match(&paths::KEYMAP)], "schema": keymap_file_json_schema(&action_names), } ] @@ -646,7 +646,7 @@ fn open_bundled_config_file( cx: &mut ViewContext, ) { workspace - .with_local_workspace(&app_state.clone(), cx, |workspace, cx| { + .with_local_workspace(&app_state, cx, |workspace, cx| { let project = workspace.project().clone(); let buffer = project.update(cx, |project, cx| { let text = Assets::get(asset_path).unwrap().data; From 2b1118f59736aab11028349138e82cb30d9005ae Mon Sep 17 00:00:00 2001 From: Julia Date: Sun, 1 Jan 2023 23:50:46 -0500 Subject: [PATCH 11/33] Add dismiss buffer search button & fix some faulty icon button styling Co-Authored-By: Nate Butler --- crates/gpui/src/app.rs | 2 +- crates/search/src/buffer_search.rs | 166 ++++++++++++-------- crates/theme/src/theme.rs | 1 + styles/src/styleTree/contactNotification.ts | 4 +- styles/src/styleTree/editor.ts | 1 - styles/src/styleTree/search.ts | 12 ++ styles/src/styleTree/tabBar.ts | 2 +- 7 files changed, 119 insertions(+), 69 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index f5ced700b6..76b9fb1aa6 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -4038,7 +4038,7 @@ pub struct RenderContext<'a, T: View> { pub refreshing: bool, } -#[derive(Clone, Default)] +#[derive(Debug, Clone, Default)] pub struct MouseState { hovered: bool, clicked: Option, diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 5877322feb..fa95dda1af 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -106,73 +106,79 @@ impl View for BufferSearchBar { .with_child( Flex::row() .with_child( - ChildView::new(&self.query_editor, cx) - .aligned() - .left() - .flex(1., true) - .boxed(), - ) - .with_children(self.active_searchable_item.as_ref().and_then( - |searchable_item| { - let matches = self - .seachable_items_with_matches - .get(&searchable_item.downgrade())?; - let message = if let Some(match_ix) = self.active_match_index { - format!("{}/{}", match_ix + 1, matches.len()) - } else { - "No matches".to_string() - }; - - Some( - Label::new(message, theme.search.match_index.text.clone()) - .contained() - .with_style(theme.search.match_index.container) + Flex::row() + .with_child( + ChildView::new(&self.query_editor, cx) .aligned() + .left() + .flex(1., true) .boxed(), ) - }, - )) - .contained() - .with_style(editor_container) - .aligned() - .constrained() - .with_min_width(theme.search.editor.min_width) - .with_max_width(theme.search.editor.max_width) - .flex(1., false) - .boxed(), - ) - .with_child( - Flex::row() - .with_child(self.render_nav_button("<", Direction::Prev, cx)) - .with_child(self.render_nav_button(">", Direction::Next, cx)) - .aligned() - .boxed(), - ) - .with_child( - Flex::row() - .with_children(self.render_search_option( - supported_options.case, - "Case", - SearchOption::CaseSensitive, - cx, - )) - .with_children(self.render_search_option( - supported_options.word, - "Word", - SearchOption::WholeWord, - cx, - )) - .with_children(self.render_search_option( - supported_options.regex, - "Regex", - SearchOption::Regex, - cx, - )) - .contained() - .with_style(theme.search.option_button_group) - .aligned() + .with_children(self.active_searchable_item.as_ref().and_then( + |searchable_item| { + let matches = self + .seachable_items_with_matches + .get(&searchable_item.downgrade())?; + let message = if let Some(match_ix) = self.active_match_index { + format!("{}/{}", match_ix + 1, matches.len()) + } else { + "No matches".to_string() + }; + + Some( + Label::new(message, theme.search.match_index.text.clone()) + .contained() + .with_style(theme.search.match_index.container) + .aligned() + .boxed(), + ) + }, + )) + .contained() + .with_style(editor_container) + .aligned() + .constrained() + .with_min_width(theme.search.editor.min_width) + .with_max_width(theme.search.editor.max_width) + .flex(1., false) + .boxed(), + ) + .with_child( + Flex::row() + .with_child(self.render_nav_button("<", Direction::Prev, cx)) + .with_child(self.render_nav_button(">", Direction::Next, cx)) + .aligned() + .boxed(), + ) + .with_child( + Flex::row() + .with_children(self.render_search_option( + supported_options.case, + "Case", + SearchOption::CaseSensitive, + cx, + )) + .with_children(self.render_search_option( + supported_options.word, + "Word", + SearchOption::WholeWord, + cx, + )) + .with_children(self.render_search_option( + supported_options.regex, + "Regex", + SearchOption::Regex, + cx, + )) + .contained() + .with_style(theme.search.option_button_group) + .aligned() + .boxed(), + ) + .flex(1., true) .boxed(), ) + .with_child(self.render_close_button(&theme.search, cx)) .contained() .with_style(theme.search.container) .named("search bar") @@ -325,7 +331,7 @@ impl BufferSearchBar { let is_active = self.is_search_option_enabled(option); Some( MouseEventHandler::::new(option as usize, cx, |state, cx| { - let style = &cx + let style = cx .global::() .theme .search @@ -373,7 +379,7 @@ impl BufferSearchBar { enum NavButton {} MouseEventHandler::::new(direction as usize, cx, |state, cx| { - let style = &cx + let style = cx .global::() .theme .search @@ -399,6 +405,38 @@ impl BufferSearchBar { .boxed() } + fn render_close_button( + &self, + theme: &theme::Search, + cx: &mut RenderContext, + ) -> ElementBox { + let action = Box::new(Dismiss); + let tooltip = "Dismiss Buffer Search"; + let tooltip_style = cx.global::().theme.tooltip.clone(); + + enum CloseButton {} + MouseEventHandler::::new(0, cx, |state, _| { + let style = theme.dismiss_button.style_for(state, false); + Svg::new("icons/x_mark_8.svg") + .with_color(style.color) + .constrained() + .with_width(style.icon_width) + .aligned() + .constrained() + .with_width(style.button_width) + .contained() + .with_style(style.container) + .boxed() + }) + .on_click(MouseButton::Left, { + let action = action.boxed_clone(); + move |_, cx| cx.dispatch_any_action(action.boxed_clone()) + }) + .with_cursor_style(CursorStyle::PointingHand) + .with_tooltip::(0, tooltip.to_string(), Some(action), tooltip_style, cx) + .boxed() + } + fn deploy(pane: &mut Pane, action: &Deploy, cx: &mut ViewContext) { if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { if search_bar.update(cx, |search_bar, cx| search_bar.show(action.focus, true, cx)) { diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index bf6cb57adb..e463310b98 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -247,6 +247,7 @@ pub struct Search { pub results_status: TextStyle, pub tab_icon_width: f32, pub tab_icon_spacing: f32, + pub dismiss_button: Interactive, } #[derive(Clone, Deserialize, Default)] diff --git a/styles/src/styleTree/contactNotification.ts b/styles/src/styleTree/contactNotification.ts index e0b967203d..e3f15baa32 100644 --- a/styles/src/styleTree/contactNotification.ts +++ b/styles/src/styleTree/contactNotification.ts @@ -32,13 +32,13 @@ export default function contactNotification(colorScheme: ColorScheme): Object { }, }, dismissButton: { - color: foreground(layer, "on"), + color: foreground(layer, "variant"), iconWidth: 8, iconHeight: 8, buttonWidth: 8, buttonHeight: 8, hover: { - color: foreground(layer, "on", "hovered"), + color: foreground(layer, "hovered"), }, }, }; diff --git a/styles/src/styleTree/editor.ts b/styles/src/styleTree/editor.ts index 63d86c8f47..e96cab0316 100644 --- a/styles/src/styleTree/editor.ts +++ b/styles/src/styleTree/editor.ts @@ -257,7 +257,6 @@ export default function editor(colorScheme: ColorScheme) { right: 6, }, hover: { - color: foreground(layer, "on", "hovered"), background: background(layer, "on", "hovered"), }, }, diff --git a/styles/src/styleTree/search.ts b/styles/src/styleTree/search.ts index 4f12c42c0c..2edd3807a5 100644 --- a/styles/src/styleTree/search.ts +++ b/styles/src/styleTree/search.ts @@ -80,5 +80,17 @@ export default function search(colorScheme: ColorScheme) { ...text(layer, "mono", "on"), size: 18, }, + dismissButton: { + color: foreground(layer, "variant"), + iconWidth: 12, + buttonWidth: 14, + padding: { + left: 10, + right: 10, + }, + hover: { + color: foreground(layer, "hovered"), + }, + }, }; } diff --git a/styles/src/styleTree/tabBar.ts b/styles/src/styleTree/tabBar.ts index fcf78dc73f..a4875cec06 100644 --- a/styles/src/styleTree/tabBar.ts +++ b/styles/src/styleTree/tabBar.ts @@ -26,7 +26,7 @@ export default function tabBar(colorScheme: ColorScheme) { // Close icons iconWidth: 8, iconClose: foreground(layer, "variant"), - iconCloseActive: foreground(layer), + iconCloseActive: foreground(layer, "hovered"), // Indicators iconConflict: foreground(layer, "warning"), From b94c2652402c7b9e69ea4b8dac6f433ba046069c Mon Sep 17 00:00:00 2001 From: Julia Date: Tue, 3 Jan 2023 13:50:08 -0500 Subject: [PATCH 12/33] Correct default settings' name key for RA in init options example --- assets/settings/default.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 0487b11c19..5b73d7643c 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -221,7 +221,7 @@ // rust-analyzer // typescript-language-server // vscode-json-languageserver - // "rust_analyzer": { + // "rust-analyzer": { // //These initialization options are merged into Zed's defaults // "initialization_options": { // "checkOnSave": { From 93a634991be58783ae2c15df3118412fe20f2578 Mon Sep 17 00:00:00 2001 From: Julia Date: Tue, 3 Jan 2023 16:37:35 -0500 Subject: [PATCH 13/33] Include Typescript completion item `detail` field in completion label --- crates/zed/src/languages/typescript.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/zed/src/languages/typescript.rs b/crates/zed/src/languages/typescript.rs index 95f56bce5b..f54b09ceda 100644 --- a/crates/zed/src/languages/typescript.rs +++ b/crates/zed/src/languages/typescript.rs @@ -128,8 +128,14 @@ impl LspAdapter for TypeScriptLspAdapter { Kind::PROPERTY | Kind::FIELD => grammar.highlight_id_for_name("property"), _ => None, }?; + + let text = match &item.detail { + Some(detail) => format!("{} {}", item.label, detail), + None => item.label.clone(), + }; + Some(language::CodeLabel { - text: item.label.clone(), + text, runs: vec![(0..len, highlight_id)], filter_range: 0..len, }) From 79f8f08caf656d7966c5e34e3038ef3c73c7436c Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 4 Jan 2023 11:45:25 -0800 Subject: [PATCH 14/33] v0.69.x dev --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ee7abcb70..bdbb3411f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8184,7 +8184,7 @@ checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" [[package]] name = "zed" -version = "0.68.0" +version = "0.69.0" dependencies = [ "activity_indicator", "anyhow", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index fc7a175355..d295deea71 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] description = "The fast, collaborative code editor." edition = "2021" name = "zed" -version = "0.68.0" +version = "0.69.0" [lib] name = "zed" From 09d57d1f26cb4b8dd8b83ee96a769d63a3ecb9b7 Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 5 Jan 2023 11:27:50 -0500 Subject: [PATCH 15/33] Prefer first max while fuzzy matching projects fixes unexpected behavior --- crates/recent_projects/src/recent_projects.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 42ff2b2f1c..02e15290ab 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -146,6 +146,7 @@ impl PickerDelegate for RecentProjectsView { .matches .iter() .enumerate() + .rev() .max_by_key(|(_, m)| OrderedFloat(m.score)) .map(|(ix, _)| ix) .unwrap_or(0); From 378f0c32fe35c104288f8f24468a0bed247defff Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 5 Jan 2023 16:41:23 -0800 Subject: [PATCH 16/33] Restructure callback subscriptions Fix a callback leak that would occur when dropping a subscription to a callback collection after triggering that callback, but before processing the effect of *adding* the handler. Co-authored-by: Kay Simmons --- crates/gpui/src/app.rs | 360 ++++----------------- crates/gpui/src/app/callback_collection.rs | 197 +++++++---- 2 files changed, 207 insertions(+), 350 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 76b9fb1aa6..dce4e68e7c 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -26,8 +26,8 @@ use smallvec::SmallVec; use smol::prelude::*; pub use action::*; -use callback_collection::{CallbackCollection, Mapping}; -use collections::{btree_map, hash_map::Entry, BTreeMap, HashMap, HashSet, VecDeque}; +use callback_collection::CallbackCollection; +use collections::{hash_map::Entry, BTreeMap, HashMap, HashSet, VecDeque}; use keymap::MatchResult; use platform::Event; #[cfg(any(test, feature = "test-support"))] @@ -1047,12 +1047,10 @@ impl MutableAppContext { callback(payload, cx) }), }); - - Subscription::GlobalSubscription { - id: subscription_id, - type_id, - subscriptions: Some(self.global_subscriptions.downgrade()), - } + Subscription::GlobalSubscription( + self.global_subscriptions + .subscribe(type_id, subscription_id), + ) } pub fn observe(&mut self, handle: &H, mut callback: F) -> Subscription @@ -1089,11 +1087,7 @@ impl MutableAppContext { } }), }); - Subscription::Subscription { - id: subscription_id, - entity_id: handle.id(), - subscriptions: Some(self.subscriptions.downgrade()), - } + Subscription::Subscription(self.subscriptions.subscribe(handle.id(), subscription_id)) } fn observe_internal(&mut self, handle: &H, mut callback: F) -> Subscription @@ -1117,11 +1111,7 @@ impl MutableAppContext { } }), }); - Subscription::Observation { - id: subscription_id, - entity_id, - observations: Some(self.observations.downgrade()), - } + Subscription::Observation(self.observations.subscribe(entity_id, subscription_id)) } fn observe_focus(&mut self, handle: &ViewHandle, mut callback: F) -> Subscription @@ -1144,12 +1134,7 @@ impl MutableAppContext { } }), }); - - Subscription::FocusObservation { - id: subscription_id, - view_id, - observations: Some(self.focus_observations.downgrade()), - } + Subscription::FocusObservation(self.focus_observations.subscribe(view_id, subscription_id)) } pub fn observe_global(&mut self, mut observe: F) -> Subscription @@ -1165,12 +1150,7 @@ impl MutableAppContext { id, Box::new(move |cx: &mut MutableAppContext| observe(cx)), ); - - Subscription::GlobalObservation { - id, - type_id, - observations: Some(self.global_observations.downgrade()), - } + Subscription::GlobalObservation(self.global_observations.subscribe(type_id, id)) } pub fn observe_default_global(&mut self, observe: F) -> Subscription @@ -1235,11 +1215,10 @@ impl MutableAppContext { subscription_id, callback: Box::new(callback), }); - Subscription::WindowActivationObservation { - id: subscription_id, - window_id, - observations: Some(self.window_activation_observations.downgrade()), - } + Subscription::WindowActivationObservation( + self.window_activation_observations + .subscribe(window_id, subscription_id), + ) } fn observe_fullscreen(&mut self, window_id: usize, callback: F) -> Subscription @@ -1253,11 +1232,10 @@ impl MutableAppContext { subscription_id, callback: Box::new(callback), }); - Subscription::WindowFullscreenObservation { - id: subscription_id, - window_id, - observations: Some(self.window_activation_observations.downgrade()), - } + Subscription::WindowActivationObservation( + self.window_activation_observations + .subscribe(window_id, subscription_id), + ) } pub fn observe_keystrokes(&mut self, window_id: usize, callback: F) -> Subscription @@ -1273,12 +1251,10 @@ impl MutableAppContext { let subscription_id = post_inc(&mut self.next_subscription_id); self.keystroke_observations .add_callback(window_id, subscription_id, Box::new(callback)); - - Subscription::KeystrokeObservation { - id: subscription_id, - window_id, - observations: Some(self.keystroke_observations.downgrade()), - } + Subscription::KeystrokeObservation( + self.keystroke_observations + .subscribe(window_id, subscription_id), + ) } pub fn defer(&mut self, callback: impl 'static + FnOnce(&mut MutableAppContext)) { @@ -1999,15 +1975,13 @@ impl MutableAppContext { entity_id, subscription_id, callback, - } => self.subscriptions.add_or_remove_callback( - entity_id, - subscription_id, - callback, - ), + } => self + .subscriptions + .add_callback(entity_id, subscription_id, callback), Effect::Event { entity_id, payload } => { let mut subscriptions = self.subscriptions.clone(); - subscriptions.emit_and_cleanup(entity_id, self, |callback, this| { + subscriptions.emit(entity_id, self, |callback, this| { callback(payload.as_ref(), this) }) } @@ -2016,7 +1990,7 @@ impl MutableAppContext { type_id, subscription_id, callback, - } => self.global_subscriptions.add_or_remove_callback( + } => self.global_subscriptions.add_callback( type_id, subscription_id, callback, @@ -2028,16 +2002,13 @@ impl MutableAppContext { entity_id, subscription_id, callback, - } => self.observations.add_or_remove_callback( - entity_id, - subscription_id, - callback, - ), + } => self + .observations + .add_callback(entity_id, subscription_id, callback), Effect::ModelNotification { model_id } => { let mut observations = self.observations.clone(); - observations - .emit_and_cleanup(model_id, self, |callback, this| callback(this)); + observations.emit(model_id, self, |callback, this| callback(this)); } Effect::ViewNotification { window_id, view_id } => { @@ -2046,7 +2017,7 @@ impl MutableAppContext { Effect::GlobalNotification { type_id } => { let mut subscriptions = self.global_observations.clone(); - subscriptions.emit_and_cleanup(type_id, self, |callback, this| { + subscriptions.emit(type_id, self, |callback, this| { callback(this); true }); @@ -2080,7 +2051,7 @@ impl MutableAppContext { subscription_id, callback, } => { - self.focus_observations.add_or_remove_callback( + self.focus_observations.add_callback( view_id, subscription_id, callback, @@ -2099,7 +2070,7 @@ impl MutableAppContext { window_id, subscription_id, callback, - } => self.window_activation_observations.add_or_remove_callback( + } => self.window_activation_observations.add_callback( window_id, subscription_id, callback, @@ -2114,7 +2085,7 @@ impl MutableAppContext { window_id, subscription_id, callback, - } => self.window_fullscreen_observations.add_or_remove_callback( + } => self.window_fullscreen_observations.add_callback( window_id, subscription_id, callback, @@ -2158,7 +2129,17 @@ impl MutableAppContext { self.pending_notifications.clear(); self.remove_dropped_entities(); } else { + self.focus_observations.gc(); + self.global_subscriptions.gc(); + self.global_observations.gc(); + self.subscriptions.gc(); + self.observations.gc(); + self.window_activation_observations.gc(); + self.window_fullscreen_observations.gc(); + self.keystroke_observations.gc(); + self.remove_dropped_entities(); + if refreshing { self.perform_window_refresh(); } else { @@ -2295,7 +2276,7 @@ impl MutableAppContext { let type_id = (&*payload).type_id(); let mut subscriptions = self.global_subscriptions.clone(); - subscriptions.emit_and_cleanup(type_id, self, |callback, this| { + subscriptions.emit(type_id, self, |callback, this| { callback(payload.as_ref(), this); true //Always alive }); @@ -2320,7 +2301,7 @@ impl MutableAppContext { } let mut observations = self.observations.clone(); - observations.emit_and_cleanup(observed_view_id, self, |callback, this| callback(this)); + observations.emit(observed_view_id, self, |callback, this| callback(this)); } } @@ -2350,7 +2331,7 @@ impl MutableAppContext { window.is_fullscreen = is_fullscreen; let mut observations = this.window_fullscreen_observations.clone(); - observations.emit_and_cleanup(window_id, this, |callback, this| { + observations.emit(window_id, this, |callback, this| { callback(is_fullscreen, this) }); @@ -2367,7 +2348,7 @@ impl MutableAppContext { ) { self.update(|this| { let mut observations = this.keystroke_observations.clone(); - observations.emit_and_cleanup(window_id, this, { + observations.emit(window_id, this, { move |callback, this| callback(&keystroke, &result, handled_by.as_ref(), this) }); }); @@ -2403,7 +2384,7 @@ impl MutableAppContext { } let mut observations = this.window_activation_observations.clone(); - observations.emit_and_cleanup(window_id, this, |callback, this| callback(active, this)); + observations.emit(window_id, this, |callback, this| callback(active, this)); Some(()) }); @@ -2443,8 +2424,7 @@ impl MutableAppContext { } let mut subscriptions = this.focus_observations.clone(); - subscriptions - .emit_and_cleanup(blurred_id, this, |callback, this| callback(false, this)); + subscriptions.emit(blurred_id, this, |callback, this| callback(false, this)); } if let Some(focused_id) = focused_id { @@ -2456,8 +2436,7 @@ impl MutableAppContext { } let mut subscriptions = this.focus_observations.clone(); - subscriptions - .emit_and_cleanup(focused_id, this, |callback, this| callback(true, this)); + subscriptions.emit(focused_id, this, |callback, this| callback(true, this)); } }) } @@ -5106,46 +5085,14 @@ impl Drop for ElementStateHandle { #[must_use] pub enum Subscription { - Subscription { - id: usize, - entity_id: usize, - subscriptions: Option>>, - }, - GlobalSubscription { - id: usize, - type_id: TypeId, - subscriptions: Option>>, - }, - Observation { - id: usize, - entity_id: usize, - observations: Option>>, - }, - GlobalObservation { - id: usize, - type_id: TypeId, - observations: Option>>, - }, - FocusObservation { - id: usize, - view_id: usize, - observations: Option>>, - }, - WindowActivationObservation { - id: usize, - window_id: usize, - observations: Option>>, - }, - WindowFullscreenObservation { - id: usize, - window_id: usize, - observations: Option>>, - }, - KeystrokeObservation { - id: usize, - window_id: usize, - observations: Option>>, - }, + Subscription(callback_collection::Subscription), + Observation(callback_collection::Subscription), + GlobalSubscription(callback_collection::Subscription), + GlobalObservation(callback_collection::Subscription), + FocusObservation(callback_collection::Subscription), + WindowActivationObservation(callback_collection::Subscription), + WindowFullscreenObservation(callback_collection::Subscription), + KeystrokeObservation(callback_collection::Subscription), ReleaseObservation { id: usize, @@ -5163,36 +5110,21 @@ pub enum Subscription { impl Subscription { pub fn detach(&mut self) { match self { - Subscription::Subscription { subscriptions, .. } => { - subscriptions.take(); - } - Subscription::GlobalSubscription { subscriptions, .. } => { - subscriptions.take(); - } - Subscription::Observation { observations, .. } => { - observations.take(); - } - Subscription::GlobalObservation { observations, .. } => { - observations.take(); - } + Subscription::Subscription(subscription) => subscription.detach(), + Subscription::GlobalSubscription(subscription) => subscription.detach(), + Subscription::Observation(subscription) => subscription.detach(), + Subscription::GlobalObservation(subscription) => subscription.detach(), + Subscription::FocusObservation(subscription) => subscription.detach(), + Subscription::KeystrokeObservation(subscription) => subscription.detach(), + Subscription::WindowActivationObservation(subscription) => subscription.detach(), + Subscription::WindowFullscreenObservation(subscription) => subscription.detach(), + Subscription::ReleaseObservation { observations, .. } => { observations.take(); } - Subscription::FocusObservation { observations, .. } => { - observations.take(); - } Subscription::ActionObservation { observations, .. } => { observations.take(); } - Subscription::KeystrokeObservation { observations, .. } => { - observations.take(); - } - Subscription::WindowActivationObservation { observations, .. } => { - observations.take(); - } - Subscription::WindowFullscreenObservation { observations, .. } => { - observations.take(); - } } } } @@ -5200,80 +5132,6 @@ impl Subscription { impl Drop for Subscription { fn drop(&mut self) { match self { - Subscription::Subscription { - id, - entity_id, - subscriptions, - } => { - if let Some(subscriptions) = subscriptions.as_ref().and_then(Weak::upgrade) { - match subscriptions - .lock() - .entry(*entity_id) - .or_default() - .entry(*id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } - Subscription::GlobalSubscription { - id, - type_id, - subscriptions, - } => { - if let Some(subscriptions) = subscriptions.as_ref().and_then(Weak::upgrade) { - match subscriptions.lock().entry(*type_id).or_default().entry(*id) { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } - Subscription::Observation { - id, - entity_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - match observations - .lock() - .entry(*entity_id) - .or_default() - .entry(*id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } - Subscription::GlobalObservation { - id, - type_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - match observations.lock().entry(*type_id).or_default().entry(*id) { - collections::btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - collections::btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } Subscription::ReleaseObservation { id, entity_id, @@ -5285,90 +5143,12 @@ impl Drop for Subscription { } } } - Subscription::FocusObservation { - id, - view_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - match observations.lock().entry(*view_id).or_default().entry(*id) { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } Subscription::ActionObservation { id, observations } => { if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { observations.lock().remove(id); } } - Subscription::KeystrokeObservation { - id, - window_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - match observations - .lock() - .entry(*window_id) - .or_default() - .entry(*id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } - Subscription::WindowActivationObservation { - id, - window_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - match observations - .lock() - .entry(*window_id) - .or_default() - .entry(*id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } - Subscription::WindowFullscreenObservation { - id, - window_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - match observations - .lock() - .entry(*window_id) - .or_default() - .entry(*id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(None); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } - } - } + _ => {} } } } diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 4ec90fbac0..4494e9073f 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -1,19 +1,70 @@ +use std::mem; use std::sync::Arc; use std::{hash::Hash, sync::Weak}; use parking_lot::Mutex; -use collections::{btree_map, BTreeMap, HashMap}; +use collections::{btree_map, BTreeMap, HashMap, HashSet}; use crate::MutableAppContext; -pub type Mapping = Mutex>>>; +// Problem 5: Current bug callbacks currently called many times after being dropped +// update +// notify +// observe (push effect) +// subscription {id : 5} +// pending: [Effect::Notify, Effect::observe { id: 5 }] +// drop observation subscription (write None into subscriptions) +// flush effects +// notify +// observe +// Problem 6: Key-value pair is leaked if you drop a callback while calling it, and then never call that set of callbacks again +// ----------------- +// Problem 1: Many very similar subscription enum variants +// Problem 2: Subscriptions and CallbackCollections use a shared mutex to update the callback status +// Problem 3: Current implementation is error prone with regard to uninitialized callbacks or dropping during callback +// Problem 4: Calling callbacks requires removing all of them from the list and adding them back -pub struct CallbackCollection { - internal: Arc>, +// Solution 1 CallbackState: +// Add more state to the CallbackCollection map to communicate dropped and initialized status +// Solves: P5 +// Solution 2 DroppedSubscriptionList: +// Store a parallel set of dropped subscriptions in the Mapping which stores the key and subscription id for all dropped subscriptions +// which can be +// Solution 3 GarbageCollection: +// Use some type of traditional garbage collection to handle dropping of callbacks +// atomic flag per callback which is looped over in remove dropped entities + +// TODO: +// - Move subscription id counter to CallbackCollection +// - Consider adding a reverse map in Mapping from subscription id to key so that the dropped subscriptions +// can be a hashset of usize and the Subscription doesn't need the key +// - Investigate why the remaining two types of callback lists can't use the same callback collection and subscriptions +pub struct Subscription { + key: K, + id: usize, + mapping: Option>>>, } -impl Clone for CallbackCollection { +struct Mapping { + callbacks: HashMap>, + dropped_subscriptions: HashSet<(K, usize)>, +} + +impl Default for Mapping { + fn default() -> Self { + Self { + callbacks: Default::default(), + dropped_subscriptions: Default::default(), + } + } +} + +pub(crate) struct CallbackCollection { + internal: Arc>>, +} + +impl Clone for CallbackCollection { fn clone(&self) -> Self { Self { internal: self.internal.clone(), @@ -21,7 +72,7 @@ impl Clone for CallbackCollection { } } -impl Default for CallbackCollection { +impl Default for CallbackCollection { fn default() -> Self { CallbackCollection { internal: Arc::new(Mutex::new(Default::default())), @@ -29,75 +80,101 @@ impl Default for CallbackCollection { } } -impl CallbackCollection { - pub fn downgrade(&self) -> Weak> { - Arc::downgrade(&self.internal) - } - +impl CallbackCollection { #[cfg(test)] pub fn is_empty(&self) -> bool { - self.internal.lock().is_empty() + self.internal.lock().callbacks.is_empty() } - pub fn add_callback(&mut self, id: K, subscription_id: usize, callback: F) { + pub fn subscribe(&mut self, key: K, subscription_id: usize) -> Subscription { + Subscription { + key, + id: subscription_id, + mapping: Some(Arc::downgrade(&self.internal)), + } + } + + pub fn count(&mut self, key: K) -> usize { self.internal .lock() - .entry(id) - .or_default() - .insert(subscription_id, Some(callback)); + .callbacks + .get(&key) + .map_or(0, |callbacks| callbacks.len()) } - pub fn remove(&mut self, id: K) { - self.internal.lock().remove(&id); + pub fn add_callback(&mut self, key: K, subscription_id: usize, callback: F) { + let mut this = self.internal.lock(); + if !this.dropped_subscriptions.contains(&(key, subscription_id)) { + this.callbacks + .entry(key) + .or_default() + .insert(subscription_id, callback); + } } - pub fn add_or_remove_callback(&mut self, id: K, subscription_id: usize, callback: F) { - match self - .internal - .lock() - .entry(id) - .or_default() - .entry(subscription_id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(Some(callback)); - } + pub fn remove(&mut self, key: K) { + self.internal.lock().callbacks.remove(&key); + } - btree_map::Entry::Occupied(entry) => { - // TODO: This seems like it should never be called because no code - // should ever attempt to remove an existing callback - debug_assert!(entry.get().is_none()); - entry.remove(); + pub fn emit bool>( + &mut self, + key: K, + cx: &mut MutableAppContext, + mut call_callback: C, + ) { + let callbacks = self.internal.lock().callbacks.remove(&key); + if let Some(callbacks) = callbacks { + for (subscription_id, mut callback) in callbacks { + if !self + .internal + .lock() + .dropped_subscriptions + .contains(&(key, subscription_id)) + { + if call_callback(&mut callback, cx) { + self.add_callback(key, subscription_id, callback); + } + } } } } - pub fn emit_and_cleanup bool>( - &mut self, - id: K, - cx: &mut MutableAppContext, - mut call_callback: C, - ) { - let callbacks = self.internal.lock().remove(&id); - if let Some(callbacks) = callbacks { - for (subscription_id, callback) in callbacks { - if let Some(mut callback) = callback { - let alive = call_callback(&mut callback, cx); - if alive { - match self - .internal - .lock() - .entry(id) - .or_default() - .entry(subscription_id) - { - btree_map::Entry::Vacant(entry) => { - entry.insert(Some(callback)); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - } - } + pub fn gc(&mut self) { + let mut this = self.internal.lock(); + + for (key, id) in mem::take(&mut this.dropped_subscriptions) { + if let Some(callbacks) = this.callbacks.get_mut(&key) { + callbacks.remove(&id); + } + } + } +} + +impl Subscription { + pub fn detach(&mut self) { + self.mapping.take(); + } +} + +impl Drop for Subscription { + // If the callback has been initialized (no callback in the list for the key and id), + // add this subscription id and key to the dropped subscriptions list + // Otherwise, just remove the associated callback from the callback collection + fn drop(&mut self) { + if let Some(mapping) = self.mapping.as_ref().and_then(|mapping| mapping.upgrade()) { + let mut mapping = mapping.lock(); + if let Some(callbacks) = mapping.callbacks.get_mut(&self.key) { + match callbacks.entry(self.id) { + btree_map::Entry::Vacant(_) => { + mapping + .dropped_subscriptions + .insert((self.key.clone(), self.id)); + } + btree_map::Entry::Occupied(entry) => { + entry.remove(); + mapping + .dropped_subscriptions + .insert((self.key.clone(), self.id)); } } } From fa620bf98fc48cbe07e9a42c3cef95273c90c672 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 5 Jan 2023 17:30:39 -0800 Subject: [PATCH 17/33] Fix logic error in dropping callback subscriptions Co-authored-by: Kay Simmons --- crates/gpui/src/app/callback_collection.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 4494e9073f..38a8dae26c 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -1,10 +1,9 @@ -use std::mem; use std::sync::Arc; use std::{hash::Hash, sync::Weak}; use parking_lot::Mutex; -use collections::{btree_map, BTreeMap, HashMap, HashSet}; +use collections::{BTreeMap, HashMap, HashSet}; use crate::MutableAppContext; @@ -142,7 +141,7 @@ impl CallbackCollection { pub fn gc(&mut self) { let mut this = self.internal.lock(); - for (key, id) in mem::take(&mut this.dropped_subscriptions) { + for (key, id) in std::mem::take(&mut this.dropped_subscriptions) { if let Some(callbacks) = this.callbacks.get_mut(&key) { callbacks.remove(&id); } @@ -164,20 +163,13 @@ impl Drop for Subscription { if let Some(mapping) = self.mapping.as_ref().and_then(|mapping| mapping.upgrade()) { let mut mapping = mapping.lock(); if let Some(callbacks) = mapping.callbacks.get_mut(&self.key) { - match callbacks.entry(self.id) { - btree_map::Entry::Vacant(_) => { - mapping - .dropped_subscriptions - .insert((self.key.clone(), self.id)); - } - btree_map::Entry::Occupied(entry) => { - entry.remove(); - mapping - .dropped_subscriptions - .insert((self.key.clone(), self.id)); - } + if callbacks.remove(&self.id).is_some() { + return; } } + mapping + .dropped_subscriptions + .insert((self.key.clone(), self.id)); } } } From 82e9f736bd9050a75129ca3cc0de33ba5a1b0894 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 5 Jan 2023 18:02:53 -0800 Subject: [PATCH 18/33] Use a CallbackCollection for release observations Co-authored-by: Kay Simmons --- crates/gpui/src/app.rs | 79 +++++++++++++++--------------------------- 1 file changed, 28 insertions(+), 51 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index dce4e68e7c..fa01196d4b 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -588,9 +588,9 @@ type GlobalActionCallback = dyn FnMut(&dyn Action, &mut MutableAppContext); type SubscriptionCallback = Box bool>; type GlobalSubscriptionCallback = Box; type ObservationCallback = Box bool>; -type FocusObservationCallback = Box bool>; type GlobalObservationCallback = Box; -type ReleaseObservationCallback = Box; +type FocusObservationCallback = Box bool>; +type ReleaseObservationCallback = Box; type ActionObservationCallback = Box; type WindowActivationCallback = Box bool>; type WindowFullscreenCallback = Box bool>; @@ -615,18 +615,17 @@ pub struct MutableAppContext { next_subscription_id: usize, frame_count: usize, - focus_observations: CallbackCollection, - global_subscriptions: CallbackCollection, - global_observations: CallbackCollection, subscriptions: CallbackCollection, + global_subscriptions: CallbackCollection, observations: CallbackCollection, + global_observations: CallbackCollection, + focus_observations: CallbackCollection, + release_observations: CallbackCollection, + action_dispatch_observations: Arc>>, window_activation_observations: CallbackCollection, window_fullscreen_observations: CallbackCollection, keystroke_observations: CallbackCollection, - release_observations: Arc>>>, - action_dispatch_observations: Arc>>, - #[allow(clippy::type_complexity)] presenters_and_platform_windows: HashMap>, Box)>, @@ -1172,22 +1171,18 @@ impl MutableAppContext { F: 'static + FnOnce(&E, &mut Self), { let id = post_inc(&mut self.next_subscription_id); - self.release_observations - .lock() - .entry(handle.id()) - .or_default() - .insert( - id, - Box::new(move |released, cx| { - let released = released.downcast_ref().unwrap(); - callback(released, cx) - }), - ); - Subscription::ReleaseObservation { + let mut callback = Some(callback); + self.release_observations.add_callback( + handle.id(), id, - entity_id: handle.id(), - observations: Some(Arc::downgrade(&self.release_observations)), - } + Box::new(move |released, cx| { + let released = released.downcast_ref().unwrap(); + if let Some(callback) = callback.take() { + callback(released, cx) + } + }), + ); + Subscription::ReleaseObservation(self.release_observations.subscribe(handle.id(), id)) } pub fn observe_actions(&mut self, callback: F) -> Subscription @@ -2137,6 +2132,7 @@ impl MutableAppContext { self.window_activation_observations.gc(); self.window_fullscreen_observations.gc(); self.keystroke_observations.gc(); + self.release_observations.gc(); self.remove_dropped_entities(); @@ -2306,12 +2302,13 @@ impl MutableAppContext { } fn handle_entity_release_effect(&mut self, entity_id: usize, entity: &dyn Any) { - let callbacks = self.release_observations.lock().remove(&entity_id); - if let Some(callbacks) = callbacks { - for (_, callback) in callbacks { - callback(entity, self); - } - } + self.release_observations + .clone() + .emit(entity_id, self, |callback, this| { + callback(entity, this); + // Release observations happen one time. So clear the callback by returning false + false + }) } fn handle_fullscreen_effect(&mut self, window_id: usize, is_fullscreen: bool) { @@ -5093,14 +5090,8 @@ pub enum Subscription { WindowActivationObservation(callback_collection::Subscription), WindowFullscreenObservation(callback_collection::Subscription), KeystrokeObservation(callback_collection::Subscription), + ReleaseObservation(callback_collection::Subscription), - ReleaseObservation { - id: usize, - entity_id: usize, - #[allow(clippy::type_complexity)] - observations: - Option>>>>, - }, ActionObservation { id: usize, observations: Option>>>, @@ -5118,10 +5109,7 @@ impl Subscription { Subscription::KeystrokeObservation(subscription) => subscription.detach(), Subscription::WindowActivationObservation(subscription) => subscription.detach(), Subscription::WindowFullscreenObservation(subscription) => subscription.detach(), - - Subscription::ReleaseObservation { observations, .. } => { - observations.take(); - } + Subscription::ReleaseObservation(subscription) => subscription.detach(), Subscription::ActionObservation { observations, .. } => { observations.take(); } @@ -5132,17 +5120,6 @@ impl Subscription { impl Drop for Subscription { fn drop(&mut self) { match self { - Subscription::ReleaseObservation { - id, - entity_id, - observations, - } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - if let Some(observations) = observations.lock().get_mut(entity_id) { - observations.remove(id); - } - } - } Subscription::ActionObservation { id, observations } => { if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { observations.lock().remove(id); From 3da69117ae3bea0f9fef7503b79d37bc295c12fb Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 6 Jan 2023 09:15:53 -0800 Subject: [PATCH 19/33] Use a CallbackCollection for action dispatch observations --- crates/gpui/src/app.rs | 66 ++++++++++------------ crates/gpui/src/app/callback_collection.rs | 51 ++++------------- 2 files changed, 41 insertions(+), 76 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index fa01196d4b..aa1fd39015 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -27,7 +27,7 @@ use smol::prelude::*; pub use action::*; use callback_collection::CallbackCollection; -use collections::{hash_map::Entry, BTreeMap, HashMap, HashSet, VecDeque}; +use collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; use keymap::MatchResult; use platform::Event; #[cfg(any(test, feature = "test-support"))] @@ -621,7 +621,7 @@ pub struct MutableAppContext { global_observations: CallbackCollection, focus_observations: CallbackCollection, release_observations: CallbackCollection, - action_dispatch_observations: Arc>>, + action_dispatch_observations: CallbackCollection<(), ActionObservationCallback>, window_activation_observations: CallbackCollection, window_fullscreen_observations: CallbackCollection, keystroke_observations: CallbackCollection, @@ -1189,14 +1189,13 @@ impl MutableAppContext { where F: 'static + FnMut(TypeId, &mut MutableAppContext), { - let id = post_inc(&mut self.next_subscription_id); + let subscription_id = post_inc(&mut self.next_subscription_id); self.action_dispatch_observations - .lock() - .insert(id, Box::new(callback)); - Subscription::ActionObservation { - id, - observations: Some(Arc::downgrade(&self.action_dispatch_observations)), - } + .add_callback((), subscription_id, Box::new(callback)); + Subscription::ActionObservation( + self.action_dispatch_observations + .subscribe((), subscription_id), + ) } fn observe_window_activation(&mut self, window_id: usize, callback: F) -> Subscription @@ -2489,11 +2488,12 @@ impl MutableAppContext { } fn handle_action_dispatch_notification_effect(&mut self, action_id: TypeId) { - let mut callbacks = mem::take(&mut *self.action_dispatch_observations.lock()); - for callback in callbacks.values_mut() { - callback(action_id, self); - } - self.action_dispatch_observations.lock().extend(callbacks); + self.action_dispatch_observations + .clone() + .emit((), self, |callback, this| { + callback(action_id, this); + true + }); } fn handle_window_should_close_subscription_effect( @@ -5091,14 +5091,25 @@ pub enum Subscription { WindowFullscreenObservation(callback_collection::Subscription), KeystrokeObservation(callback_collection::Subscription), ReleaseObservation(callback_collection::Subscription), - - ActionObservation { - id: usize, - observations: Option>>>, - }, + ActionObservation(callback_collection::Subscription<(), ActionObservationCallback>), } impl Subscription { + pub fn id(&self) -> usize { + match self { + Subscription::Subscription(subscription) => subscription.id(), + Subscription::Observation(subscription) => subscription.id(), + Subscription::GlobalSubscription(subscription) => subscription.id(), + Subscription::GlobalObservation(subscription) => subscription.id(), + Subscription::FocusObservation(subscription) => subscription.id(), + Subscription::WindowActivationObservation(subscription) => subscription.id(), + Subscription::WindowFullscreenObservation(subscription) => subscription.id(), + Subscription::KeystrokeObservation(subscription) => subscription.id(), + Subscription::ReleaseObservation(subscription) => subscription.id(), + Subscription::ActionObservation(subscription) => subscription.id(), + } + } + pub fn detach(&mut self) { match self { Subscription::Subscription(subscription) => subscription.detach(), @@ -5110,22 +5121,7 @@ impl Subscription { Subscription::WindowActivationObservation(subscription) => subscription.detach(), Subscription::WindowFullscreenObservation(subscription) => subscription.detach(), Subscription::ReleaseObservation(subscription) => subscription.detach(), - Subscription::ActionObservation { observations, .. } => { - observations.take(); - } - } - } -} - -impl Drop for Subscription { - fn drop(&mut self) { - match self { - Subscription::ActionObservation { id, observations } => { - if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { - observations.lock().remove(id); - } - } - _ => {} + Subscription::ActionObservation(subscription) => subscription.detach(), } } } diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 38a8dae26c..5bed9f7a29 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -1,44 +1,13 @@ +use crate::MutableAppContext; +use collections::{BTreeMap, HashMap, HashSet}; +use parking_lot::Mutex; use std::sync::Arc; use std::{hash::Hash, sync::Weak}; -use parking_lot::Mutex; +pub struct CallbackCollection { + internal: Arc>>, +} -use collections::{BTreeMap, HashMap, HashSet}; - -use crate::MutableAppContext; - -// Problem 5: Current bug callbacks currently called many times after being dropped -// update -// notify -// observe (push effect) -// subscription {id : 5} -// pending: [Effect::Notify, Effect::observe { id: 5 }] -// drop observation subscription (write None into subscriptions) -// flush effects -// notify -// observe -// Problem 6: Key-value pair is leaked if you drop a callback while calling it, and then never call that set of callbacks again -// ----------------- -// Problem 1: Many very similar subscription enum variants -// Problem 2: Subscriptions and CallbackCollections use a shared mutex to update the callback status -// Problem 3: Current implementation is error prone with regard to uninitialized callbacks or dropping during callback -// Problem 4: Calling callbacks requires removing all of them from the list and adding them back - -// Solution 1 CallbackState: -// Add more state to the CallbackCollection map to communicate dropped and initialized status -// Solves: P5 -// Solution 2 DroppedSubscriptionList: -// Store a parallel set of dropped subscriptions in the Mapping which stores the key and subscription id for all dropped subscriptions -// which can be -// Solution 3 GarbageCollection: -// Use some type of traditional garbage collection to handle dropping of callbacks -// atomic flag per callback which is looped over in remove dropped entities - -// TODO: -// - Move subscription id counter to CallbackCollection -// - Consider adding a reverse map in Mapping from subscription id to key so that the dropped subscriptions -// can be a hashset of usize and the Subscription doesn't need the key -// - Investigate why the remaining two types of callback lists can't use the same callback collection and subscriptions pub struct Subscription { key: K, id: usize, @@ -59,10 +28,6 @@ impl Default for Mapping { } } -pub(crate) struct CallbackCollection { - internal: Arc>>, -} - impl Clone for CallbackCollection { fn clone(&self) -> Self { Self { @@ -150,6 +115,10 @@ impl CallbackCollection { } impl Subscription { + pub fn id(&self) -> usize { + self.id + } + pub fn detach(&mut self) { self.mapping.take(); } From a165cd596bdc8b71a47130b9b8281b1f30659254 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 6 Jan 2023 09:56:29 -0800 Subject: [PATCH 20/33] Make event tests in gpui more consistent --- crates/gpui/src/app.rs | 254 +++++++++++------------------------------ 1 file changed, 68 insertions(+), 186 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index aa1fd39015..3299fbb746 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -5768,60 +5768,44 @@ mod tests { #[crate::test(self)] fn test_view_events(cx: &mut MutableAppContext) { - #[derive(Default)] - struct View { - events: Vec, - } - - impl Entity for View { - type Event = usize; - } - - impl super::View for View { - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - - fn ui_name() -> &'static str { - "View" - } - } - struct Model; impl Entity for Model { - type Event = usize; + type Event = String; } - let (_, handle_1) = cx.add_window(Default::default(), |_| View::default()); - let handle_2 = cx.add_view(&handle_1, |_| View::default()); + let (_, handle_1) = cx.add_window(Default::default(), |_| TestView::default()); + let handle_2 = cx.add_view(&handle_1, |_| TestView::default()); let handle_3 = cx.add_model(|_| Model); handle_1.update(cx, |_, cx| { cx.subscribe(&handle_2, move |me, emitter, event, cx| { - me.events.push(*event); + me.events.push(event.clone()); cx.subscribe(&emitter, |me, _, event, _| { - me.events.push(*event * 2); + me.events.push(format!("{event} from inner")); }) .detach(); }) .detach(); cx.subscribe(&handle_3, |me, _, event, _| { - me.events.push(*event); + me.events.push(event.clone()); }) .detach(); }); - handle_2.update(cx, |_, c| c.emit(7)); - assert_eq!(handle_1.read(cx).events, vec![7]); + handle_2.update(cx, |_, c| c.emit("7".into())); + assert_eq!(handle_1.read(cx).events, vec!["7"]); - handle_2.update(cx, |_, c| c.emit(5)); - assert_eq!(handle_1.read(cx).events, vec![7, 5, 10]); + handle_2.update(cx, |_, c| c.emit("5".into())); + assert_eq!(handle_1.read(cx).events, vec!["7", "5", "5 from inner"]); - handle_3.update(cx, |_, c| c.emit(9)); - assert_eq!(handle_1.read(cx).events, vec![7, 5, 10, 9]); + handle_3.update(cx, |_, c| c.emit("9".into())); + assert_eq!( + handle_1.read(cx).events, + vec!["7", "5", "5 from inner", "9"] + ); } #[crate::test(self)] @@ -6012,31 +5996,15 @@ mod tests { #[crate::test(self)] fn test_dropping_subscribers(cx: &mut MutableAppContext) { - struct View; - - impl Entity for View { - type Event = (); - } - - impl super::View for View { - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - - fn ui_name() -> &'static str { - "View" - } - } - struct Model; impl Entity for Model { type Event = (); } - let (_, root_view) = cx.add_window(Default::default(), |_| View); - let observing_view = cx.add_view(&root_view, |_| View); - let emitting_view = cx.add_view(&root_view, |_| View); + let (_, root_view) = cx.add_window(Default::default(), |_| TestView::default()); + let observing_view = cx.add_view(&root_view, |_| TestView::default()); + let emitting_view = cx.add_view(&root_view, |_| TestView::default()); let observing_model = cx.add_model(|_| Model); let observed_model = cx.add_model(|_| Model); @@ -6053,165 +6021,92 @@ mod tests { drop(observing_model); }); - emitting_view.update(cx, |_, cx| cx.emit(())); + emitting_view.update(cx, |_, cx| cx.emit(Default::default())); observed_model.update(cx, |_, cx| cx.emit(())); } #[crate::test(self)] fn test_view_emit_before_subscribe_in_same_update_cycle(cx: &mut MutableAppContext) { - #[derive(Default)] - struct TestView; - - impl Entity for TestView { - type Event = (); - } - - impl View for TestView { - fn ui_name() -> &'static str { - "TestView" - } - - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - } - - let events = Rc::new(RefCell::new(Vec::new())); - cx.add_window(Default::default(), |cx| { + let (_, view) = cx.add_window::(Default::default(), |cx| { drop(cx.subscribe(&cx.handle(), { - let events = events.clone(); - move |_, _, _, _| events.borrow_mut().push("dropped before flush") + move |this, _, _, _| this.events.push("dropped before flush".into()) })); cx.subscribe(&cx.handle(), { - let events = events.clone(); - move |_, _, _, _| events.borrow_mut().push("before emit") + move |this, _, _, _| this.events.push("before emit".into()) }) .detach(); - cx.emit(()); + cx.emit("the event".into()); cx.subscribe(&cx.handle(), { - let events = events.clone(); - move |_, _, _, _| events.borrow_mut().push("after emit") + move |this, _, _, _| this.events.push("after emit".into()) }) .detach(); - TestView + TestView { events: Vec::new() } }); - assert_eq!(*events.borrow(), ["before emit"]); + + assert_eq!(view.read(cx).events, ["before emit"]); } #[crate::test(self)] fn test_observe_and_notify_from_view(cx: &mut MutableAppContext) { - #[derive(Default)] - struct View { - events: Vec, - } - - impl Entity for View { - type Event = usize; - } - - impl super::View for View { - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - - fn ui_name() -> &'static str { - "View" - } - } - #[derive(Default)] struct Model { - count: usize, + state: String, } impl Entity for Model { type Event = (); } - let (_, view) = cx.add_window(Default::default(), |_| View::default()); - let model = cx.add_model(|_| Model::default()); + let (_, view) = cx.add_window(Default::default(), |_| TestView::default()); + let model = cx.add_model(|_| Model { + state: "old-state".into(), + }); view.update(cx, |_, c| { c.observe(&model, |me, observed, c| { - me.events.push(observed.read(c).count) + me.events.push(observed.read(c).state.clone()) }) .detach(); }); - model.update(cx, |model, c| { - model.count = 11; - c.notify(); + model.update(cx, |model, cx| { + model.state = "new-state".into(); + cx.notify(); }); - assert_eq!(view.read(cx).events, vec![11]); + assert_eq!(view.read(cx).events, vec!["new-state"]); } #[crate::test(self)] fn test_view_notify_before_observe_in_same_update_cycle(cx: &mut MutableAppContext) { - #[derive(Default)] - struct TestView; - - impl Entity for TestView { - type Event = (); - } - - impl View for TestView { - fn ui_name() -> &'static str { - "TestView" - } - - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - } - - let events = Rc::new(RefCell::new(Vec::new())); - cx.add_window(Default::default(), |cx| { + let (_, view) = cx.add_window::(Default::default(), |cx| { drop(cx.observe(&cx.handle(), { - let events = events.clone(); - move |_, _, _| events.borrow_mut().push("dropped before flush") + move |this, _, _| this.events.push("dropped before flush".into()) })); cx.observe(&cx.handle(), { - let events = events.clone(); - move |_, _, _| events.borrow_mut().push("before notify") + move |this, _, _| this.events.push("before notify".into()) }) .detach(); cx.notify(); cx.observe(&cx.handle(), { - let events = events.clone(); - move |_, _, _| events.borrow_mut().push("after notify") + move |this, _, _| this.events.push("after notify".into()) }) .detach(); - TestView + TestView { events: Vec::new() } }); - assert_eq!(*events.borrow(), ["before notify"]); + + assert_eq!(view.read(cx).events, ["before notify"]); } #[crate::test(self)] fn test_dropping_observers(cx: &mut MutableAppContext) { - struct View; - - impl Entity for View { - type Event = (); - } - - impl super::View for View { - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - - fn ui_name() -> &'static str { - "View" - } - } - struct Model; impl Entity for Model { type Event = (); } - let (_, root_view) = cx.add_window(Default::default(), |_| View); - let observing_view = cx.add_view(root_view, |_| View); + let (_, root_view) = cx.add_window(Default::default(), |_| TestView::default()); + let observing_view = cx.add_view(root_view, |_| TestView::default()); let observing_model = cx.add_model(|_| Model); let observed_model = cx.add_model(|_| Model); @@ -6927,47 +6822,15 @@ mod tests { #[crate::test(self)] #[should_panic] async fn test_view_condition_timeout(cx: &mut TestAppContext) { - struct View; - - impl super::Entity for View { - type Event = (); - } - - impl super::View for View { - fn ui_name() -> &'static str { - "test view" - } - - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - } - - let (_, view) = cx.add_window(|_| View); + let (_, view) = cx.add_window(|_| TestView::default()); view.condition(cx, |_, _| false).await; } #[crate::test(self)] #[should_panic(expected = "view dropped with pending condition")] async fn test_view_condition_panic_on_drop(cx: &mut TestAppContext) { - struct View; - - impl super::Entity for View { - type Event = (); - } - - impl super::View for View { - fn ui_name() -> &'static str { - "test view" - } - - fn render(&mut self, _: &mut RenderContext) -> ElementBox { - Empty::new().boxed() - } - } - - let (_, root_view) = cx.add_window(|_| View); - let view = cx.add_view(&root_view, |_| View); + let (_, root_view) = cx.add_window(|_| TestView::default()); + let view = cx.add_view(&root_view, |_| TestView::default()); let condition = view.condition(cx, |_, _| false); cx.update(|_| drop(view)); @@ -7180,4 +7043,23 @@ mod tests { assert!(!child_rendered.take()); assert!(child_dropped.take()); } + + #[derive(Default)] + struct TestView { + events: Vec, + } + + impl Entity for TestView { + type Event = String; + } + + impl View for TestView { + fn ui_name() -> &'static str { + "TestView" + } + + fn render(&mut self, _: &mut RenderContext) -> ElementBox { + Empty::new().boxed() + } + } } From 4708f5d88f85e6821c62deea2bfd3cfc1e0f37eb Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 6 Jan 2023 10:44:37 -0800 Subject: [PATCH 21/33] Add test for notifying and dropping subscriptions in an update cycle --- crates/gpui/src/app.rs | 25 ++++++++++++++++++++++ crates/gpui/src/app/callback_collection.rs | 12 +++-------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 3299fbb746..4a5b6f54be 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -6097,6 +6097,31 @@ mod tests { assert_eq!(view.read(cx).events, ["before notify"]); } + #[crate::test(self)] + fn test_notify_and_drop_observe_subscription_in_same_update_cycle(cx: &mut MutableAppContext) { + struct Model; + impl Entity for Model { + type Event = (); + } + + let model = cx.add_model(|_| Model); + let (_, view) = cx.add_window(Default::default(), |_| TestView::default()); + + view.update(cx, |_, cx| { + model.update(cx, |_, cx| cx.notify()); + drop(cx.observe(&model, move |this, _, _| { + this.events.push("model notified".into()); + })); + model.update(cx, |_, cx| cx.notify()); + }); + + for _ in 0..3 { + model.update(cx, |_, cx| cx.notify()); + } + + assert_eq!(view.read(cx).events, Vec::::new()); + } + #[crate::test(self)] fn test_dropping_observers(cx: &mut MutableAppContext) { struct Model; diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 5bed9f7a29..43f4f3f62e 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -58,14 +58,6 @@ impl CallbackCollection { } } - pub fn count(&mut self, key: K) -> usize { - self.internal - .lock() - .callbacks - .get(&key) - .map_or(0, |callbacks| callbacks.len()) - } - pub fn add_callback(&mut self, key: K, subscription_id: usize, callback: F) { let mut this = self.internal.lock(); if !this.dropped_subscriptions.contains(&(key, subscription_id)) { @@ -77,7 +69,9 @@ impl CallbackCollection { } pub fn remove(&mut self, key: K) { - self.internal.lock().callbacks.remove(&key); + let callbacks = self.internal.lock().callbacks.remove(&key); + // drop these after releasing the lock + drop(callbacks); } pub fn emit bool>( From ef192a902a030ee2a4a15c054f23563dc8e91ee6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 6 Jan 2023 11:03:45 -0800 Subject: [PATCH 22/33] Remove dropped subscription eagerly when removing callbacks --- crates/gpui/src/app/callback_collection.rs | 48 +++++++++++++++------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 43f4f3f62e..44db224967 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -60,17 +60,24 @@ impl CallbackCollection { pub fn add_callback(&mut self, key: K, subscription_id: usize, callback: F) { let mut this = self.internal.lock(); - if !this.dropped_subscriptions.contains(&(key, subscription_id)) { - this.callbacks - .entry(key) - .or_default() - .insert(subscription_id, callback); + + // If this callback's subscription was dropped before the callback was + // added, then just drop the callback. + if this.dropped_subscriptions.remove(&(key, subscription_id)) { + return; } + + this.callbacks + .entry(key) + .or_default() + .insert(subscription_id, callback); } pub fn remove(&mut self, key: K) { let callbacks = self.internal.lock().callbacks.remove(&key); - // drop these after releasing the lock + + // Drop these callbacks after releasing the lock, in case one of them + // owns a subscription to this callback collection. drop(callbacks); } @@ -83,15 +90,19 @@ impl CallbackCollection { let callbacks = self.internal.lock().callbacks.remove(&key); if let Some(callbacks) = callbacks { for (subscription_id, mut callback) in callbacks { - if !self + // If this callback's subscription was dropped while invoking an + // earlier callback, then just drop this callback. + if self .internal .lock() .dropped_subscriptions - .contains(&(key, subscription_id)) + .remove(&(key, subscription_id)) { - if call_callback(&mut callback, cx) { - self.add_callback(key, subscription_id, callback); - } + continue; + } + + if call_callback(&mut callback, cx) { + self.add_callback(key, subscription_id, callback); } } } @@ -119,17 +130,24 @@ impl Subscription { } impl Drop for Subscription { - // If the callback has been initialized (no callback in the list for the key and id), - // add this subscription id and key to the dropped subscriptions list - // Otherwise, just remove the associated callback from the callback collection fn drop(&mut self) { if let Some(mapping) = self.mapping.as_ref().and_then(|mapping| mapping.upgrade()) { let mut mapping = mapping.lock(); + + // If the callback is present in the mapping, then just remove it. if let Some(callbacks) = mapping.callbacks.get_mut(&self.key) { - if callbacks.remove(&self.id).is_some() { + let callback = callbacks.remove(&self.id); + if callback.is_some() { + drop(mapping); + drop(callback); return; } } + + // If this subscription's callback is not present, then either it has been + // temporarily removed during emit, or it has not yet been added. Record + // that this subscription has been dropped so that the callback can be + // removed later. mapping .dropped_subscriptions .insert((self.key.clone(), self.id)); From 53cb3a4429cbce6417fbf1dc07c87f108f8453f2 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 6 Jan 2023 11:33:50 -0800 Subject: [PATCH 23/33] Remove GC step for callback collections, always drop callbacks asap --- crates/gpui/src/app.rs | 10 ---- crates/gpui/src/app/callback_collection.rs | 65 ++++++++++++++-------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 4a5b6f54be..5568155cf7 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -2123,16 +2123,6 @@ impl MutableAppContext { self.pending_notifications.clear(); self.remove_dropped_entities(); } else { - self.focus_observations.gc(); - self.global_subscriptions.gc(); - self.global_observations.gc(); - self.subscriptions.gc(); - self.observations.gc(); - self.window_activation_observations.gc(); - self.window_fullscreen_observations.gc(); - self.keystroke_observations.gc(); - self.release_observations.gc(); - self.remove_dropped_entities(); if refreshing { diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 44db224967..45479eabad 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -16,7 +16,17 @@ pub struct Subscription { struct Mapping { callbacks: HashMap>, - dropped_subscriptions: HashSet<(K, usize)>, + dropped_subscriptions: HashMap>, +} + +impl Mapping { + fn clear_dropped_state(&mut self, key: &K, subscription_id: usize) -> bool { + if let Some(subscriptions) = self.dropped_subscriptions.get_mut(&key) { + subscriptions.remove(&subscription_id) + } else { + false + } + } } impl Default for Mapping { @@ -50,6 +60,14 @@ impl CallbackCollection { self.internal.lock().callbacks.is_empty() } + pub fn count(&self, key: K) -> usize { + self.internal + .lock() + .callbacks + .get(&key) + .map_or(0, |callbacks| callbacks.len()) + } + pub fn subscribe(&mut self, key: K, subscription_id: usize) -> Subscription { Subscription { key, @@ -63,7 +81,7 @@ impl CallbackCollection { // If this callback's subscription was dropped before the callback was // added, then just drop the callback. - if this.dropped_subscriptions.remove(&(key, subscription_id)) { + if this.clear_dropped_state(&key, subscription_id) { return; } @@ -74,10 +92,12 @@ impl CallbackCollection { } pub fn remove(&mut self, key: K) { - let callbacks = self.internal.lock().callbacks.remove(&key); - // Drop these callbacks after releasing the lock, in case one of them // owns a subscription to this callback collection. + let mut this = self.internal.lock(); + let callbacks = this.callbacks.remove(&key); + this.dropped_subscriptions.remove(&key); + drop(this); drop(callbacks); } @@ -91,29 +111,26 @@ impl CallbackCollection { if let Some(callbacks) = callbacks { for (subscription_id, mut callback) in callbacks { // If this callback's subscription was dropped while invoking an - // earlier callback, then just drop this callback. - if self - .internal - .lock() - .dropped_subscriptions - .remove(&(key, subscription_id)) - { + // earlier callback, then just drop the callback. + let mut this = self.internal.lock(); + if this.clear_dropped_state(&key, subscription_id) { continue; } - if call_callback(&mut callback, cx) { - self.add_callback(key, subscription_id, callback); + drop(this); + let alive = call_callback(&mut callback, cx); + + // If this callback's subscription was dropped while invoking the callback + // itself, or if the callback returns false, then just drop the callback. + let mut this = self.internal.lock(); + if this.clear_dropped_state(&key, subscription_id) || !alive { + continue; } - } - } - } - pub fn gc(&mut self) { - let mut this = self.internal.lock(); - - for (key, id) in std::mem::take(&mut this.dropped_subscriptions) { - if let Some(callbacks) = this.callbacks.get_mut(&key) { - callbacks.remove(&id); + this.callbacks + .entry(key) + .or_default() + .insert(subscription_id, callback); } } } @@ -150,7 +167,9 @@ impl Drop for Subscription { // removed later. mapping .dropped_subscriptions - .insert((self.key.clone(), self.id)); + .entry(self.key.clone()) + .or_default() + .insert(self.id); } } } From b762d70202ffb71b053b4f83b085d8db0b710da1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 6 Jan 2023 11:51:36 -0800 Subject: [PATCH 24/33] Remove unused CallbackCollection method --- crates/gpui/src/app/callback_collection.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/gpui/src/app/callback_collection.rs b/crates/gpui/src/app/callback_collection.rs index 45479eabad..7496d71501 100644 --- a/crates/gpui/src/app/callback_collection.rs +++ b/crates/gpui/src/app/callback_collection.rs @@ -60,14 +60,6 @@ impl CallbackCollection { self.internal.lock().callbacks.is_empty() } - pub fn count(&self, key: K) -> usize { - self.internal - .lock() - .callbacks - .get(&key) - .map_or(0, |callbacks| callbacks.len()) - } - pub fn subscribe(&mut self, key: K, subscription_id: usize) -> Subscription { Subscription { key, From 73e7967a12edaac323ff4798a21da2d4f8ff3071 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Fri, 6 Jan 2023 14:03:01 -0800 Subject: [PATCH 25/33] working f and t bindings --- assets/keymaps/vim.json | 52 +- crates/collab_ui/src/contact_list.rs | 8 +- crates/command_palette/src/command_palette.rs | 6 +- crates/context_menu/src/context_menu.rs | 8 +- crates/editor/src/editor.rs | 7 +- crates/editor/src/test/editor_test_context.rs | 4 +- crates/gpui/src/app.rs | 39 +- crates/gpui/src/app/test_app_context.rs | 10 +- crates/gpui/src/gpui.rs | 2 +- crates/gpui/src/keymap.rs | 757 ------------------ crates/gpui/src/keymap_matcher.rs | 459 +++++++++++ crates/gpui/src/keymap_matcher/binding.rs | 104 +++ crates/gpui/src/keymap_matcher/keymap.rs | 61 ++ .../gpui/src/keymap_matcher/keymap_context.rs | 123 +++ crates/gpui/src/keymap_matcher/keystroke.rs | 97 +++ crates/gpui/src/platform.rs | 4 +- crates/gpui/src/platform/event.rs | 2 +- crates/gpui/src/platform/mac/event.rs | 2 +- crates/gpui/src/platform/mac/platform.rs | 11 +- crates/gpui/src/platform/mac/window.rs | 2 +- crates/gpui/src/platform/test.rs | 5 +- crates/gpui/src/presenter.rs | 2 +- crates/live_kit_client/examples/test_app.rs | 2 +- crates/picker/src/picker.rs | 4 +- crates/project_panel/src/project_panel.rs | 5 +- crates/settings/src/keymap_file.rs | 2 +- crates/terminal/src/mappings/keys.rs | 4 +- crates/terminal/src/terminal.rs | 2 +- crates/terminal_view/src/terminal_view.rs | 4 +- crates/vim/src/motion.rs | 91 ++- crates/vim/src/normal.rs | 42 +- crates/vim/src/state.rs | 64 +- crates/vim/src/test/neovim_connection.rs | 2 +- crates/vim/src/vim.rs | 11 +- .../test_capital_f_and_capital_t.json | 1 + crates/vim/test_data/test_f_and_t.json | 1 + crates/workspace/src/workspace.rs | 3 +- 37 files changed, 1143 insertions(+), 860 deletions(-) delete mode 100644 crates/gpui/src/keymap.rs create mode 100644 crates/gpui/src/keymap_matcher.rs create mode 100644 crates/gpui/src/keymap_matcher/binding.rs create mode 100644 crates/gpui/src/keymap_matcher/keymap.rs create mode 100644 crates/gpui/src/keymap_matcher/keymap_context.rs create mode 100644 crates/gpui/src/keymap_matcher/keystroke.rs create mode 100644 crates/vim/test_data/test_capital_f_and_capital_t.json create mode 100644 crates/vim/test_data/test_f_and_t.json diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 99c94798db..bef6f48cb4 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -1,6 +1,6 @@ [ { - "context": "Editor && VimControl", + "context": "Editor && VimControl && !VimWaiting", "bindings": { "g": [ "vim::PushOperator", @@ -53,6 +53,42 @@ } ], "%": "vim::Matching", + "ctrl-y": [ + "vim::Scroll", + "LineUp" + ], + "f": [ + "vim::PushOperator", + { + "FindForward": { + "before": false + } + } + ], + "t": [ + "vim::PushOperator", + { + "FindForward": { + "before": true + } + } + ], + "shift-f": [ + "vim::PushOperator", + { + "FindBackward": { + "after": false + } + } + ], + "shift-t": [ + "vim::PushOperator", + { + "FindBackward": { + "after": true + } + } + ], "escape": "editor::Cancel", "0": "vim::StartOfLine", // When no number operator present, use start of line motion "1": [ @@ -94,7 +130,7 @@ } }, { - "context": "Editor && vim_mode == normal && vim_operator == none", + "context": "Editor && vim_mode == normal && vim_operator == none && !VimWaiting", "bindings": { "c": [ "vim::PushOperator", @@ -173,10 +209,6 @@ "ctrl-e": [ "vim::Scroll", "LineDown" - ], - "ctrl-y": [ - "vim::Scroll", - "LineUp" ] } }, @@ -255,7 +287,7 @@ } }, { - "context": "Editor && vim_mode == visual", + "context": "Editor && vim_mode == visual && !VimWaiting", "bindings": { "u": "editor::Undo", "c": "vim::VisualChange", @@ -271,5 +303,11 @@ "escape": "vim::NormalBefore", "ctrl-c": "vim::NormalBefore" } + }, + { + "context": "Editor && VimWaiting", + "bindings": { + "*": "gpui::KeyPressed" + } } ] \ No newline at end of file diff --git a/crates/collab_ui/src/contact_list.rs b/crates/collab_ui/src/contact_list.rs index ba6b236a82..743b98adb0 100644 --- a/crates/collab_ui/src/contact_list.rs +++ b/crates/collab_ui/src/contact_list.rs @@ -8,8 +8,10 @@ use fuzzy::{match_strings, StringMatchCandidate}; use gpui::{ elements::*, geometry::{rect::RectF, vector::vec2f}, - impl_actions, impl_internal_actions, keymap, AppContext, CursorStyle, Entity, ModelHandle, - MouseButton, MutableAppContext, RenderContext, Subscription, View, ViewContext, ViewHandle, + impl_actions, impl_internal_actions, + keymap_matcher::KeymapContext, + AppContext, CursorStyle, Entity, ModelHandle, MouseButton, MutableAppContext, RenderContext, + Subscription, View, ViewContext, ViewHandle, }; use menu::{Confirm, SelectNext, SelectPrev}; use project::Project; @@ -1267,7 +1269,7 @@ impl View for ContactList { "ContactList" } - fn keymap_context(&self, _: &AppContext) -> keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { let mut cx = Self::default_keymap_context(); cx.set.insert("menu".into()); cx diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 3742e36c72..d4625cbce0 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -3,7 +3,7 @@ use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, elements::{ChildView, Flex, Label, ParentElement}, - keymap::Keystroke, + keymap_matcher::Keystroke, Action, AnyViewHandle, Element, Entity, MouseState, MutableAppContext, RenderContext, View, ViewContext, ViewHandle, }; @@ -64,8 +64,10 @@ impl CommandPalette { name: humanize_action_name(name), action, keystrokes: bindings + .iter() + .filter_map(|binding| binding.keystrokes()) .last() - .map_or(Vec::new(), |binding| binding.keystrokes().to_vec()), + .map_or(Vec::new(), |keystrokes| keystrokes.to_vec()), }) }) .collect(); diff --git a/crates/context_menu/src/context_menu.rs b/crates/context_menu/src/context_menu.rs index 96d99f5109..0dc0ce6f42 100644 --- a/crates/context_menu/src/context_menu.rs +++ b/crates/context_menu/src/context_menu.rs @@ -1,7 +1,7 @@ use gpui::{ - elements::*, geometry::vector::Vector2F, impl_internal_actions, keymap, platform::CursorStyle, - Action, AnyViewHandle, AppContext, Axis, Entity, MouseButton, MutableAppContext, RenderContext, - SizeConstraint, Subscription, View, ViewContext, + elements::*, geometry::vector::Vector2F, impl_internal_actions, keymap_matcher::KeymapContext, + platform::CursorStyle, Action, AnyViewHandle, AppContext, Axis, Entity, MouseButton, + MutableAppContext, RenderContext, SizeConstraint, Subscription, View, ViewContext, }; use menu::*; use settings::Settings; @@ -75,7 +75,7 @@ impl View for ContextMenu { "ContextMenu" } - fn keymap_context(&self, _: &AppContext) -> keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { let mut cx = Self::default_keymap_context(); cx.set.insert("menu".into()); cx diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d8ee49866b..535bd4a57c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -36,6 +36,7 @@ use gpui::{ fonts::{self, HighlightStyle, TextStyle}, geometry::vector::Vector2F, impl_actions, impl_internal_actions, + keymap_matcher::KeymapContext, platform::CursorStyle, serde_json::json, AnyViewHandle, AppContext, AsyncAppContext, ClipboardItem, Element, ElementBox, Entity, @@ -464,7 +465,7 @@ pub struct Editor { searchable: bool, cursor_shape: CursorShape, workspace_id: Option, - keymap_context_layers: BTreeMap, + keymap_context_layers: BTreeMap, input_enabled: bool, leader_replica_id: Option, remote_id: Option, @@ -1225,7 +1226,7 @@ impl Editor { } } - pub fn set_keymap_context_layer(&mut self, context: gpui::keymap::Context) { + pub fn set_keymap_context_layer(&mut self, context: KeymapContext) { self.keymap_context_layers .insert(TypeId::of::(), context); } @@ -6245,7 +6246,7 @@ impl View for Editor { false } - fn keymap_context(&self, _: &AppContext) -> gpui::keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { let mut context = Self::default_keymap_context(); let mode = match self.mode { EditorMode::SingleLine => "single_line", diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 568f29d3e1..d8dbfee171 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -9,7 +9,9 @@ use indoc::indoc; use crate::{ display_map::ToDisplayPoint, AnchorRangeExt, Autoscroll, DisplayPoint, Editor, MultiBuffer, }; -use gpui::{keymap::Keystroke, AppContext, ContextHandle, ModelContext, ViewContext, ViewHandle}; +use gpui::{ + keymap_matcher::Keystroke, AppContext, ContextHandle, ModelContext, ViewContext, ViewHandle, +}; use language::{Buffer, BufferSnapshot}; use settings::Settings; use util::{ diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 5568155cf7..6b784833c7 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -28,7 +28,6 @@ use smol::prelude::*; pub use action::*; use callback_collection::CallbackCollection; use collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; -use keymap::MatchResult; use platform::Event; #[cfg(any(test, feature = "test-support"))] pub use test_app_context::{ContextHandle, TestAppContext}; @@ -37,7 +36,7 @@ use crate::{ elements::ElementBox, executor::{self, Task}, geometry::rect::RectF, - keymap::{self, Binding, Keystroke}, + keymap_matcher::{self, Binding, KeymapContext, KeymapMatcher, Keystroke, MatchResult}, platform::{self, KeyDownEvent, Platform, PromptLevel, WindowOptions}, presenter::Presenter, util::post_inc, @@ -72,11 +71,11 @@ pub trait View: Entity + Sized { false } - fn keymap_context(&self, _: &AppContext) -> keymap::Context { + fn keymap_context(&self, _: &AppContext) -> keymap_matcher::KeymapContext { Self::default_keymap_context() } - fn default_keymap_context() -> keymap::Context { - let mut cx = keymap::Context::default(); + fn default_keymap_context() -> keymap_matcher::KeymapContext { + let mut cx = keymap_matcher::KeymapContext::default(); cx.set.insert(Self::ui_name().into()); cx } @@ -609,7 +608,7 @@ pub struct MutableAppContext { capture_actions: HashMap>>>, actions: HashMap>>>, global_actions: HashMap>, - keystroke_matcher: keymap::Matcher, + keystroke_matcher: KeymapMatcher, next_entity_id: usize, next_window_id: usize, next_subscription_id: usize, @@ -668,7 +667,7 @@ impl MutableAppContext { capture_actions: Default::default(), actions: Default::default(), global_actions: Default::default(), - keystroke_matcher: keymap::Matcher::default(), + keystroke_matcher: KeymapMatcher::default(), next_entity_id: 0, next_window_id: 0, next_subscription_id: 0, @@ -1361,8 +1360,10 @@ impl MutableAppContext { .views .get(&(window_id, *view_id)) .expect("view in responder chain does not exist"); - let cx = view.keymap_context(self.as_ref()); - let keystrokes = self.keystroke_matcher.keystrokes_for_action(action, &cx); + let keymap_context = view.keymap_context(self.as_ref()); + let keystrokes = self + .keystroke_matcher + .keystrokes_for_action(action, &keymap_context); if keystrokes.is_some() { return keystrokes; } @@ -1443,7 +1444,7 @@ impl MutableAppContext { }) } - pub fn add_bindings>(&mut self, bindings: T) { + pub fn add_bindings>(&mut self, bindings: T) { self.keystroke_matcher.add_bindings(bindings); } @@ -3139,7 +3140,7 @@ pub trait AnyView { window_id: usize, view_id: usize, ) -> bool; - fn keymap_context(&self, cx: &AppContext) -> keymap::Context; + fn keymap_context(&self, cx: &AppContext) -> KeymapContext; fn debug_json(&self, cx: &AppContext) -> serde_json::Value; fn text_for_range(&self, range: Range, cx: &AppContext) -> Option; @@ -3281,7 +3282,7 @@ where View::modifiers_changed(self, event, &mut cx) } - fn keymap_context(&self, cx: &AppContext) -> keymap::Context { + fn keymap_context(&self, cx: &AppContext) -> KeymapContext { View::keymap_context(self, cx) } @@ -6633,7 +6634,7 @@ mod tests { struct View { id: usize, - keymap_context: keymap::Context, + keymap_context: KeymapContext, } impl Entity for View { @@ -6649,7 +6650,7 @@ mod tests { "View" } - fn keymap_context(&self, _: &AppContext) -> keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { self.keymap_context.clone() } } @@ -6658,7 +6659,7 @@ mod tests { fn new(id: usize) -> Self { View { id, - keymap_context: keymap::Context::default(), + keymap_context: KeymapContext::default(), } } } @@ -6682,17 +6683,13 @@ mod tests { // This keymap's only binding dispatches an action on view 2 because that view will have // "a" and "b" in its context, but not "c". - cx.add_bindings(vec![keymap::Binding::new( + cx.add_bindings(vec![Binding::new( "a", Action("a".to_string()), Some("a && b && !c"), )]); - cx.add_bindings(vec![keymap::Binding::new( - "b", - Action("b".to_string()), - None, - )]); + cx.add_bindings(vec![Binding::new("b", Action("b".to_string()), None)]); let actions = Rc::new(RefCell::new(Vec::new())); cx.add_action({ diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 72f1f546fb..67455cd2a7 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -17,11 +17,11 @@ use parking_lot::{Mutex, RwLock}; use smol::stream::StreamExt; use crate::{ - executor, geometry::vector::Vector2F, keymap::Keystroke, platform, Action, AnyViewHandle, - AppContext, Appearance, Entity, Event, FontCache, InputHandler, KeyDownEvent, LeakDetector, - ModelContext, ModelHandle, MutableAppContext, Platform, ReadModelWith, ReadViewWith, - RenderContext, Task, UpdateModel, UpdateView, View, ViewContext, ViewHandle, WeakHandle, - WindowInputHandler, + executor, geometry::vector::Vector2F, keymap_matcher::Keystroke, platform, Action, + AnyViewHandle, AppContext, Appearance, Entity, Event, FontCache, InputHandler, KeyDownEvent, + LeakDetector, ModelContext, ModelHandle, MutableAppContext, Platform, ReadModelWith, + ReadViewWith, RenderContext, Task, UpdateModel, UpdateView, View, ViewContext, ViewHandle, + WeakHandle, WindowInputHandler, }; use collections::BTreeMap; diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index d8b446ac17..dfdc269aff 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -25,7 +25,7 @@ pub mod executor; pub use executor::Task; pub mod color; pub mod json; -pub mod keymap; +pub mod keymap_matcher; pub mod platform; pub use gpui_macros::test; pub use platform::*; diff --git a/crates/gpui/src/keymap.rs b/crates/gpui/src/keymap.rs deleted file mode 100644 index e9bc228757..0000000000 --- a/crates/gpui/src/keymap.rs +++ /dev/null @@ -1,757 +0,0 @@ -use crate::Action; -use anyhow::{anyhow, Result}; -use smallvec::SmallVec; -use std::{ - any::{Any, TypeId}, - collections::{HashMap, HashSet}, - fmt::{Debug, Write}, -}; -use tree_sitter::{Language, Node, Parser}; - -extern "C" { - fn tree_sitter_context_predicate() -> Language; -} - -pub struct Matcher { - pending_views: HashMap, - pending_keystrokes: Vec, - keymap: Keymap, -} - -#[derive(Default)] -pub struct Keymap { - bindings: Vec, - binding_indices_by_action_type: HashMap>, -} - -pub struct Binding { - keystrokes: SmallVec<[Keystroke; 2]>, - action: Box, - context_predicate: Option, -} - -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct Keystroke { - pub ctrl: bool, - pub alt: bool, - pub shift: bool, - pub cmd: bool, - pub function: bool, - pub key: String, -} - -#[derive(Clone, Debug, Default, Eq, PartialEq)] -pub struct Context { - pub set: HashSet, - pub map: HashMap, -} - -#[derive(Debug, Eq, PartialEq)] -enum ContextPredicate { - Identifier(String), - Equal(String, String), - NotEqual(String, String), - Not(Box), - And(Box, Box), - Or(Box, Box), -} - -trait ActionArg { - fn boxed_clone(&self) -> Box; -} - -impl ActionArg for T -where - T: 'static + Any + Clone, -{ - fn boxed_clone(&self) -> Box { - Box::new(self.clone()) - } -} - -pub enum MatchResult { - None, - Pending, - Matches(Vec<(usize, Box)>), -} - -impl Debug for MatchResult { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - MatchResult::None => f.debug_struct("MatchResult::None").finish(), - MatchResult::Pending => f.debug_struct("MatchResult::Pending").finish(), - MatchResult::Matches(matches) => f - .debug_list() - .entries( - matches - .iter() - .map(|(view_id, action)| format!("{view_id}, {}", action.name())), - ) - .finish(), - } - } -} - -impl PartialEq for MatchResult { - fn eq(&self, other: &Self) -> bool { - match (self, other) { - (MatchResult::None, MatchResult::None) => true, - (MatchResult::Pending, MatchResult::Pending) => true, - (MatchResult::Matches(matches), MatchResult::Matches(other_matches)) => { - matches.len() == other_matches.len() - && matches.iter().zip(other_matches.iter()).all( - |((view_id, action), (other_view_id, other_action))| { - view_id == other_view_id && action.eq(other_action.as_ref()) - }, - ) - } - _ => false, - } - } -} - -impl Eq for MatchResult {} - -impl Clone for MatchResult { - fn clone(&self) -> Self { - match self { - MatchResult::None => MatchResult::None, - MatchResult::Pending => MatchResult::Pending, - MatchResult::Matches(matches) => MatchResult::Matches( - matches - .iter() - .map(|(view_id, action)| (*view_id, Action::boxed_clone(action.as_ref()))) - .collect(), - ), - } - } -} - -impl Matcher { - pub fn new(keymap: Keymap) -> Self { - Self { - pending_views: HashMap::new(), - pending_keystrokes: Vec::new(), - keymap, - } - } - - pub fn set_keymap(&mut self, keymap: Keymap) { - self.clear_pending(); - self.keymap = keymap; - } - - pub fn add_bindings>(&mut self, bindings: T) { - self.clear_pending(); - self.keymap.add_bindings(bindings); - } - - pub fn clear_bindings(&mut self) { - self.clear_pending(); - self.keymap.clear(); - } - - pub fn bindings_for_action_type(&self, action_type: TypeId) -> impl Iterator { - self.keymap.bindings_for_action_type(action_type) - } - - pub fn clear_pending(&mut self) { - self.pending_keystrokes.clear(); - self.pending_views.clear(); - } - - pub fn has_pending_keystrokes(&self) -> bool { - !self.pending_keystrokes.is_empty() - } - - pub fn push_keystroke( - &mut self, - keystroke: Keystroke, - dispatch_path: Vec<(usize, Context)>, - ) -> MatchResult { - let mut any_pending = false; - let mut matched_bindings = Vec::new(); - - let first_keystroke = self.pending_keystrokes.is_empty(); - self.pending_keystrokes.push(keystroke); - - for (view_id, context) in dispatch_path { - // Don't require pending view entry if there are no pending keystrokes - if !first_keystroke && !self.pending_views.contains_key(&view_id) { - continue; - } - - // If there is a previous view context, invalidate that view if it - // has changed - if let Some(previous_view_context) = self.pending_views.remove(&view_id) { - if previous_view_context != context { - continue; - } - } - - // Find the bindings which map the pending keystrokes and current context - for binding in self.keymap.bindings.iter().rev() { - if binding.keystrokes.starts_with(&self.pending_keystrokes) - && binding - .context_predicate - .as_ref() - .map(|c| c.eval(&context)) - .unwrap_or(true) - { - // If the binding is completed, push it onto the matches list - if binding.keystrokes.len() == self.pending_keystrokes.len() { - matched_bindings.push((view_id, binding.action.boxed_clone())); - } else { - // Otherwise, the binding is still pending - self.pending_views.insert(view_id, context.clone()); - any_pending = true; - } - } - } - } - - if !any_pending { - self.clear_pending(); - } - - if !matched_bindings.is_empty() { - MatchResult::Matches(matched_bindings) - } else if any_pending { - MatchResult::Pending - } else { - MatchResult::None - } - } - - pub fn keystrokes_for_action( - &self, - action: &dyn Action, - cx: &Context, - ) -> Option> { - for binding in self.keymap.bindings.iter().rev() { - if binding.action.eq(action) - && binding - .context_predicate - .as_ref() - .map_or(true, |predicate| predicate.eval(cx)) - { - return Some(binding.keystrokes.clone()); - } - } - None - } -} - -impl Default for Matcher { - fn default() -> Self { - Self::new(Keymap::default()) - } -} - -impl Keymap { - pub fn new(bindings: Vec) -> Self { - let mut binding_indices_by_action_type = HashMap::new(); - for (ix, binding) in bindings.iter().enumerate() { - binding_indices_by_action_type - .entry(binding.action.as_any().type_id()) - .or_insert_with(SmallVec::new) - .push(ix); - } - Self { - binding_indices_by_action_type, - bindings, - } - } - - fn bindings_for_action_type(&self, action_type: TypeId) -> impl Iterator { - self.binding_indices_by_action_type - .get(&action_type) - .map(SmallVec::as_slice) - .unwrap_or(&[]) - .iter() - .map(|ix| &self.bindings[*ix]) - } - - fn add_bindings>(&mut self, bindings: T) { - for binding in bindings { - self.binding_indices_by_action_type - .entry(binding.action.as_any().type_id()) - .or_default() - .push(self.bindings.len()); - self.bindings.push(binding); - } - } - - fn clear(&mut self) { - self.bindings.clear(); - self.binding_indices_by_action_type.clear(); - } -} - -impl Binding { - pub fn new(keystrokes: &str, action: A, context: Option<&str>) -> Self { - Self::load(keystrokes, Box::new(action), context).unwrap() - } - - pub fn load(keystrokes: &str, action: Box, context: Option<&str>) -> Result { - let context = if let Some(context) = context { - Some(ContextPredicate::parse(context)?) - } else { - None - }; - - let keystrokes = keystrokes - .split_whitespace() - .map(Keystroke::parse) - .collect::>()?; - - Ok(Self { - keystrokes, - action, - context_predicate: context, - }) - } - - pub fn keystrokes(&self) -> &[Keystroke] { - &self.keystrokes - } - - pub fn action(&self) -> &dyn Action { - self.action.as_ref() - } -} - -impl Keystroke { - pub fn parse(source: &str) -> anyhow::Result { - let mut ctrl = false; - let mut alt = false; - let mut shift = false; - let mut cmd = false; - let mut function = false; - let mut key = None; - - let mut components = source.split('-').peekable(); - while let Some(component) = components.next() { - match component { - "ctrl" => ctrl = true, - "alt" => alt = true, - "shift" => shift = true, - "cmd" => cmd = true, - "fn" => function = true, - _ => { - if let Some(component) = components.peek() { - if component.is_empty() && source.ends_with('-') { - key = Some(String::from("-")); - break; - } else { - return Err(anyhow!("Invalid keystroke `{}`", source)); - } - } else { - key = Some(String::from(component)); - } - } - } - } - - let key = key.ok_or_else(|| anyhow!("Invalid keystroke `{}`", source))?; - - Ok(Keystroke { - ctrl, - alt, - shift, - cmd, - function, - key, - }) - } - - pub fn modified(&self) -> bool { - self.ctrl || self.alt || self.shift || self.cmd - } -} - -impl std::fmt::Display for Keystroke { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if self.ctrl { - f.write_char('^')?; - } - if self.alt { - f.write_char('⎇')?; - } - if self.cmd { - f.write_char('⌘')?; - } - if self.shift { - f.write_char('⇧')?; - } - let key = match self.key.as_str() { - "backspace" => '⌫', - "up" => '↑', - "down" => '↓', - "left" => '←', - "right" => '→', - "tab" => '⇥', - "escape" => '⎋', - key => { - if key.len() == 1 { - key.chars().next().unwrap().to_ascii_uppercase() - } else { - return f.write_str(key); - } - } - }; - f.write_char(key) - } -} - -impl Context { - pub fn extend(&mut self, other: &Context) { - for v in &other.set { - self.set.insert(v.clone()); - } - for (k, v) in &other.map { - self.map.insert(k.clone(), v.clone()); - } - } -} - -impl ContextPredicate { - fn parse(source: &str) -> anyhow::Result { - let mut parser = Parser::new(); - let language = unsafe { tree_sitter_context_predicate() }; - parser.set_language(language).unwrap(); - let source = source.as_bytes(); - let tree = parser.parse(source, None).unwrap(); - Self::from_node(tree.root_node(), source) - } - - fn from_node(node: Node, source: &[u8]) -> anyhow::Result { - let parse_error = "error parsing context predicate"; - let kind = node.kind(); - - match kind { - "source" => Self::from_node(node.child(0).ok_or_else(|| anyhow!(parse_error))?, source), - "identifier" => Ok(Self::Identifier(node.utf8_text(source)?.into())), - "not" => { - let child = Self::from_node( - node.child_by_field_name("expression") - .ok_or_else(|| anyhow!(parse_error))?, - source, - )?; - Ok(Self::Not(Box::new(child))) - } - "and" | "or" => { - let left = Box::new(Self::from_node( - node.child_by_field_name("left") - .ok_or_else(|| anyhow!(parse_error))?, - source, - )?); - let right = Box::new(Self::from_node( - node.child_by_field_name("right") - .ok_or_else(|| anyhow!(parse_error))?, - source, - )?); - if kind == "and" { - Ok(Self::And(left, right)) - } else { - Ok(Self::Or(left, right)) - } - } - "equal" | "not_equal" => { - let left = node - .child_by_field_name("left") - .ok_or_else(|| anyhow!(parse_error))? - .utf8_text(source)? - .into(); - let right = node - .child_by_field_name("right") - .ok_or_else(|| anyhow!(parse_error))? - .utf8_text(source)? - .into(); - if kind == "equal" { - Ok(Self::Equal(left, right)) - } else { - Ok(Self::NotEqual(left, right)) - } - } - "parenthesized" => Self::from_node( - node.child_by_field_name("expression") - .ok_or_else(|| anyhow!(parse_error))?, - source, - ), - _ => Err(anyhow!(parse_error)), - } - } - - fn eval(&self, cx: &Context) -> bool { - match self { - Self::Identifier(name) => cx.set.contains(name.as_str()), - Self::Equal(left, right) => cx - .map - .get(left) - .map(|value| value == right) - .unwrap_or(false), - Self::NotEqual(left, right) => { - cx.map.get(left).map(|value| value != right).unwrap_or(true) - } - Self::Not(pred) => !pred.eval(cx), - Self::And(left, right) => left.eval(cx) && right.eval(cx), - Self::Or(left, right) => left.eval(cx) || right.eval(cx), - } - } -} - -#[cfg(test)] -mod tests { - use anyhow::Result; - use serde::Deserialize; - - use crate::{actions, impl_actions}; - - use super::*; - - #[test] - fn test_push_keystroke() -> Result<()> { - actions!(test, [B, AB, C, D, DA]); - - let mut ctx1 = Context::default(); - ctx1.set.insert("1".into()); - - let mut ctx2 = Context::default(); - ctx2.set.insert("2".into()); - - let dispatch_path = vec![(2, ctx2), (1, ctx1)]; - - let keymap = Keymap::new(vec![ - Binding::new("a b", AB, Some("1")), - Binding::new("b", B, Some("2")), - Binding::new("c", C, Some("2")), - Binding::new("d", D, Some("1")), - Binding::new("d", D, Some("2")), - Binding::new("d a", DA, Some("2")), - ]); - - let mut matcher = Matcher::new(keymap); - - // Binding with pending prefix always takes precedence - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone()), - MatchResult::Pending, - ); - // B alone doesn't match because a was pending, so AB is returned instead - assert_eq!( - matcher.push_keystroke(Keystroke::parse("b")?, dispatch_path.clone()), - MatchResult::Matches(vec![(1, Box::new(AB))]), - ); - assert!(!matcher.has_pending_keystrokes()); - - // Without an a prefix, B is dispatched like expected - assert_eq!( - matcher.push_keystroke(Keystroke::parse("b")?, dispatch_path.clone()), - MatchResult::Matches(vec![(2, Box::new(B))]), - ); - assert!(!matcher.has_pending_keystrokes()); - - // If a is prefixed, C will not be dispatched because there - // was a pending binding for it - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone()), - MatchResult::Pending, - ); - assert_eq!( - matcher.push_keystroke(Keystroke::parse("c")?, dispatch_path.clone()), - MatchResult::None, - ); - assert!(!matcher.has_pending_keystrokes()); - - // If a single keystroke matches multiple bindings in the tree - // all of them are returned so that we can fallback if the action - // handler decides to propagate the action - assert_eq!( - matcher.push_keystroke(Keystroke::parse("d")?, dispatch_path.clone()), - MatchResult::Matches(vec![(2, Box::new(D)), (1, Box::new(D))]), - ); - // If none of the d action handlers consume the binding, a pending - // binding may then be used - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone()), - MatchResult::Matches(vec![(2, Box::new(DA))]), - ); - assert!(!matcher.has_pending_keystrokes()); - - Ok(()) - } - - #[test] - fn test_keystroke_parsing() -> Result<()> { - assert_eq!( - Keystroke::parse("ctrl-p")?, - Keystroke { - key: "p".into(), - ctrl: true, - alt: false, - shift: false, - cmd: false, - function: false, - } - ); - - assert_eq!( - Keystroke::parse("alt-shift-down")?, - Keystroke { - key: "down".into(), - ctrl: false, - alt: true, - shift: true, - cmd: false, - function: false, - } - ); - - assert_eq!( - Keystroke::parse("shift-cmd--")?, - Keystroke { - key: "-".into(), - ctrl: false, - alt: false, - shift: true, - cmd: true, - function: false, - } - ); - - Ok(()) - } - - #[test] - fn test_context_predicate_parsing() -> Result<()> { - use ContextPredicate::*; - - assert_eq!( - ContextPredicate::parse("a && (b == c || d != e)")?, - And( - Box::new(Identifier("a".into())), - Box::new(Or( - Box::new(Equal("b".into(), "c".into())), - Box::new(NotEqual("d".into(), "e".into())), - )) - ) - ); - - assert_eq!( - ContextPredicate::parse("!a")?, - Not(Box::new(Identifier("a".into())),) - ); - - Ok(()) - } - - #[test] - fn test_context_predicate_eval() -> Result<()> { - let predicate = ContextPredicate::parse("a && b || c == d")?; - - let mut context = Context::default(); - context.set.insert("a".into()); - assert!(!predicate.eval(&context)); - - context.set.insert("b".into()); - assert!(predicate.eval(&context)); - - context.set.remove("b"); - context.map.insert("c".into(), "x".into()); - assert!(!predicate.eval(&context)); - - context.map.insert("c".into(), "d".into()); - assert!(predicate.eval(&context)); - - let predicate = ContextPredicate::parse("!a")?; - assert!(predicate.eval(&Context::default())); - - Ok(()) - } - - #[test] - fn test_matcher() -> Result<()> { - #[derive(Clone, Deserialize, PartialEq, Eq, Debug)] - pub struct A(pub String); - impl_actions!(test, [A]); - actions!(test, [B, Ab]); - - #[derive(Clone, Debug, Eq, PartialEq)] - struct ActionArg { - a: &'static str, - } - - let keymap = Keymap::new(vec![ - Binding::new("a", A("x".to_string()), Some("a")), - Binding::new("b", B, Some("a")), - Binding::new("a b", Ab, Some("a || b")), - ]); - - let mut ctx_a = Context::default(); - ctx_a.set.insert("a".into()); - - let mut ctx_b = Context::default(); - ctx_b.set.insert("b".into()); - - let mut matcher = Matcher::new(keymap); - - // Basic match - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, ctx_a.clone())]), - MatchResult::Matches(vec![(1, Box::new(A("x".to_string())))]) - ); - matcher.clear_pending(); - - // Multi-keystroke match - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, ctx_b.clone())]), - MatchResult::Pending - ); - assert_eq!( - matcher.push_keystroke(Keystroke::parse("b")?, vec![(1, ctx_b.clone())]), - MatchResult::Matches(vec![(1, Box::new(Ab))]) - ); - matcher.clear_pending(); - - // Failed matches don't interfere with matching subsequent keys - assert_eq!( - matcher.push_keystroke(Keystroke::parse("x")?, vec![(1, ctx_a.clone())]), - MatchResult::None - ); - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, ctx_a.clone())]), - MatchResult::Matches(vec![(1, Box::new(A("x".to_string())))]) - ); - matcher.clear_pending(); - - // Pending keystrokes are cleared when the context changes - assert_eq!( - matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, ctx_b.clone())]), - MatchResult::Pending - ); - assert_eq!( - matcher.push_keystroke(Keystroke::parse("b")?, vec![(1, ctx_a.clone())]), - MatchResult::None - ); - matcher.clear_pending(); - - let mut ctx_c = Context::default(); - ctx_c.set.insert("c".into()); - - // Pending keystrokes are maintained per-view - assert_eq!( - matcher.push_keystroke( - Keystroke::parse("a")?, - vec![(1, ctx_b.clone()), (2, ctx_c.clone())] - ), - MatchResult::Pending - ); - assert_eq!( - matcher.push_keystroke(Keystroke::parse("b")?, vec![(1, ctx_b.clone())]), - MatchResult::Matches(vec![(1, Box::new(Ab))]) - ); - - Ok(()) - } -} diff --git a/crates/gpui/src/keymap_matcher.rs b/crates/gpui/src/keymap_matcher.rs new file mode 100644 index 0000000000..e007605cff --- /dev/null +++ b/crates/gpui/src/keymap_matcher.rs @@ -0,0 +1,459 @@ +mod binding; +mod keymap; +mod keymap_context; +mod keystroke; + +use std::{any::TypeId, fmt::Debug}; + +use collections::HashMap; +use serde::Deserialize; +use smallvec::SmallVec; + +use crate::{impl_actions, Action}; + +pub use binding::{Binding, BindingMatchResult}; +pub use keymap::Keymap; +pub use keymap_context::{KeymapContext, KeymapContextPredicate}; +pub use keystroke::Keystroke; + +#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize)] +pub struct KeyPressed { + #[serde(default)] + pub keystroke: Keystroke, +} + +impl_actions!(gpui, [KeyPressed]); + +pub struct KeymapMatcher { + pending_views: HashMap, + pending_keystrokes: Vec, + keymap: Keymap, +} + +impl KeymapMatcher { + pub fn new(keymap: Keymap) -> Self { + Self { + pending_views: Default::default(), + pending_keystrokes: Vec::new(), + keymap, + } + } + + pub fn set_keymap(&mut self, keymap: Keymap) { + self.clear_pending(); + self.keymap = keymap; + } + + pub fn add_bindings>(&mut self, bindings: T) { + self.clear_pending(); + self.keymap.add_bindings(bindings); + } + + pub fn clear_bindings(&mut self) { + self.clear_pending(); + self.keymap.clear(); + } + + pub fn bindings_for_action_type(&self, action_type: TypeId) -> impl Iterator { + self.keymap.bindings_for_action_type(action_type) + } + + pub fn clear_pending(&mut self) { + self.pending_keystrokes.clear(); + self.pending_views.clear(); + } + + pub fn has_pending_keystrokes(&self) -> bool { + !self.pending_keystrokes.is_empty() + } + + pub fn push_keystroke( + &mut self, + keystroke: Keystroke, + dispatch_path: Vec<(usize, KeymapContext)>, + ) -> MatchResult { + let mut any_pending = false; + let mut matched_bindings: Vec<(usize, Box)> = Vec::new(); + + let first_keystroke = self.pending_keystrokes.is_empty(); + self.pending_keystrokes.push(keystroke.clone()); + + for (view_id, context) in dispatch_path { + // Don't require pending view entry if there are no pending keystrokes + if !first_keystroke && !self.pending_views.contains_key(&view_id) { + continue; + } + + // If there is a previous view context, invalidate that view if it + // has changed + if let Some(previous_view_context) = self.pending_views.remove(&view_id) { + if previous_view_context != context { + continue; + } + } + + // Find the bindings which map the pending keystrokes and current context + for binding in self.keymap.bindings().iter().rev() { + match binding.match_keys_and_context(&self.pending_keystrokes, &context) { + BindingMatchResult::Complete(mut action) => { + // Swap in keystroke for special KeyPressed action + if action.name() == "KeyPressed" && action.namespace() == "gpui" { + action = Box::new(KeyPressed { + keystroke: keystroke.clone(), + }); + } + matched_bindings.push((view_id, action)) + } + BindingMatchResult::Partial => { + self.pending_views.insert(view_id, context.clone()); + any_pending = true; + } + _ => {} + } + } + } + + if !any_pending { + self.clear_pending(); + } + + if !matched_bindings.is_empty() { + MatchResult::Matches(matched_bindings) + } else if any_pending { + MatchResult::Pending + } else { + MatchResult::None + } + } + + pub fn keystrokes_for_action( + &self, + action: &dyn Action, + context: &KeymapContext, + ) -> Option> { + self.keymap + .bindings() + .iter() + .rev() + .find_map(|binding| binding.keystrokes_for_action(action, context)) + } +} + +impl Default for KeymapMatcher { + fn default() -> Self { + Self::new(Keymap::default()) + } +} + +pub enum MatchResult { + None, + Pending, + Matches(Vec<(usize, Box)>), +} + +impl Debug for MatchResult { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + MatchResult::None => f.debug_struct("MatchResult::None").finish(), + MatchResult::Pending => f.debug_struct("MatchResult::Pending").finish(), + MatchResult::Matches(matches) => f + .debug_list() + .entries( + matches + .iter() + .map(|(view_id, action)| format!("{view_id}, {}", action.name())), + ) + .finish(), + } + } +} + +impl PartialEq for MatchResult { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (MatchResult::None, MatchResult::None) => true, + (MatchResult::Pending, MatchResult::Pending) => true, + (MatchResult::Matches(matches), MatchResult::Matches(other_matches)) => { + matches.len() == other_matches.len() + && matches.iter().zip(other_matches.iter()).all( + |((view_id, action), (other_view_id, other_action))| { + view_id == other_view_id && action.eq(other_action.as_ref()) + }, + ) + } + _ => false, + } + } +} + +impl Eq for MatchResult {} + +impl Clone for MatchResult { + fn clone(&self) -> Self { + match self { + MatchResult::None => MatchResult::None, + MatchResult::Pending => MatchResult::Pending, + MatchResult::Matches(matches) => MatchResult::Matches( + matches + .iter() + .map(|(view_id, action)| (*view_id, Action::boxed_clone(action.as_ref()))) + .collect(), + ), + } + } +} + +#[cfg(test)] +mod tests { + use anyhow::Result; + use serde::Deserialize; + + use crate::{actions, impl_actions, keymap_matcher::KeymapContext}; + + use super::*; + + #[test] + fn test_push_keystroke() -> Result<()> { + actions!(test, [B, AB, C, D, DA]); + + let mut context1 = KeymapContext::default(); + context1.set.insert("1".into()); + + let mut context2 = KeymapContext::default(); + context2.set.insert("2".into()); + + let dispatch_path = vec![(2, context2), (1, context1)]; + + let keymap = Keymap::new(vec![ + Binding::new("a b", AB, Some("1")), + Binding::new("b", B, Some("2")), + Binding::new("c", C, Some("2")), + Binding::new("d", D, Some("1")), + Binding::new("d", D, Some("2")), + Binding::new("d a", DA, Some("2")), + ]); + + let mut matcher = KeymapMatcher::new(keymap); + + // Binding with pending prefix always takes precedence + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone()), + MatchResult::Pending, + ); + // B alone doesn't match because a was pending, so AB is returned instead + assert_eq!( + matcher.push_keystroke(Keystroke::parse("b")?, dispatch_path.clone()), + MatchResult::Matches(vec![(1, Box::new(AB))]), + ); + assert!(!matcher.has_pending_keystrokes()); + + // Without an a prefix, B is dispatched like expected + assert_eq!( + matcher.push_keystroke(Keystroke::parse("b")?, dispatch_path.clone()), + MatchResult::Matches(vec![(2, Box::new(B))]), + ); + assert!(!matcher.has_pending_keystrokes()); + + // If a is prefixed, C will not be dispatched because there + // was a pending binding for it + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone()), + MatchResult::Pending, + ); + assert_eq!( + matcher.push_keystroke(Keystroke::parse("c")?, dispatch_path.clone()), + MatchResult::None, + ); + assert!(!matcher.has_pending_keystrokes()); + + // If a single keystroke matches multiple bindings in the tree + // all of them are returned so that we can fallback if the action + // handler decides to propagate the action + assert_eq!( + matcher.push_keystroke(Keystroke::parse("d")?, dispatch_path.clone()), + MatchResult::Matches(vec![(2, Box::new(D)), (1, Box::new(D))]), + ); + // If none of the d action handlers consume the binding, a pending + // binding may then be used + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, dispatch_path.clone()), + MatchResult::Matches(vec![(2, Box::new(DA))]), + ); + assert!(!matcher.has_pending_keystrokes()); + + Ok(()) + } + + #[test] + fn test_keystroke_parsing() -> Result<()> { + assert_eq!( + Keystroke::parse("ctrl-p")?, + Keystroke { + key: "p".into(), + ctrl: true, + alt: false, + shift: false, + cmd: false, + function: false, + } + ); + + assert_eq!( + Keystroke::parse("alt-shift-down")?, + Keystroke { + key: "down".into(), + ctrl: false, + alt: true, + shift: true, + cmd: false, + function: false, + } + ); + + assert_eq!( + Keystroke::parse("shift-cmd--")?, + Keystroke { + key: "-".into(), + ctrl: false, + alt: false, + shift: true, + cmd: true, + function: false, + } + ); + + Ok(()) + } + + #[test] + fn test_context_predicate_parsing() -> Result<()> { + use KeymapContextPredicate::*; + + assert_eq!( + KeymapContextPredicate::parse("a && (b == c || d != e)")?, + And( + Box::new(Identifier("a".into())), + Box::new(Or( + Box::new(Equal("b".into(), "c".into())), + Box::new(NotEqual("d".into(), "e".into())), + )) + ) + ); + + assert_eq!( + KeymapContextPredicate::parse("!a")?, + Not(Box::new(Identifier("a".into())),) + ); + + Ok(()) + } + + #[test] + fn test_context_predicate_eval() -> Result<()> { + let predicate = KeymapContextPredicate::parse("a && b || c == d")?; + + let mut context = KeymapContext::default(); + context.set.insert("a".into()); + assert!(!predicate.eval(&context)); + + context.set.insert("b".into()); + assert!(predicate.eval(&context)); + + context.set.remove("b"); + context.map.insert("c".into(), "x".into()); + assert!(!predicate.eval(&context)); + + context.map.insert("c".into(), "d".into()); + assert!(predicate.eval(&context)); + + let predicate = KeymapContextPredicate::parse("!a")?; + assert!(predicate.eval(&KeymapContext::default())); + + Ok(()) + } + + #[test] + fn test_matcher() -> Result<()> { + #[derive(Clone, Deserialize, PartialEq, Eq, Debug)] + pub struct A(pub String); + impl_actions!(test, [A]); + actions!(test, [B, Ab]); + + #[derive(Clone, Debug, Eq, PartialEq)] + struct ActionArg { + a: &'static str, + } + + let keymap = Keymap::new(vec![ + Binding::new("a", A("x".to_string()), Some("a")), + Binding::new("b", B, Some("a")), + Binding::new("a b", Ab, Some("a || b")), + ]); + + let mut context_a = KeymapContext::default(); + context_a.set.insert("a".into()); + + let mut context_b = KeymapContext::default(); + context_b.set.insert("b".into()); + + let mut matcher = KeymapMatcher::new(keymap); + + // Basic match + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, context_a.clone())]), + MatchResult::Matches(vec![(1, Box::new(A("x".to_string())))]) + ); + matcher.clear_pending(); + + // Multi-keystroke match + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, context_b.clone())]), + MatchResult::Pending + ); + assert_eq!( + matcher.push_keystroke(Keystroke::parse("b")?, vec![(1, context_b.clone())]), + MatchResult::Matches(vec![(1, Box::new(Ab))]) + ); + matcher.clear_pending(); + + // Failed matches don't interfere with matching subsequent keys + assert_eq!( + matcher.push_keystroke(Keystroke::parse("x")?, vec![(1, context_a.clone())]), + MatchResult::None + ); + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, context_a.clone())]), + MatchResult::Matches(vec![(1, Box::new(A("x".to_string())))]) + ); + matcher.clear_pending(); + + // Pending keystrokes are cleared when the context changes + assert_eq!( + matcher.push_keystroke(Keystroke::parse("a")?, vec![(1, context_b.clone())]), + MatchResult::Pending + ); + assert_eq!( + matcher.push_keystroke(Keystroke::parse("b")?, vec![(1, context_a.clone())]), + MatchResult::None + ); + matcher.clear_pending(); + + let mut context_c = KeymapContext::default(); + context_c.set.insert("c".into()); + + // Pending keystrokes are maintained per-view + assert_eq!( + matcher.push_keystroke( + Keystroke::parse("a")?, + vec![(1, context_b.clone()), (2, context_c.clone())] + ), + MatchResult::Pending + ); + assert_eq!( + matcher.push_keystroke(Keystroke::parse("b")?, vec![(1, context_b.clone())]), + MatchResult::Matches(vec![(1, Box::new(Ab))]) + ); + + Ok(()) + } +} diff --git a/crates/gpui/src/keymap_matcher/binding.rs b/crates/gpui/src/keymap_matcher/binding.rs new file mode 100644 index 0000000000..b16b7f1552 --- /dev/null +++ b/crates/gpui/src/keymap_matcher/binding.rs @@ -0,0 +1,104 @@ +use anyhow::Result; +use smallvec::SmallVec; + +use crate::Action; + +use super::{KeymapContext, KeymapContextPredicate, Keystroke}; + +pub struct Binding { + action: Box, + keystrokes: Option>, + context_predicate: Option, +} + +impl Binding { + pub fn new(keystrokes: &str, action: A, context: Option<&str>) -> Self { + Self::load(keystrokes, Box::new(action), context).unwrap() + } + + pub fn load(keystrokes: &str, action: Box, context: Option<&str>) -> Result { + let context = if let Some(context) = context { + Some(KeymapContextPredicate::parse(context)?) + } else { + None + }; + + let keystrokes = if keystrokes == "*" { + None // Catch all context + } else { + Some( + keystrokes + .split_whitespace() + .map(Keystroke::parse) + .collect::>()?, + ) + }; + + Ok(Self { + keystrokes, + action, + context_predicate: context, + }) + } + + fn match_context(&self, context: &KeymapContext) -> bool { + self.context_predicate + .as_ref() + .map(|predicate| predicate.eval(context)) + .unwrap_or(true) + } + + pub fn match_keys_and_context( + &self, + pending_keystrokes: &Vec, + context: &KeymapContext, + ) -> BindingMatchResult { + if self + .keystrokes + .as_ref() + .map(|keystrokes| keystrokes.starts_with(&pending_keystrokes)) + .unwrap_or(true) + && self.match_context(context) + { + // If the binding is completed, push it onto the matches list + if self + .keystrokes + .as_ref() + .map(|keystrokes| keystrokes.len() == pending_keystrokes.len()) + .unwrap_or(true) + { + BindingMatchResult::Complete(self.action.boxed_clone()) + } else { + BindingMatchResult::Partial + } + } else { + BindingMatchResult::Fail + } + } + + pub fn keystrokes_for_action( + &self, + action: &dyn Action, + context: &KeymapContext, + ) -> Option> { + if self.action.eq(action) && self.match_context(context) { + self.keystrokes.clone() + } else { + None + } + } + + pub fn keystrokes(&self) -> Option<&[Keystroke]> { + self.keystrokes.as_deref() + } + + pub fn action(&self) -> &dyn Action { + self.action.as_ref() + } +} + +pub enum BindingMatchResult { + Complete(Box), + Partial, + Fail, +} diff --git a/crates/gpui/src/keymap_matcher/keymap.rs b/crates/gpui/src/keymap_matcher/keymap.rs new file mode 100644 index 0000000000..2f33164690 --- /dev/null +++ b/crates/gpui/src/keymap_matcher/keymap.rs @@ -0,0 +1,61 @@ +use smallvec::SmallVec; +use std::{ + any::{Any, TypeId}, + collections::HashMap, +}; + +use super::Binding; + +#[derive(Default)] +pub struct Keymap { + bindings: Vec, + binding_indices_by_action_type: HashMap>, +} + +impl Keymap { + pub fn new(bindings: Vec) -> Self { + let mut binding_indices_by_action_type = HashMap::new(); + for (ix, binding) in bindings.iter().enumerate() { + binding_indices_by_action_type + .entry(binding.action().type_id()) + .or_insert_with(SmallVec::new) + .push(ix); + } + + Self { + binding_indices_by_action_type, + bindings, + } + } + + pub(crate) fn bindings_for_action_type( + &self, + action_type: TypeId, + ) -> impl Iterator { + self.binding_indices_by_action_type + .get(&action_type) + .map(SmallVec::as_slice) + .unwrap_or(&[]) + .iter() + .map(|ix| &self.bindings[*ix]) + } + + pub(crate) fn add_bindings>(&mut self, bindings: T) { + for binding in bindings { + self.binding_indices_by_action_type + .entry(binding.action().type_id()) + .or_default() + .push(self.bindings.len()); + self.bindings.push(binding); + } + } + + pub(crate) fn clear(&mut self) { + self.bindings.clear(); + self.binding_indices_by_action_type.clear(); + } + + pub fn bindings(&self) -> &Vec { + &self.bindings + } +} diff --git a/crates/gpui/src/keymap_matcher/keymap_context.rs b/crates/gpui/src/keymap_matcher/keymap_context.rs new file mode 100644 index 0000000000..ad7ce929fe --- /dev/null +++ b/crates/gpui/src/keymap_matcher/keymap_context.rs @@ -0,0 +1,123 @@ +use anyhow::anyhow; + +use collections::{HashMap, HashSet}; +use tree_sitter::{Language, Node, Parser}; + +extern "C" { + fn tree_sitter_context_predicate() -> Language; +} + +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct KeymapContext { + pub set: HashSet, + pub map: HashMap, +} + +impl KeymapContext { + pub fn extend(&mut self, other: &Self) { + for v in &other.set { + self.set.insert(v.clone()); + } + for (k, v) in &other.map { + self.map.insert(k.clone(), v.clone()); + } + } +} + +#[derive(Debug, Eq, PartialEq)] +pub enum KeymapContextPredicate { + Identifier(String), + Equal(String, String), + NotEqual(String, String), + Not(Box), + And(Box, Box), + Or(Box, Box), +} + +impl KeymapContextPredicate { + pub fn parse(source: &str) -> anyhow::Result { + let mut parser = Parser::new(); + let language = unsafe { tree_sitter_context_predicate() }; + parser.set_language(language).unwrap(); + let source = source.as_bytes(); + let tree = parser.parse(source, None).unwrap(); + Self::from_node(tree.root_node(), source) + } + + fn from_node(node: Node, source: &[u8]) -> anyhow::Result { + let parse_error = "error parsing context predicate"; + let kind = node.kind(); + + match kind { + "source" => Self::from_node(node.child(0).ok_or_else(|| anyhow!(parse_error))?, source), + "identifier" => Ok(Self::Identifier(node.utf8_text(source)?.into())), + "not" => { + let child = Self::from_node( + node.child_by_field_name("expression") + .ok_or_else(|| anyhow!(parse_error))?, + source, + )?; + Ok(Self::Not(Box::new(child))) + } + "and" | "or" => { + let left = Box::new(Self::from_node( + node.child_by_field_name("left") + .ok_or_else(|| anyhow!(parse_error))?, + source, + )?); + let right = Box::new(Self::from_node( + node.child_by_field_name("right") + .ok_or_else(|| anyhow!(parse_error))?, + source, + )?); + if kind == "and" { + Ok(Self::And(left, right)) + } else { + Ok(Self::Or(left, right)) + } + } + "equal" | "not_equal" => { + let left = node + .child_by_field_name("left") + .ok_or_else(|| anyhow!(parse_error))? + .utf8_text(source)? + .into(); + let right = node + .child_by_field_name("right") + .ok_or_else(|| anyhow!(parse_error))? + .utf8_text(source)? + .into(); + if kind == "equal" { + Ok(Self::Equal(left, right)) + } else { + Ok(Self::NotEqual(left, right)) + } + } + "parenthesized" => Self::from_node( + node.child_by_field_name("expression") + .ok_or_else(|| anyhow!(parse_error))?, + source, + ), + _ => Err(anyhow!(parse_error)), + } + } + + pub fn eval(&self, context: &KeymapContext) -> bool { + match self { + Self::Identifier(name) => context.set.contains(name.as_str()), + Self::Equal(left, right) => context + .map + .get(left) + .map(|value| value == right) + .unwrap_or(false), + Self::NotEqual(left, right) => context + .map + .get(left) + .map(|value| value != right) + .unwrap_or(true), + Self::Not(pred) => !pred.eval(context), + Self::And(left, right) => left.eval(context) && right.eval(context), + Self::Or(left, right) => left.eval(context) || right.eval(context), + } + } +} diff --git a/crates/gpui/src/keymap_matcher/keystroke.rs b/crates/gpui/src/keymap_matcher/keystroke.rs new file mode 100644 index 0000000000..ed3c3f6914 --- /dev/null +++ b/crates/gpui/src/keymap_matcher/keystroke.rs @@ -0,0 +1,97 @@ +use std::fmt::Write; + +use anyhow::anyhow; +use serde::Deserialize; + +#[derive(Clone, Debug, Eq, PartialEq, Default, Deserialize)] +pub struct Keystroke { + pub ctrl: bool, + pub alt: bool, + pub shift: bool, + pub cmd: bool, + pub function: bool, + pub key: String, +} + +impl Keystroke { + pub fn parse(source: &str) -> anyhow::Result { + let mut ctrl = false; + let mut alt = false; + let mut shift = false; + let mut cmd = false; + let mut function = false; + let mut key = None; + + let mut components = source.split('-').peekable(); + while let Some(component) = components.next() { + match component { + "ctrl" => ctrl = true, + "alt" => alt = true, + "shift" => shift = true, + "cmd" => cmd = true, + "fn" => function = true, + _ => { + if let Some(component) = components.peek() { + if component.is_empty() && source.ends_with('-') { + key = Some(String::from("-")); + break; + } else { + return Err(anyhow!("Invalid keystroke `{}`", source)); + } + } else { + key = Some(String::from(component)); + } + } + } + } + + let key = key.ok_or_else(|| anyhow!("Invalid keystroke `{}`", source))?; + + Ok(Keystroke { + ctrl, + alt, + shift, + cmd, + function, + key, + }) + } + + pub fn modified(&self) -> bool { + self.ctrl || self.alt || self.shift || self.cmd + } +} + +impl std::fmt::Display for Keystroke { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.ctrl { + f.write_char('^')?; + } + if self.alt { + f.write_char('⎇')?; + } + if self.cmd { + f.write_char('⌘')?; + } + if self.shift { + f.write_char('⇧')?; + } + let key = match self.key.as_str() { + "backspace" => '⌫', + "up" => '↑', + "down" => '↓', + "left" => '←', + "right" => '→', + "tab" => '⇥', + "escape" => '⎋', + key => { + if key.len() == 1 { + key.chars().next().unwrap().to_ascii_uppercase() + } else { + return f.write_str(key); + } + } + }; + f.write_char(key) + } +} diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 2920835c49..d027218040 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -14,7 +14,7 @@ use crate::{ rect::{RectF, RectI}, vector::Vector2F, }, - keymap, + keymap_matcher::KeymapMatcher, text_layout::{LineLayout, RunStyle}, Action, ClipboardItem, Menu, Scene, }; @@ -87,7 +87,7 @@ pub(crate) trait ForegroundPlatform { fn on_menu_command(&self, callback: Box); fn on_validate_menu_command(&self, callback: Box bool>); fn on_will_open_menu(&self, callback: Box); - fn set_menus(&self, menus: Vec, matcher: &keymap::Matcher); + fn set_menus(&self, menus: Vec, matcher: &KeymapMatcher); fn prompt_for_paths( &self, options: PathPromptOptions, diff --git a/crates/gpui/src/platform/event.rs b/crates/gpui/src/platform/event.rs index 862807a74d..0c08af4497 100644 --- a/crates/gpui/src/platform/event.rs +++ b/crates/gpui/src/platform/event.rs @@ -2,7 +2,7 @@ use std::ops::Deref; use pathfinder_geometry::vector::vec2f; -use crate::{geometry::vector::Vector2F, keymap::Keystroke}; +use crate::{geometry::vector::Vector2F, keymap_matcher::Keystroke}; #[derive(Clone, Debug)] pub struct KeyDownEvent { diff --git a/crates/gpui/src/platform/mac/event.rs b/crates/gpui/src/platform/mac/event.rs index 0464840e86..c527fe8d25 100644 --- a/crates/gpui/src/platform/mac/event.rs +++ b/crates/gpui/src/platform/mac/event.rs @@ -1,6 +1,6 @@ use crate::{ geometry::vector::vec2f, - keymap::Keystroke, + keymap_matcher::Keystroke, platform::{Event, NavigationDirection}, KeyDownEvent, KeyUpEvent, Modifiers, ModifiersChangedEvent, MouseButton, MouseButtonEvent, MouseMovedEvent, ScrollDelta, ScrollWheelEvent, TouchPhase, diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index 3e2ef73634..0638689cd4 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -3,7 +3,8 @@ use super::{ FontSystem, Window, }; use crate::{ - executor, keymap, + executor, + keymap_matcher::KeymapMatcher, platform::{self, CursorStyle}, Action, AppVersion, ClipboardItem, Event, Menu, MenuItem, }; @@ -135,7 +136,7 @@ impl MacForegroundPlatform { menus: Vec, delegate: id, actions: &mut Vec>, - keystroke_matcher: &keymap::Matcher, + keystroke_matcher: &KeymapMatcher, ) -> id { let application_menu = NSMenu::new(nil).autorelease(); application_menu.setDelegate_(delegate); @@ -172,7 +173,7 @@ impl MacForegroundPlatform { item: MenuItem, delegate: id, actions: &mut Vec>, - keystroke_matcher: &keymap::Matcher, + keystroke_matcher: &KeymapMatcher, ) -> id { match item { MenuItem::Separator => NSMenuItem::separatorItem(nil), @@ -183,7 +184,7 @@ impl MacForegroundPlatform { .map(|binding| binding.keystrokes()); let item; - if let Some(keystrokes) = keystrokes { + if let Some(keystrokes) = keystrokes.flatten() { if keystrokes.len() == 1 { let keystroke = &keystrokes[0]; let mut mask = NSEventModifierFlags::empty(); @@ -317,7 +318,7 @@ impl platform::ForegroundPlatform for MacForegroundPlatform { self.0.borrow_mut().validate_menu_command = Some(callback); } - fn set_menus(&self, menus: Vec, keystroke_matcher: &keymap::Matcher) { + fn set_menus(&self, menus: Vec, keystroke_matcher: &KeymapMatcher) { unsafe { let app: id = msg_send![APP_CLASS, sharedApplication]; let mut state = self.0.borrow_mut(); diff --git a/crates/gpui/src/platform/mac/window.rs b/crates/gpui/src/platform/mac/window.rs index 57c5c3711d..958ac2ebd6 100644 --- a/crates/gpui/src/platform/mac/window.rs +++ b/crates/gpui/src/platform/mac/window.rs @@ -4,7 +4,7 @@ use crate::{ rect::RectF, vector::{vec2f, Vector2F}, }, - keymap::Keystroke, + keymap_matcher::Keystroke, mac::platform::NSViewLayerContentsRedrawDuringViewResize, platform::{ self, diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index 1bd92eb6e3..00cd524c1d 100644 --- a/crates/gpui/src/platform/test.rs +++ b/crates/gpui/src/platform/test.rs @@ -4,7 +4,8 @@ use crate::{ rect::RectF, vector::{vec2f, Vector2F}, }, - keymap, Action, ClipboardItem, + keymap_matcher::KeymapMatcher, + Action, ClipboardItem, }; use anyhow::{anyhow, Result}; use collections::VecDeque; @@ -84,7 +85,7 @@ impl super::ForegroundPlatform for ForegroundPlatform { fn on_menu_command(&self, _: Box) {} fn on_validate_menu_command(&self, _: Box bool>) {} fn on_will_open_menu(&self, _: Box) {} - fn set_menus(&self, _: Vec, _: &keymap::Matcher) {} + fn set_menus(&self, _: Vec, _: &KeymapMatcher) {} fn prompt_for_paths( &self, diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index eb7554a39c..5c13f7467a 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -4,7 +4,7 @@ use crate::{ font_cache::FontCache, geometry::rect::RectF, json::{self, ToJson}, - keymap::Keystroke, + keymap_matcher::Keystroke, platform::{CursorStyle, Event}, scene::{ CursorRegion, MouseClick, MouseDown, MouseDownOut, MouseDrag, MouseEvent, MouseHover, diff --git a/crates/live_kit_client/examples/test_app.rs b/crates/live_kit_client/examples/test_app.rs index eddee785bc..a92b106d33 100644 --- a/crates/live_kit_client/examples/test_app.rs +++ b/crates/live_kit_client/examples/test_app.rs @@ -1,5 +1,5 @@ use futures::StreamExt; -use gpui::{actions, keymap::Binding, Menu, MenuItem}; +use gpui::{actions, keymap_matcher::Binding, Menu, MenuItem}; use live_kit_client::{LocalVideoTrack, RemoteVideoTrackUpdate, Room}; use live_kit_server::token::{self, VideoGrant}; use log::LevelFilter; diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index a9cf23fb3f..dd6ab78c29 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -2,7 +2,7 @@ use editor::Editor; use gpui::{ elements::*, geometry::vector::{vec2f, Vector2F}, - keymap, + keymap_matcher::KeymapContext, platform::CursorStyle, AnyViewHandle, AppContext, Axis, Entity, MouseButton, MouseState, MutableAppContext, RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle, @@ -124,7 +124,7 @@ impl View for Picker { .named("picker") } - fn keymap_context(&self, _: &AppContext) -> keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { let mut cx = Self::default_keymap_context(); cx.set.insert("menu".into()); cx diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index e88f3004eb..0042695950 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -10,7 +10,8 @@ use gpui::{ MouseEventHandler, ParentElement, ScrollTarget, Stack, Svg, UniformList, UniformListState, }, geometry::vector::Vector2F, - impl_internal_actions, keymap, + impl_internal_actions, + keymap_matcher::KeymapContext, platform::CursorStyle, AppContext, ClipboardItem, Element, ElementBox, Entity, ModelHandle, MouseButton, MutableAppContext, PromptLevel, RenderContext, Task, View, ViewContext, ViewHandle, @@ -1301,7 +1302,7 @@ impl View for ProjectPanel { .boxed() } - fn keymap_context(&self, _: &AppContext) -> keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { let mut cx = Self::default_keymap_context(); cx.set.insert("menu".into()); cx diff --git a/crates/settings/src/keymap_file.rs b/crates/settings/src/keymap_file.rs index 4dcb5a6fb0..4090bcc63a 100644 --- a/crates/settings/src/keymap_file.rs +++ b/crates/settings/src/keymap_file.rs @@ -2,7 +2,7 @@ use crate::{parse_json_with_comments, Settings}; use anyhow::{Context, Result}; use assets::Assets; use collections::BTreeMap; -use gpui::{keymap::Binding, MutableAppContext}; +use gpui::{keymap_matcher::Binding, MutableAppContext}; use schemars::{ gen::{SchemaGenerator, SchemaSettings}, schema::{InstanceType, Schema, SchemaObject, SingleOrVec, SubschemaValidation}, diff --git a/crates/terminal/src/mappings/keys.rs b/crates/terminal/src/mappings/keys.rs index ddcd6c5898..9d19625971 100644 --- a/crates/terminal/src/mappings/keys.rs +++ b/crates/terminal/src/mappings/keys.rs @@ -1,6 +1,6 @@ /// The mappings defined in this file where created from reading the alacritty source use alacritty_terminal::term::TermMode; -use gpui::keymap::Keystroke; +use gpui::keymap_matcher::Keystroke; #[derive(Debug, PartialEq, Eq)] pub enum Modifiers { @@ -273,6 +273,8 @@ fn modifier_code(keystroke: &Keystroke) -> u32 { #[cfg(test)] mod test { + use gpui::keymap_matcher::Keystroke; + use super::*; #[test] diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 7cdac33cda..dd5c5fb3b0 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -50,7 +50,7 @@ use thiserror::Error; use gpui::{ geometry::vector::{vec2f, Vector2F}, - keymap::Keystroke, + keymap_matcher::Keystroke, scene::{MouseDown, MouseDrag, MouseScrollWheel, MouseUp}, ClipboardItem, Entity, ModelContext, MouseButton, MouseMovedEvent, Task, }; diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 7602a3db22..a4f90a8d72 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -14,7 +14,7 @@ use gpui::{ elements::{AnchorCorner, ChildView, Flex, Label, ParentElement, Stack, Text}, geometry::vector::Vector2F, impl_actions, impl_internal_actions, - keymap::Keystroke, + keymap_matcher::{KeymapContext, Keystroke}, AnyViewHandle, AppContext, Element, ElementBox, Entity, ModelHandle, MutableAppContext, Task, View, ViewContext, ViewHandle, WeakViewHandle, }; @@ -465,7 +465,7 @@ impl View for TerminalView { }); } - fn keymap_context(&self, cx: &gpui::AppContext) -> gpui::keymap::Context { + fn keymap_context(&self, cx: &gpui::AppContext) -> KeymapContext { let mut context = Self::default_keymap_context(); let mode = self.terminal.read(cx).last_content.mode; diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 5aa1df6dd8..9089eebcb5 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -3,7 +3,7 @@ use editor::{ display_map::{DisplaySnapshot, ToDisplayPoint}, movement, Bias, CharKind, DisplayPoint, }; -use gpui::{actions, impl_actions, MutableAppContext}; +use gpui::{actions, impl_actions, keymap_matcher::KeyPressed, MutableAppContext}; use language::{Point, Selection, SelectionGoal}; use serde::Deserialize; use workspace::Workspace; @@ -32,6 +32,8 @@ pub enum Motion { StartOfDocument, EndOfDocument, Matching, + FindForward { before: bool, character: char }, + FindBackward { after: bool, character: char }, } #[derive(Clone, Deserialize, PartialEq)] @@ -107,10 +109,34 @@ pub fn init(cx: &mut MutableAppContext) { &PreviousWordStart { ignore_punctuation }: &PreviousWordStart, cx: _| { motion(Motion::PreviousWordStart { ignore_punctuation }, cx) }, ); + cx.add_action( + |_: &mut Workspace, KeyPressed { keystroke }: &KeyPressed, cx| match Vim::read(cx) + .active_operator() + { + Some(Operator::FindForward { before }) => motion( + Motion::FindForward { + before, + character: keystroke.key.chars().next().unwrap(), + }, + cx, + ), + Some(Operator::FindBackward { after }) => motion( + Motion::FindBackward { + after, + character: keystroke.key.chars().next().unwrap(), + }, + cx, + ), + _ => cx.propagate_action(), + }, + ) } pub(crate) fn motion(motion: Motion, cx: &mut MutableAppContext) { - if let Some(Operator::Namespace(_)) = Vim::read(cx).active_operator() { + if let Some(Operator::Namespace(_)) + | Some(Operator::FindForward { .. }) + | Some(Operator::FindBackward { .. }) = Vim::read(cx).active_operator() + { Vim::update(cx, |vim, cx| vim.pop_operator(cx)); } @@ -152,14 +178,16 @@ impl Motion { | CurrentLine | EndOfLine | NextWordEnd { .. } - | Matching => true, + | Matching + | FindForward { .. } => true, Left | Backspace | Right | StartOfLine | NextWordStart { .. } | PreviousWordStart { .. } - | FirstNonWhitespace => false, + | FirstNonWhitespace + | FindBackward { .. } => false, } } @@ -196,6 +224,14 @@ impl Motion { StartOfDocument => (start_of_document(map, point, times), SelectionGoal::None), EndOfDocument => (end_of_document(map, point, times), SelectionGoal::None), Matching => (matching(map, point), SelectionGoal::None), + FindForward { before, character } => ( + find_forward(map, point, before, character, times), + SelectionGoal::None, + ), + FindBackward { after, character } => ( + find_backward(map, point, after, character, times), + SelectionGoal::None, + ), }; (new_point != point || self.infallible()).then_some((new_point, goal)) @@ -446,3 +482,50 @@ fn matching(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint { point } } + +fn find_forward( + map: &DisplaySnapshot, + from: DisplayPoint, + before: bool, + target: char, + mut times: usize, +) -> DisplayPoint { + let mut previous_point = from; + + for (ch, point) in map.chars_at(from) { + if ch == target && point != from { + times -= 1; + if times == 0 { + return if before { previous_point } else { point }; + } + } else if ch == '\n' { + break; + } + previous_point = point; + } + + from +} + +fn find_backward( + map: &DisplaySnapshot, + from: DisplayPoint, + after: bool, + target: char, + mut times: usize, +) -> DisplayPoint { + let mut previous_point = from; + for (ch, point) in map.reverse_chars_at(from) { + if ch == target && point != from { + times -= 1; + if times == 0 { + return if after { previous_point } else { point }; + } + } else if ch == '\n' { + break; + } + previous_point = point; + } + + from +} diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index bc65fbd09e..d88d496ee9 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -18,6 +18,7 @@ use editor::{ }; use gpui::{actions, impl_actions, MutableAppContext, ViewContext}; use language::{AutoindentMode, Point, SelectionGoal}; +use log::error; use serde::Deserialize; use workspace::Workspace; @@ -101,8 +102,9 @@ pub fn normal_motion( Some(Operator::Change) => change_motion(vim, motion, times, cx), Some(Operator::Delete) => delete_motion(vim, motion, times, cx), Some(Operator::Yank) => yank_motion(vim, motion, times, cx), - _ => { + Some(operator) => { // Can't do anything for text objects or namespace operators. Ignoring + error!("Unexpected normal mode motion operator: {:?}", operator) } } }); @@ -912,4 +914,42 @@ mod test { let mut cx = NeovimBackedTestContext::new(cx).await.binding(["h"]); cx.assert_all("Testˇ├ˇ──ˇ┐ˇTest").await; } + + #[gpui::test] + async fn test_f_and_t(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + for count in 1..=3 { + let test_case = indoc! {" + ˇaaaˇbˇ ˇbˇ ˇbˇbˇ aˇaaˇbaaa + ˇ ˇbˇaaˇa ˇbˇbˇb + ˇ + ˇb + "}; + + cx.assert_binding_matches_all([&count.to_string(), "f", "b"], test_case) + .await; + + cx.assert_binding_matches_all([&count.to_string(), "t", "b"], test_case) + .await; + } + } + + #[gpui::test] + async fn test_capital_f_and_capital_t(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + for count in 1..=3 { + let test_case = indoc! {" + ˇaaaˇbˇ ˇbˇ ˇbˇbˇ aˇaaˇbaaa + ˇ ˇbˇaaˇa ˇbˇbˇb + ˇ + ˇb + "}; + + cx.assert_binding_matches_all([&count.to_string(), "shift-f", "b"], test_case) + .await; + + cx.assert_binding_matches_all([&count.to_string(), "shift-t", "b"], test_case) + .await; + } + } } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 6bbab1ae42..c5e52a520d 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -1,4 +1,4 @@ -use gpui::keymap::Context; +use gpui::keymap_matcher::KeymapContext; use language::CursorShape; use serde::{Deserialize, Serialize}; @@ -29,6 +29,8 @@ pub enum Operator { Delete, Yank, Object { around: bool }, + FindForward { before: bool }, + FindBackward { after: bool }, } #[derive(Default)] @@ -54,6 +56,10 @@ impl VimState { pub fn vim_controlled(&self) -> bool { !matches!(self.mode, Mode::Insert) + || matches!( + self.operator_stack.last(), + Some(Operator::FindForward { .. }) | Some(Operator::FindBackward { .. }) + ) } pub fn clip_at_line_end(&self) -> bool { @@ -64,8 +70,8 @@ impl VimState { !matches!(self.mode, Mode::Visual { .. }) } - pub fn keymap_context_layer(&self) -> Context { - let mut context = Context::default(); + pub fn keymap_context_layer(&self) -> KeymapContext { + let mut context = KeymapContext::default(); context.map.insert( "vim_mode".to_string(), match self.mode { @@ -81,34 +87,48 @@ impl VimState { } let active_operator = self.operator_stack.last(); - if matches!(active_operator, Some(Operator::Object { .. })) { - context.set.insert("VimObject".to_string()); + + if let Some(active_operator) = active_operator { + for context_flag in active_operator.context_flags().into_iter() { + context.set.insert(context_flag.to_string()); + } } - Operator::set_context(active_operator, &mut context); + context.map.insert( + "vim_operator".to_string(), + active_operator + .map(|op| op.id()) + .unwrap_or_else(|| "none") + .to_string(), + ); context } } impl Operator { - pub fn set_context(operator: Option<&Operator>, context: &mut Context) { - let operator_context = match operator { - Some(Operator::Number(_)) => "n", - Some(Operator::Namespace(Namespace::G)) => "g", - Some(Operator::Namespace(Namespace::Z)) => "z", - Some(Operator::Object { around: false }) => "i", - Some(Operator::Object { around: true }) => "a", - Some(Operator::Change) => "c", - Some(Operator::Delete) => "d", - Some(Operator::Yank) => "y", - - None => "none", + pub fn id(&self) -> &'static str { + match self { + Operator::Number(_) => "n", + Operator::Namespace(Namespace::G) => "g", + Operator::Namespace(Namespace::Z) => "z", + Operator::Object { around: false } => "i", + Operator::Object { around: true } => "a", + Operator::Change => "c", + Operator::Delete => "d", + Operator::Yank => "y", + Operator::FindForward { before: false } => "f", + Operator::FindForward { before: true } => "t", + Operator::FindBackward { after: false } => "F", + Operator::FindBackward { after: true } => "T", } - .to_owned(); + } - context - .map - .insert("vim_operator".to_string(), operator_context); + pub fn context_flags(&self) -> &'static [&'static str] { + match self { + Operator::Object { .. } => &["VimObject"], + Operator::FindForward { .. } | Operator::FindBackward { .. } => &["VimWaiting"], + _ => &[], + } } } diff --git a/crates/vim/src/test/neovim_connection.rs b/crates/vim/src/test/neovim_connection.rs index e2522a76aa..1850a03171 100644 --- a/crates/vim/src/test/neovim_connection.rs +++ b/crates/vim/src/test/neovim_connection.rs @@ -7,7 +7,7 @@ use async_compat::Compat; #[cfg(feature = "neovim")] use async_trait::async_trait; #[cfg(feature = "neovim")] -use gpui::keymap::Keystroke; +use gpui::keymap_matcher::Keystroke; use language::{Point, Selection}; diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 40cc414778..4d582fea6b 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -16,7 +16,6 @@ use editor::{Bias, Cancel, Editor}; use gpui::{impl_actions, MutableAppContext, Subscription, ViewContext, WeakViewHandle}; use language::CursorShape; use serde::Deserialize; - use settings::Settings; use state::{Mode, Operator, VimState}; use workspace::{self, Workspace}; @@ -55,7 +54,7 @@ pub fn init(cx: &mut MutableAppContext) { // Editor Actions cx.add_action(|_: &mut Editor, _: &Cancel, cx| { - // If we are in a non normal mode or have an active operator, swap to normal mode + // If we are in aren't in normal mode or have an active operator, swap to normal mode // Otherwise forward cancel on to the editor let vim = Vim::read(cx); if vim.state.mode != Mode::Normal || vim.active_operator().is_some() { @@ -81,17 +80,21 @@ pub fn init(cx: &mut MutableAppContext) { .detach(); } -// Any keystrokes not mapped to vim should clear the active operator pub fn observe_keypresses(window_id: usize, cx: &mut MutableAppContext) { cx.observe_keystrokes(window_id, |_keystroke, _result, handled_by, cx| { if let Some(handled_by) = handled_by { - if handled_by.namespace() == "vim" { + // Keystroke is handled by the vim system, so continue forward + // Also short circuit if it is the special cancel action + if handled_by.namespace() == "vim" + || (handled_by.namespace() == "editor" && handled_by.name() == "Cancel") + { return true; } } Vim::update(cx, |vim, cx| { if vim.active_operator().is_some() { + // If the keystroke is not handled by vim, we should clear the operator vim.clear_operator(cx); } }); diff --git a/crates/vim/test_data/test_capital_f_and_capital_t.json b/crates/vim/test_data/test_capital_f_and_capital_t.json new file mode 100644 index 0000000000..4c7f760021 --- /dev/null +++ b/crates/vim/test_data/test_capital_f_and_capital_t.json @@ -0,0 +1 @@ +[{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,0],"end":[0,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,0],"end":[0,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,0],"end":[0,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,7],"end":[1,7]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,0],"end":[0,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,7],"end":[1,7]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,0],"end":[0,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,7],"end":[1,7]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,0],"end":[0,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,6],"end":[0,6]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,7],"end":[1,7]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,5],"end":[1,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"}] \ No newline at end of file diff --git a/crates/vim/test_data/test_f_and_t.json b/crates/vim/test_data/test_f_and_t.json new file mode 100644 index 0000000000..35f4fd5e1d --- /dev/null +++ b/crates/vim/test_data/test_f_and_t.json @@ -0,0 +1 @@ +[{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,3],"end":[0,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,4],"end":[1,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,2],"end":[0,2]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,8],"end":[0,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,8],"end":[0,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,14],"end":[0,14]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,14],"end":[0,14]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,14],"end":[0,14]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,3],"end":[1,3]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,8],"end":[1,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,8],"end":[1,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,8],"end":[1,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,5],"end":[0,5]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,13],"end":[0,13]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,4],"end":[0,4]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,8],"end":[0,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,8],"end":[0,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,14],"end":[0,14]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,13],"end":[0,13]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,8],"end":[1,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,13],"end":[0,13]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,8],"end":[0,8]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,14],"end":[0,14]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,14],"end":[0,14]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,9],"end":[0,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,10],"end":[0,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,13],"end":[0,13]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,9],"end":[1,9]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,10],"end":[1,10]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[1,11],"end":[1,11]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Normal"},{"Text":"aaab b bb aaabaaa\n baaa bbb\n \nb\n"},{"Mode":"Normal"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Normal"}] \ No newline at end of file diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 06e19bbf88..5794ca7f5f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -33,6 +33,7 @@ use gpui::{ actions, elements::*, impl_actions, impl_internal_actions, + keymap_matcher::KeymapContext, platform::{CursorStyle, WindowOptions}, AnyModelHandle, AnyViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MouseButton, MutableAppContext, PathPromptOptions, PromptLevel, RenderContext, Task, View, @@ -2588,7 +2589,7 @@ impl View for Workspace { } } - fn keymap_context(&self, _: &AppContext) -> gpui::keymap::Context { + fn keymap_context(&self, _: &AppContext) -> KeymapContext { let mut keymap = Self::default_keymap_context(); if self.active_pane() == self.dock_pane() { keymap.set.insert("Dock".into()); From 213658f1e9105628a7b137058973601ea030e015 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Fri, 6 Jan 2023 17:56:21 -0700 Subject: [PATCH 26/33] Fix tests that failed due to defaulting the grouping interval to zero in tests --- crates/collab/src/tests/integration_tests.rs | 7 ++++++- crates/editor/src/editor.rs | 2 ++ crates/editor/src/editor_tests.rs | 8 +++++++- crates/language/src/buffer_tests.rs | 11 ++++++++++- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 4a1aaf64d1..729da6d109 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -1131,6 +1131,7 @@ async fn test_unshare_project( .unwrap(); let worktree_a = project_a.read_with(cx_a, |project, cx| project.worktrees(cx).next().unwrap()); let project_b = client_b.build_remote_project(project_id, cx_b).await; + deterministic.run_until_parked(); assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared())); project_b @@ -1160,6 +1161,7 @@ async fn test_unshare_project( .await .unwrap(); let project_c2 = client_c.build_remote_project(project_id, cx_c).await; + deterministic.run_until_parked(); assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared())); project_c2 .update(cx_c, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) @@ -1213,6 +1215,7 @@ async fn test_host_disconnect( .unwrap(); let project_b = client_b.build_remote_project(project_id, cx_b).await; + deterministic.run_until_parked(); assert!(worktree_a.read_with(cx_a, |tree, _| tree.as_local().unwrap().is_shared())); let (_, workspace_b) = cx_b.add_window(|cx| { @@ -1467,7 +1470,7 @@ async fn test_project_reconnect( .read_with(cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; let worktree3_id = worktree_a3.read_with(cx_a, |tree, _| { - assert!(tree.as_local().unwrap().is_shared()); + assert!(!tree.as_local().unwrap().is_shared()); tree.id() }); deterministic.run_until_parked(); @@ -1489,6 +1492,7 @@ async fn test_project_reconnect( deterministic.run_until_parked(); project_a1.read_with(cx_a, |project, cx| { assert!(project.is_shared()); + assert!(worktree_a1.read(cx).as_local().unwrap().is_shared()); assert_eq!( worktree_a1 .read(cx) @@ -1510,6 +1514,7 @@ async fn test_project_reconnect( "subdir2/i.txt" ] ); + assert!(worktree_a3.read(cx).as_local().unwrap().is_shared()); assert_eq!( worktree_a3 .read(cx) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d8ee49866b..85da12658a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3611,7 +3611,9 @@ impl Editor { } pub fn undo(&mut self, _: &Undo, cx: &mut ViewContext) { + dbg!("undo"); if let Some(tx_id) = self.buffer.update(cx, |buffer, cx| buffer.undo(cx)) { + dbg!(tx_id); if let Some((selections, _)) = self.selection_history.transaction(tx_id).cloned() { self.change_selections(None, cx, |s| { s.select_anchors(selections.to_vec()); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 2fcc5f0014..b9f3c67f38 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -29,7 +29,11 @@ use workspace::{ #[gpui::test] fn test_edit_events(cx: &mut MutableAppContext) { cx.set_global(Settings::test(cx)); - let buffer = cx.add_model(|cx| language::Buffer::new(0, "123456", cx)); + let buffer = cx.add_model(|cx| { + let mut buffer = language::Buffer::new(0, "123456", cx); + buffer.set_group_interval(Duration::from_secs(1)); + buffer + }); let events = Rc::new(RefCell::new(Vec::new())); let (_, editor1) = cx.add_window(Default::default(), { @@ -3502,6 +3506,8 @@ async fn test_surround_with_pair(cx: &mut gpui::TestAppContext) { ] ); + view.undo(&Undo, cx); + view.undo(&Undo, cx); view.undo(&Undo, cx); assert_eq!( view.text(cx), diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index e0b7d080cb..09ccc5d621 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -289,6 +289,9 @@ async fn test_reparse(cx: &mut gpui::TestAppContext) { ); buffer.update(cx, |buf, cx| { + buf.undo(cx); + buf.undo(cx); + buf.undo(cx); buf.undo(cx); assert_eq!(buf.text(), "fn a() {}"); assert!(buf.is_parsing()); @@ -304,6 +307,9 @@ async fn test_reparse(cx: &mut gpui::TestAppContext) { ); buffer.update(cx, |buf, cx| { + buf.redo(cx); + buf.redo(cx); + buf.redo(cx); buf.redo(cx); assert_eq!(buf.text(), "fn a(b: C) { d.e::(f); }"); assert!(buf.is_parsing()); @@ -1022,8 +1028,11 @@ fn test_autoindent_block_mode(cx: &mut MutableAppContext) { .unindent() ); + // Grouping is disabled in tests, so we need 2 undos + buffer.undo(cx); // Undo the auto-indent + buffer.undo(cx); // Undo the original edit + // Insert the block at a deeper indent level. The entire block is outdented. - buffer.undo(cx); buffer.edit([(Point::new(2, 0)..Point::new(2, 0), " ")], None, cx); buffer.edit( [(Point::new(2, 8)..Point::new(2, 8), inserted_text)], From 12cd712b53a3cfd825d826c8962382c5c7ff3f9b Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 6 Jan 2023 22:46:32 -0500 Subject: [PATCH 27/33] Require start autocomplete query byte to match a completion word start byte --- crates/editor/src/editor.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index d8ee49866b..354a2be97d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -827,6 +827,40 @@ impl CompletionsMenu { }) .collect() }; + + //Remove all candidates where the query's start does not match the start of any word in the candidate + if let Some(query) = query { + if let Some(&start) = query.as_bytes().get(0) { + let start = start.to_ascii_lowercase(); + matches.retain(|m| { + let bytes = m.string.as_bytes(); + let mut index = 0; + + std::iter::from_fn(move || { + let start_index = index; + while index < bytes.len() { + let current_upper = bytes[index].is_ascii_uppercase(); + let has_more = index + 1 < bytes.len(); + let next_upper = has_more && bytes[index + 1].is_ascii_uppercase(); + + index += 1; + if !current_upper && next_upper { + return Some(&m.string[start_index..index]); + } + } + + index = bytes.len(); + if start_index < bytes.len() { + return Some(&m.string[start_index..]); + } + None + }) + .flat_map(|w| w.split_inclusive('_')) + .any(|w| w.as_bytes().first().map(|&b| b.to_ascii_lowercase()) == Some(start)) + }); + } + } + matches.sort_unstable_by_key(|mat| { let completion = &self.completions[mat.candidate_id]; ( From a46ca323567aada6960467c447d789a606db3d6a Mon Sep 17 00:00:00 2001 From: Julia Date: Sat, 7 Jan 2023 15:34:28 -0500 Subject: [PATCH 28/33] Completion word start filtering which is codepoint aware --- crates/editor/src/editor.rs | 38 ++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 354a2be97d..4405fc0b33 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -830,33 +830,41 @@ impl CompletionsMenu { //Remove all candidates where the query's start does not match the start of any word in the candidate if let Some(query) = query { - if let Some(&start) = query.as_bytes().get(0) { - let start = start.to_ascii_lowercase(); - matches.retain(|m| { - let bytes = m.string.as_bytes(); + if let Some(query_start) = query.chars().next() { + matches.retain(|string_match| { + let text = &string_match.string; + let mut index = 0; + let mut codepoints = text.char_indices().peekable(); std::iter::from_fn(move || { let start_index = index; - while index < bytes.len() { - let current_upper = bytes[index].is_ascii_uppercase(); - let has_more = index + 1 < bytes.len(); - let next_upper = has_more && bytes[index + 1].is_ascii_uppercase(); + while let Some((new_index, codepoint)) = codepoints.next() { + index = new_index + codepoint.len_utf8(); + let current_upper = codepoint.is_uppercase(); + let next_upper = codepoints + .peek() + .map(|(_, c)| c.is_uppercase()) + .unwrap_or(false); - index += 1; if !current_upper && next_upper { - return Some(&m.string[start_index..index]); + return Some(&text[start_index..index]); } } - index = bytes.len(); - if start_index < bytes.len() { - return Some(&m.string[start_index..]); + index = text.len(); + if start_index < text.len() { + return Some(&text[start_index..]); } None }) - .flat_map(|w| w.split_inclusive('_')) - .any(|w| w.as_bytes().first().map(|&b| b.to_ascii_lowercase()) == Some(start)) + .flat_map(|word| word.split_inclusive('_')) + .any(|word| { + word.chars() + .flat_map(|codepoint| codepoint.to_lowercase()) + .zip(query_start.to_lowercase()) + .all(|(word_cp, query_cp)| word_cp == query_cp) + }) }); } } From 529ccbda3a9e784538e8aef5474236de2d848c21 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 7 Jan 2023 16:25:00 -0700 Subject: [PATCH 29/33] Introduce git index mutations to randomized collaboration test The test now fails at the following seed: ```bash SEED=850 ITERATIONS=1 OPERATIONS=131 cargo test --package=collab random ``` --- .../src/tests/randomized_integration_tests.rs | 68 +++++++++++++++++-- 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index a42d4f7d32..b067cac5ff 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -14,8 +14,11 @@ use language::{range_to_lsp, FakeLspAdapter, Language, LanguageConfig, PointUtf1 use lsp::FakeLanguageServer; use parking_lot::Mutex; use project::{search::SearchQuery, Project}; -use rand::prelude::*; -use std::{env, path::PathBuf, sync::Arc}; +use rand::{ + distributions::{Alphanumeric, DistString}, + prelude::*, +}; +use std::{env, ffi::OsStr, path::PathBuf, sync::Arc}; #[gpui::test(iterations = 100)] async fn test_random_collaboration( @@ -382,9 +385,15 @@ async fn test_random_collaboration( ); } (None, None) => {} - (None, _) => panic!("host's file is None, guest's isn't "), - (_, None) => panic!("guest's file is None, hosts's isn't "), + (None, _) => panic!("host's file is None, guest's isn't"), + (_, None) => panic!("guest's file is None, hosts's isn't"), } + + let host_diff_base = + host_buffer.read_with(host_cx, |b, _| b.diff_base().map(ToString::to_string)); + let guest_diff_base = guest_buffer + .read_with(client_cx, |b, _| b.diff_base().map(ToString::to_string)); + assert_eq!(guest_diff_base, host_diff_base); } } } @@ -543,9 +552,10 @@ async fn randomly_mutate_client( 50..=59 if !client.local_projects.is_empty() || !client.remote_projects.is_empty() => { randomly_mutate_worktrees(client, &rng, cx).await?; } - 60..=84 if !client.local_projects.is_empty() || !client.remote_projects.is_empty() => { + 60..=74 if !client.local_projects.is_empty() || !client.remote_projects.is_empty() => { randomly_query_and_mutate_buffers(client, &rng, cx).await?; } + 75..=84 => randomly_mutate_git(client, &rng).await, _ => randomly_mutate_fs(client, &rng).await, } @@ -605,6 +615,54 @@ async fn randomly_mutate_active_call( Ok(()) } +async fn randomly_mutate_git(client: &mut TestClient, rng: &Mutex) { + let directories = client.fs.directories().await; + let mut dir_path = directories.choose(&mut *rng.lock()).unwrap().clone(); + if dir_path.file_name() == Some(OsStr::new(".git")) { + dir_path.pop(); + } + let mut git_dir_path = dir_path.clone(); + git_dir_path.push(".git"); + + if !directories.contains(&git_dir_path) { + log::info!( + "{}: creating git directory at {:?}", + client.username, + git_dir_path + ); + client.fs.create_dir(&git_dir_path).await.unwrap(); + } + + let mut child_paths = client.fs.read_dir(&dir_path).await.unwrap(); + let mut child_file_paths = Vec::new(); + while let Some(child_path) = child_paths.next().await { + let child_path = child_path.unwrap(); + if client.fs.is_file(&child_path).await { + child_file_paths.push(child_path); + } + } + let count = rng.lock().gen_range(0..=child_file_paths.len()); + child_file_paths.shuffle(&mut *rng.lock()); + child_file_paths.truncate(count); + + let mut new_index = Vec::new(); + for abs_child_file_path in &child_file_paths { + let child_file_path = abs_child_file_path.strip_prefix(&dir_path).unwrap(); + let new_base = Alphanumeric.sample_string(&mut *rng.lock(), 16); + new_index.push((child_file_path, new_base)); + } + log::info!( + "{}: updating Git index at {:?}: {:#?}", + client.username, + git_dir_path, + new_index + ); + client + .fs + .set_index_for_repo(&git_dir_path, &new_index) + .await; +} + async fn randomly_mutate_fs(client: &mut TestClient, rng: &Mutex) { let is_dir = rng.lock().gen::(); let mut new_path = client From 95098e4f29ae235d096a6453a72d59fc052305e6 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sun, 8 Jan 2023 09:10:57 -0700 Subject: [PATCH 30/33] Update git diff base when synchronizing a guest's buffers --- crates/project/src/project.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 271d0f242b..608abc5815 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5189,20 +5189,27 @@ impl Project { let operations = buffer.serialize_ops(Some(remote_version), cx); let client = this.client.clone(); - let file = buffer.file().cloned(); + if let Some(file) = buffer.file() { + client + .send(proto::UpdateBufferFile { + project_id, + buffer_id: buffer_id as u64, + file: Some(file.to_proto()), + }) + .log_err(); + } + + client + .send(proto::UpdateDiffBase { + project_id, + buffer_id: buffer_id as u64, + diff_base: buffer.diff_base().map(Into::into), + }) + .log_err(); + cx.background() .spawn( async move { - if let Some(file) = file { - client - .send(proto::UpdateBufferFile { - project_id, - buffer_id: buffer_id as u64, - file: Some(file.to_proto()), - }) - .log_err(); - } - let operations = operations.await; for chunk in split_operations(operations) { client From ad7eaca443f7fd5c81c3823238b902666f2624ef Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sun, 8 Jan 2023 09:36:58 -0700 Subject: [PATCH 31/33] Make `Buffer::diff_base` available outside of tests --- crates/language/src/buffer.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 66f8651e9d..9ceb6b32a9 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -682,7 +682,6 @@ impl Buffer { task } - #[cfg(any(test, feature = "test-support"))] pub fn diff_base(&self) -> Option<&str> { self.diff_base.as_deref() } From 97ed89a797fb11c9bfa3a6900dc62f817b628d1d Mon Sep 17 00:00:00 2001 From: Julia Date: Mon, 9 Jan 2023 13:02:44 -0500 Subject: [PATCH 32/33] Test that completion word splitting does reasonable things --- crates/editor/src/editor.rs | 59 ++++++++++++++++--------------- crates/editor/src/editor_tests.rs | 14 ++++++++ 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 4405fc0b33..356fd0ca4f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -832,34 +832,9 @@ impl CompletionsMenu { if let Some(query) = query { if let Some(query_start) = query.chars().next() { matches.retain(|string_match| { - let text = &string_match.string; - - let mut index = 0; - let mut codepoints = text.char_indices().peekable(); - - std::iter::from_fn(move || { - let start_index = index; - while let Some((new_index, codepoint)) = codepoints.next() { - index = new_index + codepoint.len_utf8(); - let current_upper = codepoint.is_uppercase(); - let next_upper = codepoints - .peek() - .map(|(_, c)| c.is_uppercase()) - .unwrap_or(false); - - if !current_upper && next_upper { - return Some(&text[start_index..index]); - } - } - - index = text.len(); - if start_index < text.len() { - return Some(&text[start_index..]); - } - None - }) - .flat_map(|word| word.split_inclusive('_')) - .any(|word| { + split_words(&string_match.string).any(|word| { + //Check that the first codepoint of the word as lowercase matches the first + //codepoint of the query as lowercase word.chars() .flat_map(|codepoint| codepoint.to_lowercase()) .zip(query_start.to_lowercase()) @@ -6841,6 +6816,34 @@ pub fn styled_runs_for_code_label<'a>( }) } +pub fn split_words<'a>(text: &'a str) -> impl std::iter::Iterator + 'a { + let mut index = 0; + let mut codepoints = text.char_indices().peekable(); + + std::iter::from_fn(move || { + let start_index = index; + while let Some((new_index, codepoint)) = codepoints.next() { + index = new_index + codepoint.len_utf8(); + let current_upper = codepoint.is_uppercase(); + let next_upper = codepoints + .peek() + .map(|(_, c)| c.is_uppercase()) + .unwrap_or(false); + + if !current_upper && next_upper { + return Some(&text[start_index..index]); + } + } + + index = text.len(); + if start_index < text.len() { + return Some(&text[start_index..]); + } + None + }) + .flat_map(|word| word.split_inclusive('_')) +} + trait RangeExt { fn sorted(&self) -> Range; fn to_inclusive(&self) -> RangeInclusive; diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 2fcc5f0014..798636c0a1 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -5439,6 +5439,20 @@ async fn go_to_hunk(deterministic: Arc, cx: &mut gpui::TestAppCon ); } +#[test] +fn test_split_words() { + fn split<'a>(text: &'a str) -> Vec<&'a str> { + split_words(text).collect() + } + + assert_eq!(split("HelloWorld"), &["Hello", "World"]); + assert_eq!(split("hello_world"), &["hello_", "world"]); + assert_eq!(split("_hello_world_"), &["_", "hello_", "world_"]); + assert_eq!(split("Hello_World"), &["Hello_", "World"]); + assert_eq!(split("helloWOrld"), &["hello", "WOrld"]); + assert_eq!(split("helloworld"), &["helloworld"]); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point From 69e28d04b0b459edb66971941fbf2da69ba1a624 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Mon, 9 Jan 2023 10:19:11 -0800 Subject: [PATCH 33/33] Added open screenshare when following into non-followable buffer --- crates/workspace/src/workspace.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 06e19bbf88..6f37565fa4 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2142,7 +2142,6 @@ impl Workspace { let call = self.active_call()?; let room = call.read(cx).room()?.read(cx); let participant = room.remote_participant_for_peer_id(leader_id)?; - let mut items_to_add = Vec::new(); match participant.location { call::ParticipantLocation::SharedProject { project_id } => { @@ -2153,6 +2152,12 @@ impl Workspace { .and_then(|id| state.items_by_leader_view_id.get(&id)) { items_to_add.push((pane.clone(), item.boxed_clone())); + } else { + if let Some(shared_screen) = + self.shared_screen_for_peer(leader_id, pane, cx) + { + items_to_add.push((pane.clone(), Box::new(shared_screen))); + } } } }