From 3d5333691666bbb7c8ff89a8dc7bb44106183f52 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Fri, 10 Feb 2023 14:41:22 -0800 Subject: [PATCH] More vim fixes and move some more things out of app.rs --- Cargo.lock | 1 - assets/keymaps/vim.json | 2 +- crates/editor/src/display_map.rs | 91 ++++- crates/editor/src/editor.rs | 35 +- crates/gpui/src/app.rs | 356 +------------------- crates/gpui/src/app/menu.rs | 52 +++ crates/gpui/src/app/ref_counts.rs | 217 ++++++++++++ crates/gpui/src/app/test_app_context.rs | 15 +- crates/gpui/src/app/window_input_handler.rs | 98 ++++++ crates/gpui/src/platform/test.rs | 4 +- crates/gpui/src/test.rs | 25 +- crates/sqlez/Cargo.toml | 5 +- crates/vim/src/editor_events.rs | 16 +- crates/vim/src/motion.rs | 96 +++--- crates/vim/src/normal.rs | 6 +- crates/vim/src/test/vim_test_context.rs | 2 +- crates/vim/src/vim.rs | 41 +-- crates/vim/src/visual.rs | 4 +- crates/zed/src/zed.rs | 2 +- 19 files changed, 595 insertions(+), 473 deletions(-) create mode 100644 crates/gpui/src/app/menu.rs create mode 100644 crates/gpui/src/app/ref_counts.rs create mode 100644 crates/gpui/src/app/window_input_handler.rs diff --git a/Cargo.lock b/Cargo.lock index 8eb8d6987a..82e9197631 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6104,7 +6104,6 @@ dependencies = [ "libsqlite3-sys", "parking_lot 0.11.2", "smol", - "sqlez_macros", "thread_local", "uuid 1.2.2", ] diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index f52a1941a3..e8a7d767f0 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -315,7 +315,7 @@ { "context": "Editor && VimWaiting", "bindings": { - "*": "gpui::KeyPressed", + // "*": "gpui::KeyPressed", "escape": "editor::Cancel" } } diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index e32276df41..99a74fe7f2 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -337,7 +337,7 @@ impl DisplaySnapshot { .map(|h| h.text) } - // Returns text chunks starting at the end of the given display row in reverse until the start of the file + /// Returns text chunks starting at the end of the given display row in reverse until the start of the file pub fn reverse_text_chunks(&self, display_row: u32) -> impl Iterator { (0..=display_row).into_iter().rev().flat_map(|row| { self.blocks_snapshot @@ -411,6 +411,67 @@ impl DisplaySnapshot { }) } + /// Returns an iterator of the start positions of the occurances of `target` in the `self` after `from` + /// Stops if `condition` returns false for any of the character position pairs observed. + pub fn find_while<'a>( + &'a self, + from: DisplayPoint, + target: &str, + condition: impl FnMut(char, DisplayPoint) -> bool + 'a, + ) -> impl Iterator + 'a { + Self::find_internal(self.chars_at(from), target.chars().collect(), condition) + } + + /// Returns an iterator of the end positions of the occurances of `target` in the `self` before `from` + /// Stops if `condition` returns false for any of the character position pairs observed. + pub fn reverse_find_while<'a>( + &'a self, + from: DisplayPoint, + target: &str, + condition: impl FnMut(char, DisplayPoint) -> bool + 'a, + ) -> impl Iterator + 'a { + Self::find_internal( + self.reverse_chars_at(from), + target.chars().rev().collect(), + condition, + ) + } + + fn find_internal<'a>( + iterator: impl Iterator + 'a, + target: Vec, + mut condition: impl FnMut(char, DisplayPoint) -> bool + 'a, + ) -> impl Iterator + 'a { + // List of partial matches with the index of the last seen character in target and the starting point of the match + let mut partial_matches: Vec<(usize, DisplayPoint)> = Vec::new(); + iterator + .take_while(move |(ch, point)| condition(*ch, *point)) + .filter_map(move |(ch, point)| { + if Some(&ch) == target.get(0) { + partial_matches.push((0, point)); + } + + let mut found = None; + // Keep partial matches that have the correct next character + partial_matches.retain_mut(|(match_position, match_start)| { + if target.get(*match_position) == Some(&ch) { + *match_position += 1; + if *match_position == target.len() { + found = Some(match_start.clone()); + // This match is completed. No need to keep tracking it + false + } else { + true + } + } else { + false + } + }); + + found + }) + } + pub fn column_to_chars(&self, display_row: u32, target: u32) -> u32 { let mut count = 0; let mut column = 0; @@ -627,7 +688,7 @@ pub mod tests { use smol::stream::StreamExt; use std::{env, sync::Arc}; use theme::SyntaxTheme; - use util::test::{marked_text_ranges, sample_text}; + use util::test::{marked_text_offsets, marked_text_ranges, sample_text}; use Bias::*; #[gpui::test(iterations = 100)] @@ -1418,6 +1479,32 @@ pub mod tests { ) } + #[test] + fn test_find_internal() { + assert("This is a ˇtest of find internal", "test"); + assert("Some text ˇaˇaˇaa with repeated characters", "aa"); + + fn assert(marked_text: &str, target: &str) { + let (text, expected_offsets) = marked_text_offsets(marked_text); + + let chars = text + .chars() + .enumerate() + .map(|(index, ch)| (ch, DisplayPoint::new(0, index as u32))); + let target = target.chars(); + + assert_eq!( + expected_offsets + .into_iter() + .map(|offset| offset as u32) + .collect::>(), + DisplaySnapshot::find_internal(chars, target.collect(), |_, _| true) + .map(|point| point.column()) + .collect::>() + ) + } + } + fn syntax_chunks<'a>( rows: Range, map: &ModelHandle, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index fbce19d607..c649fed7ce 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6400,26 +6400,29 @@ impl View for Editor { text: &str, cx: &mut ViewContext, ) { + self.transact(cx, |this, cx| { + if this.input_enabled { + let new_selected_ranges = if let Some(range_utf16) = range_utf16 { + let range_utf16 = OffsetUtf16(range_utf16.start)..OffsetUtf16(range_utf16.end); + Some(this.selection_replacement_ranges(range_utf16, cx)) + } else { + this.marked_text_ranges(cx) + }; + + if let Some(new_selected_ranges) = new_selected_ranges { + this.change_selections(None, cx, |selections| { + selections.select_ranges(new_selected_ranges) + }); + } + } + + this.handle_input(text, cx); + }); + if !self.input_enabled { return; } - self.transact(cx, |this, cx| { - let new_selected_ranges = if let Some(range_utf16) = range_utf16 { - let range_utf16 = OffsetUtf16(range_utf16.start)..OffsetUtf16(range_utf16.end); - Some(this.selection_replacement_ranges(range_utf16, cx)) - } else { - this.marked_text_ranges(cx) - }; - - if let Some(new_selected_ranges) = new_selected_ranges { - this.change_selections(None, cx, |selections| { - selections.select_ranges(new_selected_ranges) - }); - } - this.handle_input(text, cx); - }); - if let Some(transaction) = self.ime_transaction { self.buffer.update(cx, |buffer, cx| { buffer.group_until_transaction(transaction, cx); diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 6f7199aa33..c0e0b067c4 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1,7 +1,10 @@ pub mod action; mod callback_collection; +mod menu; +pub(crate) mod ref_counts; #[cfg(any(test, feature = "test-support"))] pub mod test_app_context; +mod window_input_handler; use std::{ any::{type_name, Any, TypeId}, @@ -19,34 +22,38 @@ use std::{ }; use anyhow::{anyhow, Context, Result}; -use lazy_static::lazy_static; use parking_lot::Mutex; use pathfinder_geometry::vector::Vector2F; use postage::oneshot; use smallvec::SmallVec; use smol::prelude::*; +use uuid::Uuid; pub use action::*; use callback_collection::CallbackCollection; use collections::{hash_map::Entry, HashMap, HashSet, VecDeque}; +pub use menu::*; use platform::Event; #[cfg(any(test, feature = "test-support"))] +use ref_counts::LeakDetector; +#[cfg(any(test, feature = "test-support"))] pub use test_app_context::{ContextHandle, TestAppContext}; -use uuid::Uuid; +use window_input_handler::WindowInputHandler; use crate::{ elements::ElementBox, executor::{self, Task}, - geometry::rect::RectF, keymap_matcher::{self, Binding, KeymapContext, KeymapMatcher, Keystroke, MatchResult}, platform::{self, KeyDownEvent, Platform, PromptLevel, WindowOptions}, presenter::Presenter, util::post_inc, - Appearance, AssetCache, AssetSource, ClipboardItem, FontCache, InputHandler, KeyUpEvent, + Appearance, AssetCache, AssetSource, ClipboardItem, FontCache, KeyUpEvent, ModifiersChangedEvent, MouseButton, MouseRegionId, PathPromptOptions, TextLayoutCache, WindowBounds, }; +use self::ref_counts::RefCounts; + pub trait Entity: 'static { type Event; @@ -174,31 +181,12 @@ pub trait UpdateView { T: View; } -pub struct Menu<'a> { - pub name: &'a str, - pub items: Vec>, -} - -pub enum MenuItem<'a> { - Separator, - Submenu(Menu<'a>), - Action { - name: &'a str, - action: Box, - }, -} - #[derive(Clone)] pub struct App(Rc>); #[derive(Clone)] pub struct AsyncAppContext(Rc>); -pub struct WindowInputHandler { - app: Rc>, - window_id: usize, -} - impl App { pub fn new(asset_source: impl AssetSource) -> Result { let platform = platform::current::platform(); @@ -220,33 +208,7 @@ impl App { cx.borrow_mut().quit(); } })); - foreground_platform.on_will_open_menu(Box::new({ - let cx = app.0.clone(); - move || { - let mut cx = cx.borrow_mut(); - cx.keystroke_matcher.clear_pending(); - } - })); - foreground_platform.on_validate_menu_command(Box::new({ - let cx = app.0.clone(); - move |action| { - let cx = cx.borrow_mut(); - !cx.keystroke_matcher.has_pending_keystrokes() && cx.is_action_available(action) - } - })); - foreground_platform.on_menu_command(Box::new({ - let cx = app.0.clone(); - move |action| { - let mut cx = cx.borrow_mut(); - if let Some(key_window_id) = cx.cx.platform.key_window_id() { - if let Some(view_id) = cx.focused_view_id(key_window_id) { - cx.handle_dispatch_action_from_effect(key_window_id, Some(view_id), action); - return; - } - } - cx.dispatch_global_action_any(action); - } - })); + setup_menu_handlers(foreground_platform.as_ref(), &app); app.0.borrow_mut().weak_self = Some(Rc::downgrade(&app.0)); Ok(app) @@ -349,94 +311,6 @@ impl App { } } -impl WindowInputHandler { - fn read_focused_view(&self, f: F) -> Option - where - F: FnOnce(&dyn AnyView, &AppContext) -> T, - { - // Input-related application hooks are sometimes called by the OS during - // a call to a window-manipulation API, like prompting the user for file - // paths. In that case, the AppContext will already be borrowed, so any - // InputHandler methods need to fail gracefully. - // - // See https://github.com/zed-industries/community/issues/444 - let app = self.app.try_borrow().ok()?; - - let view_id = app.focused_view_id(self.window_id)?; - let view = app.cx.views.get(&(self.window_id, view_id))?; - let result = f(view.as_ref(), &app); - Some(result) - } - - fn update_focused_view(&mut self, f: F) -> Option - where - F: FnOnce(usize, usize, &mut dyn AnyView, &mut MutableAppContext) -> T, - { - let mut app = self.app.try_borrow_mut().ok()?; - app.update(|app| { - let view_id = app.focused_view_id(self.window_id)?; - let mut view = app.cx.views.remove(&(self.window_id, view_id))?; - let result = f(self.window_id, view_id, view.as_mut(), &mut *app); - app.cx.views.insert((self.window_id, view_id), view); - Some(result) - }) - } -} - -impl InputHandler for WindowInputHandler { - fn text_for_range(&self, range: Range) -> Option { - self.read_focused_view(|view, cx| view.text_for_range(range.clone(), cx)) - .flatten() - } - - fn selected_text_range(&self) -> Option> { - self.read_focused_view(|view, cx| view.selected_text_range(cx)) - .flatten() - } - - fn replace_text_in_range(&mut self, range: Option>, text: &str) { - self.update_focused_view(|window_id, view_id, view, cx| { - view.replace_text_in_range(range, text, cx, window_id, view_id); - }); - } - - fn marked_text_range(&self) -> Option> { - self.read_focused_view(|view, cx| view.marked_text_range(cx)) - .flatten() - } - - fn unmark_text(&mut self) { - self.update_focused_view(|window_id, view_id, view, cx| { - view.unmark_text(cx, window_id, view_id); - }); - } - - fn replace_and_mark_text_in_range( - &mut self, - range: Option>, - new_text: &str, - new_selected_range: Option>, - ) { - self.update_focused_view(|window_id, view_id, view, cx| { - view.replace_and_mark_text_in_range( - range, - new_text, - new_selected_range, - cx, - window_id, - view_id, - ); - }); - } - - fn rect_for_range(&self, range_utf16: Range) -> Option { - let app = self.app.borrow(); - let (presenter, _) = app.presenters_and_platform_windows.get(&self.window_id)?; - let presenter = presenter.borrow(); - presenter.rect_for_text_range(range_utf16, &app) - } -} - impl AsyncAppContext { pub fn spawn(&self, f: F) -> Task where @@ -984,11 +858,6 @@ impl MutableAppContext { result } - pub fn set_menus(&mut self, menus: Vec) { - self.foreground_platform - .set_menus(menus, &self.keystroke_matcher); - } - fn show_character_palette(&self, window_id: usize) { let (_, window) = &self.presenters_and_platform_windows[&window_id]; window.show_character_palette(); @@ -4025,7 +3894,7 @@ impl<'a, T: View> ViewContext<'a, T> { }) } - pub fn observe_keystroke(&mut self, mut callback: F) -> Subscription + pub fn observe_keystrokes(&mut self, mut callback: F) -> Subscription where F: 'static + FnMut( @@ -5280,205 +5149,6 @@ impl Subscription { } } -lazy_static! { - static ref LEAK_BACKTRACE: bool = - std::env::var("LEAK_BACKTRACE").map_or(false, |b| !b.is_empty()); -} - -#[cfg(any(test, feature = "test-support"))] -#[derive(Default)] -pub struct LeakDetector { - next_handle_id: usize, - #[allow(clippy::type_complexity)] - handle_backtraces: HashMap< - usize, - ( - Option<&'static str>, - HashMap>, - ), - >, -} - -#[cfg(any(test, feature = "test-support"))] -impl LeakDetector { - fn handle_created(&mut self, type_name: Option<&'static str>, entity_id: usize) -> usize { - let handle_id = post_inc(&mut self.next_handle_id); - let entry = self.handle_backtraces.entry(entity_id).or_default(); - let backtrace = if *LEAK_BACKTRACE { - Some(backtrace::Backtrace::new_unresolved()) - } else { - None - }; - if let Some(type_name) = type_name { - entry.0.get_or_insert(type_name); - } - entry.1.insert(handle_id, backtrace); - handle_id - } - - fn handle_dropped(&mut self, entity_id: usize, handle_id: usize) { - if let Some((_, backtraces)) = self.handle_backtraces.get_mut(&entity_id) { - assert!(backtraces.remove(&handle_id).is_some()); - if backtraces.is_empty() { - self.handle_backtraces.remove(&entity_id); - } - } - } - - pub fn assert_dropped(&mut self, entity_id: usize) { - if let Some((type_name, backtraces)) = self.handle_backtraces.get_mut(&entity_id) { - for trace in backtraces.values_mut().flatten() { - trace.resolve(); - eprintln!("{:?}", crate::util::CwdBacktrace(trace)); - } - - let hint = if *LEAK_BACKTRACE { - "" - } else { - " – set LEAK_BACKTRACE=1 for more information" - }; - - panic!( - "{} handles to {} {} still exist{}", - backtraces.len(), - type_name.unwrap_or("entity"), - entity_id, - hint - ); - } - } - - pub fn detect(&mut self) { - let mut found_leaks = false; - for (id, (type_name, backtraces)) in self.handle_backtraces.iter_mut() { - eprintln!( - "leaked {} handles to {} {}", - backtraces.len(), - type_name.unwrap_or("entity"), - id - ); - for trace in backtraces.values_mut().flatten() { - trace.resolve(); - eprintln!("{:?}", crate::util::CwdBacktrace(trace)); - } - found_leaks = true; - } - - let hint = if *LEAK_BACKTRACE { - "" - } else { - " – set LEAK_BACKTRACE=1 for more information" - }; - assert!(!found_leaks, "detected leaked handles{}", hint); - } -} - -#[derive(Default)] -struct RefCounts { - entity_counts: HashMap, - element_state_counts: HashMap, - dropped_models: HashSet, - dropped_views: HashSet<(usize, usize)>, - dropped_element_states: HashSet, - - #[cfg(any(test, feature = "test-support"))] - leak_detector: Arc>, -} - -struct ElementStateRefCount { - ref_count: usize, - frame_id: usize, -} - -impl RefCounts { - fn inc_model(&mut self, model_id: usize) { - match self.entity_counts.entry(model_id) { - Entry::Occupied(mut entry) => { - *entry.get_mut() += 1; - } - Entry::Vacant(entry) => { - entry.insert(1); - self.dropped_models.remove(&model_id); - } - } - } - - fn inc_view(&mut self, window_id: usize, view_id: usize) { - match self.entity_counts.entry(view_id) { - Entry::Occupied(mut entry) => *entry.get_mut() += 1, - Entry::Vacant(entry) => { - entry.insert(1); - self.dropped_views.remove(&(window_id, view_id)); - } - } - } - - fn inc_element_state(&mut self, id: ElementStateId, frame_id: usize) { - match self.element_state_counts.entry(id) { - Entry::Occupied(mut entry) => { - let entry = entry.get_mut(); - if entry.frame_id == frame_id || entry.ref_count >= 2 { - panic!("used the same element state more than once in the same frame"); - } - entry.ref_count += 1; - entry.frame_id = frame_id; - } - Entry::Vacant(entry) => { - entry.insert(ElementStateRefCount { - ref_count: 1, - frame_id, - }); - self.dropped_element_states.remove(&id); - } - } - } - - fn dec_model(&mut self, model_id: usize) { - let count = self.entity_counts.get_mut(&model_id).unwrap(); - *count -= 1; - if *count == 0 { - self.entity_counts.remove(&model_id); - self.dropped_models.insert(model_id); - } - } - - fn dec_view(&mut self, window_id: usize, view_id: usize) { - let count = self.entity_counts.get_mut(&view_id).unwrap(); - *count -= 1; - if *count == 0 { - self.entity_counts.remove(&view_id); - self.dropped_views.insert((window_id, view_id)); - } - } - - fn dec_element_state(&mut self, id: ElementStateId) { - let entry = self.element_state_counts.get_mut(&id).unwrap(); - entry.ref_count -= 1; - if entry.ref_count == 0 { - self.element_state_counts.remove(&id); - self.dropped_element_states.insert(id); - } - } - - fn is_entity_alive(&self, entity_id: usize) -> bool { - self.entity_counts.contains_key(&entity_id) - } - - fn take_dropped( - &mut self, - ) -> ( - HashSet, - HashSet<(usize, usize)>, - HashSet, - ) { - ( - std::mem::take(&mut self.dropped_models), - std::mem::take(&mut self.dropped_views), - std::mem::take(&mut self.dropped_element_states), - ) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/gpui/src/app/menu.rs b/crates/gpui/src/app/menu.rs new file mode 100644 index 0000000000..2234bfa391 --- /dev/null +++ b/crates/gpui/src/app/menu.rs @@ -0,0 +1,52 @@ +use crate::{Action, App, ForegroundPlatform, MutableAppContext}; + +pub struct Menu<'a> { + pub name: &'a str, + pub items: Vec>, +} + +pub enum MenuItem<'a> { + Separator, + Submenu(Menu<'a>), + Action { + name: &'a str, + action: Box, + }, +} + +impl MutableAppContext { + pub fn set_menus(&mut self, menus: Vec) { + self.foreground_platform + .set_menus(menus, &self.keystroke_matcher); + } +} + +pub(crate) fn setup_menu_handlers(foreground_platform: &dyn ForegroundPlatform, app: &App) { + foreground_platform.on_will_open_menu(Box::new({ + let cx = app.0.clone(); + move || { + let mut cx = cx.borrow_mut(); + cx.keystroke_matcher.clear_pending(); + } + })); + foreground_platform.on_validate_menu_command(Box::new({ + let cx = app.0.clone(); + move |action| { + let cx = cx.borrow_mut(); + !cx.keystroke_matcher.has_pending_keystrokes() && cx.is_action_available(action) + } + })); + foreground_platform.on_menu_command(Box::new({ + let cx = app.0.clone(); + move |action| { + let mut cx = cx.borrow_mut(); + if let Some(key_window_id) = cx.cx.platform.key_window_id() { + if let Some(view_id) = cx.focused_view_id(key_window_id) { + cx.handle_dispatch_action_from_effect(key_window_id, Some(view_id), action); + return; + } + } + cx.dispatch_global_action_any(action); + } + })); +} diff --git a/crates/gpui/src/app/ref_counts.rs b/crates/gpui/src/app/ref_counts.rs new file mode 100644 index 0000000000..a9ae6e7a6c --- /dev/null +++ b/crates/gpui/src/app/ref_counts.rs @@ -0,0 +1,217 @@ +use std::sync::Arc; + +use lazy_static::lazy_static; +use parking_lot::Mutex; + +use collections::{hash_map::Entry, HashMap, HashSet}; + +use crate::{util::post_inc, ElementStateId}; + +lazy_static! { + static ref LEAK_BACKTRACE: bool = + std::env::var("LEAK_BACKTRACE").map_or(false, |b| !b.is_empty()); +} + +struct ElementStateRefCount { + ref_count: usize, + frame_id: usize, +} + +#[derive(Default)] +pub struct RefCounts { + entity_counts: HashMap, + element_state_counts: HashMap, + dropped_models: HashSet, + dropped_views: HashSet<(usize, usize)>, + dropped_element_states: HashSet, + + #[cfg(any(test, feature = "test-support"))] + pub leak_detector: Arc>, +} + +impl RefCounts { + pub fn new( + #[cfg(any(test, feature = "test-support"))] leak_detector: Arc>, + ) -> Self { + Self { + #[cfg(any(test, feature = "test-support"))] + leak_detector, + ..Default::default() + } + } + + pub fn inc_model(&mut self, model_id: usize) { + match self.entity_counts.entry(model_id) { + Entry::Occupied(mut entry) => { + *entry.get_mut() += 1; + } + Entry::Vacant(entry) => { + entry.insert(1); + self.dropped_models.remove(&model_id); + } + } + } + + pub fn inc_view(&mut self, window_id: usize, view_id: usize) { + match self.entity_counts.entry(view_id) { + Entry::Occupied(mut entry) => *entry.get_mut() += 1, + Entry::Vacant(entry) => { + entry.insert(1); + self.dropped_views.remove(&(window_id, view_id)); + } + } + } + + pub fn inc_element_state(&mut self, id: ElementStateId, frame_id: usize) { + match self.element_state_counts.entry(id) { + Entry::Occupied(mut entry) => { + let entry = entry.get_mut(); + if entry.frame_id == frame_id || entry.ref_count >= 2 { + panic!("used the same element state more than once in the same frame"); + } + entry.ref_count += 1; + entry.frame_id = frame_id; + } + Entry::Vacant(entry) => { + entry.insert(ElementStateRefCount { + ref_count: 1, + frame_id, + }); + self.dropped_element_states.remove(&id); + } + } + } + + pub fn dec_model(&mut self, model_id: usize) { + let count = self.entity_counts.get_mut(&model_id).unwrap(); + *count -= 1; + if *count == 0 { + self.entity_counts.remove(&model_id); + self.dropped_models.insert(model_id); + } + } + + pub fn dec_view(&mut self, window_id: usize, view_id: usize) { + let count = self.entity_counts.get_mut(&view_id).unwrap(); + *count -= 1; + if *count == 0 { + self.entity_counts.remove(&view_id); + self.dropped_views.insert((window_id, view_id)); + } + } + + pub fn dec_element_state(&mut self, id: ElementStateId) { + let entry = self.element_state_counts.get_mut(&id).unwrap(); + entry.ref_count -= 1; + if entry.ref_count == 0 { + self.element_state_counts.remove(&id); + self.dropped_element_states.insert(id); + } + } + + pub fn is_entity_alive(&self, entity_id: usize) -> bool { + self.entity_counts.contains_key(&entity_id) + } + + pub fn take_dropped( + &mut self, + ) -> ( + HashSet, + HashSet<(usize, usize)>, + HashSet, + ) { + ( + std::mem::take(&mut self.dropped_models), + std::mem::take(&mut self.dropped_views), + std::mem::take(&mut self.dropped_element_states), + ) + } +} + +#[cfg(any(test, feature = "test-support"))] +#[derive(Default)] +pub struct LeakDetector { + next_handle_id: usize, + #[allow(clippy::type_complexity)] + handle_backtraces: HashMap< + usize, + ( + Option<&'static str>, + HashMap>, + ), + >, +} + +#[cfg(any(test, feature = "test-support"))] +impl LeakDetector { + pub fn handle_created(&mut self, type_name: Option<&'static str>, entity_id: usize) -> usize { + let handle_id = post_inc(&mut self.next_handle_id); + let entry = self.handle_backtraces.entry(entity_id).or_default(); + let backtrace = if *LEAK_BACKTRACE { + Some(backtrace::Backtrace::new_unresolved()) + } else { + None + }; + if let Some(type_name) = type_name { + entry.0.get_or_insert(type_name); + } + entry.1.insert(handle_id, backtrace); + handle_id + } + + pub fn handle_dropped(&mut self, entity_id: usize, handle_id: usize) { + if let Some((_, backtraces)) = self.handle_backtraces.get_mut(&entity_id) { + assert!(backtraces.remove(&handle_id).is_some()); + if backtraces.is_empty() { + self.handle_backtraces.remove(&entity_id); + } + } + } + + pub fn assert_dropped(&mut self, entity_id: usize) { + if let Some((type_name, backtraces)) = self.handle_backtraces.get_mut(&entity_id) { + for trace in backtraces.values_mut().flatten() { + trace.resolve(); + eprintln!("{:?}", crate::util::CwdBacktrace(trace)); + } + + let hint = if *LEAK_BACKTRACE { + "" + } else { + " – set LEAK_BACKTRACE=1 for more information" + }; + + panic!( + "{} handles to {} {} still exist{}", + backtraces.len(), + type_name.unwrap_or("entity"), + entity_id, + hint + ); + } + } + + pub fn detect(&mut self) { + let mut found_leaks = false; + for (id, (type_name, backtraces)) in self.handle_backtraces.iter_mut() { + eprintln!( + "leaked {} handles to {} {}", + backtraces.len(), + type_name.unwrap_or("entity"), + id + ); + for trace in backtraces.values_mut().flatten() { + trace.resolve(); + eprintln!("{:?}", crate::util::CwdBacktrace(trace)); + } + found_leaks = true; + } + + let hint = if *LEAK_BACKTRACE { + "" + } else { + " – set LEAK_BACKTRACE=1 for more information" + }; + assert!(!found_leaks, "detected leaked handles{}", hint); + } +} diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 67455cd2a7..0805cdd865 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -19,13 +19,14 @@ use smol::stream::StreamExt; use crate::{ executor, geometry::vector::Vector2F, keymap_matcher::Keystroke, platform, Action, AnyViewHandle, AppContext, Appearance, Entity, Event, FontCache, InputHandler, KeyDownEvent, - LeakDetector, ModelContext, ModelHandle, MutableAppContext, Platform, ReadModelWith, - ReadViewWith, RenderContext, Task, UpdateModel, UpdateView, View, ViewContext, ViewHandle, - WeakHandle, WindowInputHandler, + ModelContext, ModelHandle, MutableAppContext, Platform, ReadModelWith, ReadViewWith, + RenderContext, Task, UpdateModel, UpdateView, View, ViewContext, ViewHandle, WeakHandle, }; use collections::BTreeMap; -use super::{AsyncAppContext, RefCounts}; +use super::{ + ref_counts::LeakDetector, window_input_handler::WindowInputHandler, AsyncAppContext, RefCounts, +}; pub struct TestAppContext { cx: Rc>, @@ -52,11 +53,7 @@ impl TestAppContext { platform, foreground_platform.clone(), font_cache, - RefCounts { - #[cfg(any(test, feature = "test-support"))] - leak_detector, - ..Default::default() - }, + RefCounts::new(leak_detector), (), ); cx.next_entity_id = first_entity_id; diff --git a/crates/gpui/src/app/window_input_handler.rs b/crates/gpui/src/app/window_input_handler.rs new file mode 100644 index 0000000000..855f0e3041 --- /dev/null +++ b/crates/gpui/src/app/window_input_handler.rs @@ -0,0 +1,98 @@ +use std::{cell::RefCell, ops::Range, rc::Rc}; + +use pathfinder_geometry::rect::RectF; + +use crate::{AnyView, AppContext, InputHandler, MutableAppContext}; + +pub struct WindowInputHandler { + pub app: Rc>, + pub window_id: usize, +} + +impl WindowInputHandler { + fn read_focused_view(&self, f: F) -> Option + where + F: FnOnce(&dyn AnyView, &AppContext) -> T, + { + // Input-related application hooks are sometimes called by the OS during + // a call to a window-manipulation API, like prompting the user for file + // paths. In that case, the AppContext will already be borrowed, so any + // InputHandler methods need to fail gracefully. + // + // See https://github.com/zed-industries/community/issues/444 + let app = self.app.try_borrow().ok()?; + + let view_id = app.focused_view_id(self.window_id)?; + let view = app.cx.views.get(&(self.window_id, view_id))?; + let result = f(view.as_ref(), &app); + Some(result) + } + + fn update_focused_view(&mut self, f: F) -> Option + where + F: FnOnce(usize, usize, &mut dyn AnyView, &mut MutableAppContext) -> T, + { + let mut app = self.app.try_borrow_mut().ok()?; + app.update(|app| { + let view_id = app.focused_view_id(self.window_id)?; + let mut view = app.cx.views.remove(&(self.window_id, view_id))?; + let result = f(self.window_id, view_id, view.as_mut(), &mut *app); + app.cx.views.insert((self.window_id, view_id), view); + Some(result) + }) + } +} + +impl InputHandler for WindowInputHandler { + fn text_for_range(&self, range: Range) -> Option { + self.read_focused_view(|view, cx| view.text_for_range(range.clone(), cx)) + .flatten() + } + + fn selected_text_range(&self) -> Option> { + self.read_focused_view(|view, cx| view.selected_text_range(cx)) + .flatten() + } + + fn replace_text_in_range(&mut self, range: Option>, text: &str) { + self.update_focused_view(|window_id, view_id, view, cx| { + view.replace_text_in_range(range, text, cx, window_id, view_id); + }); + } + + fn marked_text_range(&self) -> Option> { + self.read_focused_view(|view, cx| view.marked_text_range(cx)) + .flatten() + } + + fn unmark_text(&mut self) { + self.update_focused_view(|window_id, view_id, view, cx| { + view.unmark_text(cx, window_id, view_id); + }); + } + + fn replace_and_mark_text_in_range( + &mut self, + range: Option>, + new_text: &str, + new_selected_range: Option>, + ) { + self.update_focused_view(|window_id, view_id, view, cx| { + view.replace_and_mark_text_in_range( + range, + new_text, + new_selected_range, + cx, + window_id, + view_id, + ); + }); + } + + fn rect_for_range(&self, range_utf16: Range) -> Option { + let app = self.app.borrow(); + let (presenter, _) = app.presenters_and_platform_windows.get(&self.window_id)?; + let presenter = presenter.borrow(); + presenter.rect_for_text_range(range_utf16, &app) + } +} diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index aa73aebc90..173c8d8505 100644 --- a/crates/gpui/src/platform/test.rs +++ b/crates/gpui/src/platform/test.rs @@ -5,7 +5,7 @@ use crate::{ vector::{vec2f, Vector2F}, }, keymap_matcher::KeymapMatcher, - Action, ClipboardItem, + Action, ClipboardItem, Menu, }; use anyhow::{anyhow, Result}; use collections::VecDeque; @@ -77,7 +77,7 @@ impl super::ForegroundPlatform for ForegroundPlatform { fn on_menu_command(&self, _: Box) {} fn on_validate_menu_command(&self, _: Box bool>) {} fn on_will_open_menu(&self, _: Box) {} - fn set_menus(&self, _: Vec, _: &KeymapMatcher) {} + fn set_menus(&self, _: Vec, _: &KeymapMatcher) {} fn prompt_for_paths( &self, diff --git a/crates/gpui/src/test.rs b/crates/gpui/src/test.rs index eb992b638a..d784d43ece 100644 --- a/crates/gpui/src/test.rs +++ b/crates/gpui/src/test.rs @@ -1,14 +1,3 @@ -use crate::{ - elements::Empty, - executor::{self, ExecutorEvent}, - platform, - util::CwdBacktrace, - Element, ElementBox, Entity, FontCache, Handle, LeakDetector, MutableAppContext, Platform, - RenderContext, Subscription, TestAppContext, View, -}; -use futures::StreamExt; -use parking_lot::Mutex; -use smol::channel; use std::{ fmt::Write, panic::{self, RefUnwindSafe}, @@ -19,6 +8,20 @@ use std::{ }, }; +use futures::StreamExt; +use parking_lot::Mutex; +use smol::channel; + +use crate::{ + app::ref_counts::LeakDetector, + elements::Empty, + executor::{self, ExecutorEvent}, + platform, + util::CwdBacktrace, + Element, ElementBox, Entity, FontCache, Handle, MutableAppContext, Platform, RenderContext, + Subscription, TestAppContext, View, +}; + #[cfg(test)] #[ctor::ctor] fn init_logger() { diff --git a/crates/sqlez/Cargo.toml b/crates/sqlez/Cargo.toml index f247f3e537..716ec76644 100644 --- a/crates/sqlez/Cargo.toml +++ b/crates/sqlez/Cargo.toml @@ -15,7 +15,4 @@ thread_local = "1.1.4" lazy_static = "1.4" parking_lot = "0.11.1" futures = "0.3" -uuid = { version = "1.1.2", features = ["v4"] } - -[dev-dependencies] -sqlez_macros = { path = "../sqlez_macros"} \ No newline at end of file +uuid = { version = "1.1.2", features = ["v4"] } \ No newline at end of file diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index d452d4b790..c58f66478f 100644 --- a/crates/vim/src/editor_events.rs +++ b/crates/vim/src/editor_events.rs @@ -1,4 +1,4 @@ -use editor::{EditorBlurred, EditorFocused, EditorMode, EditorReleased}; +use editor::{EditorBlurred, EditorFocused, EditorMode, EditorReleased, Event}; use gpui::MutableAppContext; use crate::{state::Mode, Vim}; @@ -20,14 +20,18 @@ fn focused(EditorFocused(editor): &EditorFocused, cx: &mut MutableAppContext) { } vim.active_editor = Some(editor.downgrade()); - dbg!("Active editor changed", editor.read(cx).mode()); - vim.editor_subscription = Some(cx.subscribe(editor, |editor, event, cx| { - if editor.read(cx).leader_replica_id().is_none() { - if let editor::Event::SelectionsChanged { local: true } = event { - let newest_empty = editor.read(cx).selections.newest::(cx).is_empty(); + vim.editor_subscription = Some(cx.subscribe(editor, |editor, event, cx| match event { + Event::SelectionsChanged { local: true } => { + let editor = editor.read(cx); + if editor.leader_replica_id().is_none() { + let newest_empty = editor.selections.newest::(cx).is_empty(); local_selections_changed(newest_empty, cx); } } + Event::InputIgnored { text } => { + Vim::active_editor_input_ignored(text.clone(), cx); + } + _ => {} })); if vim.enabled { diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 62b30730e8..8bc7c756e0 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use editor::{ char_kind, display_map::{DisplaySnapshot, ToDisplayPoint}, @@ -15,7 +17,7 @@ use crate::{ Vim, }; -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum Motion { Left, Backspace, @@ -32,8 +34,8 @@ pub enum Motion { StartOfDocument, EndOfDocument, Matching, - FindForward { before: bool, character: char }, - FindBackward { after: bool, character: char }, + FindForward { before: bool, text: Arc }, + FindBackward { after: bool, text: Arc }, } #[derive(Clone, Deserialize, PartialEq)] @@ -134,7 +136,7 @@ pub(crate) fn motion(motion: Motion, cx: &mut MutableAppContext) { // Motion handling is specified here: // https://github.com/vim/vim/blob/master/runtime/doc/motion.txt impl Motion { - pub fn linewise(self) -> bool { + pub fn linewise(&self) -> bool { use Motion::*; matches!( self, @@ -142,12 +144,12 @@ impl Motion { ) } - pub fn infallible(self) -> bool { + pub fn infallible(&self) -> bool { use Motion::*; matches!(self, StartOfDocument | CurrentLine | EndOfDocument) } - pub fn inclusive(self) -> bool { + pub fn inclusive(&self) -> bool { use Motion::*; match self { Down @@ -171,13 +173,14 @@ impl Motion { } pub fn move_point( - self, + &self, map: &DisplaySnapshot, point: DisplayPoint, goal: SelectionGoal, times: usize, ) -> Option<(DisplayPoint, SelectionGoal)> { use Motion::*; + let infallible = self.infallible(); let (new_point, goal) = match self { Left => (left(map, point, times), SelectionGoal::None), Backspace => (backspace(map, point, times), SelectionGoal::None), @@ -185,15 +188,15 @@ impl Motion { Up => up(map, point, goal, times), Right => (right(map, point, times), SelectionGoal::None), NextWordStart { ignore_punctuation } => ( - next_word_start(map, point, ignore_punctuation, times), + next_word_start(map, point, *ignore_punctuation, times), SelectionGoal::None, ), NextWordEnd { ignore_punctuation } => ( - next_word_end(map, point, ignore_punctuation, times), + next_word_end(map, point, *ignore_punctuation, times), SelectionGoal::None, ), PreviousWordStart { ignore_punctuation } => ( - previous_word_start(map, point, ignore_punctuation, times), + previous_word_start(map, point, *ignore_punctuation, times), SelectionGoal::None, ), FirstNonWhitespace => (first_non_whitespace(map, point), SelectionGoal::None), @@ -203,22 +206,22 @@ impl Motion { StartOfDocument => (start_of_document(map, point, times), SelectionGoal::None), EndOfDocument => (end_of_document(map, point, times), SelectionGoal::None), Matching => (matching(map, point), SelectionGoal::None), - FindForward { before, character } => ( - find_forward(map, point, before, character, times), + FindForward { before, text } => ( + find_forward(map, point, *before, text.clone(), times), SelectionGoal::None, ), - FindBackward { after, character } => ( - find_backward(map, point, after, character, times), + FindBackward { after, text } => ( + find_backward(map, point, *after, text.clone(), times), SelectionGoal::None, ), }; - (new_point != point || self.infallible()).then_some((new_point, goal)) + (new_point != point || infallible).then_some((new_point, goal)) } // Expands a selection using self motion for an operator pub fn expand_selection( - self, + &self, map: &DisplaySnapshot, selection: &mut Selection, times: usize, @@ -254,7 +257,7 @@ impl Motion { // but "d}" will not include that line. let mut inclusive = self.inclusive(); if !inclusive - && self != Motion::Backspace + && self != &Motion::Backspace && selection.end.row() > selection.start.row() && selection.end.column() == 0 { @@ -466,45 +469,42 @@ fn find_forward( map: &DisplaySnapshot, from: DisplayPoint, before: bool, - target: char, - mut times: usize, + target: Arc, + times: usize, ) -> DisplayPoint { - let mut previous_point = from; - - for (ch, point) in map.chars_at(from) { - if ch == target && point != from { - times -= 1; - if times == 0 { - return if before { previous_point } else { point }; + map.find_while(from, target.as_ref(), |ch, _| ch != '\n') + .skip_while(|found_at| found_at == &from) + .nth(times - 1) + .map(|mut found| { + if before { + *found.column_mut() -= 1; + found = map.clip_point(found, Bias::Right); + found + } else { + found } - } else if ch == '\n' { - break; - } - previous_point = point; - } - - from + }) + .unwrap_or(from) } fn find_backward( map: &DisplaySnapshot, from: DisplayPoint, after: bool, - target: char, - mut times: usize, + target: Arc, + times: usize, ) -> DisplayPoint { - let mut previous_point = from; - for (ch, point) in map.reverse_chars_at(from) { - if ch == target && point != from { - times -= 1; - if times == 0 { - return if after { previous_point } else { point }; + map.reverse_find_while(from, target.as_ref(), |ch, _| ch != '\n') + .skip_while(|found_at| found_at == &from) + .nth(times - 1) + .map(|mut found| { + if after { + *found.column_mut() += 1; + found = map.clip_point(found, Bias::Left); + found + } else { + found } - } else if ch == '\n' { - break; - } - previous_point = point; - } - - from + }) + .unwrap_or(from) } diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index d6391353cf..742f2426c8 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -2,7 +2,7 @@ mod change; mod delete; mod yank; -use std::{borrow::Cow, cmp::Ordering}; +use std::{borrow::Cow, cmp::Ordering, sync::Arc}; use crate::{ motion::Motion, @@ -424,7 +424,7 @@ fn scroll(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContext, cx: &mut MutableAppContext) { Vim::update(cx, |vim, cx| { vim.update_active_editor(cx, |editor, cx| { editor.transact(cx, |editor, cx| { @@ -453,7 +453,7 @@ pub(crate) fn normal_replace(text: &str, cx: &mut MutableAppContext) { ( range.start.to_offset(&map, Bias::Left) ..range.end.to_offset(&map, Bias::Left), - text, + text.clone(), ) }) .collect::>(); diff --git a/crates/vim/src/test/vim_test_context.rs b/crates/vim/src/test/vim_test_context.rs index 723dac0581..539ab0a8ff 100644 --- a/crates/vim/src/test/vim_test_context.rs +++ b/crates/vim/src/test/vim_test_context.rs @@ -53,7 +53,7 @@ impl<'a> VimTestContext<'a> { // Setup search toolbars and keypress hook workspace.update(cx, |workspace, cx| { - observe_keypresses(window_id, cx); + observe_keystrokes(window_id, cx); workspace.active_pane().update(cx, |pane, cx| { pane.toolbar().update(cx, |toolbar, cx| { let buffer_search_bar = cx.add_view(BufferSearchBar::new); diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 699ff01dcc..9fe9969680 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -10,12 +10,12 @@ mod state; mod utils; mod visual; +use std::sync::Arc; + use command_palette::CommandPaletteFilter; use editor::{Bias, Cancel, Editor, EditorMode}; use gpui::{ - impl_actions, - keymap_matcher::{KeyPressed, Keystroke}, - MutableAppContext, Subscription, ViewContext, ViewHandle, WeakViewHandle, + impl_actions, MutableAppContext, Subscription, ViewContext, ViewHandle, WeakViewHandle, }; use language::CursorShape; use motion::Motion; @@ -57,11 +57,6 @@ pub fn init(cx: &mut MutableAppContext) { cx.add_action(|_: &mut Workspace, n: &Number, cx: _| { Vim::update(cx, |vim, cx| vim.push_number(n, cx)); }); - cx.add_action( - |_: &mut Workspace, KeyPressed { keystroke }: &KeyPressed, cx| { - Vim::key_pressed(keystroke, cx); - }, - ); // Editor Actions cx.add_action(|_: &mut Editor, _: &Cancel, cx| { @@ -91,7 +86,7 @@ pub fn init(cx: &mut MutableAppContext) { .detach(); } -pub fn observe_keypresses(window_id: usize, cx: &mut MutableAppContext) { +pub fn observe_keystrokes(window_id: usize, cx: &mut MutableAppContext) { cx.observe_keystrokes(window_id, |_keystroke, _result, handled_by, cx| { if let Some(handled_by) = handled_by { // Keystroke is handled by the vim system, so continue forward @@ -103,11 +98,12 @@ pub fn observe_keypresses(window_id: usize, cx: &mut MutableAppContext) { } } - Vim::update(cx, |vim, cx| { - if vim.active_operator().is_some() { - // If the keystroke is not handled by vim, we should clear the operator + Vim::update(cx, |vim, cx| match vim.active_operator() { + Some(Operator::FindForward { .. } | Operator::FindBackward { .. }) => {} + Some(_) => { vim.clear_operator(cx); } + _ => {} }); true }) @@ -164,7 +160,6 @@ impl Vim { .and_then(|editor| editor.upgrade(cx)) { editor.update(cx, |editor, cx| { - dbg!(&mode, editor.mode()); editor.change_selections(None, cx, |s| { s.move_with(|map, selection| { if self.state.empty_selections_only() { @@ -221,24 +216,24 @@ impl Vim { self.state.operator_stack.last().copied() } - fn key_pressed(keystroke: &Keystroke, cx: &mut ViewContext) { + fn active_editor_input_ignored(text: Arc, cx: &mut MutableAppContext) { + if text.is_empty() { + return; + } + match Vim::read(cx).active_operator() { Some(Operator::FindForward { before }) => { - if let Some(character) = keystroke.key.chars().next() { - motion::motion(Motion::FindForward { before, character }, cx) - } + motion::motion(Motion::FindForward { before, text }, cx) } Some(Operator::FindBackward { after }) => { - if let Some(character) = keystroke.key.chars().next() { - motion::motion(Motion::FindBackward { after, character }, cx) - } + motion::motion(Motion::FindBackward { after, text }, cx) } Some(Operator::Replace) => match Vim::read(cx).state.mode { - Mode::Normal => normal_replace(&keystroke.key, cx), - Mode::Visual { line } => visual_replace(&keystroke.key, line, cx), + Mode::Normal => normal_replace(text, cx), + Mode::Visual { line } => visual_replace(text, line, cx), _ => Vim::update(cx, |vim, cx| vim.clear_operator(cx)), }, - _ => cx.propagate_action(), + _ => {} } } diff --git a/crates/vim/src/visual.rs b/crates/vim/src/visual.rs index ac8771f969..b890e4e41b 100644 --- a/crates/vim/src/visual.rs +++ b/crates/vim/src/visual.rs @@ -1,4 +1,4 @@ -use std::borrow::Cow; +use std::{borrow::Cow, sync::Arc}; use collections::HashMap; use editor::{ @@ -313,7 +313,7 @@ pub fn paste(_: &mut Workspace, _: &VisualPaste, cx: &mut ViewContext }); } -pub(crate) fn visual_replace(text: &str, line: bool, cx: &mut MutableAppContext) { +pub(crate) fn visual_replace(text: Arc, line: bool, cx: &mut MutableAppContext) { Vim::update(cx, |vim, cx| { vim.update_active_editor(cx, |editor, cx| { editor.transact(cx, |editor, cx| { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 9195c36940..bf9afe136e 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -363,7 +363,7 @@ pub fn initialize_workspace( auto_update::notify_of_any_new_update(cx.weak_handle(), cx); let window_id = cx.window_id(); - vim::observe_keypresses(window_id, cx); + vim::observe_keystrokes(window_id, cx); cx.on_window_should_close(|workspace, cx| { if let Some(task) = workspace.close(&Default::default(), cx) {