From c485fc86a21567d694fd8b25d2f7b450f0d9241c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 27 Apr 2023 18:06:52 -0700 Subject: [PATCH 1/7] Add copilot.disabled_globs setting --- Cargo.lock | 2 + Cargo.toml | 1 + assets/settings/default.json | 7 +++ crates/editor/Cargo.toml | 1 + crates/editor/src/editor.rs | 37 +++++++++++-- crates/editor/src/editor_tests.rs | 91 +++++++++++++++++++++++++++++++ crates/editor/src/multi_buffer.rs | 11 +++- crates/project/Cargo.toml | 2 +- crates/settings/Cargo.toml | 1 + crates/settings/src/settings.rs | 57 ++++++++++--------- 10 files changed, 176 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4b6ebbe5edcf3b83762e7abf7ba6b1db1a74adff..af780197e8e3b28856a1494167ecf5744f908338 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1983,6 +1983,7 @@ dependencies = [ "futures 0.3.25", "fuzzy", "git", + "glob", "gpui", "indoc", "itertools", @@ -5964,6 +5965,7 @@ dependencies = [ "collections", "fs", "futures 0.3.25", + "glob", "gpui", "json_comments", "postage", diff --git a/Cargo.toml b/Cargo.toml index 1ef283d135128a5bc60dea30a5d00d91efd511b3..15df687d41d3a5933250bb4d2cd66a7ab0ed4075 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,6 +77,7 @@ async-trait = { version = "0.1" } ctor = { version = "0.1" } env_logger = { version = "0.9" } futures = { version = "0.3" } +glob = { version = "0.3.1" } lazy_static = { version = "1.4.0" } log = { version = "0.4.16", features = ["kv_unstable_serde"] } ordered-float = { version = "2.1.1" } diff --git a/assets/settings/default.json b/assets/settings/default.json index b12bd00efa7d85857dc4604c44d98d067aedd6e3..b72dab05c7467ce1a54093b7ff976c67e96e1ecb 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -115,6 +115,13 @@ // "git_gutter": "hide" "git_gutter": "tracked_files" }, + "copilot": { + // The set of glob patterns for which copilot should be disabled + // in any matching file. + "disabled_globs": [ + ".env" + ] + }, // Settings specific to journaling "journal": { // The path of the directory where journal entries are stored diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index feb55e1b2fc09516cf80b8e191216a43bcaa3fe4..e8cb323a27c65d3a08f2557cee9756a5696a3c5a 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -79,6 +79,7 @@ workspace = { path = "../workspace", features = ["test-support"] } ctor.workspace = true env_logger.workspace = true +glob.workspace = true rand.workspace = true unindent.workspace = true tree-sitter = "0.20" diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index eea418b211e27caf27ec1168cddd60ec62b022c2..9128cdb0acf1681dac6e15ed6d90442138a8a56f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2925,11 +2925,7 @@ impl Editor { let snapshot = self.buffer.read(cx).snapshot(cx); let cursor = self.selections.newest_anchor().head(); - let language_name = snapshot.language_at(cursor).map(|language| language.name()); - if !cx - .global::() - .show_copilot_suggestions(language_name.as_deref()) - { + if !self.is_copilot_enabled_at(cursor, &snapshot, cx) { self.clear_copilot_suggestions(cx); return None; } @@ -3080,6 +3076,37 @@ impl Editor { } } + fn is_copilot_enabled_at( + &self, + location: Anchor, + snapshot: &MultiBufferSnapshot, + cx: &mut ViewContext, + ) -> bool { + let settings = cx.global::(); + + let language_name = snapshot + .language_at(location) + .map(|language| language.name()); + if !settings.show_copilot_suggestions(language_name.as_deref()) { + return false; + } + + let file = snapshot.file_at(location); + if let Some(file) = file { + let path = file.path(); + if settings + .copilot + .disabled_globs + .iter() + .any(|glob| glob.matches_path(path)) + { + return false; + } + } + + true + } + fn has_active_copilot_suggestion(&self, cx: &AppContext) -> bool { self.display_map.read(cx).has_suggestion() } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index af4bc1674b8e010b1f2e357122b63b07b9d8ff8f..3099bb640d615bbbf0393d74f161cccb5babb52d 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -6387,6 +6387,97 @@ async fn test_copilot_multibuffer( }); } +#[gpui::test] +async fn test_copilot_disabled_globs( + deterministic: Arc, + cx: &mut gpui::TestAppContext, +) { + let (copilot, copilot_lsp) = Copilot::fake(cx); + cx.update(|cx| { + let mut settings = Settings::test(cx); + settings.copilot.disabled_globs = vec![glob::Pattern::new(".env*").unwrap()]; + cx.set_global(settings); + cx.set_global(copilot) + }); + + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/test", + json!({ + ".env": "SECRET=something\n", + "README.md": "hello\n" + }), + ) + .await; + let project = Project::test(fs, ["/test".as_ref()], cx).await; + + let private_buffer = project + .update(cx, |project, cx| { + project.open_local_buffer("/test/.env", cx) + }) + .await + .unwrap(); + let public_buffer = project + .update(cx, |project, cx| { + project.open_local_buffer("/test/README.md", cx) + }) + .await + .unwrap(); + + let multibuffer = cx.add_model(|cx| { + let mut multibuffer = MultiBuffer::new(0); + multibuffer.push_excerpts( + private_buffer.clone(), + [ExcerptRange { + context: Point::new(0, 0)..Point::new(1, 0), + primary: None, + }], + cx, + ); + multibuffer.push_excerpts( + public_buffer.clone(), + [ExcerptRange { + context: Point::new(0, 0)..Point::new(1, 0), + primary: None, + }], + cx, + ); + multibuffer + }); + let (_, editor) = cx.add_window(|cx| build_editor(multibuffer, cx)); + + let mut copilot_requests = copilot_lsp + .handle_request::(move |_params, _cx| async move { + Ok(copilot::request::GetCompletionsResult { + completions: vec![copilot::request::Completion { + text: "next line".into(), + range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 0)), + ..Default::default() + }], + }) + }); + + editor.update(cx, |editor, cx| { + editor.change_selections(None, cx, |selections| { + selections.select_ranges([Point::new(0, 0)..Point::new(0, 0)]) + }); + editor.next_copilot_suggestion(&Default::default(), cx); + }); + + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + assert!(copilot_requests.try_next().is_err()); + + editor.update(cx, |editor, cx| { + editor.change_selections(None, cx, |s| { + s.select_ranges([Point::new(2, 0)..Point::new(2, 0)]) + }); + editor.next_copilot_suggestion(&Default::default(), cx); + }); + + deterministic.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); + assert!(copilot_requests.try_next().is_ok()); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index bf2f12e82e95ef58fddbbf4fdcabb40f0d7a66a3..39e66d0f93c15f23da9ff93c9f7e5be5a460c2b4 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -10,9 +10,9 @@ use gpui::{AppContext, Entity, ModelContext, ModelHandle, Task}; pub use language::Completion; use language::{ char_kind, AutoindentMode, Buffer, BufferChunks, BufferSnapshot, CharKind, Chunk, CursorShape, - DiagnosticEntry, IndentSize, Language, LanguageScope, OffsetRangeExt, OffsetUtf16, Outline, - OutlineItem, Point, PointUtf16, Selection, TextDimension, ToOffset as _, ToOffsetUtf16 as _, - ToPoint as _, ToPointUtf16 as _, TransactionId, Unclipped, + DiagnosticEntry, File, IndentSize, Language, LanguageScope, OffsetRangeExt, OffsetUtf16, + Outline, OutlineItem, Point, PointUtf16, Selection, TextDimension, ToOffset as _, + ToOffsetUtf16 as _, ToPoint as _, ToPointUtf16 as _, TransactionId, Unclipped, }; use std::{ borrow::Cow, @@ -2754,6 +2754,11 @@ impl MultiBufferSnapshot { self.trailing_excerpt_update_count } + pub fn file_at<'a, T: ToOffset>(&'a self, point: T) -> Option<&'a Arc> { + self.point_to_buffer_offset(point) + .and_then(|(buffer, _)| buffer.file()) + } + pub fn language_at<'a, T: ToOffset>(&'a self, point: T) -> Option<&'a Arc> { self.point_to_buffer_offset(point) .and_then(|(buffer, offset)| buffer.language_at(offset)) diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index 56803bb0620009dab2e6e983110a44b644ea2911..46b00fc6eeb05a7942836e2c3fa109dcab43bde6 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -28,7 +28,6 @@ fs = { path = "../fs" } fsevent = { path = "../fsevent" } fuzzy = { path = "../fuzzy" } git = { path = "../git" } -glob = { version = "0.3.1" } gpui = { path = "../gpui" } language = { path = "../language" } lsp = { path = "../lsp" } @@ -43,6 +42,7 @@ anyhow.workspace = true async-trait.workspace = true backtrace = "0.3" futures.workspace = true +glob.workspace = true ignore = "0.4" lazy_static.workspace = true log.workspace = true diff --git a/crates/settings/Cargo.toml b/crates/settings/Cargo.toml index 3a87832e15fbd5d41bd28b05391de7f74b83847d..00f6cda3a718b5b8038fd9b2823a5f564efe461a 100644 --- a/crates/settings/Cargo.toml +++ b/crates/settings/Cargo.toml @@ -23,6 +23,7 @@ theme = { path = "../theme" } staff_mode = { path = "../staff_mode" } util = { path = "../util" } +glob.workspace = true json_comments = "0.2" postage.workspace = true schemars = "0.8" diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index f2082be6bb0883adcece8a5d53168d1d018b6c00..260f9531a8f4f88648114cbf1f938dd59782ded3 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -47,6 +47,7 @@ pub struct Settings { pub editor_overrides: EditorSettings, pub git: GitSettings, pub git_overrides: GitSettings, + pub copilot: CopilotSettings, pub journal_defaults: JournalSettings, pub journal_overrides: JournalSettings, pub terminal_defaults: TerminalSettings, @@ -61,29 +62,6 @@ pub struct Settings { pub base_keymap: BaseKeymap, } -#[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, Default)] -#[serde(rename_all = "snake_case")] -pub enum CopilotSettings { - #[default] - On, - Off, -} - -impl From for bool { - fn from(value: CopilotSettings) -> Self { - match value { - CopilotSettings::On => true, - CopilotSettings::Off => false, - } - } -} - -impl CopilotSettings { - pub fn is_on(&self) -> bool { - >::into(*self) - } -} - #[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq, Default)] pub enum BaseKeymap { #[default] @@ -150,6 +128,29 @@ impl TelemetrySettings { } } +#[derive(Clone, Debug, Default)] +pub struct CopilotSettings { + pub disabled_globs: Vec, +} + +#[derive(Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] +pub struct CopilotSettingsContent { + #[serde(default)] + disabled_globs: Vec, +} + +impl From for CopilotSettings { + fn from(value: CopilotSettingsContent) -> Self { + Self { + disabled_globs: value + .disabled_globs + .into_iter() + .filter_map(|p| glob::Pattern::new(&p).ok()) + .collect(), + } + } +} + #[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] pub struct GitSettings { pub git_gutter: Option, @@ -390,6 +391,8 @@ pub struct SettingsFileContent { #[serde(default)] pub buffer_font_features: Option, #[serde(default)] + pub copilot: Option, + #[serde(default)] pub active_pane_magnification: Option, #[serde(default)] pub cursor_blink: Option, @@ -438,8 +441,7 @@ pub struct LspSettings { pub initialization_options: Option, } -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] -#[serde(rename_all = "snake_case")] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct Features { pub copilot: bool, } @@ -506,6 +508,7 @@ impl Settings { show_copilot_suggestions: required(defaults.editor.show_copilot_suggestions), }, editor_overrides: Default::default(), + copilot: defaults.copilot.unwrap().into(), git: defaults.git.unwrap(), git_overrides: Default::default(), journal_defaults: defaults.journal, @@ -576,6 +579,9 @@ impl Settings { merge(&mut self.base_keymap, data.base_keymap); merge(&mut self.features.copilot, data.features.copilot); + if let Some(copilot) = data.copilot.map(CopilotSettings::from) { + self.copilot = copilot; + } self.editor_overrides = data.editor; self.git_overrides = data.git.unwrap_or_default(); self.journal_overrides = data.journal; @@ -751,6 +757,7 @@ impl Settings { show_copilot_suggestions: Some(true), }, editor_overrides: Default::default(), + copilot: Default::default(), journal_defaults: Default::default(), journal_overrides: Default::default(), terminal_defaults: Default::default(), From 8eb1312debea806ca5ef5d0d948cb017bd0ba25c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 2 May 2023 19:56:45 -0700 Subject: [PATCH 2/7] Add copilot menu item for enabling paths by glob --- Cargo.lock | 3 + crates/copilot_button/Cargo.toml | 1 + crates/copilot_button/src/copilot_button.rs | 133 ++++++-- crates/editor/src/editor.rs | 16 +- crates/settings/Cargo.toml | 1 + crates/settings/src/settings.rs | 352 +++++++++++++------- crates/settings/src/settings_file.rs | 42 +-- crates/workspace/Cargo.toml | 1 + crates/workspace/src/workspace.rs | 50 ++- crates/zed/src/main.rs | 4 +- crates/zed/src/menus.rs | 2 +- crates/zed/src/zed.rs | 59 +--- 12 files changed, 432 insertions(+), 232 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af780197e8e3b28856a1494167ecf5744f908338..d1f5631f6f14359fe16d299b67903f687fc23c11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1362,6 +1362,7 @@ name = "copilot_button" version = "0.1.0" dependencies = [ "anyhow", + "assets", "context_menu", "copilot", "editor", @@ -5968,6 +5969,7 @@ dependencies = [ "glob", "gpui", "json_comments", + "lazy_static", "postage", "pretty_assertions", "schemars", @@ -8455,6 +8457,7 @@ name = "workspace" version = "0.1.0" dependencies = [ "anyhow", + "assets", "async-recursion 1.0.0", "bincode", "call", diff --git a/crates/copilot_button/Cargo.toml b/crates/copilot_button/Cargo.toml index 04e9165a758f058210cebb9b5724328e6341967a..2d42b192d95bdfdf528a96140289c4e28709e3b8 100644 --- a/crates/copilot_button/Cargo.toml +++ b/crates/copilot_button/Cargo.toml @@ -9,6 +9,7 @@ path = "src/copilot_button.rs" doctest = false [dependencies] +assets = { path = "../assets" } copilot = { path = "../copilot" } editor = { path = "../editor" } context_menu = { path = "../context_menu" } diff --git a/crates/copilot_button/src/copilot_button.rs b/crates/copilot_button/src/copilot_button.rs index a597bb7e47c1bd724a076e54802291408190243b..93adc95efd12c0f028247ee37fc5d70df1075566 100644 --- a/crates/copilot_button/src/copilot_button.rs +++ b/crates/copilot_button/src/copilot_button.rs @@ -1,18 +1,19 @@ +use anyhow::Result; use context_menu::{ContextMenu, ContextMenuItem}; use copilot::{Copilot, Reinstall, SignOut, Status}; -use editor::Editor; +use editor::{scroll::autoscroll::Autoscroll, Editor}; use gpui::{ elements::*, platform::{CursorStyle, MouseButton}, - AnyElement, AppContext, Element, Entity, MouseState, Subscription, View, ViewContext, - ViewHandle, WindowContext, + AnyElement, AppContext, AsyncAppContext, Element, Entity, MouseState, Subscription, View, + ViewContext, ViewHandle, WeakViewHandle, WindowContext, }; use settings::{settings_file::SettingsFile, Settings}; -use std::sync::Arc; -use util::ResultExt; +use std::{path::Path, sync::Arc}; +use util::{paths, ResultExt}; use workspace::{ - item::ItemHandle, notifications::simple_message_notification::OsOpen, StatusItemView, Toast, - Workspace, + create_and_open_local_file, item::ItemHandle, + notifications::simple_message_notification::OsOpen, AppState, StatusItemView, Toast, Workspace, }; const COPILOT_SETTINGS_URL: &str = "https://github.com/settings/copilot"; @@ -20,10 +21,12 @@ const COPILOT_STARTING_TOAST_ID: usize = 1337; const COPILOT_ERROR_TOAST_ID: usize = 1338; pub struct CopilotButton { + app_state: Arc, popup_menu: ViewHandle, editor_subscription: Option<(Subscription, usize)>, editor_enabled: Option, language: Option>, + path: Option>, } impl Entity for CopilotButton { @@ -51,7 +54,7 @@ impl View for CopilotButton { let enabled = self .editor_enabled - .unwrap_or(settings.show_copilot_suggestions(None)); + .unwrap_or(settings.show_copilot_suggestions(None, None)); Stack::new() .with_child( @@ -131,7 +134,7 @@ impl View for CopilotButton { } impl CopilotButton { - pub fn new(cx: &mut ViewContext) -> Self { + pub fn new(app_state: Arc, cx: &mut ViewContext) -> Self { let menu = cx.add_view(|cx| { let mut menu = ContextMenu::new(cx); menu.set_position_mode(OverlayPositionMode::Local); @@ -146,10 +149,12 @@ impl CopilotButton { .detach(); Self { + app_state, popup_menu: menu, editor_subscription: None, editor_enabled: None, language: None, + path: None, } } @@ -176,10 +181,10 @@ impl CopilotButton { pub fn deploy_copilot_menu(&mut self, cx: &mut ViewContext) { let settings = cx.global::(); - let mut menu_options = Vec::with_capacity(6); + let mut menu_options = Vec::with_capacity(8); if let Some(language) = self.language.clone() { - let language_enabled = settings.show_copilot_suggestions(Some(language.as_ref())); + let language_enabled = settings.copilot_enabled_for_language(Some(language.as_ref())); menu_options.push(ContextMenuItem::handler( format!( "{} Suggestions for {}", @@ -190,7 +195,38 @@ impl CopilotButton { )); } - let globally_enabled = cx.global::().show_copilot_suggestions(None); + if let Some(path) = self.path.as_ref() { + let path_enabled = settings.copilot_enabled_for_path(path); + let app_state = Arc::downgrade(&self.app_state); + let path = path.clone(); + menu_options.push(ContextMenuItem::handler( + format!( + "{} Suggestions for This Path", + if path_enabled { "Hide" } else { "Show" } + ), + move |cx| { + if let Some((workspace, app_state)) = cx + .root_view() + .clone() + .downcast::() + .zip(app_state.upgrade()) + { + let workspace = workspace.downgrade(); + cx.spawn(|_, cx| { + configure_disabled_globs( + workspace, + app_state, + path_enabled.then_some(path.clone()), + cx, + ) + }) + .detach_and_log_err(cx); + } + }, + )); + } + + let globally_enabled = cx.global::().features.copilot; menu_options.push(ContextMenuItem::handler( if globally_enabled { "Hide Suggestions for All Files" @@ -236,10 +272,14 @@ impl CopilotButton { let language_name = snapshot .language_at(suggestion_anchor) .map(|language| language.name()); + let path = snapshot + .file_at(suggestion_anchor) + .map(|file| file.path().clone()); - self.language = language_name.clone(); - - self.editor_enabled = Some(settings.show_copilot_suggestions(language_name.as_deref())); + self.editor_enabled = + Some(settings.show_copilot_suggestions(language_name.as_deref(), path.as_deref())); + self.language = language_name; + self.path = path; cx.notify() } @@ -260,8 +300,63 @@ impl StatusItemView for CopilotButton { } } +async fn configure_disabled_globs( + workspace: WeakViewHandle, + app_state: Arc, + path_to_disable: Option>, + mut cx: AsyncAppContext, +) -> Result<()> { + let settings_editor = workspace + .update(&mut cx, |_, cx| { + create_and_open_local_file(&paths::SETTINGS, app_state, cx, || { + Settings::initial_user_settings_content(&assets::Assets) + .as_ref() + .into() + }) + })? + .await? + .downcast::() + .unwrap(); + + settings_editor.downgrade().update(&mut cx, |item, cx| { + let text = item.buffer().read(cx).snapshot(cx).text(); + + let edits = SettingsFile::update_unsaved(&text, cx, |file| { + let copilot = file.copilot.get_or_insert_with(Default::default); + let globs = copilot.disabled_globs.get_or_insert_with(|| { + cx.global::() + .copilot + .disabled_globs + .clone() + .iter() + .map(|glob| glob.as_str().to_string()) + .collect::>() + }); + + if let Some(path_to_disable) = &path_to_disable { + globs.push(path_to_disable.to_string_lossy().into_owned()); + } else { + globs.clear(); + } + }); + + if !edits.is_empty() { + item.change_selections(Some(Autoscroll::newest()), cx, |selections| { + selections.select_ranges(edits.iter().map(|e| e.0.clone())); + }); + + // When *enabling* a path, don't actually perform an edit, just select the range. + if path_to_disable.is_some() { + item.edit(edits.iter().cloned(), cx); + } + } + })?; + + anyhow::Ok(()) +} + fn toggle_copilot_globally(cx: &mut AppContext) { - let show_copilot_suggestions = cx.global::().show_copilot_suggestions(None); + let show_copilot_suggestions = cx.global::().show_copilot_suggestions(None, None); SettingsFile::update(cx, move |file_contents| { file_contents.editor.show_copilot_suggestions = Some((!show_copilot_suggestions).into()) }); @@ -270,7 +365,7 @@ fn toggle_copilot_globally(cx: &mut AppContext) { fn toggle_copilot_for_language(language: Arc, cx: &mut AppContext) { let show_copilot_suggestions = cx .global::() - .show_copilot_suggestions(Some(&language)); + .show_copilot_suggestions(Some(&language), None); SettingsFile::update(cx, move |file_contents| { file_contents.languages.insert( @@ -280,13 +375,13 @@ fn toggle_copilot_for_language(language: Arc, cx: &mut AppContext) { ..Default::default() }, ); - }) + }); } fn hide_copilot(cx: &mut AppContext) { SettingsFile::update(cx, move |file_contents| { file_contents.features.copilot = Some(false) - }) + }); } fn initiate_sign_in(cx: &mut WindowContext) { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9128cdb0acf1681dac6e15ed6d90442138a8a56f..69eb8170ac6a37678bb3111dcc0a024877e80691 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3084,26 +3084,14 @@ impl Editor { ) -> bool { let settings = cx.global::(); + let path = snapshot.file_at(location).map(|file| file.path()); let language_name = snapshot .language_at(location) .map(|language| language.name()); - if !settings.show_copilot_suggestions(language_name.as_deref()) { + if !settings.show_copilot_suggestions(language_name.as_deref(), path.map(|p| p.as_ref())) { return false; } - let file = snapshot.file_at(location); - if let Some(file) = file { - let path = file.path(); - if settings - .copilot - .disabled_globs - .iter() - .any(|glob| glob.matches_path(path)) - { - return false; - } - } - true } diff --git a/crates/settings/Cargo.toml b/crates/settings/Cargo.toml index 00f6cda3a718b5b8038fd9b2823a5f564efe461a..ca59f996c370722cc283f35b2ac1552dc8d6a0db 100644 --- a/crates/settings/Cargo.toml +++ b/crates/settings/Cargo.toml @@ -25,6 +25,7 @@ util = { path = "../util" } glob.workspace = true json_comments = "0.2" +lazy_static.workspace = true postage.workspace = true schemars = "0.8" serde.workspace = true diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 260f9531a8f4f88648114cbf1f938dd59782ded3..80c9da82ba06324b1151e22e7258c3a250582d89 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -7,6 +7,7 @@ use gpui::{ font_cache::{FamilyId, FontCache}, fonts, AssetSource, }; +use lazy_static::lazy_static; use schemars::{ gen::{SchemaGenerator, SchemaSettings}, schema::{InstanceType, ObjectValidation, Schema, SchemaObject, SingleOrVec}, @@ -18,14 +19,19 @@ use sqlez::{ bindable::{Bind, Column, StaticColumnCount}, statement::Statement, }; -use std::{collections::HashMap, num::NonZeroU32, str, sync::Arc}; +use std::{ + borrow::Cow, collections::HashMap, num::NonZeroU32, ops::Range, path::Path, str, sync::Arc, +}; use theme::{Theme, ThemeRegistry}; -use tree_sitter::Query; +use tree_sitter::{Query, Tree}; use util::{RangeExt, ResultExt as _}; pub use keymap_file::{keymap_file_json_schema, KeymapFileContent}; pub use watched_json::watch_files; +pub const DEFAULT_SETTINGS_ASSET_PATH: &str = "settings/default.json"; +pub const INITIAL_USER_SETTINGS_ASSET_PATH: &str = "settings/initial_user_settings.json"; + #[derive(Clone)] pub struct Settings { pub features: Features, @@ -136,19 +142,7 @@ pub struct CopilotSettings { #[derive(Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] pub struct CopilotSettingsContent { #[serde(default)] - disabled_globs: Vec, -} - -impl From for CopilotSettings { - fn from(value: CopilotSettingsContent) -> Self { - Self { - disabled_globs: value - .disabled_globs - .into_iter() - .filter_map(|p| glob::Pattern::new(&p).ok()) - .collect(), - } - } + pub disabled_globs: Option>, } #[derive(Copy, Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] @@ -453,6 +447,13 @@ pub struct FeaturesContent { } impl Settings { + pub fn initial_user_settings_content(assets: &'static impl AssetSource) -> Cow<'static, str> { + match assets.load(INITIAL_USER_SETTINGS_ASSET_PATH).unwrap() { + Cow::Borrowed(s) => Cow::Borrowed(str::from_utf8(s).unwrap()), + Cow::Owned(s) => Cow::Owned(String::from_utf8(s).unwrap()), + } + } + /// Fill out the settings corresponding to the default.json file, overrides will be set later pub fn defaults( assets: impl AssetSource, @@ -466,7 +467,7 @@ impl Settings { } let defaults: SettingsFileContent = parse_json_with_comments( - str::from_utf8(assets.load("settings/default.json").unwrap().as_ref()).unwrap(), + str::from_utf8(assets.load(DEFAULT_SETTINGS_ASSET_PATH).unwrap().as_ref()).unwrap(), ) .unwrap(); @@ -508,7 +509,16 @@ impl Settings { show_copilot_suggestions: required(defaults.editor.show_copilot_suggestions), }, editor_overrides: Default::default(), - copilot: defaults.copilot.unwrap().into(), + copilot: CopilotSettings { + disabled_globs: defaults + .copilot + .unwrap() + .disabled_globs + .unwrap() + .into_iter() + .map(|s| glob::Pattern::new(&s).unwrap()) + .collect(), + }, git: defaults.git.unwrap(), git_overrides: Default::default(), journal_defaults: defaults.journal, @@ -579,8 +589,13 @@ impl Settings { merge(&mut self.base_keymap, data.base_keymap); merge(&mut self.features.copilot, data.features.copilot); - if let Some(copilot) = data.copilot.map(CopilotSettings::from) { - self.copilot = copilot; + if let Some(copilot) = data.copilot { + if let Some(disabled_globs) = copilot.disabled_globs { + self.copilot.disabled_globs = disabled_globs + .into_iter() + .filter_map(|s| glob::Pattern::new(&s).ok()) + .collect() + } } self.editor_overrides = data.editor; self.git_overrides = data.git.unwrap_or_default(); @@ -608,11 +623,34 @@ impl Settings { &self.features } - pub fn show_copilot_suggestions(&self, language: Option<&str>) -> bool { - self.features.copilot - && self.language_setting(language, |settings| { - settings.show_copilot_suggestions.map(Into::into) - }) + pub fn show_copilot_suggestions(&self, language: Option<&str>, path: Option<&Path>) -> bool { + if !self.features.copilot { + return false; + } + + if !self.copilot_enabled_for_language(language) { + return false; + } + + if let Some(path) = path { + if !self.copilot_enabled_for_path(path) { + return false; + } + } + + true + } + + pub fn copilot_enabled_for_path(&self, path: &Path) -> bool { + !self + .copilot + .disabled_globs + .iter() + .any(|glob| glob.matches_path(path)) + } + + pub fn copilot_enabled_for_language(&self, language: Option<&str>) -> bool { + self.language_setting(language, |settings| settings.show_copilot_suggestions) } pub fn tab_size(&self, language: Option<&str>) -> NonZeroU32 { @@ -866,17 +904,8 @@ pub fn parse_json_with_comments(content: &str) -> Result )?) } -fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_value: &Value) { - const LANGUAGE_OVERRIDES: &'static str = "language_overrides"; - const LANGAUGES: &'static str = "languages"; - - let mut parser = tree_sitter::Parser::new(); - parser.set_language(tree_sitter_json::language()).unwrap(); - let tree = parser.parse(&settings_content, None).unwrap(); - - let mut cursor = tree_sitter::QueryCursor::new(); - - let query = Query::new( +lazy_static! { + static ref PAIR_QUERY: Query = Query::new( tree_sitter_json::language(), " (pair @@ -885,14 +914,65 @@ fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_valu ", ) .unwrap(); +} + +fn update_object_in_settings_file<'a>( + old_object: &'a serde_json::Map, + new_object: &'a serde_json::Map, + text: &str, + syntax_tree: &Tree, + tab_size: usize, + key_path: &mut Vec<&'a str>, + edits: &mut Vec<(Range, String)>, +) { + for (key, old_value) in old_object.iter() { + key_path.push(key); + let new_value = new_object.get(key).unwrap_or(&Value::Null); + + // If the old and new values are both objects, then compare them key by key, + // preserving the comments and formatting of the unchanged parts. Otherwise, + // replace the old value with the new value. + if let (Value::Object(old_sub_object), Value::Object(new_sub_object)) = + (old_value, new_value) + { + update_object_in_settings_file( + old_sub_object, + new_sub_object, + text, + syntax_tree, + tab_size, + key_path, + edits, + ) + } else if old_value != new_value { + let (range, replacement) = + update_key_in_settings_file(text, syntax_tree, &key_path, tab_size, &new_value); + edits.push((range, replacement)); + } - let has_language_overrides = settings_content.contains(LANGUAGE_OVERRIDES); + key_path.pop(); + } +} + +fn update_key_in_settings_file( + text: &str, + syntax_tree: &Tree, + key_path: &[&str], + tab_size: usize, + new_value: impl Serialize, +) -> (Range, String) { + const LANGUAGE_OVERRIDES: &'static str = "language_overrides"; + const LANGUAGES: &'static str = "languages"; + + let mut cursor = tree_sitter::QueryCursor::new(); + + let has_language_overrides = text.contains(LANGUAGE_OVERRIDES); let mut depth = 0; let mut last_value_range = 0..0; let mut first_key_start = None; - let mut existing_value_range = 0..settings_content.len(); - let matches = cursor.matches(&query, tree.root_node(), settings_content.as_bytes()); + let mut existing_value_range = 0..text.len(); + let matches = cursor.matches(&PAIR_QUERY, syntax_tree.root_node(), text.as_bytes()); for mat in matches { if mat.captures.len() != 2 { continue; @@ -915,10 +995,10 @@ fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_valu first_key_start.get_or_insert_with(|| key_range.start); - let found_key = settings_content + let found_key = text .get(key_range.clone()) .map(|key_text| { - if key_path[depth] == LANGAUGES && has_language_overrides { + if key_path[depth] == LANGUAGES && has_language_overrides { return key_text == format!("\"{}\"", LANGUAGE_OVERRIDES); } else { return key_text == format!("\"{}\"", key_path[depth]); @@ -942,12 +1022,11 @@ fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_valu // We found the exact key we want, insert the new value if depth == key_path.len() { - let new_val = serde_json::to_string_pretty(new_value) - .expect("Could not serialize new json field to string"); - settings_content.replace_range(existing_value_range, &new_val); + let new_val = to_pretty_json(&new_value, tab_size, tab_size * depth); + (existing_value_range, new_val) } else { // We have key paths, construct the sub objects - let new_key = if has_language_overrides && key_path[depth] == LANGAUGES { + let new_key = if has_language_overrides && key_path[depth] == LANGUAGES { LANGUAGE_OVERRIDES } else { key_path[depth] @@ -956,7 +1035,7 @@ fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_valu // We don't have the key, construct the nested objects let mut new_value = serde_json::to_value(new_value).unwrap(); for key in key_path[(depth + 1)..].iter().rev() { - if has_language_overrides && key == &LANGAUGES { + if has_language_overrides && key == &LANGUAGES { new_value = serde_json::json!({ LANGUAGE_OVERRIDES.to_string(): new_value }); } else { new_value = serde_json::json!({ key.to_string(): new_value }); @@ -966,7 +1045,7 @@ fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_valu if let Some(first_key_start) = first_key_start { let mut row = 0; let mut column = 0; - for (ix, char) in settings_content.char_indices() { + for (ix, char) in text.char_indices() { if ix == first_key_start { break; } @@ -981,37 +1060,29 @@ fn write_settings_key(settings_content: &mut String, key_path: &[&str], new_valu if row > 0 { // depth is 0 based, but division needs to be 1 based. let new_val = to_pretty_json(&new_value, column / (depth + 1), column); - let content = format!(r#""{new_key}": {new_val},"#); - settings_content.insert_str(first_key_start, &content); - - settings_content.insert_str( - first_key_start + content.len(), - &format!("\n{:width$}", ' ', width = column), - ) + let space = ' '; + let content = format!("\"{new_key}\": {new_val},\n{space:width$}", width = column); + (first_key_start..first_key_start, content) } else { let new_val = serde_json::to_string(&new_value).unwrap(); let mut content = format!(r#""{new_key}": {new_val},"#); content.push(' '); - settings_content.insert_str(first_key_start, &content); + (first_key_start..first_key_start, content) } } else { new_value = serde_json::json!({ new_key.to_string(): new_value }); let indent_prefix_len = 4 * depth; - let new_val = to_pretty_json(&new_value, 4, indent_prefix_len); - - settings_content.replace_range(existing_value_range, &new_val); + let mut new_val = to_pretty_json(&new_value, 4, indent_prefix_len); if depth == 0 { - settings_content.push('\n'); + new_val.push('\n'); } + + (existing_value_range, new_val) } } } -fn to_pretty_json( - value: &serde_json::Value, - indent_size: usize, - indent_prefix_len: usize, -) -> String { +fn to_pretty_json(value: &impl Serialize, indent_size: usize, indent_prefix_len: usize) -> String { const SPACES: [u8; 32] = [b' '; 32]; debug_assert!(indent_size <= SPACES.len()); @@ -1038,13 +1109,16 @@ fn to_pretty_json( adjusted_text } -pub fn update_settings_file( - mut text: String, +/// Update the settings file with the given callback. +/// +/// Returns a new JSON string and the offset where the first edit occurred. +fn update_settings_file( + text: &str, mut old_file_content: SettingsFileContent, + tab_size: NonZeroU32, update: impl FnOnce(&mut SettingsFileContent), -) -> String { +) -> Vec<(Range, String)> { let mut new_file_content = old_file_content.clone(); - update(&mut new_file_content); if new_file_content.languages.len() != old_file_content.languages.len() { @@ -1062,51 +1136,25 @@ pub fn update_settings_file( } } + let mut parser = tree_sitter::Parser::new(); + parser.set_language(tree_sitter_json::language()).unwrap(); + let tree = parser.parse(text, None).unwrap(); + let old_object = to_json_object(old_file_content); let new_object = to_json_object(new_file_content); - - fn apply_changes_to_json_text( - old_object: &serde_json::Map, - new_object: &serde_json::Map, - current_key_path: Vec<&str>, - json_text: &mut String, - ) { - for (key, old_value) in old_object.iter() { - // We know that these two are from the same shape of object, so we can just unwrap - let new_value = new_object.get(key).unwrap(); - - if old_value != new_value { - match new_value { - Value::Bool(_) | Value::Number(_) | Value::String(_) => { - let mut key_path = current_key_path.clone(); - key_path.push(key); - write_settings_key(json_text, &key_path, &new_value); - } - Value::Object(new_sub_object) => { - let mut key_path = current_key_path.clone(); - key_path.push(key); - if let Value::Object(old_sub_object) = old_value { - apply_changes_to_json_text( - old_sub_object, - new_sub_object, - key_path, - json_text, - ); - } else { - unimplemented!("This function doesn't support changing values from simple values to objects yet"); - } - } - Value::Null | Value::Array(_) => { - unimplemented!("We only support objects and simple values"); - } - } - } - } - } - - apply_changes_to_json_text(&old_object, &new_object, vec![], &mut text); - - text + let mut key_path = Vec::new(); + let mut edits = Vec::new(); + update_object_in_settings_file( + &old_object, + &new_object, + &text, + &tree, + tab_size.get() as usize, + &mut key_path, + &mut edits, + ); + edits.sort_unstable_by_key(|e| e.0.start); + return edits; } fn to_json_object(settings_file: SettingsFileContent) -> serde_json::Map { @@ -1122,15 +1170,18 @@ mod tests { use super::*; use unindent::Unindent; - fn assert_new_settings, S2: Into>( - old_json: S1, + fn assert_new_settings( + old_json: String, update: fn(&mut SettingsFileContent), - expected_new_json: S2, + expected_new_json: String, ) { - let old_json = old_json.into(); let old_content: SettingsFileContent = serde_json::from_str(&old_json).unwrap_or_default(); - let new_json = update_settings_file(old_json, old_content, update); - pretty_assertions::assert_eq!(new_json, expected_new_json.into()); + let edits = update_settings_file(&old_json, old_content, 4.try_into().unwrap(), update); + let mut new_json = old_json; + for (range, replacement) in edits.into_iter().rev() { + new_json.replace_range(range, &replacement); + } + pretty_assertions::assert_eq!(new_json, expected_new_json); } #[test] @@ -1171,6 +1222,63 @@ mod tests { ); } + #[test] + fn test_update_copilot_globs() { + assert_new_settings( + r#" + { + } + "# + .unindent(), + |settings| { + settings.copilot = Some(CopilotSettingsContent { + disabled_globs: Some(vec![]), + }); + }, + r#" + { + "copilot": { + "disabled_globs": [] + } + } + "# + .unindent(), + ); + + assert_new_settings( + r#" + { + "copilot": { + "disabled_globs": [ + "**/*.json" + ] + } + } + "# + .unindent(), + |settings| { + settings + .copilot + .get_or_insert(Default::default()) + .disabled_globs + .as_mut() + .unwrap() + .push(".env".into()); + }, + r#" + { + "copilot": { + "disabled_globs": [ + "**/*.json", + ".env" + ] + } + } + "# + .unindent(), + ); + } + #[test] fn test_update_copilot() { assert_new_settings( @@ -1354,7 +1462,7 @@ mod tests { #[test] fn test_update_telemetry_setting() { assert_new_settings( - "{}", + "{}".into(), |settings| settings.telemetry.set_diagnostics(true), r#" { @@ -1370,7 +1478,7 @@ mod tests { #[test] fn test_update_object_empty_doc() { assert_new_settings( - "", + "".into(), |settings| settings.telemetry.set_diagnostics(true), r#" { @@ -1423,7 +1531,7 @@ mod tests { #[test] fn write_key_no_document() { assert_new_settings( - "", + "".to_string(), |settings| settings.theme = Some("summerfruit-light".to_string()), r#" { @@ -1437,16 +1545,16 @@ mod tests { #[test] fn test_write_theme_into_single_line_settings_without_theme() { assert_new_settings( - r#"{ "a": "", "ok": true }"#, + r#"{ "a": "", "ok": true }"#.to_string(), |settings| settings.theme = Some("summerfruit-light".to_string()), - r#"{ "theme": "summerfruit-light", "a": "", "ok": true }"#, + r#"{ "theme": "summerfruit-light", "a": "", "ok": true }"#.to_string(), ); } #[test] fn test_write_theme_pre_object_whitespace() { assert_new_settings( - r#" { "a": "", "ok": true }"#, + r#" { "a": "", "ok": true }"#.to_string(), |settings| settings.theme = Some("summerfruit-light".to_string()), r#" { "theme": "summerfruit-light", "a": "", "ok": true }"#.unindent(), ); diff --git a/crates/settings/src/settings_file.rs b/crates/settings/src/settings_file.rs index c261879a58750d1247c7c6f9801f8b0d1fcded19..eb9f1508c2d416cbe5f2af164148bf9d1d202592 100644 --- a/crates/settings/src/settings_file.rs +++ b/crates/settings/src/settings_file.rs @@ -1,9 +1,9 @@ -use crate::{update_settings_file, watched_json::WatchedJsonFile, SettingsFileContent}; +use crate::{update_settings_file, watched_json::WatchedJsonFile, Settings, SettingsFileContent}; use anyhow::Result; use assets::Assets; use fs::Fs; -use gpui::{AppContext, AssetSource}; -use std::{io::ErrorKind, path::Path, sync::Arc}; +use gpui::AppContext; +use std::{io::ErrorKind, ops::Range, path::Path, sync::Arc}; // TODO: Switch SettingsFile to open a worktree and buffer for synchronization // And instant updates in the Zed editor @@ -33,14 +33,7 @@ impl SettingsFile { Err(err) => { if let Some(e) = err.downcast_ref::() { if e.kind() == ErrorKind::NotFound { - return Ok(std::str::from_utf8( - Assets - .load("settings/initial_user_settings.json") - .unwrap() - .as_ref(), - ) - .unwrap() - .to_string()); + return Ok(Settings::initial_user_settings_content(&Assets).to_string()); } } return Err(err); @@ -48,28 +41,39 @@ impl SettingsFile { } } + pub fn update_unsaved( + text: &str, + cx: &AppContext, + update: impl FnOnce(&mut SettingsFileContent), + ) -> Vec<(Range, String)> { + let this = cx.global::(); + let tab_size = cx.global::().tab_size(Some("JSON")); + let current_file_content = this.settings_file_content.current(); + update_settings_file(&text, current_file_content, tab_size, update) + } + pub fn update( cx: &mut AppContext, update: impl 'static + Send + FnOnce(&mut SettingsFileContent), ) { let this = cx.global::(); - + let tab_size = cx.global::().tab_size(Some("JSON")); let current_file_content = this.settings_file_content.current(); - let fs = this.fs.clone(); let path = this.path.clone(); cx.background() .spawn(async move { let old_text = SettingsFile::load_settings(path, &fs).await?; - - let new_text = update_settings_file(old_text, current_file_content, update); - + let edits = update_settings_file(&old_text, current_file_content, tab_size, update); + let mut new_text = old_text; + for (range, replacement) in edits.into_iter().rev() { + new_text.replace_range(range, &replacement); + } fs.atomic_write(path.to_path_buf(), new_text).await?; - - Ok(()) as Result<()> + anyhow::Ok(()) }) - .detach_and_log_err(cx); + .detach_and_log_err(cx) } } diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index bd32522e4db1847ecb08fe35a79c4540cc1d984b..177dc0a292f387fefee9aa5b68c1d0e6e07ac8c3 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -19,6 +19,7 @@ test-support = [ ] [dependencies] +assets = { path = "../assets" } db = { path = "../db" } call = { path = "../call" } client = { path = "../client" } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 1a622babb34d12cee2669103497718fb84822774..64fa9c70629d5e50d7635880dfa5f7fcff04b307 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -14,9 +14,8 @@ pub mod sidebar; mod status_bar; mod toolbar; -pub use smallvec; - use anyhow::{anyhow, Context, Result}; +use assets::Assets; use call::ActiveCall; use client::{ proto::{self, PeerId}, @@ -47,13 +46,14 @@ use gpui::{ ModelHandle, SizeConstraint, Subscription, Task, View, ViewContext, ViewHandle, WeakViewHandle, }; use item::{FollowableItem, FollowableItemHandle, Item, ItemHandle, ProjectItem}; -use language::LanguageRegistry; +use language::{LanguageRegistry, Rope}; use std::{ any::TypeId, borrow::Cow, cmp, env, future::Future, path::{Path, PathBuf}, + str, sync::Arc, time::Duration, }; @@ -82,7 +82,7 @@ use status_bar::StatusBar; pub use status_bar::StatusItemView; use theme::{Theme, ThemeRegistry}; pub use toolbar::{ToolbarItemLocation, ToolbarItemView}; -use util::ResultExt; +use util::{paths, ResultExt}; lazy_static! { static ref ZED_WINDOW_SIZE: Option = env::var("ZED_WINDOW_SIZE") @@ -126,6 +126,8 @@ actions!( ] ); +actions!(zed, [OpenSettings]); + #[derive(Clone, PartialEq)] pub struct OpenPaths { pub paths: Vec, @@ -314,6 +316,18 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { .detach(); }); + cx.add_action({ + let app_state = app_state.clone(); + move |_: &mut Workspace, _: &OpenSettings, cx: &mut ViewContext| { + create_and_open_local_file(&paths::SETTINGS, app_state.clone(), cx, || { + Settings::initial_user_settings_content(&Assets) + .as_ref() + .into() + }) + .detach_and_log_err(cx); + } + }); + let client = &app_state.client; client.add_view_request_handler(Workspace::handle_follow); client.add_view_message_handler(Workspace::handle_unfollow); @@ -2981,6 +2995,34 @@ pub fn open_new( }) } +pub fn create_and_open_local_file( + path: &'static Path, + app_state: Arc, + cx: &mut ViewContext, + default_content: impl 'static + Send + FnOnce() -> Rope, +) -> Task>> { + cx.spawn(|workspace, mut cx| async move { + let fs = &app_state.fs; + if !fs.is_file(path).await { + fs.create_file(path, Default::default()).await?; + fs.save(path, &default_content(), Default::default()) + .await?; + } + + let mut items = workspace + .update(&mut cx, |workspace, cx| { + workspace.with_local_workspace(&app_state, cx, |workspace, cx| { + workspace.open_paths(vec![path.to_path_buf()], false, cx) + }) + })? + .await? + .await; + + let item = items.pop().flatten(); + item.ok_or_else(|| anyhow!("path {path:?} is not a file"))? + }) +} + pub fn join_remote_project( project_id: u64, follow_user_id: u64, diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 679fd39e2f9455d461b946507bd800c23110bf27..771775de57b20ed63ee60a6b8724066ef3e0fc3c 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -44,9 +44,9 @@ use theme::ThemeRegistry; use util::{channel::RELEASE_CHANNEL, paths, ResultExt, TryFutureExt}; use workspace::{ self, dock::FocusDock, item::ItemHandle, notifications::NotifyResultExt, AppState, NewFile, - Workspace, + OpenSettings, Workspace, }; -use zed::{self, build_window_options, initialize_workspace, languages, menus, OpenSettings}; +use zed::{self, build_window_options, initialize_workspace, languages, menus}; fn main() { let http = http::client(); diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index e0a2f7473193582b5c3e6e9eb19ec697e625cda9..741b61f323d13cf3682a1c5c9460fb2f97a16661 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -12,7 +12,7 @@ pub fn menus() -> Vec> { MenuItem::submenu(Menu { name: "Preferences", items: vec![ - MenuItem::action("Open Settings", super::OpenSettings), + MenuItem::action("Open Settings", workspace::OpenSettings), MenuItem::action("Open Key Bindings", super::OpenKeymap), MenuItem::action("Open Default Settings", super::OpenDefaultSettings), MenuItem::action("Open Default Key Bindings", super::OpenDefaultKeymap), diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 494739a9678f936fbb486aef4292321e3e4f5390..4478a88837c02e49a43e83f3f52d63f1e505789d 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -20,22 +20,21 @@ use gpui::{ geometry::vector::vec2f, impl_actions, platform::{Platform, PromptLevel, TitlebarOptions, WindowBounds, WindowKind, WindowOptions}, - AssetSource, ViewContext, + ViewContext, }; -use language::Rope; pub use lsp; pub use project; use project_panel::ProjectPanel; use search::{BufferSearchBar, ProjectSearchBar}; use serde::Deserialize; use serde_json::to_string_pretty; -use settings::Settings; -use std::{borrow::Cow, env, path::Path, str, sync::Arc}; +use settings::{Settings, DEFAULT_SETTINGS_ASSET_PATH}; +use std::{borrow::Cow, str, sync::Arc}; use terminal_view::terminal_button::TerminalButton; use util::{channel::ReleaseChannel, paths, ResultExt}; use uuid::Uuid; pub use workspace; -use workspace::{sidebar::SidebarSide, AppState, Restart, Workspace}; +use workspace::{create_and_open_local_file, sidebar::SidebarSide, AppState, Restart, Workspace}; #[derive(Deserialize, Clone, PartialEq)] pub struct OpenBrowser { @@ -56,7 +55,6 @@ actions!( ToggleFullScreen, Quit, DebugElements, - OpenSettings, OpenLog, OpenLicenses, OpenTelemetryLog, @@ -148,21 +146,6 @@ pub fn init(app_state: &Arc, cx: &mut gpui::AppContext) { }) .detach_and_log_err(cx); }); - cx.add_action({ - let app_state = app_state.clone(); - move |_: &mut Workspace, _: &OpenSettings, cx: &mut ViewContext| { - open_config_file(&paths::SETTINGS, app_state.clone(), cx, || { - str::from_utf8( - Assets - .load("settings/initial_user_settings.json") - .unwrap() - .as_ref(), - ) - .unwrap() - .into() - }); - } - }); cx.add_action({ let app_state = app_state.clone(); move |workspace: &mut Workspace, _: &OpenLog, cx: &mut ViewContext| { @@ -190,7 +173,8 @@ pub fn init(app_state: &Arc, cx: &mut gpui::AppContext) { cx.add_action({ let app_state = app_state.clone(); move |_: &mut Workspace, _: &OpenKeymap, cx: &mut ViewContext| { - open_config_file(&paths::KEYMAP, app_state.clone(), cx, Default::default); + create_and_open_local_file(&paths::KEYMAP, app_state.clone(), cx, Default::default) + .detach_and_log_err(cx); } }); cx.add_action({ @@ -210,7 +194,7 @@ pub fn init(app_state: &Arc, cx: &mut gpui::AppContext) { move |_: &mut Workspace, _: &OpenDefaultSettings, cx: &mut ViewContext| { open_bundled_file( app_state.clone(), - "settings/default.json", + DEFAULT_SETTINGS_ASSET_PATH, "Default Settings", "JSON", cx, @@ -316,7 +300,7 @@ pub fn initialize_workspace( }); let toggle_terminal = cx.add_view(|cx| TerminalButton::new(workspace_handle.clone(), cx)); - let copilot = cx.add_view(|cx| copilot_button::CopilotButton::new(cx)); + let copilot = cx.add_view(|cx| copilot_button::CopilotButton::new(app_state.clone(), cx)); let diagnostic_summary = cx.add_view(|cx| diagnostics::items::DiagnosticIndicator::new(workspace.project(), cx)); let activity_indicator = @@ -480,33 +464,6 @@ fn about(_: &mut Workspace, _: &About, cx: &mut gpui::ViewContext) { cx.prompt(PromptLevel::Info, &format!("{app_name} {version}"), &["OK"]); } -fn open_config_file( - path: &'static Path, - app_state: Arc, - cx: &mut ViewContext, - default_content: impl 'static + Send + FnOnce() -> Rope, -) { - cx.spawn(|workspace, mut cx| async move { - let fs = &app_state.fs; - if !fs.is_file(path).await { - fs.create_file(path, Default::default()).await?; - fs.save(path, &default_content(), Default::default()) - .await?; - } - - workspace - .update(&mut cx, |workspace, cx| { - workspace.with_local_workspace(&app_state, cx, |workspace, cx| { - workspace.open_paths(vec![path.to_path_buf()], false, cx) - }) - })? - .await? - .await; - Ok::<_, anyhow::Error>(()) - }) - .detach_and_log_err(cx) -} - fn open_log_file( workspace: &mut Workspace, app_state: Arc, From 653ea3a85dd536baf380b1cfef121181209c745e Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Wed, 3 May 2023 14:38:41 -0400 Subject: [PATCH 3/7] v0.86.x dev --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e2b29c7aeb5f394ade28bb86184d3a099b4f69c1..5738f9ab0568fecd4914d2a522bf1929151f3590 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8546,7 +8546,7 @@ checksum = "09041cd90cf85f7f8b2df60c646f853b7f535ce68f85244eb6731cf89fa498ec" [[package]] name = "zed" -version = "0.85.0" +version = "0.86.0" dependencies = [ "activity_indicator", "anyhow", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 7157404640acc2012a31dda8aea32162d7c29132..141af8ee3745e03289eee27135a453f98984e78b 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] description = "The fast, collaborative code editor." edition = "2021" name = "zed" -version = "0.85.0" +version = "0.86.0" publish = false [lib] From 053b34875b73088dc2a1374981a664831505d4e2 Mon Sep 17 00:00:00 2001 From: Joseph Lyons Date: Wed, 3 May 2023 14:59:04 -0400 Subject: [PATCH 4/7] collab 0.11.0 --- Cargo.lock | 2 +- crates/collab/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5738f9ab0568fecd4914d2a522bf1929151f3590..9c6ec6046c581bbbeeb4886345441a4d4d1fbc9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1189,7 +1189,7 @@ dependencies = [ [[package]] name = "collab" -version = "0.10.0" +version = "0.11.0" dependencies = [ "anyhow", "async-tungstenite", diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 2a209b29736ac796c1e9364b57b49911bdf54853..9a8af636b03044419a7e3cfeda38c9af9f577988 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] default-run = "collab" edition = "2021" name = "collab" -version = "0.10.0" +version = "0.11.0" publish = false [[bin]] From 18e39ef2fa3a20493fc9827641df4f0b9d170ed7 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 10:35:13 +0200 Subject: [PATCH 5/7] Cache view's type id and keymap context into a separate map During `layout`, we now pass a mutable reference to the element's parent view. This is a recursive process that causes the view to be removed from `AppContext` and then re-inserted back into it once the layout is complete. As such, querying parent views during `layout` does not work as such views will have been removed from `AppContext` and not yet re-inserted into it. This caused a bug in `KeystrokeLabel`, which used the `keystrokes_for_action` method to query its ancestors to determine their type id and keymap context. Now, any time a view notifies, we will cache its keymap context so that we don't need to query the parent view during `layout`. --- crates/gpui/src/app.rs | 39 +++++++++++++++++++++++------------ crates/gpui/src/app/window.rs | 16 ++++++++++---- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 4d84f7c070fd179ec3220054b1f3fff2041ed3e5..f44012685be3880d5e2139f94ed8f224c852337b 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -440,6 +440,7 @@ type WindowShouldCloseSubscriptionCallback = Box b pub struct AppContext { models: HashMap>, views: HashMap<(usize, usize), Box>, + views_metadata: HashMap<(usize, usize), ViewMetadata>, pub(crate) parents: HashMap<(usize, usize), ParentId>, windows: HashMap, globals: HashMap>, @@ -502,6 +503,7 @@ impl AppContext { Self { models: Default::default(), views: Default::default(), + views_metadata: Default::default(), parents: Default::default(), windows: Default::default(), globals: Default::default(), @@ -727,9 +729,9 @@ impl AppContext { } pub fn view_type_id(&self, window_id: usize, view_id: usize) -> Option { - self.views + self.views_metadata .get(&(window_id, view_id)) - .map(|view| view.as_any().type_id()) + .map(|metadata| metadata.type_id) } pub fn active_labeled_tasks<'a>( @@ -1045,9 +1047,10 @@ impl AppContext { .read_window(window_id, |cx| { if let Some(focused_view_id) = cx.focused_view_id() { for view_id in cx.ancestors(focused_view_id) { - if let Some(view) = cx.views.get(&(window_id, view_id)) { - let view_type = view.as_any().type_id(); - if let Some(actions) = cx.actions.get(&view_type) { + if let Some(view_metadata) = + cx.views_metadata.get(&(window_id, view_id)) + { + if let Some(actions) = cx.actions.get(&view_metadata.type_id) { if actions.contains_key(&action_type) { return true; } @@ -1448,6 +1451,7 @@ impl AppContext { for (window_id, view_id) in dropped_views { self.subscriptions.remove(view_id); self.observations.remove(view_id); + self.views_metadata.remove(&(window_id, view_id)); let mut view = self.views.remove(&(window_id, view_id)).unwrap(); view.release(self); let change_focus_to = self.windows.get_mut(&window_id).and_then(|window| { @@ -1779,9 +1783,11 @@ impl AppContext { observed_window_id: usize, observed_view_id: usize, ) { - if self + let view_key = (observed_window_id, observed_view_id); + if let Some((view, mut view_metadata)) = self .views - .contains_key(&(observed_window_id, observed_view_id)) + .remove(&view_key) + .zip(self.views_metadata.remove(&view_key)) { if let Some(window) = self.windows.get_mut(&observed_window_id) { window @@ -1791,6 +1797,10 @@ impl AppContext { .insert(observed_view_id); } + view_metadata.keymap_context = view.keymap_context(self); + self.views.insert(view_key, view); + self.views_metadata.insert(view_key, view_metadata); + let mut observations = self.observations.clone(); observations.emit(observed_view_id, |callback| callback(self)); } @@ -2037,6 +2047,11 @@ pub enum ParentId { Root, } +struct ViewMetadata { + type_id: TypeId, + keymap_context: KeymapContext, +} + #[derive(Default, Clone)] pub struct WindowInvalidation { pub updated: HashSet, @@ -2437,11 +2452,10 @@ where cx.handle().into_any() } else { let focused_type = cx - .views + .views_metadata .get(&(cx.window_id, focused_id)) .unwrap() - .as_any() - .type_id(); + .type_id; AnyViewHandle::new( cx.window_id, focused_id, @@ -2458,11 +2472,10 @@ where cx.handle().into_any() } else { let blurred_type = cx - .views + .views_metadata .get(&(cx.window_id, blurred_id)) .unwrap() - .as_any() - .type_id(); + .type_id; AnyViewHandle::new( cx.window_id, blurred_id, diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 49befafbec4bfca2fac1782f2d5c8d249b24a951..091a2ab1b28b5a9798bcb243ea459d4f8cabcabe 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -34,7 +34,7 @@ use std::{ use util::ResultExt; use uuid::Uuid; -use super::Reference; +use super::{Reference, ViewMetadata}; pub struct Window { pub(crate) root_view: Option, @@ -369,13 +369,13 @@ impl<'a> WindowContext<'a> { let mut contexts = Vec::new(); let mut handler_depth = None; for (i, view_id) in self.ancestors(view_id).enumerate() { - if let Some(view) = self.views.get(&(window_id, view_id)) { - if let Some(actions) = self.actions.get(&view.as_any().type_id()) { + if let Some(view_metadata) = self.views_metadata.get(&(window_id, view_id)) { + if let Some(actions) = self.actions.get(&view_metadata.type_id) { if actions.contains_key(&action.as_any().type_id()) { handler_depth = Some(i); } } - contexts.push(view.keymap_context(self)); + contexts.push(view_metadata.keymap_context.clone()); } } @@ -1177,6 +1177,14 @@ impl<'a> WindowContext<'a> { self.parents.insert((window_id, view_id), parent_id); let mut cx = ViewContext::mutable(self, view_id); let handle = if let Some(view) = build_view(&mut cx) { + let keymap_context = view.keymap_context(cx.app_context()); + self.views_metadata.insert( + (window_id, view_id), + ViewMetadata { + type_id: TypeId::of::(), + keymap_context, + }, + ); self.views.insert((window_id, view_id), Box::new(view)); self.window .invalidation From 3d679ddb26009d5e8afd867ccfde28571f55625b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 12:04:30 +0200 Subject: [PATCH 6/7] Avoid re-allocating `KeymapContext` after every view notification --- crates/collab_ui/src/contact_list.rs | 7 ++-- crates/context_menu/src/context_menu.rs | 7 ++-- crates/editor/src/editor.rs | 26 ++++++++------ crates/gpui/src/app.rs | 25 ++++++------- crates/gpui/src/app/window.rs | 16 ++++----- .../gpui/src/keymap_matcher/keymap_context.rs | 5 +++ crates/picker/src/picker.rs | 7 ++-- crates/project_panel/src/project_panel.rs | 7 ++-- crates/terminal_view/src/terminal_view.rs | 35 +++++++++---------- crates/vim/src/vim.rs | 4 +-- crates/workspace/src/pane.rs | 5 ++- crates/workspace/src/workspace.rs | 5 --- 12 files changed, 74 insertions(+), 75 deletions(-) diff --git a/crates/collab_ui/src/contact_list.rs b/crates/collab_ui/src/contact_list.rs index 87aa41b7a4658af0c171b8d9b39eeb4431c0cc77..452867b8c4dd6d74a8ba23b77120f7472dbde7e1 100644 --- a/crates/collab_ui/src/contact_list.rs +++ b/crates/collab_ui/src/contact_list.rs @@ -1306,10 +1306,9 @@ impl View for ContactList { "ContactList" } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } fn render(&mut self, cx: &mut ViewContext) -> AnyElement { diff --git a/crates/context_menu/src/context_menu.rs b/crates/context_menu/src/context_menu.rs index e0429bd01b6203ad2a75d7709b2d1f0c0c5e1e2e..0a4cd59384b1ee4590fcd2d8fd11df0815a238dc 100644 --- a/crates/context_menu/src/context_menu.rs +++ b/crates/context_menu/src/context_menu.rs @@ -140,10 +140,9 @@ impl View for ContextMenu { "ContextMenu" } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } fn render(&mut self, cx: &mut ViewContext) -> AnyElement { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index be57596584e318145bb3191b374d9f66b813d8ef..641ef4d133a40b11b1605660aa42eaa1510472fe 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1425,13 +1425,19 @@ impl Editor { } } - pub fn set_keymap_context_layer(&mut self, context: KeymapContext) { + pub fn set_keymap_context_layer( + &mut self, + context: KeymapContext, + cx: &mut ViewContext, + ) { self.keymap_context_layers .insert(TypeId::of::(), context); + cx.notify(); } - pub fn remove_keymap_context_layer(&mut self) { + pub fn remove_keymap_context_layer(&mut self, cx: &mut ViewContext) { self.keymap_context_layers.remove(&TypeId::of::()); + cx.notify(); } pub fn set_input_enabled(&mut self, input_enabled: bool) { @@ -7157,28 +7163,26 @@ impl View for Editor { false } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut context = Self::default_keymap_context(); + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); let mode = match self.mode { EditorMode::SingleLine => "single_line", EditorMode::AutoHeight { .. } => "auto_height", EditorMode::Full => "full", }; - context.add_key("mode", mode); + keymap.add_key("mode", mode); if self.pending_rename.is_some() { - context.add_identifier("renaming"); + keymap.add_identifier("renaming"); } match self.context_menu.as_ref() { - Some(ContextMenu::Completions(_)) => context.add_identifier("showing_completions"), - Some(ContextMenu::CodeActions(_)) => context.add_identifier("showing_code_actions"), + Some(ContextMenu::Completions(_)) => keymap.add_identifier("showing_completions"), + Some(ContextMenu::CodeActions(_)) => keymap.add_identifier("showing_code_actions"), None => {} } for layer in self.keymap_context_layers.values() { - context.extend(layer); + keymap.extend(layer); } - - context } fn text_for_range(&self, range_utf16: Range, cx: &AppContext) -> Option { diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index f44012685be3880d5e2139f94ed8f224c852337b..fa538d3e440f71efb3d6f414fd49433bf10a8b1e 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -83,14 +83,15 @@ pub trait View: Entity + Sized { false } - fn keymap_context(&self, _: &AppContext) -> keymap_matcher::KeymapContext { - Self::default_keymap_context() + fn update_keymap_context(&self, keymap: &mut keymap_matcher::KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); } - fn default_keymap_context() -> keymap_matcher::KeymapContext { - let mut cx = keymap_matcher::KeymapContext::default(); - cx.add_identifier(Self::ui_name()); - cx + + fn reset_to_default_keymap_context(keymap: &mut keymap_matcher::KeymapContext) { + keymap.clear(); + keymap.add_identifier(Self::ui_name()); } + fn debug_json(&self, _: &AppContext) -> serde_json::Value { serde_json::Value::Null } @@ -1797,7 +1798,7 @@ impl AppContext { .insert(observed_view_id); } - view_metadata.keymap_context = view.keymap_context(self); + view.update_keymap_context(&mut view_metadata.keymap_context, self); self.views.insert(view_key, view); self.views_metadata.insert(view_key, view_metadata); @@ -2380,7 +2381,7 @@ pub trait AnyView { cx: &mut WindowContext, view_id: usize, ) -> bool; - fn keymap_context(&self, cx: &AppContext) -> KeymapContext; + fn update_keymap_context(&self, keymap: &mut KeymapContext, cx: &AppContext); fn debug_json(&self, cx: &WindowContext) -> serde_json::Value; fn text_for_range(&self, range: Range, cx: &WindowContext) -> Option; @@ -2506,8 +2507,8 @@ where View::modifiers_changed(self, event, &mut cx) } - fn keymap_context(&self, cx: &AppContext) -> KeymapContext { - View::keymap_context(self, cx) + fn update_keymap_context(&self, keymap: &mut KeymapContext, cx: &AppContext) { + View::update_keymap_context(self, keymap, cx) } fn debug_json(&self, cx: &WindowContext) -> serde_json::Value { @@ -5572,8 +5573,8 @@ mod tests { "View" } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - self.keymap_context.clone() + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + *keymap = self.keymap_context.clone(); } } diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 091a2ab1b28b5a9798bcb243ea459d4f8cabcabe..94c4df23850ef7076e5432822a73b7ac9d0bb66e 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -2,7 +2,7 @@ use crate::{ elements::AnyRootElement, geometry::rect::RectF, json::ToJson, - keymap_matcher::{Binding, Keystroke, MatchResult}, + keymap_matcher::{Binding, KeymapContext, Keystroke, MatchResult}, platform::{ self, Appearance, CursorStyle, Event, KeyDownEvent, KeyUpEvent, ModifiersChangedEvent, MouseButton, MouseMovedEvent, PromptLevel, WindowBounds, @@ -406,10 +406,9 @@ impl<'a> WindowContext<'a> { let mut contexts = Vec::new(); let mut handler_depths_by_action_type = HashMap::::default(); for (depth, view_id) in self.ancestors(view_id).enumerate() { - if let Some(view) = self.views.get(&(window_id, view_id)) { - contexts.push(view.keymap_context(self)); - let view_type = view.as_any().type_id(); - if let Some(actions) = self.actions.get(&view_type) { + if let Some(view_metadata) = self.views_metadata.get(&(window_id, view_id)) { + contexts.push(view_metadata.keymap_context.clone()); + if let Some(actions) = self.actions.get(&view_metadata.type_id) { handler_depths_by_action_type.extend( actions .keys() @@ -458,9 +457,9 @@ impl<'a> WindowContext<'a> { let dispatch_path = self .ancestors(focused_view_id) .filter_map(|view_id| { - self.views + self.views_metadata .get(&(window_id, view_id)) - .map(|view| (view_id, view.keymap_context(self))) + .map(|view| (view_id, view.keymap_context.clone())) }) .collect(); @@ -1177,7 +1176,8 @@ impl<'a> WindowContext<'a> { self.parents.insert((window_id, view_id), parent_id); let mut cx = ViewContext::mutable(self, view_id); let handle = if let Some(view) = build_view(&mut cx) { - let keymap_context = view.keymap_context(cx.app_context()); + let mut keymap_context = KeymapContext::default(); + view.update_keymap_context(&mut keymap_context, cx.app_context()); self.views_metadata.insert( (window_id, view_id), ViewMetadata { diff --git a/crates/gpui/src/keymap_matcher/keymap_context.rs b/crates/gpui/src/keymap_matcher/keymap_context.rs index bbf6bfc14bad820cd95c1525c5b673f8b8c04dcc..b1a449edf3ed9e005a0b7a4d4711c7792b3c023c 100644 --- a/crates/gpui/src/keymap_matcher/keymap_context.rs +++ b/crates/gpui/src/keymap_matcher/keymap_context.rs @@ -17,6 +17,11 @@ impl KeymapContext { } } + pub fn clear(&mut self) { + self.set.clear(); + self.map.clear(); + } + pub fn extend(&mut self, other: &Self) { for v in &other.set { self.set.insert(v.clone()); diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 62ffd417fe7b463e77f12804303fd01292c8a26d..01749ccf84d72f5cc5907bf66f7d9c75c4d3d06f 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -126,10 +126,9 @@ impl View for Picker { .into_any_named("picker") } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } fn focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 373417b167047ac80cd2d04aa8d7eb22068fa1df..ec80bd245aef9feaa68252414f1d732431b9fcf6 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1316,10 +1316,9 @@ impl View for ProjectPanel { } } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } } diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 5e04fc982556cc638752cd0a7a4774a27c0d2654..3fcbf3988721a1282eb17672d46ef639d33ae1a0 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -446,11 +446,11 @@ impl View for TerminalView { }); } - fn keymap_context(&self, cx: &gpui::AppContext) -> KeymapContext { - let mut context = Self::default_keymap_context(); + fn update_keymap_context(&self, keymap: &mut KeymapContext, cx: &gpui::AppContext) { + Self::reset_to_default_keymap_context(keymap); let mode = self.terminal.read(cx).last_content.mode; - context.add_key( + keymap.add_key( "screen", if mode.contains(TermMode::ALT_SCREEN) { "alt" @@ -460,40 +460,40 @@ impl View for TerminalView { ); if mode.contains(TermMode::APP_CURSOR) { - context.add_identifier("DECCKM"); + keymap.add_identifier("DECCKM"); } if mode.contains(TermMode::APP_KEYPAD) { - context.add_identifier("DECPAM"); + keymap.add_identifier("DECPAM"); } else { - context.add_identifier("DECPNM"); + keymap.add_identifier("DECPNM"); } if mode.contains(TermMode::SHOW_CURSOR) { - context.add_identifier("DECTCEM"); + keymap.add_identifier("DECTCEM"); } if mode.contains(TermMode::LINE_WRAP) { - context.add_identifier("DECAWM"); + keymap.add_identifier("DECAWM"); } if mode.contains(TermMode::ORIGIN) { - context.add_identifier("DECOM"); + keymap.add_identifier("DECOM"); } if mode.contains(TermMode::INSERT) { - context.add_identifier("IRM"); + keymap.add_identifier("IRM"); } //LNM is apparently the name for this. https://vt100.net/docs/vt510-rm/LNM.html if mode.contains(TermMode::LINE_FEED_NEW_LINE) { - context.add_identifier("LNM"); + keymap.add_identifier("LNM"); } if mode.contains(TermMode::FOCUS_IN_OUT) { - context.add_identifier("report_focus"); + keymap.add_identifier("report_focus"); } if mode.contains(TermMode::ALTERNATE_SCROLL) { - context.add_identifier("alternate_scroll"); + keymap.add_identifier("alternate_scroll"); } if mode.contains(TermMode::BRACKETED_PASTE) { - context.add_identifier("bracketed_paste"); + keymap.add_identifier("bracketed_paste"); } if mode.intersects(TermMode::MOUSE_MODE) { - context.add_identifier("any_mouse_reporting"); + keymap.add_identifier("any_mouse_reporting"); } { let mouse_reporting = if mode.contains(TermMode::MOUSE_REPORT_CLICK) { @@ -505,7 +505,7 @@ impl View for TerminalView { } else { "off" }; - context.add_key("mouse_reporting", mouse_reporting); + keymap.add_key("mouse_reporting", mouse_reporting); } { let format = if mode.contains(TermMode::SGR_MOUSE) { @@ -515,9 +515,8 @@ impl View for TerminalView { } else { "normal" }; - context.add_key("mouse_format", format); + keymap.add_key("mouse_format", format); } - context } } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 7ce925944cede7325b9c2b6248250f895d12cc99..a0a12210ec36d73a6fd468331622231f2b7802c0 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -309,7 +309,7 @@ impl Vim { editor.set_input_enabled(!state.vim_controlled()); editor.selections.line_mode = matches!(state.mode, Mode::Visual { line: true }); let context_layer = state.keymap_context_layer(); - editor.set_keymap_context_layer::(context_layer); + editor.set_keymap_context_layer::(context_layer, cx); } else { Self::unhook_vim_settings(editor, cx); } @@ -321,7 +321,7 @@ impl Vim { editor.set_clip_at_line_ends(false, cx); editor.set_input_enabled(true); editor.selections.line_mode = false; - editor.remove_keymap_context_layer::(); + editor.remove_keymap_context_layer::(cx); } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 8bd42fed045ec80f51ea7b9432f588dcef6ca2a6..3d1e477941f2d40d5983b2f4cf2f4734b784260a 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1831,12 +1831,11 @@ impl View for Pane { }); } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut keymap = Self::default_keymap_context(); + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); if self.docked.is_some() { keymap.add_identifier("docked"); } - keymap } } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index bb2629862d3aeceb8c0e3c18895d355e6df6ce51..e1007043dcfc91d0f66204782b2b8cfa158ea76e 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -37,7 +37,6 @@ use gpui::{ vector::{vec2f, Vector2F}, }, impl_actions, - keymap_matcher::KeymapContext, platform::{ CursorStyle, MouseButton, PathPromptOptions, Platform, PromptLevel, WindowBounds, WindowOptions, @@ -2809,10 +2808,6 @@ impl View for Workspace { } } } - - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - Self::default_keymap_context() - } } impl ViewId { From 5cc6304fa607744c9f1f0e352aebe5ed95b609ac Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 12:09:32 +0200 Subject: [PATCH 7/7] Verify keystrokes can be queried while views are on the stack --- crates/gpui/src/app.rs | 114 ++++++++++++++++++++++------------------- 1 file changed, 60 insertions(+), 54 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index fa538d3e440f71efb3d6f414fd49433bf10a8b1e..caebc2714a5fd66920c3b3efe1f9ad2112f64bc7 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -5678,7 +5678,7 @@ mod tests { } #[crate::test(self)] - fn test_keystrokes_for_action(cx: &mut AppContext) { + fn test_keystrokes_for_action(cx: &mut TestAppContext) { actions!(test, [Action1, Action2, GlobalAction]); struct View1 {} @@ -5708,70 +5708,76 @@ mod tests { } } - let (window_id, view_1) = cx.add_window(Default::default(), |_| View1 {}); + let (_, view_1) = cx.add_window(|_| View1 {}); let view_2 = cx.add_view(&view_1, |cx| { cx.focus_self(); View2 {} }); - cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); - cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); - cx.add_global_action(|_: &GlobalAction, _| {}); - - cx.add_bindings(vec![ - Binding::new("a", Action1, Some("View1")), - Binding::new("b", Action2, Some("View1 > View2")), - Binding::new("c", GlobalAction, Some("View3")), // View 3 does not exist - ]); + cx.update(|cx| { + cx.add_action(|_: &mut View1, _: &Action1, _cx| {}); + cx.add_action(|_: &mut View2, _: &Action2, _cx| {}); + cx.add_global_action(|_: &GlobalAction, _| {}); + + cx.add_bindings(vec![ + Binding::new("a", Action1, Some("View1")), + Binding::new("b", Action2, Some("View1 > View2")), + Binding::new("c", GlobalAction, Some("View3")), // View 3 does not exist + ]); + }); - cx.update_window(window_id, |cx| { - // Sanity check - assert_eq!( - cx.keystrokes_for_action(view_1.id(), &Action1) - .unwrap() - .as_slice(), - &[Keystroke::parse("a").unwrap()] - ); - assert_eq!( - cx.keystrokes_for_action(view_2.id(), &Action2) - .unwrap() - .as_slice(), - &[Keystroke::parse("b").unwrap()] - ); + // Here we update the views to ensure that, even if they are on the stack, + // we can still retrieve keystrokes correctly. + view_1.update(cx, |_, cx| { + view_2.update(cx, |_, cx| { + // Sanity check + assert_eq!( + cx.keystrokes_for_action(view_1.id(), &Action1) + .unwrap() + .as_slice(), + &[Keystroke::parse("a").unwrap()] + ); + assert_eq!( + cx.keystrokes_for_action(view_2.id(), &Action2) + .unwrap() + .as_slice(), + &[Keystroke::parse("b").unwrap()] + ); - // The 'a' keystroke propagates up the view tree from view_2 - // to view_1. The action, Action1, is handled by view_1. - assert_eq!( - cx.keystrokes_for_action(view_2.id(), &Action1) - .unwrap() - .as_slice(), - &[Keystroke::parse("a").unwrap()] - ); + // The 'a' keystroke propagates up the view tree from view_2 + // to view_1. The action, Action1, is handled by view_1. + assert_eq!( + cx.keystrokes_for_action(view_2.id(), &Action1) + .unwrap() + .as_slice(), + &[Keystroke::parse("a").unwrap()] + ); - // Actions that are handled below the current view don't have bindings - assert_eq!(cx.keystrokes_for_action(view_1.id(), &Action2), None); + // Actions that are handled below the current view don't have bindings + assert_eq!(cx.keystrokes_for_action(view_1.id(), &Action2), None); - // Actions that are handled in other branches of the tree should not have a binding - assert_eq!(cx.keystrokes_for_action(view_2.id(), &GlobalAction), None); + // Actions that are handled in other branches of the tree should not have a binding + assert_eq!(cx.keystrokes_for_action(view_2.id(), &GlobalAction), None); - // Check that global actions do not have a binding, even if a binding does exist in another view - assert_eq!( - &available_actions(view_1.id(), cx), - &[ - ("test::Action1", vec![Keystroke::parse("a").unwrap()]), - ("test::GlobalAction", vec![]) - ], - ); + // Check that global actions do not have a binding, even if a binding does exist in another view + assert_eq!( + &available_actions(view_1.id(), cx), + &[ + ("test::Action1", vec![Keystroke::parse("a").unwrap()]), + ("test::GlobalAction", vec![]) + ], + ); - // Check that view 1 actions and bindings are available even when called from view 2 - assert_eq!( - &available_actions(view_2.id(), cx), - &[ - ("test::Action1", vec![Keystroke::parse("a").unwrap()]), - ("test::Action2", vec![Keystroke::parse("b").unwrap()]), - ("test::GlobalAction", vec![]), - ], - ); + // Check that view 1 actions and bindings are available even when called from view 2 + assert_eq!( + &available_actions(view_2.id(), cx), + &[ + ("test::Action1", vec![Keystroke::parse("a").unwrap()]), + ("test::Action2", vec![Keystroke::parse("b").unwrap()]), + ("test::GlobalAction", vec![]), + ], + ); + }); }); // Produces a list of actions and key bindings