From c71e522b4ea1b48865ceb5db3ee1d6a0803813ed Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 11:37:46 -0500 Subject: [PATCH 01/24] Allow users to set UI font properties in their settings Co-Authored-By: Marshall Bowers <1486634+maxdeviant@users.noreply.github.com> --- assets/settings/default.json | 9 +++++++++ crates/editor2/src/editor.rs | 2 +- crates/theme2/src/settings.rs | 17 ++++++++++++++--- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 85f8a8fbc4b290c8fc7f913a44fbe0f767341510..08d85dd723cc13ca98b0b239a199b263f738d99a 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -35,6 +35,15 @@ // "custom": 2 // }, "buffer_line_height": "comfortable", + // The name of a font to use for rendering text in the UI + "ui_font_family": "Zed Mono", + // The OpenType features to enable for text in the UI + "ui_font_features": { + // Disable ligatures: + "calt": false + }, + // The default font size for text in the UI + "ui_font_size": 14, // The factor to grow the active pane by. Defaults to 1.0 // which gives the same size as all other panes. "active_pane_magnification": 1.0, diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index 8e7bd5876f7d69c3254464ff9d32b31ad5877037..0063bf2ce4274de298ec8d5a2a2a5c4d2574219a 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -9387,7 +9387,7 @@ impl Render for Editor { font_size: rems(0.875).into(), font_weight: FontWeight::NORMAL, font_style: FontStyle::Normal, - line_height: relative(1.3).into(), // TODO relative(settings.buffer_line_height.value()), + line_height: relative(1.).into(), underline: None, } } diff --git a/crates/theme2/src/settings.rs b/crates/theme2/src/settings.rs index 5e3329ffa149b1f744f7031162c7235961c42a96..c705617db04f2a554c4853ceac370087ecc52edf 100644 --- a/crates/theme2/src/settings.rs +++ b/crates/theme2/src/settings.rs @@ -34,6 +34,10 @@ pub struct ThemeSettingsContent { #[serde(default)] pub ui_font_size: Option, #[serde(default)] + pub ui_font_family: Option, + #[serde(default)] + pub ui_font_features: Option, + #[serde(default)] pub buffer_font_family: Option, #[serde(default)] pub buffer_font_size: Option, @@ -120,10 +124,10 @@ impl settings::Settings for ThemeSettings { let themes = cx.default_global::>(); let mut this = Self { - ui_font_size: defaults.ui_font_size.unwrap_or(16.).into(), + ui_font_size: defaults.ui_font_size.unwrap().into(), ui_font: Font { - family: "Helvetica".into(), - features: Default::default(), + family: defaults.ui_font_family.clone().unwrap().into(), + features: defaults.ui_font_features.clone().unwrap(), weight: Default::default(), style: Default::default(), }, @@ -149,6 +153,13 @@ impl settings::Settings for ThemeSettings { this.buffer_font.features = value; } + if let Some(value) = value.ui_font_family { + this.ui_font.family = value.into(); + } + if let Some(value) = value.ui_font_features { + this.ui_font.features = value; + } + if let Some(value) = &value.theme { if let Some(theme) = themes.get(value).log_err() { this.active_theme = theme; From b2f9c454b0215a8387e8c50c270eabb577896a41 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 11:38:04 -0500 Subject: [PATCH 02/24] Change the default buffer font size to 16 Co-Authored-By: Marshall Bowers <1486634+maxdeviant@users.noreply.github.com> --- 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 08d85dd723cc13ca98b0b239a199b263f738d99a..0b87808e22cff23a85ae9f47ff7670906c786ce5 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -23,7 +23,7 @@ // "calt": false }, // The default font size for text in the editor - "buffer_font_size": 15, + "buffer_font_size": 16, // Set the buffer's line height. // May take 3 values: // 1. Use a line height that's comfortable for reading (1.618) From 38d0fdc09aafbeb9388396fd149fd47d568f59e4 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 11:42:23 -0500 Subject: [PATCH 03/24] Remove todo Co-Authored-By: Marshall Bowers <1486634+maxdeviant@users.noreply.github.com> --- crates/editor2/src/editor.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index 0063bf2ce4274de298ec8d5a2a2a5c4d2574219a..c4788ebd71421f17f55567c1cb21e43d5593514d 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -9379,18 +9379,16 @@ impl Render for Editor { fn render(&mut self, cx: &mut ViewContext) -> Self::Element { let settings = ThemeSettings::get_global(cx); let text_style = match self.mode { - EditorMode::SingleLine => { - TextStyle { - color: cx.theme().colors().text, - font_family: settings.ui_font.family.clone(), // todo!() - font_features: settings.ui_font.features, - font_size: rems(0.875).into(), - font_weight: FontWeight::NORMAL, - font_style: FontStyle::Normal, - line_height: relative(1.).into(), - underline: None, - } - } + EditorMode::SingleLine => TextStyle { + color: cx.theme().colors().text, + font_family: settings.ui_font.family.clone(), + font_features: settings.ui_font.features, + font_size: rems(0.875).into(), + font_weight: FontWeight::NORMAL, + font_style: FontStyle::Normal, + line_height: relative(1.).into(), + underline: None, + }, EditorMode::AutoHeight { max_lines } => todo!(), From b559bfd80f23758e482e53e4384143a72aef2556 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 16 Nov 2023 13:03:30 -0500 Subject: [PATCH 04/24] Parameterize `theme2::init` to allow loading just the base theme (#3345) This PR adds a parameter to the `theme2::init` method to indicate what the theme-loading behavior should be. This allows us to indicate when we want to load all of the additional built-in user themes (like in the Zed binary and in the storybook), and when we don't want to load the user themes (like in tests). We're using an enum over just a `bool` here for clarity at the call site. Release Notes: - N/A --- crates/collab2/src/tests/test_server.rs | 2 +- .../command_palette2/src/command_palette.rs | 2 +- crates/editor2/src/display_map/inlay_map.rs | 2 +- crates/editor2/src/editor_tests.rs | 2 +- crates/editor2/src/inlay_hint_cache.rs | 2 +- crates/file_finder2/src/file_finder.rs | 2 +- crates/project_panel2/src/project_panel.rs | 4 ++-- crates/storybook2/src/storybook2.rs | 2 +- crates/terminal_view2/src/terminal_view.rs | 2 +- crates/theme2/src/registry.rs | 8 +++++--- crates/theme2/src/settings.rs | 2 +- crates/theme2/src/theme2.rs | 19 ++++++++++++++++++- crates/workspace2/src/workspace2.rs | 2 +- crates/zed2/src/main.rs | 2 +- 14 files changed, 36 insertions(+), 17 deletions(-) diff --git a/crates/collab2/src/tests/test_server.rs b/crates/collab2/src/tests/test_server.rs index de6f3e92a1be9057df060d082a204312eb905318..090a32d4caef33386da38f0b5cd60f83a6db5afc 100644 --- a/crates/collab2/src/tests/test_server.rs +++ b/crates/collab2/src/tests/test_server.rs @@ -224,7 +224,7 @@ impl TestServer { }); cx.update(|cx| { - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); Project::init(&client, cx); client::init(&client, cx); language::init(cx); diff --git a/crates/command_palette2/src/command_palette.rs b/crates/command_palette2/src/command_palette.rs index 8138f025d3da4af5cfb285c48be0ea8b88879f8f..31eb608ef56bc56bbfdbc1bdf1eca776ee3c6811 100644 --- a/crates/command_palette2/src/command_palette.rs +++ b/crates/command_palette2/src/command_palette.rs @@ -456,7 +456,7 @@ mod tests { fn init_test(cx: &mut TestAppContext) -> Arc { cx.update(|cx| { let app_state = AppState::test(cx); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); language::init(cx); editor::init(cx); workspace::init(app_state.clone(), cx); diff --git a/crates/editor2/src/display_map/inlay_map.rs b/crates/editor2/src/display_map/inlay_map.rs index a6bc6343f4fc58e7fc7240fc186845edbdb6a115..84fad96a48026c4ab7a08f3f6674a0a2958b89a7 100644 --- a/crates/editor2/src/display_map/inlay_map.rs +++ b/crates/editor2/src/display_map/inlay_map.rs @@ -1891,6 +1891,6 @@ mod tests { fn init_test(cx: &mut AppContext) { let store = SettingsStore::test(cx); cx.set_global(store); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); } } diff --git a/crates/editor2/src/editor_tests.rs b/crates/editor2/src/editor_tests.rs index 63bc6179c20080e6015068d2c68fd0aac46479a5..bd69e7acdf5c01875c9908a7d56665c6260fb0ce 100644 --- a/crates/editor2/src/editor_tests.rs +++ b/crates/editor2/src/editor_tests.rs @@ -8277,7 +8277,7 @@ pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsC cx.update(|cx| { let store = SettingsStore::test(cx); cx.set_global(store); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); client::init_settings(cx); language::init(cx); Project::init_settings(cx); diff --git a/crates/editor2/src/inlay_hint_cache.rs b/crates/editor2/src/inlay_hint_cache.rs index af9febf376b2e74eb9ccac99d84fbe588ece612b..eba49ccbf749d5a7b4279955de8a74d43a23d8ae 100644 --- a/crates/editor2/src/inlay_hint_cache.rs +++ b/crates/editor2/src/inlay_hint_cache.rs @@ -3179,7 +3179,7 @@ all hints should be invalidated and requeried for all of its visible excerpts" cx.update(|cx| { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); client::init_settings(cx); language::init(cx); Project::init_settings(cx); diff --git a/crates/file_finder2/src/file_finder.rs b/crates/file_finder2/src/file_finder.rs index 236bc152441e4eabb097616c7cc12e7c4da1236a..b2850761a97a373f9fcdb6b01ea0c5c9acd80b22 100644 --- a/crates/file_finder2/src/file_finder.rs +++ b/crates/file_finder2/src/file_finder.rs @@ -1763,7 +1763,7 @@ mod tests { fn init_test(cx: &mut TestAppContext) -> Arc { cx.update(|cx| { let state = AppState::test(cx); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); language::init(cx); super::init(cx); editor::init(cx); diff --git a/crates/project_panel2/src/project_panel.rs b/crates/project_panel2/src/project_panel.rs index f33633afb9bdd081ff671f8867f325ce96a38a0c..87edabab5217084dffa8693dc5121de4bc6166fa 100644 --- a/crates/project_panel2/src/project_panel.rs +++ b/crates/project_panel2/src/project_panel.rs @@ -2785,7 +2785,7 @@ mod tests { let settings_store = SettingsStore::test(cx); cx.set_global(settings_store); init_settings(cx); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); language::init(cx); editor::init_settings(cx); crate::init((), cx); @@ -2798,7 +2798,7 @@ mod tests { fn init_test_with_editor(cx: &mut TestAppContext) { cx.update(|cx| { let app_state = AppState::test(cx); - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); init_settings(cx); language::init(cx); editor::init(cx); diff --git a/crates/storybook2/src/storybook2.rs b/crates/storybook2/src/storybook2.rs index c4c1d75eac26b245ede5b79b6042e33cbeec67f7..20adc44c1aa84ec96491ffacd884de5b73efbfb2 100644 --- a/crates/storybook2/src/storybook2.rs +++ b/crates/storybook2/src/storybook2.rs @@ -60,7 +60,7 @@ fn main() { .unwrap(); cx.set_global(store); - theme2::init(cx); + theme2::init(theme2::LoadThemes::All, cx); let selector = story_selector.unwrap_or(StorySelector::Component(ComponentStory::Workspace)); diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index 14391ca2b2f357f56f084a5688601182bad780cf..ad337163845b965e94d7f9f3eb1c3d8b02dbf325 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -1126,7 +1126,7 @@ mod tests { pub async fn init_test(cx: &mut TestAppContext) -> (Model, View) { let params = cx.update(AppState::test); cx.update(|cx| { - theme::init(cx); + theme::init(theme::LoadThemes::JustBase, cx); Project::init_settings(cx); language::init(cx); }); diff --git a/crates/theme2/src/registry.rs b/crates/theme2/src/registry.rs index 19af0ede51c1ce4467ac57d91dd8ffd86c8eb93c..919dd1b1099ecb35853142a2b5a66f14904a7653 100644 --- a/crates/theme2/src/registry.rs +++ b/crates/theme2/src/registry.rs @@ -100,6 +100,11 @@ impl ThemeRegistry { .ok_or_else(|| anyhow!("theme not found: {}", name)) .cloned() } + + pub fn load_user_themes(&mut self) { + #[cfg(not(feature = "importing-themes"))] + self.insert_user_theme_familes(crate::all_user_themes()); + } } impl Default for ThemeRegistry { @@ -110,9 +115,6 @@ impl Default for ThemeRegistry { this.insert_theme_families([one_family()]); - #[cfg(not(feature = "importing-themes"))] - this.insert_user_theme_familes(crate::all_user_themes()); - this } } diff --git a/crates/theme2/src/settings.rs b/crates/theme2/src/settings.rs index 5e3329ffa149b1f744f7031162c7235961c42a96..071199d3d6a771ec5a044d7c2d386ac5391ab07c 100644 --- a/crates/theme2/src/settings.rs +++ b/crates/theme2/src/settings.rs @@ -117,7 +117,7 @@ impl settings::Settings for ThemeSettings { user_values: &[&Self::FileContent], cx: &mut AppContext, ) -> Result { - let themes = cx.default_global::>(); + let themes = cx.default_global::(); let mut this = Self { ui_font_size: defaults.ui_font_size.unwrap_or(16.).into(), diff --git a/crates/theme2/src/theme2.rs b/crates/theme2/src/theme2.rs index b6790b5a6ff534cb8080e7ae88a28c72cbeeefb7..05e41ba368bdaf60616383e24f37df4445b07306 100644 --- a/crates/theme2/src/theme2.rs +++ b/crates/theme2/src/theme2.rs @@ -31,8 +31,25 @@ pub enum Appearance { Dark, } -pub fn init(cx: &mut AppContext) { +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum LoadThemes { + /// Only load the base theme. + /// + /// No user themes will be loaded. + JustBase, + + /// Load all of the built-in themes. + All, +} + +pub fn init(themes_to_load: LoadThemes, cx: &mut AppContext) { cx.set_global(ThemeRegistry::default()); + + match themes_to_load { + LoadThemes::JustBase => (), + LoadThemes::All => cx.global_mut::().load_user_themes(), + } + ThemeSettings::register(cx); } diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index f28675661de250b03f9e3d174da21d9835a2d2fd..a146ad1822b75a9c9ea25cfe581c480cd5f92742 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -355,7 +355,7 @@ impl AppState { let user_store = cx.build_model(|cx| UserStore::new(client.clone(), http_client, cx)); let workspace_store = cx.build_model(|cx| WorkspaceStore::new(client.clone(), cx)); - theme2::init(cx); + theme2::init(theme2::LoadThemes::JustBase, cx); client2::init(&client, cx); crate::init_settings(cx); diff --git a/crates/zed2/src/main.rs b/crates/zed2/src/main.rs index ef65d00fc386503d68a6aaa457ecadabbf21e1a0..700660a9b7e69b30cd28366cda2b7d2a325162a1 100644 --- a/crates/zed2/src/main.rs +++ b/crates/zed2/src/main.rs @@ -140,7 +140,7 @@ fn main() { cx.set_global(client.clone()); - theme::init(cx); + theme::init(theme::LoadThemes::All, cx); project::Project::init(&client, cx); client::init(&client, cx); command_palette::init(cx); From 08dddf0b262605d9d5a75c20c31521fdd5f7eaab Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 13:13:03 -0500 Subject: [PATCH 05/24] Revert change to default buffer font size --- 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 0b87808e22cff23a85ae9f47ff7670906c786ce5..08d85dd723cc13ca98b0b239a199b263f738d99a 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -23,7 +23,7 @@ // "calt": false }, // The default font size for text in the editor - "buffer_font_size": 16, + "buffer_font_size": 15, // Set the buffer's line height. // May take 3 values: // 1. Use a line height that's comfortable for reading (1.618) From a0e976599c19de4d6a3d5975c779b35151b365c5 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Thu, 16 Nov 2023 10:32:55 -0800 Subject: [PATCH 06/24] Salvage old distributed slice code --- Cargo.lock | 21 ++++++++++++ crates/gpui2/Cargo.toml | 1 + crates/gpui2/src/action.rs | 40 ++++++++++++++++++++++ crates/gpui2/src/gpui2.rs | 1 + crates/gpui2_macros/Cargo.toml | 2 +- crates/gpui2_macros/src/register_action.rs | 27 ++++++++++++--- 6 files changed, 86 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf2e964ea8f533a8a0c0ada0e60077fdd140caa1..3b45a329183efcbf686be97aaefb7dbf29990fb0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3797,6 +3797,7 @@ dependencies = [ "image", "itertools 0.10.5", "lazy_static", + "linkme", "log", "media", "metal", @@ -4815,6 +4816,26 @@ version = "0.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" +[[package]] +name = "linkme" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91ed2ee9464ff9707af8e9ad834cffa4802f072caad90639c583dd3c62e6e608" +dependencies = [ + "linkme-impl", +] + +[[package]] +name = "linkme-impl" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba125974b109d512fccbc6c0244e7580143e460895dfd6ea7f8bbb692fd94396" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.37", +] + [[package]] name = "linux-raw-sys" version = "0.0.42" diff --git a/crates/gpui2/Cargo.toml b/crates/gpui2/Cargo.toml index df461af7b87c40044f3ad8cc8b4c198e222c0263..1bec9d43dc34f8ebcd13b65ec962e67b16bf363c 100644 --- a/crates/gpui2/Cargo.toml +++ b/crates/gpui2/Cargo.toml @@ -22,6 +22,7 @@ sqlez = { path = "../sqlez" } async-task = "4.0.3" backtrace = { version = "0.3", optional = true } ctor.workspace = true +linkme = "0.3" derive_more.workspace = true dhat = { version = "0.3", optional = true } env_logger = { version = "0.9", optional = true } diff --git a/crates/gpui2/src/action.rs b/crates/gpui2/src/action.rs index a81bcfcdbc010262b8e36c33ea88b07e4273ebfc..0a5ea781bdf04ed930ecb2baa2970fc099e0fb0b 100644 --- a/crates/gpui2/src/action.rs +++ b/crates/gpui2/src/action.rs @@ -200,3 +200,43 @@ macro_rules! actions { actions!($($rest)*); }; } + +/// This type must be public so that our macros can build it in other crates. +/// But this is an implementation detail and should not be used directly. +#[doc(hidden)] +pub struct ActionData { + pub name: &'static str, + pub build: ActionBuilder, + pub type_id: TypeId, +} + +/// This type must be public so that our macros can build it in other crates. +/// But this is an implementation detail and should not be used directly. +#[doc(hidden)] +pub type MacroActionBuilder = fn() -> ActionData; + +/// This constant must be public to be accessible from other crates. +/// But it's existence is an implementation detail and should not be used directly. +#[doc(hidden)] +#[linkme::distributed_slice] +pub static __GPUI_ACTIONS: [MacroActionBuilder]; + +fn qualify_name(action_name: &'static str) -> SharedString { + let mut separator_matches = action_name.rmatch_indices("::"); + separator_matches.next().unwrap(); + let name_start_ix = separator_matches.next().map_or(0, |(ix, _)| ix + 2); + // todo!() remove the 2 replacement when migration is done + action_name[name_start_ix..].replace("2::", "::").into() +} + +pub(crate) fn load_actions_2() { + let mut lock = ACTION_REGISTRY.write(); + + for action in __GPUI_ACTIONS { + let action = action(); + let name = qualify_name(action.name); + lock.builders_by_name.insert(name.clone(), action.build); + lock.names_by_type_id.insert(action.type_id, name.clone()); + lock.all_names.push(name); + } +} diff --git a/crates/gpui2/src/gpui2.rs b/crates/gpui2/src/gpui2.rs index 3b98b846c475daac41075734e9523353089a84ec..c2d10154dec3c102191460cccac0e71a7248ca99 100644 --- a/crates/gpui2/src/gpui2.rs +++ b/crates/gpui2/src/gpui2.rs @@ -49,6 +49,7 @@ pub use input::*; pub use interactive::*; pub use key_dispatch::*; pub use keymap::*; +pub use linkme; pub use platform::*; use private::Sealed; pub use refineable::*; diff --git a/crates/gpui2_macros/Cargo.toml b/crates/gpui2_macros/Cargo.toml index eb44334095d40f3231dfac5df498641699cbaba2..aab669c1b71653a76a95561fbd429a534efb2cab 100644 --- a/crates/gpui2_macros/Cargo.toml +++ b/crates/gpui2_macros/Cargo.toml @@ -9,6 +9,6 @@ path = "src/gpui2_macros.rs" proc-macro = true [dependencies] -syn = "1.0.72" +syn = { version = "1.0.72", features = ["full"] } quote = "1.0.9" proc-macro2 = "1.0.66" diff --git a/crates/gpui2_macros/src/register_action.rs b/crates/gpui2_macros/src/register_action.rs index 68c39ad9bd7347be7d01e139e37b5a14c8c05d92..e6c47e8c5284c13db43a4ba29cf2c36fd10d9f9d 100644 --- a/crates/gpui2_macros/src/register_action.rs +++ b/crates/gpui2_macros/src/register_action.rs @@ -18,14 +18,31 @@ use syn::{parse_macro_input, DeriveInput}; pub fn register_action(_attr: TokenStream, item: TokenStream) -> TokenStream { let input = parse_macro_input!(item as DeriveInput); let type_name = &input.ident; - let ctor_fn_name = format_ident!("register_{}_builder", type_name.to_string().to_lowercase()); + + let static_slice_name = + format_ident!("__GPUI_ACTIONS_{}", type_name.to_string().to_uppercase()); + + let action_builder_fn_name = format_ident!( + "__gpui_actions_builder_{}", + type_name.to_string().to_lowercase() + ); let expanded = quote! { #input - #[allow(non_snake_case)] - #[gpui::ctor] - fn #ctor_fn_name() { - gpui::register_action::<#type_name>() + + #[doc(hidden)] + #[gpui::linkme::distributed_slice(gpui::__GPUI_ACTIONS)] + #[linkme(crate = gpui::linkme)] + static #static_slice_name: gpui::MacroActionBuilder = #action_builder_fn_name; + + /// This is an auto generated function, do not use. + #[doc(hidden)] + fn #action_builder_fn_name() -> gpui::ActionData { + gpui::ActionData { + name: ::std::any::type_name::<#type_name>(), + type_id: ::std::any::TypeId::of::<#type_name>(), + build: <#type_name as gpui::Action>::build, + } } }; From fa9f4a935554758ee9a3f17485ba3b50ebe913af Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 13:43:36 -0500 Subject: [PATCH 07/24] Init rem_size in the workspace at the start of the render Co-Authored-By: Mikayla Maki --- crates/workspace2/src/workspace2.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index f28675661de250b03f9e3d174da21d9835a2d2fd..e09624bd2cb695975cf46e5818f0b2cab0d5bad2 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -3614,7 +3614,16 @@ impl Render for Workspace { fn render(&mut self, cx: &mut ViewContext) -> Self::Element { let mut context = KeyContext::default(); context.add("Workspace"); - let ui_font = ThemeSettings::get_global(cx).ui_font.family.clone(); + + let (ui_font, ui_font_size) = { + let theme_settings = ThemeSettings::get_global(cx); + ( + theme_settings.ui_font.family.clone(), + theme_settings.ui_font_size.clone(), + ) + }; + + cx.set_rem_size(ui_font_size); self.add_workspace_actions_listeners(div()) .key_context(context) From ffd092a098c54a8b8c4f8f497038b9f6e626d93f Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 15:30:50 -0500 Subject: [PATCH 08/24] Add ui_font_* for tests --- crates/settings2/src/settings_file.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/settings2/src/settings_file.rs b/crates/settings2/src/settings_file.rs index 6f2c8d374f1aa52c1285e96fb87f1765f171b4ce..fc4ad5882e25b6c452a412f107e7535000a6cff8 100644 --- a/crates/settings2/src/settings_file.rs +++ b/crates/settings2/src/settings_file.rs @@ -16,6 +16,9 @@ pub fn test_settings() -> String { .unwrap(); util::merge_non_null_json_value_into( serde_json::json!({ + "ui_font_family": "Courier", + "ui_font_features": {}, + "ui_font_size": 14, "buffer_font_family": "Courier", "buffer_font_features": {}, "buffer_font_size": 14, From c2d6d2495267f4ba78a04e41c38e1dfb095b1f72 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 16:01:42 -0500 Subject: [PATCH 09/24] Ensure the titlebar stays large enough even with small ui sizes --- crates/collab_ui2/src/collab_titlebar_item.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/collab_ui2/src/collab_titlebar_item.rs b/crates/collab_ui2/src/collab_titlebar_item.rs index c9d16c7a5de68a7004980c4abb1a9dd0f25a8279..ca5118cd8de5d4c04b251bcedf4fb58509dd64f9 100644 --- a/crates/collab_ui2/src/collab_titlebar_item.rs +++ b/crates/collab_ui2/src/collab_titlebar_item.rs @@ -31,9 +31,9 @@ use std::sync::Arc; use call::ActiveCall; use client::{Client, UserStore}; use gpui::{ - div, rems, AppContext, Component, Div, InteractiveComponent, Model, ParentComponent, Render, - Stateful, StatefulInteractiveComponent, Styled, Subscription, ViewContext, VisualContext, - WeakView, WindowBounds, + div, px, rems, AppContext, Component, Div, InteractiveComponent, Model, ParentComponent, + Render, Stateful, StatefulInteractiveComponent, Styled, Subscription, ViewContext, + VisualContext, WeakView, WindowBounds, }; use project::Project; use theme::ActiveTheme; @@ -88,12 +88,17 @@ impl Render for CollabTitlebarItem { h_stack() .id("titlebar") .justify_between() + .w_full() + .h(rems(1.75)) + // Set a non-scaling min-height here to ensure the titlebar is + // always at least the height of the traffic lights. + .min_h(px(32.)) .when( !matches!(cx.window_bounds(), WindowBounds::Fullscreen), - |s| s.pl_20(), + // Use pixels here instead of a rem-based size because the macOS traffic + // lights are a static size, and don't scale with the rest of the UI. + |s| s.pl(px(68.)), ) - .w_full() - .h(rems(1.75)) .bg(cx.theme().colors().title_bar_background) .on_click(|_, event, cx| { if event.up.click_count == 2 { @@ -102,6 +107,7 @@ impl Render for CollabTitlebarItem { }) .child( h_stack() + .gap_1() // TODO - Add player menu .child( div() From 3d8e63b93bef7c0e79d0e389de81d70ddff1849c Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 16:09:11 -0500 Subject: [PATCH 10/24] Buttons should always use `cursor_pointer` --- crates/ui2/src/components/button.rs | 1 + crates/ui2/src/components/icon_button.rs | 11 +++++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/ui2/src/components/button.rs b/crates/ui2/src/components/button.rs index 397ce4f4c48314339e340f717ebccddda63453e4..de055bcd5c3f185a7c049cb58eaa64f523fec9e5 100644 --- a/crates/ui2/src/components/button.rs +++ b/crates/ui2/src/components/button.rs @@ -178,6 +178,7 @@ impl Button { .text_ui() .rounded_md() .bg(self.variant.bg_color(cx)) + .cursor_pointer() .hover(|style| style.bg(self.variant.bg_color_hover(cx))) .active(|style| style.bg(self.variant.bg_color_active(cx))); diff --git a/crates/ui2/src/components/icon_button.rs b/crates/ui2/src/components/icon_button.rs index 4408c51f62128cbe27b76dcd28f976b225abedda..5dd77711610527d0fa3c42ea232a1283a10a6b58 100644 --- a/crates/ui2/src/components/icon_button.rs +++ b/crates/ui2/src/components/icon_button.rs @@ -95,17 +95,16 @@ impl IconButton { .rounded_md() .p_1() .bg(bg_color) + .cursor_pointer() .hover(|style| style.bg(bg_hover_color)) .active(|style| style.bg(bg_active_color)) .child(IconElement::new(self.icon).color(icon_color)); if let Some(click_handler) = self.handlers.click.clone() { - button = button - .on_mouse_down(MouseButton::Left, move |state, event, cx| { - cx.stop_propagation(); - click_handler(state, cx); - }) - .cursor_pointer(); + button = button.on_mouse_down(MouseButton::Left, move |state, event, cx| { + cx.stop_propagation(); + click_handler(state, cx); + }) } if let Some(tooltip) = self.tooltip.take() { From 9c5f580012820686c6789e64392760ac1ffb7c72 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 16:17:10 -0500 Subject: [PATCH 11/24] Use `Selected` for active IconButtons --- crates/ui2/src/components/icon_button.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui2/src/components/icon_button.rs b/crates/ui2/src/components/icon_button.rs index 5dd77711610527d0fa3c42ea232a1283a10a6b58..5512da2b34638e25e6450fd92f59b4a8d60c23f6 100644 --- a/crates/ui2/src/components/icon_button.rs +++ b/crates/ui2/src/components/icon_button.rs @@ -72,7 +72,7 @@ impl IconButton { fn render(mut self, _view: &mut V, cx: &mut ViewContext) -> impl Component { let icon_color = match (self.state, self.color) { (InteractionState::Disabled, _) => TextColor::Disabled, - (InteractionState::Active, _) => TextColor::Error, + (InteractionState::Active, _) => TextColor::Selected, _ => self.color, }; From 3223e21d9fcf12d8b22ae63c3d561a74177a4d5b Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Thu, 16 Nov 2023 16:17:17 -0500 Subject: [PATCH 12/24] Add dock borders --- crates/workspace2/src/dock.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/workspace2/src/dock.rs b/crates/workspace2/src/dock.rs index 8e7f08252c80d8d69a3349dbe21daf2be18eb780..ee45ca862c96a4ca034645748ca4ac730c40223e 100644 --- a/crates/workspace2/src/dock.rs +++ b/crates/workspace2/src/dock.rs @@ -7,6 +7,7 @@ use gpui::{ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::sync::Arc; +use theme2::ActiveTheme; use ui::{h_stack, IconButton, InteractionState, Tooltip}; pub enum PanelEvent { @@ -443,10 +444,16 @@ impl Render for Dock { let size = entry.panel.size(cx); div() + .border_color(cx.theme().colors().border) .map(|this| match self.position().axis() { Axis::Horizontal => this.w(px(size)).h_full(), Axis::Vertical => this.h(px(size)).w_full(), }) + .map(|this| match self.position() { + DockPosition::Left => this.border_r(), + DockPosition::Right => this.border_l(), + DockPosition::Bottom => this.border_t(), + }) .child(entry.panel.to_any()) } else { div() @@ -675,7 +682,7 @@ impl Render for PanelButtons { Some(button) }); - h_stack().children(buttons) + h_stack().gap_0p5().children(buttons) } } From 6397c05835886abbdc7ac47ce655cecbae8483f7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 14:09:01 -0800 Subject: [PATCH 13/24] Add the ability to deprioritize specific labeled tasks in tests --- crates/gpui2/src/executor.rs | 64 ++++++++++++---- crates/gpui2/src/platform.rs | 4 +- crates/gpui2/src/platform/mac/dispatcher.rs | 4 +- crates/gpui2/src/platform/test/dispatcher.rs | 79 +++++++++++++------- 4 files changed, 108 insertions(+), 43 deletions(-) diff --git a/crates/gpui2/src/executor.rs b/crates/gpui2/src/executor.rs index bb9b5d0d79dbee945ba508267314684eb98a613c..b29fbbb5a192ea8a2081571bb2008f295c8cf06f 100644 --- a/crates/gpui2/src/executor.rs +++ b/crates/gpui2/src/executor.rs @@ -5,10 +5,11 @@ use std::{ fmt::Debug, marker::PhantomData, mem, + num::NonZeroUsize, pin::Pin, rc::Rc, sync::{ - atomic::{AtomicBool, Ordering::SeqCst}, + atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst}, Arc, }, task::{Context, Poll}, @@ -71,30 +72,57 @@ impl Future for Task { } } } + +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +pub struct TaskLabel(NonZeroUsize); + +impl TaskLabel { + pub fn new() -> Self { + static NEXT_TASK_LABEL: AtomicUsize = AtomicUsize::new(1); + Self(NEXT_TASK_LABEL.fetch_add(1, SeqCst).try_into().unwrap()) + } +} + type AnyLocalFuture = Pin>>; + type AnyFuture = Pin>>; + impl BackgroundExecutor { pub fn new(dispatcher: Arc) -> Self { Self { dispatcher } } - /// Enqueues the given closure to be run on any thread. The closure returns - /// a future which will be run to completion on any available thread. + /// Enqueues the given future to be run to completion on a background thread. pub fn spawn(&self, future: impl Future + Send + 'static) -> Task where R: Send + 'static, { + self.spawn_internal::(Box::pin(future), None) + } + + /// Enqueues the given future to be run to completion on a background thread. + /// The given label can be used to control the priority of the task in tests. + pub fn spawn_labeled( + &self, + label: TaskLabel, + future: impl Future + Send + 'static, + ) -> Task + where + R: Send + 'static, + { + self.spawn_internal::(Box::pin(future), Some(label)) + } + + fn spawn_internal( + &self, + future: AnyFuture, + label: Option, + ) -> Task { let dispatcher = self.dispatcher.clone(); - fn inner( - dispatcher: Arc, - future: AnyFuture, - ) -> Task { - let (runnable, task) = - async_task::spawn(future, move |runnable| dispatcher.dispatch(runnable)); - runnable.schedule(); - Task::Spawned(task) - } - inner::(dispatcher, Box::pin(future)) + let (runnable, task) = + async_task::spawn(future, move |runnable| dispatcher.dispatch(runnable, label)); + runnable.schedule(); + Task::Spawned(task) } #[cfg(any(test, feature = "test-support"))] @@ -216,11 +244,21 @@ impl BackgroundExecutor { self.dispatcher.as_test().unwrap().simulate_random_delay() } + #[cfg(any(test, feature = "test-support"))] + pub fn deprioritize_task(&self, task_label: TaskLabel) { + self.dispatcher.as_test().unwrap().deprioritize(task_label) + } + #[cfg(any(test, feature = "test-support"))] pub fn advance_clock(&self, duration: Duration) { self.dispatcher.as_test().unwrap().advance_clock(duration) } + #[cfg(any(test, feature = "test-support"))] + pub fn run_step(&self) -> bool { + self.dispatcher.as_test().unwrap().poll(false) + } + #[cfg(any(test, feature = "test-support"))] pub fn run_until_parked(&self) { self.dispatcher.as_test().unwrap().run_until_parked() diff --git a/crates/gpui2/src/platform.rs b/crates/gpui2/src/platform.rs index 00ce3340f82b6466990a0bc75cfdea175bad4edb..882dc332ef91aa9cd1b0460e5e2ff5e126328455 100644 --- a/crates/gpui2/src/platform.rs +++ b/crates/gpui2/src/platform.rs @@ -8,7 +8,7 @@ use crate::{ point, size, AnyWindowHandle, BackgroundExecutor, Bounds, DevicePixels, Font, FontId, FontMetrics, FontRun, ForegroundExecutor, GlobalPixels, GlyphId, InputEvent, LineLayout, Pixels, Point, RenderGlyphParams, RenderImageParams, RenderSvgParams, Result, Scene, - SharedString, Size, + SharedString, Size, TaskLabel, }; use anyhow::{anyhow, bail}; use async_task::Runnable; @@ -162,7 +162,7 @@ pub(crate) trait PlatformWindow { pub trait PlatformDispatcher: Send + Sync { fn is_main_thread(&self) -> bool; - fn dispatch(&self, runnable: Runnable); + fn dispatch(&self, runnable: Runnable, label: Option); fn dispatch_on_main_thread(&self, runnable: Runnable); fn dispatch_after(&self, duration: Duration, runnable: Runnable); fn poll(&self, background_only: bool) -> bool; diff --git a/crates/gpui2/src/platform/mac/dispatcher.rs b/crates/gpui2/src/platform/mac/dispatcher.rs index 68c0e3b4f53c4040899ac0d344dc1dec095a9bc1..1752f78601f6c7f785207d8ada49181ee54f9545 100644 --- a/crates/gpui2/src/platform/mac/dispatcher.rs +++ b/crates/gpui2/src/platform/mac/dispatcher.rs @@ -2,7 +2,7 @@ #![allow(non_camel_case_types)] #![allow(non_snake_case)] -use crate::PlatformDispatcher; +use crate::{PlatformDispatcher, TaskLabel}; use async_task::Runnable; use objc::{ class, msg_send, @@ -37,7 +37,7 @@ impl PlatformDispatcher for MacDispatcher { is_main_thread == YES } - fn dispatch(&self, runnable: Runnable) { + fn dispatch(&self, runnable: Runnable, _: Option) { unsafe { dispatch_async_f( dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT.try_into().unwrap(), 0), diff --git a/crates/gpui2/src/platform/test/dispatcher.rs b/crates/gpui2/src/platform/test/dispatcher.rs index 258c484063c534afca163ac3cec99ef3d00a43a9..3abe4796b3ff985178e609e05f503fbde563adf0 100644 --- a/crates/gpui2/src/platform/test/dispatcher.rs +++ b/crates/gpui2/src/platform/test/dispatcher.rs @@ -1,7 +1,7 @@ -use crate::PlatformDispatcher; +use crate::{PlatformDispatcher, TaskLabel}; use async_task::Runnable; use backtrace::Backtrace; -use collections::{HashMap, VecDeque}; +use collections::{HashMap, HashSet, VecDeque}; use parking::{Parker, Unparker}; use parking_lot::Mutex; use rand::prelude::*; @@ -28,12 +28,14 @@ struct TestDispatcherState { random: StdRng, foreground: HashMap>, background: Vec, + deprioritized_background: Vec, delayed: Vec<(Duration, Runnable)>, time: Duration, is_main_thread: bool, next_id: TestDispatcherId, allow_parking: bool, waiting_backtrace: Option, + deprioritized_task_labels: HashSet, } impl TestDispatcher { @@ -43,12 +45,14 @@ impl TestDispatcher { random, foreground: HashMap::default(), background: Vec::new(), + deprioritized_background: Vec::new(), delayed: Vec::new(), time: Duration::ZERO, is_main_thread: true, next_id: TestDispatcherId(1), allow_parking: false, waiting_backtrace: None, + deprioritized_task_labels: Default::default(), }; TestDispatcher { @@ -101,6 +105,13 @@ impl TestDispatcher { } } + pub fn deprioritize(&self, task_label: TaskLabel) { + self.state + .lock() + .deprioritized_task_labels + .insert(task_label); + } + pub fn run_until_parked(&self) { while self.poll(false) {} } @@ -150,8 +161,17 @@ impl PlatformDispatcher for TestDispatcher { self.state.lock().is_main_thread } - fn dispatch(&self, runnable: Runnable) { - self.state.lock().background.push(runnable); + fn dispatch(&self, runnable: Runnable, label: Option) { + { + let mut state = self.state.lock(); + if label.map_or(false, |label| { + state.deprioritized_task_labels.contains(&label) + }) { + state.deprioritized_background.push(runnable); + } else { + state.background.push(runnable); + } + } self.unparker.unpark(); } @@ -196,34 +216,41 @@ impl PlatformDispatcher for TestDispatcher { }; let background_len = state.background.len(); + let runnable; + let main_thread; if foreground_len == 0 && background_len == 0 { - return false; - } - - let main_thread = state.random.gen_ratio( - foreground_len as u32, - (foreground_len + background_len) as u32, - ); - let was_main_thread = state.is_main_thread; - state.is_main_thread = main_thread; - - let runnable = if main_thread { - let state = &mut *state; - let runnables = state - .foreground - .values_mut() - .filter(|runnables| !runnables.is_empty()) - .choose(&mut state.random) - .unwrap(); - runnables.pop_front().unwrap() + let deprioritized_background_len = state.deprioritized_background.len(); + if deprioritized_background_len == 0 { + return false; + } + let ix = state.random.gen_range(0..deprioritized_background_len); + main_thread = false; + runnable = state.deprioritized_background.swap_remove(ix); } else { - let ix = state.random.gen_range(0..background_len); - state.background.swap_remove(ix) + main_thread = state.random.gen_ratio( + foreground_len as u32, + (foreground_len + background_len) as u32, + ); + if main_thread { + let state = &mut *state; + runnable = state + .foreground + .values_mut() + .filter(|runnables| !runnables.is_empty()) + .choose(&mut state.random) + .unwrap() + .pop_front() + .unwrap(); + } else { + let ix = state.random.gen_range(0..background_len); + runnable = state.background.swap_remove(ix); + }; }; + let was_main_thread = state.is_main_thread; + state.is_main_thread = main_thread; drop(state); runnable.run(); - self.state.lock().is_main_thread = was_main_thread; true From f9650b3111f7a1ae071fd14adc4bc9554cf0545b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 14:09:33 -0800 Subject: [PATCH 14/24] Don't run until all the way until parked when waiting for a model's next event --- crates/gpui2/src/app/test_context.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/gpui2/src/app/test_context.rs b/crates/gpui2/src/app/test_context.rs index 5397a2214d4324ba31816d332072a120f1c75b03..37f81da15b9fd92882d58fede5e755ce65f3c1e5 100644 --- a/crates/gpui2/src/app/test_context.rs +++ b/crates/gpui2/src/app/test_context.rs @@ -354,10 +354,18 @@ impl Model { }) }); - cx.executor().run_until_parked(); - rx.try_next() - .expect("no event received") - .expect("model was dropped") + loop { + match rx.try_next() { + Ok(Some(event)) => return event, + Ok(None) => panic!("model was dropped"), + Err(_) => { + if !cx.executor().run_step() { + break; + } + } + } + } + panic!("no event received") } } From b2451d9dd688841e848f15a84aa1107fcc39e728 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 14:44:32 -0800 Subject: [PATCH 15/24] Combine adjacent edits in buffer's diff --- crates/language2/src/buffer.rs | 61 ++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/crates/language2/src/buffer.rs b/crates/language2/src/buffer.rs index 2c8c55d5776d1970e4597f86372aef9d65627c16..d3f005278f1fd5c4f2fbebe60df0557b52706687 100644 --- a/crates/language2/src/buffer.rs +++ b/crates/language2/src/buffer.rs @@ -1122,26 +1122,59 @@ impl Buffer { let old_text = old_text.to_string(); let line_ending = LineEnding::detect(&new_text); LineEnding::normalize(&mut new_text); + let diff = TextDiff::from_chars(old_text.as_str(), new_text.as_str()); - let mut edits = Vec::new(); - let mut offset = 0; let empty: Arc = "".into(); - for change in diff.iter_all_changes() { - let value = change.value(); - let end_offset = offset + value.len(); - match change.tag() { - ChangeTag::Equal => { - offset = end_offset; - } - ChangeTag::Delete => { - edits.push((offset..end_offset, empty.clone())); - offset = end_offset; + + let mut edits = Vec::new(); + let mut old_offset = 0; + let mut new_offset = 0; + let mut last_edit: Option<(Range, Range)> = None; + for change in diff.iter_all_changes().map(Some).chain([None]) { + if let Some(change) = &change { + let len = change.value().len(); + match change.tag() { + ChangeTag::Equal => { + old_offset += len; + new_offset += len; + } + ChangeTag::Delete => { + let old_end_offset = old_offset + len; + if let Some((last_old_range, _)) = &mut last_edit { + last_old_range.end = old_end_offset; + } else { + last_edit = + Some((old_offset..old_end_offset, new_offset..new_offset)); + } + old_offset = old_end_offset; + } + ChangeTag::Insert => { + let new_end_offset = new_offset + len; + if let Some((_, last_new_range)) = &mut last_edit { + last_new_range.end = new_end_offset; + } else { + last_edit = + Some((old_offset..old_offset, new_offset..new_end_offset)); + } + new_offset = new_end_offset; + } } - ChangeTag::Insert => { - edits.push((offset..offset, value.into())); + } + + if let Some((old_range, new_range)) = &last_edit { + if old_offset > old_range.end || new_offset > new_range.end || change.is_none() + { + let text = if new_range.is_empty() { + empty.clone() + } else { + new_text[new_range.clone()].into() + }; + edits.push((old_range.clone(), text)); + last_edit.take(); } } } + Diff { base_version, line_ending, From 89d73f713ad62b7645880536c841afe2e1b9c3d9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 15:51:13 -0800 Subject: [PATCH 16/24] Label the buffer's diff task so it can be deprioritized in tests --- crates/language2/src/buffer.rs | 120 ++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 56 deletions(-) diff --git a/crates/language2/src/buffer.rs b/crates/language2/src/buffer.rs index d3f005278f1fd5c4f2fbebe60df0557b52706687..739fe8508dac57f01459abe2a9b022729862fb66 100644 --- a/crates/language2/src/buffer.rs +++ b/crates/language2/src/buffer.rs @@ -17,7 +17,8 @@ use crate::{ use anyhow::{anyhow, Result}; pub use clock::ReplicaId; use futures::FutureExt as _; -use gpui::{AppContext, EventEmitter, HighlightStyle, ModelContext, Task}; +use gpui::{AppContext, EventEmitter, HighlightStyle, ModelContext, Task, TaskLabel}; +use lazy_static::lazy_static; use lsp::LanguageServerId; use parking_lot::Mutex; use similar::{ChangeTag, TextDiff}; @@ -51,6 +52,10 @@ pub use {tree_sitter_rust, tree_sitter_typescript}; pub use lsp::DiagnosticSeverity; +lazy_static! { + pub static ref BUFFER_DIFF_TASK: TaskLabel = TaskLabel::new(); +} + pub struct Buffer { text: TextBuffer, diff_base: Option, @@ -1118,69 +1123,72 @@ impl Buffer { pub fn diff(&self, mut new_text: String, cx: &AppContext) -> Task { let old_text = self.as_rope().clone(); let base_version = self.version(); - cx.background_executor().spawn(async move { - let old_text = old_text.to_string(); - let line_ending = LineEnding::detect(&new_text); - LineEnding::normalize(&mut new_text); - - let diff = TextDiff::from_chars(old_text.as_str(), new_text.as_str()); - let empty: Arc = "".into(); - - let mut edits = Vec::new(); - let mut old_offset = 0; - let mut new_offset = 0; - let mut last_edit: Option<(Range, Range)> = None; - for change in diff.iter_all_changes().map(Some).chain([None]) { - if let Some(change) = &change { - let len = change.value().len(); - match change.tag() { - ChangeTag::Equal => { - old_offset += len; - new_offset += len; - } - ChangeTag::Delete => { - let old_end_offset = old_offset + len; - if let Some((last_old_range, _)) = &mut last_edit { - last_old_range.end = old_end_offset; - } else { - last_edit = - Some((old_offset..old_end_offset, new_offset..new_offset)); + cx.background_executor() + .spawn_labeled(*BUFFER_DIFF_TASK, async move { + let old_text = old_text.to_string(); + let line_ending = LineEnding::detect(&new_text); + LineEnding::normalize(&mut new_text); + + let diff = TextDiff::from_chars(old_text.as_str(), new_text.as_str()); + let empty: Arc = "".into(); + + let mut edits = Vec::new(); + let mut old_offset = 0; + let mut new_offset = 0; + let mut last_edit: Option<(Range, Range)> = None; + for change in diff.iter_all_changes().map(Some).chain([None]) { + if let Some(change) = &change { + let len = change.value().len(); + match change.tag() { + ChangeTag::Equal => { + old_offset += len; + new_offset += len; } - old_offset = old_end_offset; - } - ChangeTag::Insert => { - let new_end_offset = new_offset + len; - if let Some((_, last_new_range)) = &mut last_edit { - last_new_range.end = new_end_offset; - } else { - last_edit = - Some((old_offset..old_offset, new_offset..new_end_offset)); + ChangeTag::Delete => { + let old_end_offset = old_offset + len; + if let Some((last_old_range, _)) = &mut last_edit { + last_old_range.end = old_end_offset; + } else { + last_edit = + Some((old_offset..old_end_offset, new_offset..new_offset)); + } + old_offset = old_end_offset; + } + ChangeTag::Insert => { + let new_end_offset = new_offset + len; + if let Some((_, last_new_range)) = &mut last_edit { + last_new_range.end = new_end_offset; + } else { + last_edit = + Some((old_offset..old_offset, new_offset..new_end_offset)); + } + new_offset = new_end_offset; } - new_offset = new_end_offset; } } - } - if let Some((old_range, new_range)) = &last_edit { - if old_offset > old_range.end || new_offset > new_range.end || change.is_none() - { - let text = if new_range.is_empty() { - empty.clone() - } else { - new_text[new_range.clone()].into() - }; - edits.push((old_range.clone(), text)); - last_edit.take(); + if let Some((old_range, new_range)) = &last_edit { + if old_offset > old_range.end + || new_offset > new_range.end + || change.is_none() + { + let text = if new_range.is_empty() { + empty.clone() + } else { + new_text[new_range.clone()].into() + }; + edits.push((old_range.clone(), text)); + last_edit.take(); + } } } - } - Diff { - base_version, - line_ending, - edits, - } - }) + Diff { + base_version, + line_ending, + edits, + } + }) } /// Spawn a background task that searches the buffer for any whitespace From 5f1acae0d323cd7a17cc6f5f322d230e6c2f1d79 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 15:52:24 -0800 Subject: [PATCH 17/24] Fix race conditions and bugs in Buffer::reload --- crates/language2/src/buffer.rs | 90 +++++++++++++++------------- crates/project2/src/project2.rs | 4 +- crates/project2/src/project_tests.rs | 64 ++++++++++++++++++++ crates/project2/src/worktree.rs | 3 +- crates/rope2/src/rope2.rs | 6 +- 5 files changed, 123 insertions(+), 44 deletions(-) diff --git a/crates/language2/src/buffer.rs b/crates/language2/src/buffer.rs index 739fe8508dac57f01459abe2a9b022729862fb66..e191f408e7f37e616fc2293e0d60e805db82a1e4 100644 --- a/crates/language2/src/buffer.rs +++ b/crates/language2/src/buffer.rs @@ -16,7 +16,7 @@ use crate::{ }; use anyhow::{anyhow, Result}; pub use clock::ReplicaId; -use futures::FutureExt as _; +use futures::channel::oneshot; use gpui::{AppContext, EventEmitter, HighlightStyle, ModelContext, Task, TaskLabel}; use lazy_static::lazy_static; use lsp::LanguageServerId; @@ -45,7 +45,7 @@ pub use text::{Buffer as TextBuffer, BufferSnapshot as TextBufferSnapshot, *}; use theme::SyntaxTheme; #[cfg(any(test, feature = "test-support"))] use util::RandomCharIter; -use util::{RangeExt, TryFutureExt as _}; +use util::RangeExt; #[cfg(any(test, feature = "test-support"))] pub use {tree_sitter_rust, tree_sitter_typescript}; @@ -66,6 +66,7 @@ pub struct Buffer { saved_mtime: SystemTime, transaction_depth: usize, was_dirty_before_starting_transaction: Option, + reload_task: Option>>, language: Option>, autoindent_requests: Vec>, pending_autoindent: Option>, @@ -473,6 +474,7 @@ impl Buffer { saved_mtime, saved_version: buffer.version(), saved_version_fingerprint: buffer.as_rope().fingerprint(), + reload_task: None, transaction_depth: 0, was_dirty_before_starting_transaction: None, text: buffer, @@ -572,37 +574,52 @@ impl Buffer { cx.notify(); } - pub fn reload(&mut self, cx: &mut ModelContext) -> Task>> { - cx.spawn(|this, mut cx| async move { - if let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| { + pub fn reload( + &mut self, + cx: &mut ModelContext, + ) -> oneshot::Receiver> { + let (tx, rx) = futures::channel::oneshot::channel(); + let prev_version = self.text.version(); + self.reload_task = Some(cx.spawn(|this, mut cx| async move { + let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| { let file = this.file.as_ref()?.as_local()?; Some((file.mtime(), file.load(cx))) - })? { - let new_text = new_text.await?; - let diff = this - .update(&mut cx, |this, cx| this.diff(new_text, cx))? - .await; - this.update(&mut cx, |this, cx| { - if this.version() == diff.base_version { - this.finalize_last_transaction(); - this.apply_diff(diff, cx); - if let Some(transaction) = this.finalize_last_transaction().cloned() { - this.did_reload( - this.version(), - this.as_rope().fingerprint(), - this.line_ending(), - new_mtime, - cx, - ); - return Some(transaction); - } - } - None - }) - } else { - Ok(None) - } - }) + })? + else { + return Ok(()); + }; + + let new_text = new_text.await?; + let diff = this + .update(&mut cx, |this, cx| this.diff(new_text.clone(), cx))? + .await; + this.update(&mut cx, |this, cx| { + if this.version() == diff.base_version { + this.finalize_last_transaction(); + this.apply_diff(diff, cx); + tx.send(this.finalize_last_transaction().cloned()).ok(); + + this.did_reload( + this.version(), + this.as_rope().fingerprint(), + this.line_ending(), + new_mtime, + cx, + ); + } else { + this.did_reload( + prev_version, + Rope::text_fingerprint(&new_text), + this.line_ending(), + new_mtime, + cx, + ); + } + + this.reload_task.take(); + }) + })); + rx } pub fn did_reload( @@ -631,13 +648,8 @@ impl Buffer { cx.notify(); } - pub fn file_updated( - &mut self, - new_file: Arc, - cx: &mut ModelContext, - ) -> Task<()> { + pub fn file_updated(&mut self, new_file: Arc, cx: &mut ModelContext) { let mut file_changed = false; - let mut task = Task::ready(()); if let Some(old_file) = self.file.as_ref() { if new_file.path() != old_file.path() { @@ -657,8 +669,7 @@ impl Buffer { file_changed = true; if !self.is_dirty() { - let reload = self.reload(cx).log_err().map(drop); - task = cx.background_executor().spawn(reload); + self.reload(cx).close(); } } } @@ -672,7 +683,6 @@ impl Buffer { cx.emit(Event::FileHandleChanged); cx.notify(); } - task } pub fn diff_base(&self) -> Option<&str> { diff --git a/crates/project2/src/project2.rs b/crates/project2/src/project2.rs index 61ad500a73413d795bc9fcf7bcc38c5355aba07c..f2e47b71842c0ec3aedd94c20b57bd90123149ca 100644 --- a/crates/project2/src/project2.rs +++ b/crates/project2/src/project2.rs @@ -6262,7 +6262,7 @@ impl Project { .log_err(); } - buffer.file_updated(Arc::new(new_file), cx).detach(); + buffer.file_updated(Arc::new(new_file), cx); } } }); @@ -7256,7 +7256,7 @@ impl Project { .ok_or_else(|| anyhow!("no such worktree"))?; let file = File::from_proto(file, worktree, cx)?; buffer.update(cx, |buffer, cx| { - buffer.file_updated(Arc::new(file), cx).detach(); + buffer.file_updated(Arc::new(file), cx); }); this.detect_language_for_buffer(&buffer, cx); } diff --git a/crates/project2/src/project_tests.rs b/crates/project2/src/project_tests.rs index 97b6ed9c7429be0640aff708447a017ed86f30ef..4c5905ff7e98be4d8046d365759d59900c82f448 100644 --- a/crates/project2/src/project_tests.rs +++ b/crates/project2/src/project_tests.rs @@ -2587,6 +2587,70 @@ async fn test_save_file(cx: &mut gpui::TestAppContext) { assert_eq!(new_text, buffer.update(cx, |buffer, _| buffer.text())); } +#[gpui::test(iterations = 10)] +async fn test_file_changes_multiple_times_on_disk(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/dir", + json!({ + "file1": "the original contents", + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/dir".as_ref()], cx).await; + let worktree = project.read_with(cx, |project, _| project.worktrees().next().unwrap()); + let buffer = project + .update(cx, |p, cx| p.open_local_buffer("/dir/file1", cx)) + .await + .unwrap(); + + // Simulate buffer diffs being slow, so that they don't complete before + // the next file change occurs. + cx.executor().deprioritize_task(*language::BUFFER_DIFF_TASK); + + // Change the buffer's file on disk, and then wait for the file change + // to be detected by the worktree, so that the buffer starts reloading. + fs.save( + "/dir/file1".as_ref(), + &"the first contents".into(), + Default::default(), + ) + .await + .unwrap(); + worktree.next_event(cx); + + // Change the buffer's file again. Depending on the random seed, the + // previous file change may still be in progress. + fs.save( + "/dir/file1".as_ref(), + &"the second contents".into(), + Default::default(), + ) + .await + .unwrap(); + worktree.next_event(cx); + + cx.executor().run_until_parked(); + let on_disk_text = fs.load(Path::new("/dir/file1")).await.unwrap(); + buffer.read_with(cx, |buffer, _| { + let buffer_text = buffer.text(); + if buffer_text == on_disk_text { + assert!(!buffer.is_dirty(), "buffer shouldn't be dirty. text: {buffer_text:?}, disk text: {on_disk_text:?}"); + } + // If the file change occurred while the buffer was processing the first + // change, the buffer may be in a conflicting state. + else { + assert!( + buffer.is_dirty(), + "buffer should report that it is dirty. text: {buffer_text:?}, disk text: {on_disk_text:?}" + ); + } + }); +} + #[gpui::test] async fn test_save_in_single_file_worktree(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/crates/project2/src/worktree.rs b/crates/project2/src/worktree.rs index 9444dd9185440e7fbc953ac4a96dfe4e89e8c50f..a020e8db4c311a78404169b5a31a23db8563b570 100644 --- a/crates/project2/src/worktree.rs +++ b/crates/project2/src/worktree.rs @@ -276,6 +276,7 @@ struct ShareState { _maintain_remote_snapshot: Task>, } +#[derive(Clone)] pub enum Event { UpdatedEntries(UpdatedEntriesSet), UpdatedGitRepositories(UpdatedGitRepositoriesSet), @@ -961,7 +962,7 @@ impl LocalWorktree { buffer_handle.update(&mut cx, |buffer, cx| { if has_changed_file { - buffer.file_updated(new_file, cx).detach(); + buffer.file_updated(new_file, cx); } })?; } diff --git a/crates/rope2/src/rope2.rs b/crates/rope2/src/rope2.rs index 9c764c468e493013935bf9aa6552065e3e145b62..4cea1d4759032f710c92a4dcaea98a3bf4f488b9 100644 --- a/crates/rope2/src/rope2.rs +++ b/crates/rope2/src/rope2.rs @@ -41,6 +41,10 @@ impl Rope { Self::default() } + pub fn text_fingerprint(text: &str) -> RopeFingerprint { + bromberg_sl2::hash_strict(text.as_bytes()) + } + pub fn append(&mut self, rope: Rope) { let mut chunks = rope.chunks.cursor::<()>(); chunks.next(&()); @@ -931,7 +935,7 @@ impl<'a> From<&'a str> for ChunkSummary { fn from(text: &'a str) -> Self { Self { text: TextSummary::from(text), - fingerprint: bromberg_sl2::hash_strict(text.as_bytes()), + fingerprint: Rope::text_fingerprint(text), } } } From 0bed5e4562b778ba129424c2d4b7e938cc332a9c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 16:01:49 -0800 Subject: [PATCH 18/24] Port buffer reload bug fixes back to gpui1 crates --- crates/copilot/src/copilot.rs | 20 ++++---- crates/language/src/buffer.rs | 90 +++++++++++++++++++--------------- crates/project/src/project.rs | 4 +- crates/project/src/worktree.rs | 2 +- crates/rope/src/rope.rs | 6 ++- 5 files changed, 67 insertions(+), 55 deletions(-) diff --git a/crates/copilot/src/copilot.rs b/crates/copilot/src/copilot.rs index 3b383c2ac90bb9ede84a8bcf27a4e6a4d2626dfc..92d430e3fb06699ff86fba1f7c4e6de8eb222182 100644 --- a/crates/copilot/src/copilot.rs +++ b/crates/copilot/src/copilot.rs @@ -1051,17 +1051,15 @@ mod tests { ); // Ensure updates to the file are reflected in the LSP. - buffer_1 - .update(cx, |buffer, cx| { - buffer.file_updated( - Arc::new(File { - abs_path: "/root/child/buffer-1".into(), - path: Path::new("child/buffer-1").into(), - }), - cx, - ) - }) - .await; + buffer_1.update(cx, |buffer, cx| { + buffer.file_updated( + Arc::new(File { + abs_path: "/root/child/buffer-1".into(), + path: Path::new("child/buffer-1").into(), + }), + cx, + ) + }); assert_eq!( lsp.receive_notification::() .await, diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 0194123bd2359b3a71fb48c527ff1da295613d79..6d5684e6d7a9f9196c9257f9fd98905a398c7ac6 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -17,7 +17,7 @@ use crate::{ }; use anyhow::{anyhow, Result}; pub use clock::ReplicaId; -use futures::FutureExt as _; +use futures::channel::oneshot; use gpui::{fonts::HighlightStyle, AppContext, Entity, ModelContext, Task}; use lsp::LanguageServerId; use parking_lot::Mutex; @@ -45,7 +45,7 @@ pub use text::{Buffer as TextBuffer, BufferSnapshot as TextBufferSnapshot, *}; use theme::SyntaxTheme; #[cfg(any(test, feature = "test-support"))] use util::RandomCharIter; -use util::{RangeExt, TryFutureExt as _}; +use util::RangeExt; #[cfg(any(test, feature = "test-support"))] pub use {tree_sitter_rust, tree_sitter_typescript}; @@ -62,6 +62,7 @@ pub struct Buffer { saved_mtime: SystemTime, transaction_depth: usize, was_dirty_before_starting_transaction: Option, + reload_task: Option>>, language: Option>, autoindent_requests: Vec>, pending_autoindent: Option>, @@ -509,6 +510,7 @@ impl Buffer { saved_mtime, saved_version: buffer.version(), saved_version_fingerprint: buffer.as_rope().fingerprint(), + reload_task: None, transaction_depth: 0, was_dirty_before_starting_transaction: None, text: buffer, @@ -608,37 +610,52 @@ impl Buffer { cx.notify(); } - pub fn reload(&mut self, cx: &mut ModelContext) -> Task>> { - cx.spawn(|this, mut cx| async move { - if let Some((new_mtime, new_text)) = this.read_with(&cx, |this, cx| { + pub fn reload( + &mut self, + cx: &mut ModelContext, + ) -> oneshot::Receiver> { + let (tx, rx) = futures::channel::oneshot::channel(); + let prev_version = self.text.version(); + self.reload_task = Some(cx.spawn(|this, mut cx| async move { + let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| { let file = this.file.as_ref()?.as_local()?; Some((file.mtime(), file.load(cx))) - }) { - let new_text = new_text.await?; - let diff = this - .read_with(&cx, |this, cx| this.diff(new_text, cx)) - .await; - this.update(&mut cx, |this, cx| { - if this.version() == diff.base_version { - this.finalize_last_transaction(); - this.apply_diff(diff, cx); - if let Some(transaction) = this.finalize_last_transaction().cloned() { - this.did_reload( - this.version(), - this.as_rope().fingerprint(), - this.line_ending(), - new_mtime, - cx, - ); - return Ok(Some(transaction)); - } - } - Ok(None) - }) - } else { - Ok(None) - } - }) + }) else { + return Ok(()); + }; + + let new_text = new_text.await?; + let diff = this + .update(&mut cx, |this, cx| this.diff(new_text.clone(), cx)) + .await; + this.update(&mut cx, |this, cx| { + if this.version() == diff.base_version { + this.finalize_last_transaction(); + this.apply_diff(diff, cx); + tx.send(this.finalize_last_transaction().cloned()).ok(); + + this.did_reload( + this.version(), + this.as_rope().fingerprint(), + this.line_ending(), + new_mtime, + cx, + ); + } else { + this.did_reload( + prev_version, + Rope::text_fingerprint(&new_text), + this.line_ending(), + new_mtime, + cx, + ); + } + + this.reload_task.take(); + }); + Ok(()) + })); + rx } pub fn did_reload( @@ -667,13 +684,8 @@ impl Buffer { cx.notify(); } - pub fn file_updated( - &mut self, - new_file: Arc, - cx: &mut ModelContext, - ) -> Task<()> { + pub fn file_updated(&mut self, new_file: Arc, cx: &mut ModelContext) { let mut file_changed = false; - let mut task = Task::ready(()); if let Some(old_file) = self.file.as_ref() { if new_file.path() != old_file.path() { @@ -693,8 +705,7 @@ impl Buffer { file_changed = true; if !self.is_dirty() { - let reload = self.reload(cx).log_err().map(drop); - task = cx.foreground().spawn(reload); + self.reload(cx).close(); } } } @@ -708,7 +719,6 @@ impl Buffer { cx.emit(Event::FileHandleChanged); cx.notify(); } - task } pub fn diff_base(&self) -> Option<&str> { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 322b2ae894fbfa636643968a686eb572fddaee5b..ab6cbd88c07ec8721d3adc2431964f5c69668d99 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -6190,7 +6190,7 @@ impl Project { .log_err(); } - buffer.file_updated(Arc::new(new_file), cx).detach(); + buffer.file_updated(Arc::new(new_file), cx); } } }); @@ -7182,7 +7182,7 @@ impl Project { .ok_or_else(|| anyhow!("no such worktree"))?; let file = File::from_proto(file, worktree, cx)?; buffer.update(cx, |buffer, cx| { - buffer.file_updated(Arc::new(file), cx).detach(); + buffer.file_updated(Arc::new(file), cx); }); this.detect_language_for_buffer(&buffer, cx); } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 785ce58bb8d2b40266a25106deb15fe666d2c823..d59885225acbff208153370e7ed3ec14050661ef 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -959,7 +959,7 @@ impl LocalWorktree { buffer_handle.update(&mut cx, |buffer, cx| { if has_changed_file { - buffer.file_updated(new_file, cx).detach(); + buffer.file_updated(new_file, cx); } }); } diff --git a/crates/rope/src/rope.rs b/crates/rope/src/rope.rs index 9c764c468e493013935bf9aa6552065e3e145b62..4cea1d4759032f710c92a4dcaea98a3bf4f488b9 100644 --- a/crates/rope/src/rope.rs +++ b/crates/rope/src/rope.rs @@ -41,6 +41,10 @@ impl Rope { Self::default() } + pub fn text_fingerprint(text: &str) -> RopeFingerprint { + bromberg_sl2::hash_strict(text.as_bytes()) + } + pub fn append(&mut self, rope: Rope) { let mut chunks = rope.chunks.cursor::<()>(); chunks.next(&()); @@ -931,7 +935,7 @@ impl<'a> From<&'a str> for ChunkSummary { fn from(text: &'a str) -> Self { Self { text: TextSummary::from(text), - fingerprint: bromberg_sl2::hash_strict(text.as_bytes()), + fingerprint: Rope::text_fingerprint(text), } } } From 4de2c0f7ef5ce209f71c9f4afb71d2013d6c46d7 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Thu, 16 Nov 2023 17:32:02 -0800 Subject: [PATCH 19/24] Re-implement actions as derive macros instead of blanket impls --- crates/collab_ui2/src/collab_titlebar_item.rs | 6 +- crates/editor2/src/editor.rs | 28 +- crates/gpui2/src/action.rs | 262 +++++++----------- crates/gpui2/src/app.rs | 27 +- crates/gpui2/src/elements/div.rs | 10 +- crates/gpui2/src/gpui2.rs | 1 + crates/gpui2/src/key_dispatch.rs | 13 +- crates/gpui2/src/keymap/keymap.rs | 30 +- crates/gpui2/src/window.rs | 4 +- crates/gpui2/tests/action_macros.rs | 45 +++ crates/gpui2_macros/src/action.rs | 103 ++++--- crates/gpui2_macros/src/gpui2_macros.rs | 8 +- crates/gpui2_macros/src/register_action.rs | 55 +++- crates/live_kit_client2/examples/test_app.rs | 4 +- crates/settings2/src/keymap_file.rs | 4 +- crates/terminal_view2/src/terminal_view.rs | 8 +- crates/ui2/src/components/context_menu.rs | 5 +- crates/ui2/src/components/keybinding.rs | 5 +- crates/workspace2/src/pane.rs | 10 +- crates/workspace2/src/workspace2.rs | 13 +- crates/zed2/src/languages/json.rs | 2 +- crates/zed_actions2/src/lib.rs | 7 +- 22 files changed, 361 insertions(+), 289 deletions(-) create mode 100644 crates/gpui2/tests/action_macros.rs diff --git a/crates/collab_ui2/src/collab_titlebar_item.rs b/crates/collab_ui2/src/collab_titlebar_item.rs index c9d16c7a5de68a7004980c4abb1a9dd0f25a8279..a5a40c48b6a3d684462c9292b258586913386a2a 100644 --- a/crates/collab_ui2/src/collab_titlebar_item.rs +++ b/crates/collab_ui2/src/collab_titlebar_item.rs @@ -130,14 +130,12 @@ impl Render for CollabTitlebarItem { .color(Some(TextColor::Muted)), ) .tooltip(move |_, cx| { - // todo!() Replace with real action. - #[gpui::action] - struct NoAction {} cx.build_view(|_| { Tooltip::new("Recent Branches") .key_binding(KeyBinding::new(gpui::KeyBinding::new( "cmd-b", - NoAction {}, + // todo!() Replace with real action. + gpui::NoAction, None, ))) .meta("Only local branches shown") diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index 8e7bd5876f7d69c3254464ff9d32b31ad5877037..8f4b9ccf644d4f9f3db9320882b6b71a41b6cb08 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -39,7 +39,7 @@ use futures::FutureExt; use fuzzy::{StringMatch, StringMatchCandidate}; use git::diff_hunk_to_display; use gpui::{ - action, actions, div, point, prelude::*, px, relative, rems, size, uniform_list, AnyElement, + actions, div, point, prelude::*, px, relative, rems, size, uniform_list, Action, AnyElement, AppContext, AsyncWindowContext, BackgroundExecutor, Bounds, ClipboardItem, Component, Context, EventEmitter, FocusHandle, FocusableView, FontFeatures, FontStyle, FontWeight, HighlightStyle, Hsla, InputHandler, KeyContext, Model, MouseButton, ParentComponent, Pixels, Render, Styled, @@ -180,78 +180,78 @@ pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); // // .with_soft_wrap(true) // } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct SelectNext { #[serde(default)] pub replace_newest: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct SelectPrevious { #[serde(default)] pub replace_newest: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct SelectAllMatches { #[serde(default)] pub replace_newest: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct SelectToBeginningOfLine { #[serde(default)] stop_at_soft_wraps: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct MovePageUp { #[serde(default)] center_cursor: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct MovePageDown { #[serde(default)] center_cursor: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct SelectToEndOfLine { #[serde(default)] stop_at_soft_wraps: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct ToggleCodeActions { #[serde(default)] pub deployed_from_indicator: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct ConfirmCompletion { #[serde(default)] pub item_ix: Option, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct ConfirmCodeAction { #[serde(default)] pub item_ix: Option, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct ToggleComments { #[serde(default)] pub advance_downwards: bool, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct FoldAt { pub buffer_row: u32, } -#[action] +#[derive(PartialEq, Clone, Deserialize, Action)] pub struct UnfoldAt { pub buffer_row: u32, } diff --git a/crates/gpui2/src/action.rs b/crates/gpui2/src/action.rs index 0a5ea781bdf04ed930ecb2baa2970fc099e0fb0b..8656f31a5ecd14b158139d5d772a85ab9db49716 100644 --- a/crates/gpui2/src/action.rs +++ b/crates/gpui2/src/action.rs @@ -1,10 +1,9 @@ use crate::SharedString; use anyhow::{anyhow, Context, Result}; use collections::HashMap; -use lazy_static::lazy_static; -use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard}; -use serde::Deserialize; -use std::any::{type_name, Any, TypeId}; +pub use no_action::NoAction; +use serde_json::json; +use std::any::{Any, TypeId}; /// Actions are used to implement keyboard-driven UI. /// When you declare an action, you can bind keys to the action in the keymap and @@ -15,24 +14,16 @@ use std::any::{type_name, Any, TypeId}; /// ```rust /// actions!(MoveUp, MoveDown, MoveLeft, MoveRight, Newline); /// ``` -/// More complex data types can also be actions. If you annotate your type with the `#[action]` proc macro, -/// it will automatically +/// More complex data types can also be actions. If you annotate your type with the action derive macro +/// it will be implemented and registered automatically. /// ``` -/// #[action] +/// #[derive(Clone, PartialEq, serde_derive::Deserialize, Action)] /// pub struct SelectNext { /// pub replace_newest: bool, /// } /// -/// Any type A that satisfies the following bounds is automatically an action: -/// -/// ``` -/// A: for<'a> Deserialize<'a> + PartialEq + Clone + Default + std::fmt::Debug + 'static, -/// ``` -/// -/// The `#[action]` annotation will derive these implementations for your struct automatically. If you -/// want to control them manually, you can use the lower-level `#[register_action]` macro, which only -/// generates the code needed to register your action before `main`. Then you'll need to implement all -/// the traits manually. +/// If you want to control the behavior of the action trait manually, you can use the lower-level `#[register_action]` +/// macro, which only generates the code needed to register your action before `main`. /// /// ``` /// #[gpui::register_action] @@ -41,77 +32,29 @@ use std::any::{type_name, Any, TypeId}; /// pub content: SharedString, /// } /// -/// impl std::default::Default for Paste { -/// fn default() -> Self { -/// Self { -/// content: SharedString::from("🍝"), -/// } -/// } +/// impl gpui::Action for Paste { +/// ///... /// } /// ``` -pub trait Action: std::fmt::Debug + 'static { - fn qualified_name() -> SharedString - where - Self: Sized; - fn build(value: Option) -> Result> +pub trait Action: 'static { + fn boxed_clone(&self) -> Box; + fn as_any(&self) -> &dyn Any; + fn partial_eq(&self, action: &dyn Action) -> bool; + fn name(&self) -> &str; + + fn debug_name() -> &'static str where Self: Sized; - fn is_registered() -> bool + fn build(value: serde_json::Value) -> Result> where Self: Sized; - - fn partial_eq(&self, action: &dyn Action) -> bool; - fn boxed_clone(&self) -> Box; - fn as_any(&self) -> &dyn Any; } -// Types become actions by satisfying a list of trait bounds. -impl Action for A -where - A: for<'a> Deserialize<'a> + PartialEq + Default + Clone + std::fmt::Debug + 'static, -{ - fn qualified_name() -> SharedString { - let name = type_name::(); - let mut separator_matches = name.rmatch_indices("::"); - separator_matches.next().unwrap(); - let name_start_ix = separator_matches.next().map_or(0, |(ix, _)| ix + 2); - // todo!() remove the 2 replacement when migration is done - name[name_start_ix..].replace("2::", "::").into() - } - - fn build(params: Option) -> Result> - where - Self: Sized, - { - let action = if let Some(params) = params { - serde_json::from_value(params).context("failed to deserialize action")? - } else { - Self::default() - }; - Ok(Box::new(action)) - } - - fn is_registered() -> bool { - ACTION_REGISTRY - .read() - .names_by_type_id - .get(&TypeId::of::()) - .is_some() - } - - fn partial_eq(&self, action: &dyn Action) -> bool { - action - .as_any() - .downcast_ref::() - .map_or(false, |a| self == a) - } - - fn boxed_clone(&self) -> Box { - Box::new(self.clone()) - } - - fn as_any(&self) -> &dyn Any { - self +impl std::fmt::Debug for dyn Action { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("dyn Action") + .field("type_name", &self.name()) + .finish() } } @@ -119,69 +62,90 @@ impl dyn Action { pub fn type_id(&self) -> TypeId { self.as_any().type_id() } - - pub fn name(&self) -> SharedString { - ACTION_REGISTRY - .read() - .names_by_type_id - .get(&self.type_id()) - .expect("type is not a registered action") - .clone() - } } -type ActionBuilder = fn(json: Option) -> anyhow::Result>; - -lazy_static! { - static ref ACTION_REGISTRY: RwLock = RwLock::default(); -} +type ActionBuilder = fn(json: serde_json::Value) -> anyhow::Result>; -#[derive(Default)] -struct ActionRegistry { +pub(crate) struct ActionRegistry { builders_by_name: HashMap, names_by_type_id: HashMap, all_names: Vec, // So we can return a static slice. } -/// Register an action type to allow it to be referenced in keymaps. -pub fn register_action() { - let name = A::qualified_name(); - let mut lock = ACTION_REGISTRY.write(); - lock.builders_by_name.insert(name.clone(), A::build); - lock.names_by_type_id - .insert(TypeId::of::(), name.clone()); - lock.all_names.push(name); -} +impl Default for ActionRegistry { + fn default() -> Self { + let mut this = ActionRegistry { + builders_by_name: Default::default(), + names_by_type_id: Default::default(), + all_names: Default::default(), + }; -/// Construct an action based on its name and optional JSON parameters sourced from the keymap. -pub fn build_action_from_type(type_id: &TypeId) -> Result> { - let lock = ACTION_REGISTRY.read(); - let name = lock - .names_by_type_id - .get(type_id) - .ok_or_else(|| anyhow!("no action type registered for {:?}", type_id))? - .clone(); - drop(lock); - - build_action(&name, None) + this.load_actions(); + + this + } } -/// Construct an action based on its name and optional JSON parameters sourced from the keymap. -pub fn build_action(name: &str, params: Option) -> Result> { - let lock = ACTION_REGISTRY.read(); +/// This type must be public so that our macros can build it in other crates. +/// But this is an implementation detail and should not be used directly. +#[doc(hidden)] +pub type MacroActionBuilder = fn() -> ActionData; - let build_action = lock - .builders_by_name - .get(name) - .ok_or_else(|| anyhow!("no action type registered for {}", name))?; - (build_action)(params) +/// This type must be public so that our macros can build it in other crates. +/// But this is an implementation detail and should not be used directly. +#[doc(hidden)] +pub struct ActionData { + pub name: &'static str, + pub type_id: TypeId, + pub build: ActionBuilder, } -pub fn all_action_names() -> MappedRwLockReadGuard<'static, [SharedString]> { - let lock = ACTION_REGISTRY.read(); - RwLockReadGuard::map(lock, |registry: &ActionRegistry| { - registry.all_names.as_slice() - }) +/// This constant must be public to be accessible from other crates. +/// But it's existence is an implementation detail and should not be used directly. +#[doc(hidden)] +#[linkme::distributed_slice] +pub static __GPUI_ACTIONS: [MacroActionBuilder]; + +impl ActionRegistry { + /// Load all registered actions into the registry. + pub(crate) fn load_actions(&mut self) { + for builder in __GPUI_ACTIONS { + let action = builder(); + let name: SharedString = qualify_action(action.name).into(); + self.builders_by_name.insert(name.clone(), action.build); + self.names_by_type_id.insert(action.type_id, name.clone()); + self.all_names.push(name); + } + } + + /// Construct an action based on its name and optional JSON parameters sourced from the keymap. + pub fn build_action_type(&self, type_id: &TypeId) -> Result> { + let name = self + .names_by_type_id + .get(type_id) + .ok_or_else(|| anyhow!("no action type registered for {:?}", type_id))? + .clone(); + + self.build_action(&name, None) + } + + /// Construct an action based on its name and optional JSON parameters sourced from the keymap. + pub fn build_action( + &self, + name: &str, + params: Option, + ) -> Result> { + let build_action = self + .builders_by_name + .get(name) + .ok_or_else(|| anyhow!("no action type registered for {}", name))?; + (build_action)(params.unwrap_or_else(|| json!({}))) + .with_context(|| format!("Attempting to build action {}", name)) + } + + pub fn all_action_names(&self) -> &[SharedString] { + self.all_names.as_slice() + } } /// Defines unit structs that can be used as actions. @@ -191,7 +155,7 @@ macro_rules! actions { () => {}; ( $name:ident ) => { - #[gpui::action] + #[derive(::std::cmp::PartialEq, ::std::clone::Clone, ::std::default::Default, gpui::serde_derive::Deserialize, gpui::Action)] pub struct $name; }; @@ -201,42 +165,20 @@ macro_rules! actions { }; } -/// This type must be public so that our macros can build it in other crates. -/// But this is an implementation detail and should not be used directly. -#[doc(hidden)] -pub struct ActionData { - pub name: &'static str, - pub build: ActionBuilder, - pub type_id: TypeId, -} - -/// This type must be public so that our macros can build it in other crates. -/// But this is an implementation detail and should not be used directly. -#[doc(hidden)] -pub type MacroActionBuilder = fn() -> ActionData; - -/// This constant must be public to be accessible from other crates. -/// But it's existence is an implementation detail and should not be used directly. +/// This used by our macros to pre-process the action name deterministically #[doc(hidden)] -#[linkme::distributed_slice] -pub static __GPUI_ACTIONS: [MacroActionBuilder]; - -fn qualify_name(action_name: &'static str) -> SharedString { +pub fn qualify_action(action_name: &'static str) -> String { let mut separator_matches = action_name.rmatch_indices("::"); separator_matches.next().unwrap(); let name_start_ix = separator_matches.next().map_or(0, |(ix, _)| ix + 2); // todo!() remove the 2 replacement when migration is done - action_name[name_start_ix..].replace("2::", "::").into() + action_name[name_start_ix..] + .replace("2::", "::") + .to_string() } -pub(crate) fn load_actions_2() { - let mut lock = ACTION_REGISTRY.write(); +mod no_action { + use crate as gpui; - for action in __GPUI_ACTIONS { - let action = action(); - let name = qualify_name(action.name); - lock.builders_by_name.insert(name.clone(), action.build); - lock.names_by_type_id.insert(action.type_id, name.clone()); - lock.all_names.push(name); - } + actions!(NoAction); } diff --git a/crates/gpui2/src/app.rs b/crates/gpui2/src/app.rs index c76b62b510c44fd18e714313777d94ccd9602051..b5083b97c2432d0d3d025b08b5b09cebbf21779a 100644 --- a/crates/gpui2/src/app.rs +++ b/crates/gpui2/src/app.rs @@ -14,12 +14,13 @@ use smallvec::SmallVec; pub use test_context::*; use crate::{ - current_platform, image_cache::ImageCache, Action, AnyBox, AnyView, AnyWindowHandle, - AppMetadata, AssetSource, BackgroundExecutor, ClipboardItem, Context, DispatchPhase, DisplayId, - Entity, EventEmitter, FocusEvent, FocusHandle, FocusId, ForegroundExecutor, KeyBinding, Keymap, - LayoutId, PathPromptOptions, Pixels, Platform, PlatformDisplay, Point, Render, SubscriberSet, - Subscription, SvgRenderer, Task, TextStyle, TextStyleRefinement, TextSystem, View, ViewContext, - Window, WindowContext, WindowHandle, WindowId, + current_platform, image_cache::ImageCache, Action, ActionRegistry, AnyBox, AnyView, + AnyWindowHandle, AppMetadata, AssetSource, BackgroundExecutor, ClipboardItem, Context, + DispatchPhase, DisplayId, Entity, EventEmitter, FocusEvent, FocusHandle, FocusId, + ForegroundExecutor, KeyBinding, Keymap, LayoutId, PathPromptOptions, Pixels, Platform, + PlatformDisplay, Point, Render, SharedString, SubscriberSet, Subscription, SvgRenderer, Task, + TextStyle, TextStyleRefinement, TextSystem, View, ViewContext, Window, WindowContext, + WindowHandle, WindowId, }; use anyhow::{anyhow, Result}; use collections::{HashMap, HashSet, VecDeque}; @@ -182,6 +183,7 @@ pub struct AppContext { text_system: Arc, flushing_effects: bool, pending_updates: usize, + pub(crate) actions: Rc, pub(crate) active_drag: Option, pub(crate) active_tooltip: Option, pub(crate) next_frame_callbacks: HashMap>, @@ -240,6 +242,7 @@ impl AppContext { platform: platform.clone(), app_metadata, text_system, + actions: Rc::new(ActionRegistry::default()), flushing_effects: false, pending_updates: 0, active_drag: None, @@ -964,6 +967,18 @@ impl AppContext { pub fn propagate(&mut self) { self.propagate_event = true; } + + pub fn build_action( + &self, + name: &str, + data: Option, + ) -> Result> { + self.actions.build_action(name, data) + } + + pub fn all_action_names(&self) -> &[SharedString] { + self.actions.all_action_names() + } } impl Context for AppContext { diff --git a/crates/gpui2/src/elements/div.rs b/crates/gpui2/src/elements/div.rs index 31a8827109d922546f312b507844d3996388e009..ebbc34a48a08d7cab7f2978c8d5bdf730f569e68 100644 --- a/crates/gpui2/src/elements/div.rs +++ b/crates/gpui2/src/elements/div.rs @@ -237,11 +237,11 @@ pub trait InteractiveComponent: Sized + Element { // // if we are relying on this side-effect still, removing the debug_assert! // likely breaks the command_palette tests. - debug_assert!( - A::is_registered(), - "{:?} is not registered as an action", - A::qualified_name() - ); + // debug_assert!( + // A::is_registered(), + // "{:?} is not registered as an action", + // A::qualified_name() + // ); self.interactivity().action_listeners.push(( TypeId::of::(), Box::new(move |view, action, phase, cx| { diff --git a/crates/gpui2/src/gpui2.rs b/crates/gpui2/src/gpui2.rs index c2d10154dec3c102191460cccac0e71a7248ca99..88ecd52c037415696cefc6e663758ed7dcfcab8f 100644 --- a/crates/gpui2/src/gpui2.rs +++ b/crates/gpui2/src/gpui2.rs @@ -55,6 +55,7 @@ use private::Sealed; pub use refineable::*; pub use scene::*; pub use serde; +pub use serde_derive; pub use serde_json; pub use smallvec; pub use smol::Timer; diff --git a/crates/gpui2/src/key_dispatch.rs b/crates/gpui2/src/key_dispatch.rs index 962a0308445b48afc85dbc4492d19574917a2e9c..5fbf83bfbab8e45320ee856fd7542298abdc9649 100644 --- a/crates/gpui2/src/key_dispatch.rs +++ b/crates/gpui2/src/key_dispatch.rs @@ -1,6 +1,6 @@ use crate::{ - build_action_from_type, Action, DispatchPhase, FocusId, KeyBinding, KeyContext, KeyMatch, - Keymap, Keystroke, KeystrokeMatcher, WindowContext, + Action, ActionRegistry, DispatchPhase, FocusId, KeyBinding, KeyContext, KeyMatch, Keymap, + Keystroke, KeystrokeMatcher, WindowContext, }; use collections::HashMap; use parking_lot::Mutex; @@ -10,7 +10,6 @@ use std::{ rc::Rc, sync::Arc, }; -use util::ResultExt; #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] pub struct DispatchNodeId(usize); @@ -22,6 +21,7 @@ pub(crate) struct DispatchTree { focusable_node_ids: HashMap, keystroke_matchers: HashMap, KeystrokeMatcher>, keymap: Arc>, + action_registry: Rc, } #[derive(Default)] @@ -41,7 +41,7 @@ pub(crate) struct DispatchActionListener { } impl DispatchTree { - pub fn new(keymap: Arc>) -> Self { + pub fn new(keymap: Arc>, action_registry: Rc) -> Self { Self { node_stack: Vec::new(), context_stack: Vec::new(), @@ -49,6 +49,7 @@ impl DispatchTree { focusable_node_ids: HashMap::default(), keystroke_matchers: HashMap::default(), keymap, + action_registry, } } @@ -153,7 +154,9 @@ impl DispatchTree { for node_id in self.dispatch_path(*node) { let node = &self.nodes[node_id.0]; for DispatchActionListener { action_type, .. } in &node.action_listeners { - actions.extend(build_action_from_type(action_type).log_err()); + // Intentionally silence these errors without logging. + // If an action cannot be built by default, it's not available. + actions.extend(self.action_registry.build_action_type(action_type).ok()); } } } diff --git a/crates/gpui2/src/keymap/keymap.rs b/crates/gpui2/src/keymap/keymap.rs index 989ee7a8d5515182769eb10cfb5bacc3eeaf8501..8152693c07a5ab316ce03ab3600ed855595b2ccd 100644 --- a/crates/gpui2/src/keymap/keymap.rs +++ b/crates/gpui2/src/keymap/keymap.rs @@ -1,7 +1,10 @@ -use crate::{KeyBinding, KeyBindingContextPredicate, Keystroke}; +use crate::{KeyBinding, KeyBindingContextPredicate, Keystroke, NoAction}; use collections::HashSet; use smallvec::SmallVec; -use std::{any::TypeId, collections::HashMap}; +use std::{ + any::{Any, TypeId}, + collections::HashMap, +}; #[derive(Copy, Clone, Eq, PartialEq, Default)] pub struct KeymapVersion(usize); @@ -37,20 +40,19 @@ impl Keymap { } pub fn add_bindings>(&mut self, bindings: T) { - // todo!("no action") - // let no_action_id = (NoAction {}).id(); + let no_action_id = &(NoAction {}).type_id(); let mut new_bindings = Vec::new(); - let has_new_disabled_keystrokes = false; + let mut has_new_disabled_keystrokes = false; for binding in bindings { - // if binding.action().id() == no_action_id { - // has_new_disabled_keystrokes |= self - // .disabled_keystrokes - // .entry(binding.keystrokes) - // .or_default() - // .insert(binding.context_predicate); - // } else { - new_bindings.push(binding); - // } + if binding.action.type_id() == *no_action_id { + has_new_disabled_keystrokes |= self + .disabled_keystrokes + .entry(binding.keystrokes) + .or_default() + .insert(binding.context_predicate); + } else { + new_bindings.push(binding); + } } if has_new_disabled_keystrokes { diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index b0d9d07df2db7b3b84db7d9950d3f6364b3b8c9d..ff4c13abce5148768ba89549370173b6b34cb44a 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -311,8 +311,8 @@ impl Window { layout_engine: TaffyLayoutEngine::new(), root_view: None, element_id_stack: GlobalElementId::default(), - previous_frame: Frame::new(DispatchTree::new(cx.keymap.clone())), - current_frame: Frame::new(DispatchTree::new(cx.keymap.clone())), + previous_frame: Frame::new(DispatchTree::new(cx.keymap.clone(), cx.actions.clone())), + current_frame: Frame::new(DispatchTree::new(cx.keymap.clone(), cx.actions.clone())), focus_handles: Arc::new(RwLock::new(SlotMap::with_key())), focus_listeners: SubscriberSet::new(), default_prevented: true, diff --git a/crates/gpui2/tests/action_macros.rs b/crates/gpui2/tests/action_macros.rs new file mode 100644 index 0000000000000000000000000000000000000000..49064ffd86e371e0fcc4547e8ba34dad3843a6df --- /dev/null +++ b/crates/gpui2/tests/action_macros.rs @@ -0,0 +1,45 @@ +use serde_derive::Deserialize; + +#[test] +fn test_derive() { + use gpui2 as gpui; + + #[derive(PartialEq, Clone, Deserialize, gpui2_macros::Action)] + struct AnotherTestAction; + + #[gpui2_macros::register_action] + #[derive(PartialEq, Clone, gpui::serde_derive::Deserialize)] + struct RegisterableAction {} + + impl gpui::Action for RegisterableAction { + fn boxed_clone(&self) -> Box { + todo!() + } + + fn as_any(&self) -> &dyn std::any::Any { + todo!() + } + + fn partial_eq(&self, _action: &dyn gpui::Action) -> bool { + todo!() + } + + fn name(&self) -> &str { + todo!() + } + + fn debug_name() -> &'static str + where + Self: Sized, + { + todo!() + } + + fn build(_value: serde_json::Value) -> anyhow::Result> + where + Self: Sized, + { + todo!() + } + } +} diff --git a/crates/gpui2_macros/src/action.rs b/crates/gpui2_macros/src/action.rs index 564f35d6a4da2d873afb5e4a059c295b11d1ef58..abc75a875984842496c077f6e1841f5ec9915730 100644 --- a/crates/gpui2_macros/src/action.rs +++ b/crates/gpui2_macros/src/action.rs @@ -15,48 +15,81 @@ use proc_macro::TokenStream; use quote::quote; -use syn::{parse_macro_input, DeriveInput}; +use syn::{parse_macro_input, DeriveInput, Error}; + +use crate::register_action::register_action; + +pub fn action(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); -pub fn action(_attr: TokenStream, item: TokenStream) -> TokenStream { - let input = parse_macro_input!(item as DeriveInput); let name = &input.ident; - let attrs = input - .attrs - .into_iter() - .filter(|attr| !attr.path.is_ident("action")) - .collect::>(); - - let attributes = quote! { - #[gpui::register_action] - #[derive(gpui::serde::Deserialize, std::cmp::PartialEq, std::clone::Clone, std::default::Default, std::fmt::Debug)] - #(#attrs)* + + if input.generics.lt_token.is_some() { + return Error::new(name.span(), "Actions must be a concrete type") + .into_compile_error() + .into(); + } + + let is_unit_struct = match input.data { + syn::Data::Struct(struct_data) => struct_data.fields.is_empty(), + syn::Data::Enum(_) => false, + syn::Data::Union(_) => false, + }; + + let build_impl = if is_unit_struct { + quote! { + Ok(std::boxed::Box::new(Self {})) + } + } else { + quote! { + Ok(std::boxed::Box::new(gpui::serde_json::from_value::(value)?)) + } }; - let visibility = input.vis; - - let output = match input.data { - syn::Data::Struct(ref struct_data) => match &struct_data.fields { - syn::Fields::Named(_) | syn::Fields::Unnamed(_) => { - let fields = &struct_data.fields; - quote! { - #attributes - #visibility struct #name #fields - } + + let register_action = register_action(&name); + + let output = quote! { + const _: fn() = || { + fn assert_impl gpui::serde::Deserialize<'a> + ::std::cmp::PartialEq + ::std::clone::Clone>() {} + assert_impl::<#name>(); + }; + + impl gpui::Action for #name { + fn name(&self) -> &'static str + { + ::std::any::type_name::<#name>() + } + + fn debug_name() -> &'static str + where + Self: ::std::marker::Sized + { + ::std::any::type_name::<#name>() + } + + fn build(value: gpui::serde_json::Value) -> gpui::Result<::std::boxed::Box> + where + Self: ::std::marker::Sized { + #build_impl } - syn::Fields::Unit => { - quote! { - #attributes - #visibility struct #name; - } + + fn partial_eq(&self, action: &dyn gpui::Action) -> bool { + action + .as_any() + .downcast_ref::() + .map_or(false, |a| self == a) } - }, - syn::Data::Enum(ref enum_data) => { - let variants = &enum_data.variants; - quote! { - #attributes - #visibility enum #name { #variants } + + fn boxed_clone(&self) -> std::boxed::Box { + ::std::boxed::Box::new(self.clone()) + } + + fn as_any(&self) -> &dyn ::std::any::Any { + self } } - _ => panic!("Expected a struct or an enum."), + + #register_action }; TokenStream::from(output) diff --git a/crates/gpui2_macros/src/gpui2_macros.rs b/crates/gpui2_macros/src/gpui2_macros.rs index 80b67e1a12393d76d1ecffc5a342dc8eaefacc40..3ce8373689d077f64702584d91394f2285e909f5 100644 --- a/crates/gpui2_macros/src/gpui2_macros.rs +++ b/crates/gpui2_macros/src/gpui2_macros.rs @@ -11,14 +11,14 @@ pub fn style_helpers(args: TokenStream) -> TokenStream { style_helpers::style_helpers(args) } -#[proc_macro_attribute] -pub fn action(attr: TokenStream, item: TokenStream) -> TokenStream { - action::action(attr, item) +#[proc_macro_derive(Action)] +pub fn action(input: TokenStream) -> TokenStream { + action::action(input) } #[proc_macro_attribute] pub fn register_action(attr: TokenStream, item: TokenStream) -> TokenStream { - register_action::register_action(attr, item) + register_action::register_action_macro(attr, item) } #[proc_macro_derive(Component, attributes(component))] diff --git a/crates/gpui2_macros/src/register_action.rs b/crates/gpui2_macros/src/register_action.rs index e6c47e8c5284c13db43a4ba29cf2c36fd10d9f9d..3d398c873c87e033d970a0f64b3af4b80ef2195b 100644 --- a/crates/gpui2_macros/src/register_action.rs +++ b/crates/gpui2_macros/src/register_action.rs @@ -12,13 +12,54 @@ // gpui2::register_action_builder::() // } use proc_macro::TokenStream; +use proc_macro2::Ident; use quote::{format_ident, quote}; -use syn::{parse_macro_input, DeriveInput}; +use syn::{parse_macro_input, DeriveInput, Error}; -pub fn register_action(_attr: TokenStream, item: TokenStream) -> TokenStream { +pub fn register_action_macro(_attr: TokenStream, item: TokenStream) -> TokenStream { let input = parse_macro_input!(item as DeriveInput); - let type_name = &input.ident; + let registration = register_action(&input.ident); + let has_action_derive = input + .attrs + .iter() + .find(|attr| { + (|| { + let meta = attr.parse_meta().ok()?; + meta.path().is_ident("derive").then(|| match meta { + syn::Meta::Path(_) => None, + syn::Meta::NameValue(_) => None, + syn::Meta::List(list) => list + .nested + .iter() + .find(|list| match list { + syn::NestedMeta::Meta(meta) => meta.path().is_ident("Action"), + syn::NestedMeta::Lit(_) => false, + }) + .map(|_| true), + })? + })() + .unwrap_or(false) + }) + .is_some(); + + if has_action_derive { + return Error::new( + input.ident.span(), + "The Action derive macro has already registered this action", + ) + .into_compile_error() + .into(); + } + + TokenStream::from(quote! { + #input + + #registration + }) +} + +pub(crate) fn register_action(type_name: &Ident) -> proc_macro2::TokenStream { let static_slice_name = format_ident!("__GPUI_ACTIONS_{}", type_name.to_string().to_uppercase()); @@ -27,9 +68,7 @@ pub fn register_action(_attr: TokenStream, item: TokenStream) -> TokenStream { type_name.to_string().to_lowercase() ); - let expanded = quote! { - #input - + quote! { #[doc(hidden)] #[gpui::linkme::distributed_slice(gpui::__GPUI_ACTIONS)] #[linkme(crate = gpui::linkme)] @@ -44,7 +83,5 @@ pub fn register_action(_attr: TokenStream, item: TokenStream) -> TokenStream { build: <#type_name as gpui::Action>::build, } } - }; - - TokenStream::from(expanded) + } } diff --git a/crates/live_kit_client2/examples/test_app.rs b/crates/live_kit_client2/examples/test_app.rs index 0b9e54f9b0dc7801f695adcf31b352d9997a48f1..00aec53baff83478d59365d2aed4d896e99e87f7 100644 --- a/crates/live_kit_client2/examples/test_app.rs +++ b/crates/live_kit_client2/examples/test_app.rs @@ -1,7 +1,7 @@ use std::{sync::Arc, time::Duration}; use futures::StreamExt; -use gpui::KeyBinding; +use gpui::{Action, KeyBinding}; use live_kit_client2::{ LocalAudioTrack, LocalVideoTrack, RemoteAudioTrackUpdate, RemoteVideoTrackUpdate, Room, }; @@ -10,7 +10,7 @@ use log::LevelFilter; use serde_derive::Deserialize; use simplelog::SimpleLogger; -#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Eq, Default)] +#[derive(Deserialize, Debug, Clone, Copy, PartialEq, Action)] struct Quit; fn main() { diff --git a/crates/settings2/src/keymap_file.rs b/crates/settings2/src/keymap_file.rs index 9f279864ee739243c9c475395aef6d228582fe0a..93635935cbd746b7e6fc3d96294e5895ee5c01e4 100644 --- a/crates/settings2/src/keymap_file.rs +++ b/crates/settings2/src/keymap_file.rs @@ -73,9 +73,9 @@ impl KeymapFile { "Expected first item in array to be a string." ))); }; - gpui::build_action(&name, Some(data)) + cx.build_action(&name, Some(data)) } - Value::String(name) => gpui::build_action(&name, None), + Value::String(name) => cx.build_action(&name, None), Value::Null => Ok(no_action()), _ => { return Some(Err(anyhow!("Expected two-element array, got {action:?}"))) diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index ad337163845b965e94d7f9f3eb1c3d8b02dbf325..f815dbe0ea9a05d95880917eae255ef63c797546 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -9,7 +9,7 @@ pub mod terminal_panel; // use crate::terminal_element::TerminalElement; use editor::{scroll::autoscroll::Autoscroll, Editor}; use gpui::{ - actions, div, img, red, register_action, AnyElement, AppContext, Component, DispatchPhase, Div, + actions, div, img, red, Action, AnyElement, AppContext, Component, DispatchPhase, Div, EventEmitter, FocusEvent, FocusHandle, Focusable, FocusableComponent, FocusableView, InputHandler, InteractiveComponent, KeyDownEvent, Keystroke, Model, MouseButton, ParentComponent, Pixels, Render, SharedString, Styled, Task, View, ViewContext, VisualContext, @@ -55,12 +55,10 @@ const CURSOR_BLINK_INTERVAL: Duration = Duration::from_millis(500); #[derive(Clone, Debug, PartialEq)] pub struct ScrollTerminal(pub i32); -#[register_action] -#[derive(Clone, Debug, Default, Deserialize, PartialEq)] +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Action)] pub struct SendText(String); -#[register_action] -#[derive(Clone, Debug, Default, Deserialize, PartialEq)] +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Action)] pub struct SendKeystroke(String); actions!(Clear, Copy, Paste, ShowCharacterPalette, SearchTest); diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index 8f32c3ed5664ede770af66580e0abe2f7379f767..6b3371e33861c5475e42596b9de6b57afd183617 100644 --- a/crates/ui2/src/components/context_menu.rs +++ b/crates/ui2/src/components/context_menu.rs @@ -84,7 +84,8 @@ pub use stories::*; mod stories { use super::*; use crate::story::Story; - use gpui::{action, Div, Render}; + use gpui::{Div, Render}; + use serde::Deserialize; pub struct ContextMenuStory; @@ -92,7 +93,7 @@ mod stories { type Element = Div; fn render(&mut self, cx: &mut ViewContext) -> Self::Element { - #[action] + #[derive(PartialEq, Clone, Deserialize, gpui::Action)] struct PrintCurrentDate {} Story::container(cx) diff --git a/crates/ui2/src/components/keybinding.rs b/crates/ui2/src/components/keybinding.rs index 04e036f3655c50f2110a5675fa75aa131f139de8..8da5273bf5f03afac68f898e3a52702568a92281 100644 --- a/crates/ui2/src/components/keybinding.rs +++ b/crates/ui2/src/components/keybinding.rs @@ -81,13 +81,12 @@ pub use stories::*; mod stories { use super::*; use crate::Story; - use gpui::{action, Div, Render}; + use gpui::{actions, Div, Render}; use itertools::Itertools; pub struct KeybindingStory; - #[action] - struct NoAction {} + actions!(NoAction); pub fn binding(key: &str) -> gpui::KeyBinding { gpui::KeyBinding::new(key, NoAction {}, None) diff --git a/crates/workspace2/src/pane.rs b/crates/workspace2/src/pane.rs index dcc8a4a14f182e118a2620a22578044f5c268925..b86240f4190983b02b68cc6f96f75b7a15853942 100644 --- a/crates/workspace2/src/pane.rs +++ b/crates/workspace2/src/pane.rs @@ -7,7 +7,7 @@ use crate::{ use anyhow::Result; use collections::{HashMap, HashSet, VecDeque}; use gpui::{ - actions, prelude::*, register_action, AppContext, AsyncWindowContext, Component, Div, EntityId, + actions, prelude::*, Action, AppContext, AsyncWindowContext, Component, Div, EntityId, EventEmitter, FocusHandle, Focusable, FocusableView, Model, PromptLevel, Render, Task, View, ViewContext, VisualContext, WeakView, WindowContext, }; @@ -70,15 +70,13 @@ pub struct ActivateItem(pub usize); // pub pane: WeakView, // } -#[register_action] -#[derive(Clone, PartialEq, Debug, Deserialize, Default)] +#[derive(Clone, PartialEq, Debug, Deserialize, Default, Action)] #[serde(rename_all = "camelCase")] pub struct CloseActiveItem { pub save_intent: Option, } -#[register_action] -#[derive(Clone, PartialEq, Debug, Deserialize, Default)] +#[derive(Clone, PartialEq, Debug, Deserialize, Default, Action)] #[serde(rename_all = "camelCase")] pub struct CloseAllItems { pub save_intent: Option, @@ -1917,7 +1915,7 @@ impl Render for Pane { .on_action(|pane: &mut Pane, _: &SplitRight, cx| pane.split(SplitDirection::Right, cx)) .on_action(|pane: &mut Pane, _: &SplitDown, cx| pane.split(SplitDirection::Down, cx)) .size_full() - .on_action(|pane: &mut Self, action, cx| { + .on_action(|pane: &mut Self, action: &CloseActiveItem, cx| { pane.close_active_item(action, cx) .map(|task| task.detach_and_log_err(cx)); }) diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index a146ad1822b75a9c9ea25cfe581c480cd5f92742..ab7a69e75ce13b7cdcd31e6748358e1c0589b984 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -29,11 +29,11 @@ use futures::{ Future, FutureExt, StreamExt, }; use gpui::{ - actions, div, point, register_action, size, Action, AnyModel, AnyView, AnyWeakView, AppContext, - AsyncAppContext, AsyncWindowContext, Bounds, Context, Div, Entity, EntityId, EventEmitter, - FocusHandle, FocusableView, GlobalPixels, InteractiveComponent, KeyContext, Model, - ModelContext, ParentComponent, Point, Render, Size, Styled, Subscription, Task, View, - ViewContext, VisualContext, WeakView, WindowBounds, WindowContext, WindowHandle, WindowOptions, + actions, div, point, size, Action, AnyModel, AnyView, AnyWeakView, AppContext, AsyncAppContext, + AsyncWindowContext, Bounds, Context, Div, Entity, EntityId, EventEmitter, FocusHandle, + FocusableView, GlobalPixels, InteractiveComponent, KeyContext, Model, ModelContext, + ParentComponent, Point, Render, Size, Styled, Subscription, Task, View, ViewContext, + VisualContext, WeakView, WindowBounds, WindowContext, WindowHandle, WindowOptions, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ItemSettings, ProjectItem}; use itertools::Itertools; @@ -194,8 +194,7 @@ impl Clone for Toast { } } -#[register_action] -#[derive(Debug, Default, Clone, Deserialize, PartialEq)] +#[derive(Debug, Default, Clone, Deserialize, PartialEq, Action)] pub struct OpenTerminal { pub working_directory: PathBuf, } diff --git a/crates/zed2/src/languages/json.rs b/crates/zed2/src/languages/json.rs index cf9b33d9683cadd9f5b27c41104d215becd1110d..f04f59cf6d2e23ed7d594636e41b7e173d5d4081 100644 --- a/crates/zed2/src/languages/json.rs +++ b/crates/zed2/src/languages/json.rs @@ -107,7 +107,7 @@ impl LspAdapter for JsonLspAdapter { &self, cx: &mut AppContext, ) -> BoxFuture<'static, serde_json::Value> { - let action_names = gpui::all_action_names(); + let action_names = cx.all_action_names(); let staff_mode = cx.is_staff(); let language_names = &self.languages.language_names(); let settings_schema = cx.global::().json_schema( diff --git a/crates/zed_actions2/src/lib.rs b/crates/zed_actions2/src/lib.rs index 7f0c19853e2978b5bd63ee9abf00996ca1985812..456d1f5973ba5ef628fa8e969e6696d647596776 100644 --- a/crates/zed_actions2/src/lib.rs +++ b/crates/zed_actions2/src/lib.rs @@ -1,4 +1,5 @@ -use gpui::action; +use gpui::Action; +use serde::Deserialize; // If the zed binary doesn't use anything in this crate, it will be optimized away // and the actions won't initialize. So we just provide an empty initialization function @@ -9,12 +10,12 @@ use gpui::action; // https://github.com/mmastrac/rust-ctor/issues/280 pub fn init() {} -#[action] +#[derive(Clone, PartialEq, Deserialize, Action)] pub struct OpenBrowser { pub url: String, } -#[action] +#[derive(Clone, PartialEq, Deserialize, Action)] pub struct OpenZedURL { pub url: String, } From 49d3e1cc4bebc0e66b1c995aa7b38456e2f0a6e9 Mon Sep 17 00:00:00 2001 From: Mikayla Date: Thu, 16 Nov 2023 17:39:05 -0800 Subject: [PATCH 20/24] Add default derive --- crates/editor2/src/editor.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/editor2/src/editor.rs b/crates/editor2/src/editor.rs index 8f4b9ccf644d4f9f3db9320882b6b71a41b6cb08..8d394270a79c837451ee4a35558441231e1d97b0 100644 --- a/crates/editor2/src/editor.rs +++ b/crates/editor2/src/editor.rs @@ -180,78 +180,78 @@ pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); // // .with_soft_wrap(true) // } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct SelectNext { #[serde(default)] pub replace_newest: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct SelectPrevious { #[serde(default)] pub replace_newest: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct SelectAllMatches { #[serde(default)] pub replace_newest: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct SelectToBeginningOfLine { #[serde(default)] stop_at_soft_wraps: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct MovePageUp { #[serde(default)] center_cursor: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct MovePageDown { #[serde(default)] center_cursor: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct SelectToEndOfLine { #[serde(default)] stop_at_soft_wraps: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct ToggleCodeActions { #[serde(default)] pub deployed_from_indicator: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct ConfirmCompletion { #[serde(default)] pub item_ix: Option, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct ConfirmCodeAction { #[serde(default)] pub item_ix: Option, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct ToggleComments { #[serde(default)] pub advance_downwards: bool, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct FoldAt { pub buffer_row: u32, } -#[derive(PartialEq, Clone, Deserialize, Action)] +#[derive(PartialEq, Clone, Deserialize, Default, Action)] pub struct UnfoldAt { pub buffer_row: u32, } From 17b2b112bc0f1f1af59e3dae6cde37752837dac1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 18:02:49 -0800 Subject: [PATCH 21/24] Don't update file's saved mtime when reload is aborted --- crates/language/src/buffer.rs | 2 +- crates/language2/src/buffer.rs | 30 ++++++++++++++++------------ crates/project2/src/project_tests.rs | 13 +++++++----- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 6d5684e6d7a9f9196c9257f9fd98905a398c7ac6..7feffbf3ed58f0c6ea3d18bc77af9e53580c9e6b 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -646,7 +646,7 @@ impl Buffer { prev_version, Rope::text_fingerprint(&new_text), this.line_ending(), - new_mtime, + this.saved_mtime, cx, ); } diff --git a/crates/language2/src/buffer.rs b/crates/language2/src/buffer.rs index e191f408e7f37e616fc2293e0d60e805db82a1e4..51ed192b993cdc6442a06056a7c502ead35ba977 100644 --- a/crates/language2/src/buffer.rs +++ b/crates/language2/src/buffer.rs @@ -61,9 +61,14 @@ pub struct Buffer { diff_base: Option, git_diff: git::diff::BufferDiff, file: Option>, - saved_version: clock::Global, - saved_version_fingerprint: RopeFingerprint, + /// The mtime of the file when this buffer was last loaded from + /// or saved to disk. saved_mtime: SystemTime, + /// The version vector when this buffer was last loaded from + /// or saved to disk. + saved_version: clock::Global, + /// A hash of the current contents of the buffer's file. + file_fingerprint: RopeFingerprint, transaction_depth: usize, was_dirty_before_starting_transaction: Option, reload_task: Option>>, @@ -386,8 +391,7 @@ impl Buffer { .ok_or_else(|| anyhow!("missing line_ending"))?, )); this.saved_version = proto::deserialize_version(&message.saved_version); - this.saved_version_fingerprint = - proto::deserialize_fingerprint(&message.saved_version_fingerprint)?; + this.file_fingerprint = proto::deserialize_fingerprint(&message.saved_version_fingerprint)?; this.saved_mtime = message .saved_mtime .ok_or_else(|| anyhow!("invalid saved_mtime"))? @@ -403,7 +407,7 @@ impl Buffer { diff_base: self.diff_base.as_ref().map(|h| h.to_string()), line_ending: proto::serialize_line_ending(self.line_ending()) as i32, saved_version: proto::serialize_version(&self.saved_version), - saved_version_fingerprint: proto::serialize_fingerprint(self.saved_version_fingerprint), + saved_version_fingerprint: proto::serialize_fingerprint(self.file_fingerprint), saved_mtime: Some(self.saved_mtime.into()), } } @@ -473,7 +477,7 @@ impl Buffer { Self { saved_mtime, saved_version: buffer.version(), - saved_version_fingerprint: buffer.as_rope().fingerprint(), + file_fingerprint: buffer.as_rope().fingerprint(), reload_task: None, transaction_depth: 0, was_dirty_before_starting_transaction: None, @@ -540,7 +544,7 @@ impl Buffer { } pub fn saved_version_fingerprint(&self) -> RopeFingerprint { - self.saved_version_fingerprint + self.file_fingerprint } pub fn saved_mtime(&self) -> SystemTime { @@ -568,7 +572,7 @@ impl Buffer { cx: &mut ModelContext, ) { self.saved_version = version; - self.saved_version_fingerprint = fingerprint; + self.file_fingerprint = fingerprint; self.saved_mtime = mtime; cx.emit(Event::Saved); cx.notify(); @@ -611,7 +615,7 @@ impl Buffer { prev_version, Rope::text_fingerprint(&new_text), this.line_ending(), - new_mtime, + this.saved_mtime, cx, ); } @@ -631,14 +635,14 @@ impl Buffer { cx: &mut ModelContext, ) { self.saved_version = version; - self.saved_version_fingerprint = fingerprint; + self.file_fingerprint = fingerprint; self.text.set_line_ending(line_ending); self.saved_mtime = mtime; if let Some(file) = self.file.as_ref().and_then(|f| f.as_local()) { file.buffer_reloaded( self.remote_id(), &self.saved_version, - self.saved_version_fingerprint, + self.file_fingerprint, self.line_ending(), self.saved_mtime, cx, @@ -1282,12 +1286,12 @@ impl Buffer { } pub fn is_dirty(&self) -> bool { - self.saved_version_fingerprint != self.as_rope().fingerprint() + self.file_fingerprint != self.as_rope().fingerprint() || self.file.as_ref().map_or(false, |file| file.is_deleted()) } pub fn has_conflict(&self) -> bool { - self.saved_version_fingerprint != self.as_rope().fingerprint() + self.file_fingerprint != self.as_rope().fingerprint() && self .file .as_ref() diff --git a/crates/project2/src/project_tests.rs b/crates/project2/src/project_tests.rs index 4c5905ff7e98be4d8046d365759d59900c82f448..e607b307661977fd7505d64eca6b43523b50a6c3 100644 --- a/crates/project2/src/project_tests.rs +++ b/crates/project2/src/project_tests.rs @@ -2587,7 +2587,7 @@ async fn test_save_file(cx: &mut gpui::TestAppContext) { assert_eq!(new_text, buffer.update(cx, |buffer, _| buffer.text())); } -#[gpui::test(iterations = 10)] +#[gpui::test(iterations = 30)] async fn test_file_changes_multiple_times_on_disk(cx: &mut gpui::TestAppContext) { init_test(cx); @@ -2638,14 +2638,17 @@ async fn test_file_changes_multiple_times_on_disk(cx: &mut gpui::TestAppContext) buffer.read_with(cx, |buffer, _| { let buffer_text = buffer.text(); if buffer_text == on_disk_text { - assert!(!buffer.is_dirty(), "buffer shouldn't be dirty. text: {buffer_text:?}, disk text: {on_disk_text:?}"); + assert!( + !buffer.is_dirty() && !buffer.has_conflict(), + "buffer shouldn't be dirty. text: {buffer_text:?}, disk text: {on_disk_text:?}", + ); } // If the file change occurred while the buffer was processing the first - // change, the buffer may be in a conflicting state. + // change, the buffer will be in a conflicting state. else { assert!( - buffer.is_dirty(), - "buffer should report that it is dirty. text: {buffer_text:?}, disk text: {on_disk_text:?}" + buffer.is_dirty() && buffer.has_conflict(), + "buffer should report that it has a conflict. text: {buffer_text:?}, disk text: {on_disk_text:?}" ); } }); From 432572c592c4d735a218be0e04d5757641bde97f Mon Sep 17 00:00:00 2001 From: Mikayla Date: Thu, 16 Nov 2023 18:04:35 -0800 Subject: [PATCH 22/24] #RemoveThe2 --- crates/command_palette2/src/command_palette.rs | 2 +- crates/gpui2/src/action.rs | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/command_palette2/src/command_palette.rs b/crates/command_palette2/src/command_palette.rs index 31eb608ef56bc56bbfdbc1bdf1eca776ee3c6811..6264606ed9c7db0116d475ac48a57eef872434a5 100644 --- a/crates/command_palette2/src/command_palette.rs +++ b/crates/command_palette2/src/command_palette.rs @@ -47,7 +47,7 @@ impl CommandPalette { .available_actions() .into_iter() .filter_map(|action| { - let name = action.name(); + let name = gpui::remove_the_2(action.name()); let namespace = name.split("::").next().unwrap_or("malformed action name"); if filter.is_some_and(|f| f.filtered_namespaces.contains(namespace)) { return None; diff --git a/crates/gpui2/src/action.rs b/crates/gpui2/src/action.rs index 8656f31a5ecd14b158139d5d772a85ab9db49716..958eaabdb83076dcd8114ba1f06ae2ac4fc6c50b 100644 --- a/crates/gpui2/src/action.rs +++ b/crates/gpui2/src/action.rs @@ -3,7 +3,10 @@ use anyhow::{anyhow, Context, Result}; use collections::HashMap; pub use no_action::NoAction; use serde_json::json; -use std::any::{Any, TypeId}; +use std::{ + any::{Any, TypeId}, + ops::Deref, +}; /// Actions are used to implement keyboard-driven UI. /// When you declare an action, you can bind keys to the action in the keymap and @@ -111,7 +114,8 @@ impl ActionRegistry { pub(crate) fn load_actions(&mut self) { for builder in __GPUI_ACTIONS { let action = builder(); - let name: SharedString = qualify_action(action.name).into(); + //todo(remove) + let name: SharedString = remove_the_2(action.name).into(); self.builders_by_name.insert(name.clone(), action.build); self.names_by_type_id.insert(action.type_id, name.clone()); self.all_names.push(name); @@ -135,9 +139,11 @@ impl ActionRegistry { name: &str, params: Option, ) -> Result> { + //todo(remove) + let name = remove_the_2(name); let build_action = self .builders_by_name - .get(name) + .get(name.deref()) .ok_or_else(|| anyhow!("no action type registered for {}", name))?; (build_action)(params.unwrap_or_else(|| json!({}))) .with_context(|| format!("Attempting to build action {}", name)) @@ -165,9 +171,8 @@ macro_rules! actions { }; } -/// This used by our macros to pre-process the action name deterministically -#[doc(hidden)] -pub fn qualify_action(action_name: &'static str) -> String { +//todo!(remove) +pub fn remove_the_2(action_name: &str) -> String { let mut separator_matches = action_name.rmatch_indices("::"); separator_matches.next().unwrap(); let name_start_ix = separator_matches.next().map_or(0, |(ix, _)| ix + 2); From f3b6719c76a9d540470fca417491ec5c2f1d21d4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 19:58:18 -0800 Subject: [PATCH 23/24] Rename both PlatformDispatcher::poll and Executor::run_step to 'tick' Co-authored-by: Nathan Sobo --- crates/gpui2/src/app/test_context.rs | 2 +- crates/gpui2/src/executor.rs | 6 +++--- crates/gpui2/src/platform.rs | 2 +- crates/gpui2/src/platform/mac/dispatcher.rs | 2 +- crates/gpui2/src/platform/test/dispatcher.rs | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/gpui2/src/app/test_context.rs b/crates/gpui2/src/app/test_context.rs index 37f81da15b9fd92882d58fede5e755ce65f3c1e5..f78162589ead3c73eded6d562a69111cc1f7ebef 100644 --- a/crates/gpui2/src/app/test_context.rs +++ b/crates/gpui2/src/app/test_context.rs @@ -359,7 +359,7 @@ impl Model { Ok(Some(event)) => return event, Ok(None) => panic!("model was dropped"), Err(_) => { - if !cx.executor().run_step() { + if !cx.executor().tick() { break; } } diff --git a/crates/gpui2/src/executor.rs b/crates/gpui2/src/executor.rs index b29fbbb5a192ea8a2081571bb2008f295c8cf06f..21b67b445d0c964d99fdd6e52ceb40671b69eedc 100644 --- a/crates/gpui2/src/executor.rs +++ b/crates/gpui2/src/executor.rs @@ -158,7 +158,7 @@ impl BackgroundExecutor { match future.as_mut().poll(&mut cx) { Poll::Ready(result) => return result, Poll::Pending => { - if !self.dispatcher.poll(background_only) { + if !self.dispatcher.tick(background_only) { if awoken.swap(false, SeqCst) { continue; } @@ -255,8 +255,8 @@ impl BackgroundExecutor { } #[cfg(any(test, feature = "test-support"))] - pub fn run_step(&self) -> bool { - self.dispatcher.as_test().unwrap().poll(false) + pub fn tick(&self) -> bool { + self.dispatcher.as_test().unwrap().tick(false) } #[cfg(any(test, feature = "test-support"))] diff --git a/crates/gpui2/src/platform.rs b/crates/gpui2/src/platform.rs index 882dc332ef91aa9cd1b0460e5e2ff5e126328455..3027c05fbd84c27c6f51fe02d5da84f38815d067 100644 --- a/crates/gpui2/src/platform.rs +++ b/crates/gpui2/src/platform.rs @@ -165,7 +165,7 @@ pub trait PlatformDispatcher: Send + Sync { fn dispatch(&self, runnable: Runnable, label: Option); fn dispatch_on_main_thread(&self, runnable: Runnable); fn dispatch_after(&self, duration: Duration, runnable: Runnable); - fn poll(&self, background_only: bool) -> bool; + fn tick(&self, background_only: bool) -> bool; fn park(&self); fn unparker(&self) -> Unparker; diff --git a/crates/gpui2/src/platform/mac/dispatcher.rs b/crates/gpui2/src/platform/mac/dispatcher.rs index 1752f78601f6c7f785207d8ada49181ee54f9545..2fb0eef3e5f03375cb6bfc1dba653f4f8043aca9 100644 --- a/crates/gpui2/src/platform/mac/dispatcher.rs +++ b/crates/gpui2/src/platform/mac/dispatcher.rs @@ -71,7 +71,7 @@ impl PlatformDispatcher for MacDispatcher { } } - fn poll(&self, _background_only: bool) -> bool { + fn tick(&self, _background_only: bool) -> bool { false } diff --git a/crates/gpui2/src/platform/test/dispatcher.rs b/crates/gpui2/src/platform/test/dispatcher.rs index 3abe4796b3ff985178e609e05f503fbde563adf0..e77c1c052903f44b2e346af5b3d7a6fb57cda65f 100644 --- a/crates/gpui2/src/platform/test/dispatcher.rs +++ b/crates/gpui2/src/platform/test/dispatcher.rs @@ -113,7 +113,7 @@ impl TestDispatcher { } pub fn run_until_parked(&self) { - while self.poll(false) {} + while self.tick(false) {} } pub fn parking_allowed(&self) -> bool { @@ -194,7 +194,7 @@ impl PlatformDispatcher for TestDispatcher { state.delayed.insert(ix, (next_time, runnable)); } - fn poll(&self, background_only: bool) -> bool { + fn tick(&self, background_only: bool) -> bool { let mut state = self.state.lock(); while let Some((deadline, _)) = state.delayed.first() { From 32979f3acad549c9c2db7ba530ae5c377bf8f19c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 Nov 2023 20:03:18 -0800 Subject: [PATCH 24/24] Rename deprioritize_task -> deprioritize It applies to a family of tasks, not a task. --- crates/gpui2/src/app/test_context.rs | 1 + crates/gpui2/src/executor.rs | 2 +- crates/project2/src/project_tests.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/gpui2/src/app/test_context.rs b/crates/gpui2/src/app/test_context.rs index f78162589ead3c73eded6d562a69111cc1f7ebef..8b26bb18150fb2d48e1632cda95e6382f9bfa3dd 100644 --- a/crates/gpui2/src/app/test_context.rs +++ b/crates/gpui2/src/app/test_context.rs @@ -354,6 +354,7 @@ impl Model { }) }); + // Run other tasks until the event is emitted. loop { match rx.try_next() { Ok(Some(event)) => return event, diff --git a/crates/gpui2/src/executor.rs b/crates/gpui2/src/executor.rs index 21b67b445d0c964d99fdd6e52ceb40671b69eedc..cf138a90db1b177e052d79788754d446474ce5be 100644 --- a/crates/gpui2/src/executor.rs +++ b/crates/gpui2/src/executor.rs @@ -245,7 +245,7 @@ impl BackgroundExecutor { } #[cfg(any(test, feature = "test-support"))] - pub fn deprioritize_task(&self, task_label: TaskLabel) { + pub fn deprioritize(&self, task_label: TaskLabel) { self.dispatcher.as_test().unwrap().deprioritize(task_label) } diff --git a/crates/project2/src/project_tests.rs b/crates/project2/src/project_tests.rs index e607b307661977fd7505d64eca6b43523b50a6c3..9eb9a49e49e2930ba1b133b27b8f8fcf799a261e 100644 --- a/crates/project2/src/project_tests.rs +++ b/crates/project2/src/project_tests.rs @@ -2609,7 +2609,7 @@ async fn test_file_changes_multiple_times_on_disk(cx: &mut gpui::TestAppContext) // Simulate buffer diffs being slow, so that they don't complete before // the next file change occurs. - cx.executor().deprioritize_task(*language::BUFFER_DIFF_TASK); + cx.executor().deprioritize(*language::BUFFER_DIFF_TASK); // Change the buffer's file on disk, and then wait for the file change // to be detected by the worktree, so that the buffer starts reloading.