From eeb5b03d6344baa91e3f24cb6ef2f84bdef7edbe Mon Sep 17 00:00:00 2001
From: Joseph Lyons <JosephTLyons@gmail.com>
Date: Thu, 22 Dec 2022 03:24:21 -0500
Subject: [PATCH 01/24] 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 a53f20aeda1f10c2eb890205a95187ada8608cd5..c9c8120eb7251f501a911ce9bd807ab5f7fba9f6 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<AppState>, 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<Self>) {
+        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::<ReleaseChannel>().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::<Vec<_>>()
+            .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 18e7d6fe9b1ef8be5c634ce04d25ee5a1cd09893..b8b0f69bd46e94d4600a70b651c849c28f910489 100644
--- a/crates/zed/src/menus.rs
+++ b/crates/zed/src/menus.rs
@@ -339,10 +339,8 @@ pub fn menus() -> Vec<Menu<'static>> {
                 },
                 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<Menu<'static>> {
                         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 <JosephTLyons@gmail.com>
Date: Thu, 22 Dec 2022 14:27:32 -0500
Subject: [PATCH 02/24] Factored data into a SystemSpecs struct

Co-Authored-By: Mikayla Maki <mikayla.c.maki@gmail.com>
---
 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 c9c8120eb7251f501a911ce9bd807ab5f7fba9f6..06e19bbf88c1dcabb1827254209a08d8b104cd85 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<AppState>, 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<Self>) {
-        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::<ReleaseChannel>().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::<Vec<_>>()
-            .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 b8b0f69bd46e94d4600a70b651c849c28f910489..3865389e99f7e30cabccb2bf427130fffaaf866e 100644
--- a/crates/zed/src/menus.rs
+++ b/crates/zed/src/menus.rs
@@ -339,8 +339,8 @@ pub fn menus() -> Vec<Menu<'static>> {
                 },
                 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 0000000000000000000000000000000000000000..56eeeb8167ec26f9e7377ba29bc62028457e6c54
--- /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<String>,
+    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::<ReleaseChannel>().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 099fb4eea934cf286da36959040595f1bbb0774f..0936e317f4ab405ed02d0f9c6a46005fb9bf19fc 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<AppState>, cx: &mut gpui::MutableAppContext) {
         },
     );
 
+    cx.add_action(
+        |_: &mut Workspace, _: &CopySystemSpecsIntoClipboard, cx: &mut ViewContext<Workspace>| {
+            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 <JosephTLyons@gmail.com>
Date: Thu, 22 Dec 2022 18:10:49 -0500
Subject: [PATCH 03/24] 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 57a7dde194c899107baba60869ebbef8536798dc..8bad1349cc9655ee7aef5ba65f4bc7a20a7d73ac 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 574111848c38545cec10123f157bc520fe896ef5..1e3167d2b8ddd69e9a6baa14dcccc6c74a7f7f25 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 56eeeb8167ec26f9e7377ba29bc62028457e6c54..b6c2c0fcba70347815c239b2adf052dea951e2b3 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<String>,
     app_version: &'static str,
     release_channel: &'static str,
+    os_name: &'static str,
+    os_version: Option<String>,
+    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::<ReleaseChannel>().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::<ReleaseChannel>().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 <JosephTLyons@gmail.com>
Date: Thu, 22 Dec 2022 23:04:33 -0500
Subject: [PATCH 04/24] 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 8bad1349cc9655ee7aef5ba65f4bc7a20a7d73ac..6ee7abcb707e91ef8e610b6dfabd3ea736d41c5d 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 1e3167d2b8ddd69e9a6baa14dcccc6c74a7f7f25..fc7a1753555039842d7ca3a6ce1936d808a08322 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 3865389e99f7e30cabccb2bf427130fffaaf866e..b46e8ad703721f81a9b5e3cf4c925be2e28545f5 100644
--- a/crates/zed/src/menus.rs
+++ b/crates/zed/src/menus.rs
@@ -343,10 +343,12 @@ pub fn menus() -> Vec<Menu<'static>> {
                     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 0936e317f4ab405ed02d0f9c6a46005fb9bf19fc..e6d50f43af87966edbb2788008ca770c7c41aaa9 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<AppState>, cx: &mut gpui::MutableAppContext) {
         },
     );
 
+    cx.add_action(
+        |_: &mut Workspace, _: &RequestFeature, cx: &mut ViewContext<Workspace>| {
+            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<Workspace>| {
+            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 <JosephTLyons@gmail.com>
Date: Mon, 26 Dec 2022 00:24:26 -0500
Subject: [PATCH 05/24] 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 783d90f8e1deceb7c40d0babbafae536813e9662..7af7426f194348157e38616de42949c84ad1d17a 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 36dab7314925eda17052d37fe8fc2b7e619f0c70..0464840e86fef3001291d31b608437f2a4cc7c91 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<str> {
         "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 <JosephTLyons@gmail.com>
Date: Thu, 29 Dec 2022 01:43:49 -0500
Subject: [PATCH 06/24] 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 7af7426f194348157e38616de42949c84ad1d17a..5402686ecfcdebee7878e50f58c10fbfae5ea558 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 428865ec3b1024b550a0698352d2221b257de554..4b787af20da6ce8da1be048aa9808364a8e0d995 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<DockAnchor>, cx: &mut ViewContext<Self>) -> Self {
         let handle = cx.weak_handle();
@@ -696,26 +704,29 @@ impl Pane {
         _: &CloseActiveItem,
         cx: &mut ViewContext<Workspace>,
     ) -> Option<Task<Result<()>>> {
-        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<Workspace>,
+    ) -> Option<Task<Result<()>>> {
+        Self::close_main(workspace, ItemType::Inactive, cx)
+    }
+
+    pub fn close_all_items(
+        workspace: &mut Workspace,
+        _: &CloseAllItems,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Option<Task<Result<()>>> {
+        Self::close_main(workspace, ItemType::All, cx)
+    }
+
+    fn close_main(
+        workspace: &mut Workspace,
+        close_item_type: ItemType,
+        cx: &mut ViewContext<Workspace>,
     ) -> Option<Task<Result<()>>> {
         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 <JosephTLyons@gmail.com>
Date: Thu, 29 Dec 2022 13:28:52 -0500
Subject: [PATCH 07/24] 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 5402686ecfcdebee7878e50f58c10fbfae5ea558..fbbb517c936b194318d409bd56c965e81d99d9a7 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 4b787af20da6ce8da1be048aa9808364a8e0d995..0bd4befd778075f799442db1fa300262be4d0fa7 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<Workspace>,
+    ) -> Option<Task<Result<()>>> {
+        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 <JosephTLyons@gmail.com>
Date: Thu, 29 Dec 2022 13:43:56 -0500
Subject: [PATCH 08/24] 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 0bd4befd778075f799442db1fa300262be4d0fa7..618c650e02a538b53a70b0bd3083847f4d1e0425 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 <mikayla.c.maki@gmail.com>
Date: Fri, 30 Dec 2022 18:18:02 -0800
Subject: [PATCH 09/24] 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 d44aebce0fae5a5ba33a8fd3db6f0ec69ef19e27..d2f690e0e06cf9d8c4d1708c8c853b58ab2efd62 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<str>) -> Arc<str> {
-        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 <JosephTLyons@gmail.com>
Date: Sun, 1 Jan 2023 23:50:45 -0500
Subject: [PATCH 10/24] 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 3fb0119782cdd10aa8698362b8c6e9ce3812ff29..23f6f04992ea65e0f63603558d23d8f2a6a242a8 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 f703d863be4113d62a0b68f60c6d09aae0b53f3d..3e2ef73634cfd17d2cf1a3a8d78ca7e0d6b7d128 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 e28d1711d2ceda9cfcedf28310575e1cbc3cc620..cabae1ac0a14c290da73af306dc80db824273bc6 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 bceb4fb927da826920cab1af91b67a92325ea8a5..bcd3f76dca99d5cf22ee01ebe1e1d51cc13e103f 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 1ee8f23e41e97c9e9f24fd1eb163242256c1407c..7e66efd39de86ebd12efc98b5e850a5a9aa2edae 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 fe69e684e70df437573ecd60a17e29b984291d6d..1c76e6befbf42e0069dc95c2cfbd3b7a227183cf 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 5254ef9ca4587ac44e591996798b9357a9305b05..39c8c39a4423de758807e67751a313e091e4f929 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 808701569a40376858a47012fb1b8e02b730cd74..2957f415daa80a9e06570dc977324faaed8addb2 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 3342845d148ca2588c4650a8579b9ebd335640cd..3c6cb31196b881054f0a503d41242039c51bd037 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<Self> {
         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::<sqlite3_stmt>();
                 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 86035f5d0acd322a0edb31c534fa14b0fbc9257d..f3ec6d48541b8f2fde85a1cffdcaba99501315c2 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::<sqlite3_stmt>();
                 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 2c51b776edc73ca941d75a9fcb85ce0630acbe7d..c3fe4657ee9182dbf1c229e32b19ebf3c30c0ec7 100644
--- a/crates/sqlez/src/thread_safe_connection.rs
+++ b/crates/sqlez/src/thread_safe_connection.rs
@@ -114,12 +114,12 @@ impl<M: Migrator> ThreadSafeConnection<M> {
             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<M> {
@@ -187,10 +187,9 @@ impl<M: Migrator> ThreadSafeConnection<M> {
         *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<M: Migrator> Clone for ThreadSafeConnection<M> {
         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 c7d8b20aa556d93fd08024a32667c7337d9b1013..df4a2987b5f0198fb380d5014d963dd2f69c2aff 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<impl 'a + FnMut() -> 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<impl 'a + FnMut(B) -> 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<impl 'a + FnMut() -> Result<Vec<C>>> {
-        let mut statement = Statement::prepare(&self, query)?;
+        let mut statement = Statement::prepare(self, query)?;
         Ok(move || statement.rows::<C>())
     }
 
@@ -32,7 +32,7 @@ impl Connection {
         &'a self,
         query: &str,
     ) -> Result<impl 'a + FnMut(B) -> Result<Vec<C>>> {
-        let mut statement = Statement::prepare(&self, query)?;
+        let mut statement = Statement::prepare(self, query)?;
         Ok(move |bindings| statement.with_bindings(bindings)?.rows::<C>())
     }
 
@@ -40,7 +40,7 @@ impl Connection {
         &'a self,
         query: &str,
     ) -> Result<impl 'a + FnMut() -> Result<Option<C>>> {
-        let mut statement = Statement::prepare(&self, query)?;
+        let mut statement = Statement::prepare(self, query)?;
         Ok(move || statement.maybe_row::<C>())
     }
 
@@ -48,7 +48,7 @@ impl Connection {
         &'a self,
         query: &str,
     ) -> Result<impl 'a + FnMut(B) -> Result<Option<C>>> {
-        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 429f45db7e55442773fd1b6f5b92bdb577c1c5da..675576c997a139a501f8b66b034643ee67165b87 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 8672ca398f9d13d5c9348d94f7d781adafa61dd7..112366cdf583cbd3f247a8a0cf9496c9bc8936c3 100644
--- a/crates/sum_tree/src/tree_map.rs
+++ b/crates/sum_tree/src/tree_map.rs
@@ -58,7 +58,7 @@ impl<K: Clone + Debug + Default + Ord, V: Clone + Debug> TreeMap<K, V> {
         self.0.insert_or_replace(MapEntry { key, value }, &());
     }
 
-    pub fn remove<'a>(&mut self, key: &'a K) -> Option<V> {
+    pub fn remove(&mut self, key: &K) -> Option<V> {
         let mut removed = None;
         let mut cursor = self.0.cursor::<MapKeyRef<'_, K>>();
         let key = MapKeyRef(Some(key));
diff --git a/crates/util/src/channel.rs b/crates/util/src/channel.rs
index 3edf26dc95e4386f3f89d67e4b2c9ad13fbed83c..03b0f04271b1bd017a12396e72727914018a9399 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 d9015ca6c0b2e49829d9507aa5bc442990aee300..e79cc269c9068a85fefe920ddecaf0c937a015d7 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 6aad293d34a1ac674aa1b01c30a1dc218d4aa92d..cbfb5e35a70f06da84c5fa551845bb63385e3f94 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 5ac7cb36b6db6ce66f7b6efbbc21e716cf4f9268..f4fa919c48e5b2e2640adcde991af6a1447904c7 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 e6d50f43af87966edbb2788008ca770c7c41aaa9..c3af91306dce4b1ba79bf46f52fdce79daa34cff 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>,
 ) {
     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 <floc@unpromptedtirade.com>
Date: Sun, 1 Jan 2023 23:50:46 -0500
Subject: [PATCH 11/24] Add dismiss buffer search button & fix some faulty icon
 button styling

Co-Authored-By: Nate Butler <nate@zed.dev>
---
 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 f5ced700b63c370814caa4b538c99366d654395f..76b9fb1aa68db86f3197947a7bfe4be6d756d009 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<MouseButton>,
diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs
index 5877322feb64c96d843dffbec0f328af93e4ac97..fa95dda1af924c3c005b282afd97b34d5a149e3e 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::<Self>::new(option as usize, cx, |state, cx| {
-                let style = &cx
+                let style = cx
                     .global::<Settings>()
                     .theme
                     .search
@@ -373,7 +379,7 @@ impl BufferSearchBar {
 
         enum NavButton {}
         MouseEventHandler::<NavButton>::new(direction as usize, cx, |state, cx| {
-            let style = &cx
+            let style = cx
                 .global::<Settings>()
                 .theme
                 .search
@@ -399,6 +405,38 @@ impl BufferSearchBar {
         .boxed()
     }
 
+    fn render_close_button(
+        &self,
+        theme: &theme::Search,
+        cx: &mut RenderContext<Self>,
+    ) -> ElementBox {
+        let action = Box::new(Dismiss);
+        let tooltip = "Dismiss Buffer Search";
+        let tooltip_style = cx.global::<Settings>().theme.tooltip.clone();
+
+        enum CloseButton {}
+        MouseEventHandler::<CloseButton>::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::<CloseButton, _>(0, tooltip.to_string(), Some(action), tooltip_style, cx)
+        .boxed()
+    }
+
     fn deploy(pane: &mut Pane, action: &Deploy, cx: &mut ViewContext<Pane>) {
         if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::<BufferSearchBar>() {
             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 bf6cb57adb3ce6378fed2bc5db3f4ed2d8b22962..e463310b9810eb2d6f30e924b1b66166c9b2e295 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<IconButton>,
 }
 
 #[derive(Clone, Deserialize, Default)]
diff --git a/styles/src/styleTree/contactNotification.ts b/styles/src/styleTree/contactNotification.ts
index e0b967203d1f72a3eb2fd40c6f88786e6376aafc..e3f15baa32285ce202a279736f1d5f9ed45850cb 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 63d86c8f471f3574e87cb05414c75e5a39c33c04..e96cab0316a4116d7738efeb7f2336b78cb6c532 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 4f12c42c0c267e2ec98578ae46955f915ed9f974..2edd3807a53928bf01b796e0e9d306f27c03a420 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 fcf78dc73f26854b73ef13113c2856e96f7c490c..a4875cec06a57bafde5455bff1d9d2bd9322f4f0 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 <floc@unpromptedtirade.com>
Date: Tue, 3 Jan 2023 13:50:08 -0500
Subject: [PATCH 12/24] 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 0487b11c19fdcbcd3f2ef2d7db0e66cc3a4c73c8..5b73d7643c84d344bd8f133f2b52427c0b952adf 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 <floc@unpromptedtirade.com>
Date: Tue, 3 Jan 2023 16:37:35 -0500
Subject: [PATCH 13/24] 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 95f56bce5b57941e15d63d1186aa09b45984fad6..f54b09ceda55c5e94666fe16bbd7b29fc9fe0002 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 <mikayla.c.maki@gmail.com>
Date: Wed, 4 Jan 2023 11:45:25 -0800
Subject: [PATCH 14/24] 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 6ee7abcb707e91ef8e610b6dfabd3ea736d41c5d..bdbb3411f21cbb60e190434ae512d6b6f647e614 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 fc7a1753555039842d7ca3a6ce1936d808a08322..d295deea717c3d8ac9529c8fbc50431757b22946 100644
--- a/crates/zed/Cargo.toml
+++ b/crates/zed/Cargo.toml
@@ -3,7 +3,7 @@ authors = ["Nathan Sobo <nathansobo@gmail.com>"]
 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 <floc@unpromptedtirade.com>
Date: Thu, 5 Jan 2023 11:27:50 -0500
Subject: [PATCH 15/24] 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 42ff2b2f1c3cb0a4e34ecc02c10415accc8f749e..02e15290ab269b5f4fee7cdcc365f44e0777f31f 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 <maxbrunsfeld@gmail.com>
Date: Thu, 5 Jan 2023 16:41:23 -0800
Subject: [PATCH 16/24] 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 <kay@zed.dev>
---
 crates/gpui/src/app.rs                     | 360 ++++-----------------
 crates/gpui/src/app/callback_collection.rs | 193 +++++++----
 2 files changed, 205 insertions(+), 348 deletions(-)

diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs
index 76b9fb1aa68db86f3197947a7bfe4be6d756d009..dce4e68e7c9a44298cf45a564736e158e1965cf6 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<E, H, F>(&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<E, H, F>(&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<F, V>(&mut self, handle: &ViewHandle<V>, 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<G, F>(&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<G, F>(&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<F>(&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<F>(&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<T> Drop for ElementStateHandle<T> {
 
 #[must_use]
 pub enum Subscription {
-    Subscription {
-        id: usize,
-        entity_id: usize,
-        subscriptions: Option<Weak<Mapping<usize, SubscriptionCallback>>>,
-    },
-    GlobalSubscription {
-        id: usize,
-        type_id: TypeId,
-        subscriptions: Option<Weak<Mapping<TypeId, GlobalSubscriptionCallback>>>,
-    },
-    Observation {
-        id: usize,
-        entity_id: usize,
-        observations: Option<Weak<Mapping<usize, ObservationCallback>>>,
-    },
-    GlobalObservation {
-        id: usize,
-        type_id: TypeId,
-        observations: Option<Weak<Mapping<TypeId, GlobalObservationCallback>>>,
-    },
-    FocusObservation {
-        id: usize,
-        view_id: usize,
-        observations: Option<Weak<Mapping<usize, FocusObservationCallback>>>,
-    },
-    WindowActivationObservation {
-        id: usize,
-        window_id: usize,
-        observations: Option<Weak<Mapping<usize, WindowActivationCallback>>>,
-    },
-    WindowFullscreenObservation {
-        id: usize,
-        window_id: usize,
-        observations: Option<Weak<Mapping<usize, WindowFullscreenCallback>>>,
-    },
-    KeystrokeObservation {
-        id: usize,
-        window_id: usize,
-        observations: Option<Weak<Mapping<usize, KeystrokeCallback>>>,
-    },
+    Subscription(callback_collection::Subscription<usize, SubscriptionCallback>),
+    Observation(callback_collection::Subscription<usize, ObservationCallback>),
+    GlobalSubscription(callback_collection::Subscription<TypeId, GlobalSubscriptionCallback>),
+    GlobalObservation(callback_collection::Subscription<TypeId, GlobalObservationCallback>),
+    FocusObservation(callback_collection::Subscription<usize, FocusObservationCallback>),
+    WindowActivationObservation(callback_collection::Subscription<usize, WindowActivationCallback>),
+    WindowFullscreenObservation(callback_collection::Subscription<usize, WindowFullscreenCallback>),
+    KeystrokeObservation(callback_collection::Subscription<usize, KeystrokeCallback>),
 
     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 4ec90fbac0b4962520945f2c3914bd3a324f2e1b..4494e9073faed0078c72b1ccfcc4019712dfbbbe 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<K, F> = Mutex<HashMap<K, BTreeMap<usize, Option<F>>>>;
+// 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<K: Hash + Eq, F> {
-    internal: Arc<Mapping<K, F>>,
+// 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<K: Clone + Hash + Eq, F> {
+    key: K,
+    id: usize,
+    mapping: Option<Weak<Mutex<Mapping<K, F>>>>,
+}
+
+struct Mapping<K, F> {
+    callbacks: HashMap<K, BTreeMap<usize, F>>,
+    dropped_subscriptions: HashSet<(K, usize)>,
+}
+
+impl<K, F> Default for Mapping<K, F> {
+    fn default() -> Self {
+        Self {
+            callbacks: Default::default(),
+            dropped_subscriptions: Default::default(),
+        }
+    }
 }
 
-impl<K: Hash + Eq, F> Clone for CallbackCollection<K, F> {
+pub(crate) struct CallbackCollection<K: Clone + Hash + Eq, F> {
+    internal: Arc<Mutex<Mapping<K, F>>>,
+}
+
+impl<K: Clone + Hash + Eq, F> Clone for CallbackCollection<K, F> {
     fn clone(&self) -> Self {
         Self {
             internal: self.internal.clone(),
@@ -21,7 +72,7 @@ impl<K: Hash + Eq, F> Clone for CallbackCollection<K, F> {
     }
 }
 
-impl<K: Hash + Eq + Copy, F> Default for CallbackCollection<K, F> {
+impl<K: Clone + Hash + Eq + Copy, F> Default for CallbackCollection<K, F> {
     fn default() -> Self {
         CallbackCollection {
             internal: Arc::new(Mutex::new(Default::default())),
@@ -29,75 +80,101 @@ impl<K: Hash + Eq + Copy, F> Default for CallbackCollection<K, F> {
     }
 }
 
-impl<K: Hash + Eq + Copy, F> CallbackCollection<K, F> {
-    pub fn downgrade(&self) -> Weak<Mapping<K, F>> {
-        Arc::downgrade(&self.internal)
-    }
-
+impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
     #[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<K, F> {
+        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));
-            }
-
-            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 remove(&mut self, key: K) {
+        self.internal.lock().callbacks.remove(&key);
     }
 
-    pub fn emit_and_cleanup<C: FnMut(&mut F, &mut MutableAppContext) -> bool>(
+    pub fn emit<C: FnMut(&mut F, &mut MutableAppContext) -> bool>(
         &mut self,
-        id: K,
+        key: K,
         cx: &mut MutableAppContext,
         mut call_callback: C,
     ) {
-        let callbacks = self.internal.lock().remove(&id);
+        let callbacks = self.internal.lock().callbacks.remove(&key);
         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();
-                            }
-                        }
+            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 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<K: Clone + Hash + Eq, F> Subscription<K, F> {
+    pub fn detach(&mut self) {
+        self.mapping.take();
+    }
+}
+
+impl<K: Clone + Hash + Eq, F> Drop for Subscription<K, F> {
+    // 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 <maxbrunsfeld@gmail.com>
Date: Thu, 5 Jan 2023 17:30:39 -0800
Subject: [PATCH 17/24] Fix logic error in dropping callback subscriptions

Co-authored-by: Kay Simmons <kay@zed.dev>
---
 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 4494e9073faed0078c72b1ccfcc4019712dfbbbe..38a8dae26c0629a10d2b06ab90548db6c31e5d6f 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<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
     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<K: Clone + Hash + Eq, F> Drop for Subscription<K, F> {
         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 <maxbrunsfeld@gmail.com>
Date: Thu, 5 Jan 2023 18:02:53 -0800
Subject: [PATCH 18/24] Use a CallbackCollection for release observations

Co-authored-by: Kay Simmons <kay@zed.dev>
---
 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 dce4e68e7c9a44298cf45a564736e158e1965cf6..fa01196d4bb84b006880c17029002489b3902f0c 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<dyn FnMut(&dyn Any, &mut MutableAppContext) -> bool>;
 type GlobalSubscriptionCallback = Box<dyn FnMut(&dyn Any, &mut MutableAppContext)>;
 type ObservationCallback = Box<dyn FnMut(&mut MutableAppContext) -> bool>;
-type FocusObservationCallback = Box<dyn FnMut(bool, &mut MutableAppContext) -> bool>;
 type GlobalObservationCallback = Box<dyn FnMut(&mut MutableAppContext)>;
-type ReleaseObservationCallback = Box<dyn FnOnce(&dyn Any, &mut MutableAppContext)>;
+type FocusObservationCallback = Box<dyn FnMut(bool, &mut MutableAppContext) -> bool>;
+type ReleaseObservationCallback = Box<dyn FnMut(&dyn Any, &mut MutableAppContext)>;
 type ActionObservationCallback = Box<dyn FnMut(TypeId, &mut MutableAppContext)>;
 type WindowActivationCallback = Box<dyn FnMut(bool, &mut MutableAppContext) -> bool>;
 type WindowFullscreenCallback = Box<dyn FnMut(bool, &mut MutableAppContext) -> bool>;
@@ -615,18 +615,17 @@ pub struct MutableAppContext {
     next_subscription_id: usize,
     frame_count: usize,
 
-    focus_observations: CallbackCollection<usize, FocusObservationCallback>,
-    global_subscriptions: CallbackCollection<TypeId, GlobalSubscriptionCallback>,
-    global_observations: CallbackCollection<TypeId, GlobalObservationCallback>,
     subscriptions: CallbackCollection<usize, SubscriptionCallback>,
+    global_subscriptions: CallbackCollection<TypeId, GlobalSubscriptionCallback>,
     observations: CallbackCollection<usize, ObservationCallback>,
+    global_observations: CallbackCollection<TypeId, GlobalObservationCallback>,
+    focus_observations: CallbackCollection<usize, FocusObservationCallback>,
+    release_observations: CallbackCollection<usize, ReleaseObservationCallback>,
+    action_dispatch_observations: Arc<Mutex<BTreeMap<usize, ActionObservationCallback>>>,
     window_activation_observations: CallbackCollection<usize, WindowActivationCallback>,
     window_fullscreen_observations: CallbackCollection<usize, WindowFullscreenCallback>,
     keystroke_observations: CallbackCollection<usize, KeystrokeCallback>,
 
-    release_observations: Arc<Mutex<HashMap<usize, BTreeMap<usize, ReleaseObservationCallback>>>>,
-    action_dispatch_observations: Arc<Mutex<BTreeMap<usize, ActionObservationCallback>>>,
-
     #[allow(clippy::type_complexity)]
     presenters_and_platform_windows:
         HashMap<usize, (Rc<RefCell<Presenter>>, Box<dyn platform::Window>)>,
@@ -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<F>(&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<usize, WindowActivationCallback>),
     WindowFullscreenObservation(callback_collection::Subscription<usize, WindowFullscreenCallback>),
     KeystrokeObservation(callback_collection::Subscription<usize, KeystrokeCallback>),
+    ReleaseObservation(callback_collection::Subscription<usize, ReleaseObservationCallback>),
 
-    ReleaseObservation {
-        id: usize,
-        entity_id: usize,
-        #[allow(clippy::type_complexity)]
-        observations:
-            Option<Weak<Mutex<HashMap<usize, BTreeMap<usize, ReleaseObservationCallback>>>>>,
-    },
     ActionObservation {
         id: usize,
         observations: Option<Weak<Mutex<BTreeMap<usize, ActionObservationCallback>>>>,
@@ -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 <maxbrunsfeld@gmail.com>
Date: Fri, 6 Jan 2023 09:15:53 -0800
Subject: [PATCH 19/24] 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 fa01196d4bb84b006880c17029002489b3902f0c..aa1fd390159134ac4796efb17d7d1ed484e071bb 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<TypeId, GlobalObservationCallback>,
     focus_observations: CallbackCollection<usize, FocusObservationCallback>,
     release_observations: CallbackCollection<usize, ReleaseObservationCallback>,
-    action_dispatch_observations: Arc<Mutex<BTreeMap<usize, ActionObservationCallback>>>,
+    action_dispatch_observations: CallbackCollection<(), ActionObservationCallback>,
     window_activation_observations: CallbackCollection<usize, WindowActivationCallback>,
     window_fullscreen_observations: CallbackCollection<usize, WindowFullscreenCallback>,
     keystroke_observations: CallbackCollection<usize, KeystrokeCallback>,
@@ -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<F>(&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<usize, WindowFullscreenCallback>),
     KeystrokeObservation(callback_collection::Subscription<usize, KeystrokeCallback>),
     ReleaseObservation(callback_collection::Subscription<usize, ReleaseObservationCallback>),
-
-    ActionObservation {
-        id: usize,
-        observations: Option<Weak<Mutex<BTreeMap<usize, ActionObservationCallback>>>>,
-    },
+    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 38a8dae26c0629a10d2b06ab90548db6c31e5d6f..5bed9f7a295a246b1c02915bc3bfbc6da5d1a5c5 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;
-
-use collections::{BTreeMap, HashMap, HashSet};
-
-use crate::MutableAppContext;
+pub struct CallbackCollection<K: Clone + Hash + Eq, F> {
+    internal: Arc<Mutex<Mapping<K, F>>>,
+}
 
-// 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<K: Clone + Hash + Eq, F> {
     key: K,
     id: usize,
@@ -59,10 +28,6 @@ impl<K, F> Default for Mapping<K, F> {
     }
 }
 
-pub(crate) struct CallbackCollection<K: Clone + Hash + Eq, F> {
-    internal: Arc<Mutex<Mapping<K, F>>>,
-}
-
 impl<K: Clone + Hash + Eq, F> Clone for CallbackCollection<K, F> {
     fn clone(&self) -> Self {
         Self {
@@ -150,6 +115,10 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
 }
 
 impl<K: Clone + Hash + Eq, F> Subscription<K, F> {
+    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 <maxbrunsfeld@gmail.com>
Date: Fri, 6 Jan 2023 09:56:29 -0800
Subject: [PATCH 20/24] 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 aa1fd390159134ac4796efb17d7d1ed484e071bb..3299fbb746e6c5831ebb45ac56243165f020a9e5 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<usize>,
-        }
-
-        impl Entity for View {
-            type Event = usize;
-        }
-
-        impl super::View for View {
-            fn render(&mut self, _: &mut RenderContext<Self>) -> 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<Self>) -> 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<Self>) -> ElementBox {
-                Empty::new().boxed()
-            }
-        }
-
-        let events = Rc::new(RefCell::new(Vec::new()));
-        cx.add_window(Default::default(), |cx| {
+        let (_, view) = cx.add_window::<TestView, _>(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<usize>,
-        }
-
-        impl Entity for View {
-            type Event = usize;
-        }
-
-        impl super::View for View {
-            fn render(&mut self, _: &mut RenderContext<Self>) -> 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<Self>) -> ElementBox {
-                Empty::new().boxed()
-            }
-        }
-
-        let events = Rc::new(RefCell::new(Vec::new()));
-        cx.add_window(Default::default(), |cx| {
+        let (_, view) = cx.add_window::<TestView, _>(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<Self>) -> 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<Self>) -> 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<Self>) -> 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<String>,
+    }
+
+    impl Entity for TestView {
+        type Event = String;
+    }
+
+    impl View for TestView {
+        fn ui_name() -> &'static str {
+            "TestView"
+        }
+
+        fn render(&mut self, _: &mut RenderContext<Self>) -> ElementBox {
+            Empty::new().boxed()
+        }
+    }
 }

From 4708f5d88f85e6821c62deea2bfd3cfc1e0f37eb Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Fri, 6 Jan 2023 10:44:37 -0800
Subject: [PATCH 21/24] 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 3299fbb746e6c5831ebb45ac56243165f020a9e5..4a5b6f54be4bc5a0fe4d6c4979febe5d797dfd9b 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::<String>::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 5bed9f7a295a246b1c02915bc3bfbc6da5d1a5c5..43f4f3f62ef918b21595499ab38922864a3388dd 100644
--- a/crates/gpui/src/app/callback_collection.rs
+++ b/crates/gpui/src/app/callback_collection.rs
@@ -58,14 +58,6 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
         }
     }
 
-    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<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
     }
 
     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<C: FnMut(&mut F, &mut MutableAppContext) -> bool>(

From ef192a902a030ee2a4a15c054f23563dc8e91ee6 Mon Sep 17 00:00:00 2001
From: Max Brunsfeld <maxbrunsfeld@gmail.com>
Date: Fri, 6 Jan 2023 11:03:45 -0800
Subject: [PATCH 22/24] 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 43f4f3f62ef918b21595499ab38922864a3388dd..44db22496722d763d7fe06a1d5a0474b4b0ef2fd 100644
--- a/crates/gpui/src/app/callback_collection.rs
+++ b/crates/gpui/src/app/callback_collection.rs
@@ -60,17 +60,24 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
 
     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<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
         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<K: Clone + Hash + Eq, F> Subscription<K, F> {
 }
 
 impl<K: Clone + Hash + Eq, F> Drop for Subscription<K, F> {
-    // 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 <maxbrunsfeld@gmail.com>
Date: Fri, 6 Jan 2023 11:33:50 -0800
Subject: [PATCH 23/24] 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 4a5b6f54be4bc5a0fe4d6c4979febe5d797dfd9b..5568155cf7e2527bc7c1c6e8662991ed0f9c4a48 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 44db22496722d763d7fe06a1d5a0474b4b0ef2fd..45479eabadee3a443f886292b5cd45277c6eb898 100644
--- a/crates/gpui/src/app/callback_collection.rs
+++ b/crates/gpui/src/app/callback_collection.rs
@@ -16,7 +16,17 @@ pub struct Subscription<K: Clone + Hash + Eq, F> {
 
 struct Mapping<K, F> {
     callbacks: HashMap<K, BTreeMap<usize, F>>,
-    dropped_subscriptions: HashSet<(K, usize)>,
+    dropped_subscriptions: HashMap<K, HashSet<usize>>,
+}
+
+impl<K: Hash + Eq, F> Mapping<K, F> {
+    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<K, F> Default for Mapping<K, F> {
@@ -50,6 +60,14 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
         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<K, F> {
         Subscription {
             key,
@@ -63,7 +81,7 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
 
         // 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<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
     }
 
     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<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
         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);
 
-    pub fn gc(&mut self) {
-        let mut this = self.internal.lock();
+                // 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;
+                }
 
-        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<K: Clone + Hash + Eq, F> Drop for Subscription<K, F> {
             // 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 <maxbrunsfeld@gmail.com>
Date: Fri, 6 Jan 2023 11:51:36 -0800
Subject: [PATCH 24/24] 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 45479eabadee3a443f886292b5cd45277c6eb898..7496d715014159f7b445414519f834f0c849e0ab 100644
--- a/crates/gpui/src/app/callback_collection.rs
+++ b/crates/gpui/src/app/callback_collection.rs
@@ -60,14 +60,6 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
         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<K, F> {
         Subscription {
             key,