diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 98b70ad834..6987022f7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -748,7 +748,7 @@ jobs: timeout-minutes: 120 name: Create a Windows installer runs-on: [self-hosted, Windows, X64] - if: ${{ startsWith(github.ref, 'refs/tags/v') || contains(github.event.pull_request.labels.*.name, 'run-bundling') }} + if: false && (startsWith(github.ref, 'refs/tags/v') || contains(github.event.pull_request.labels.*.name, 'run-bundling')) needs: [windows_tests] env: AZURE_TENANT_ID: ${{ secrets.AZURE_SIGNING_TENANT_ID }} @@ -787,7 +787,7 @@ jobs: - name: Upload Artifacts to release uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844 # v1 # Re-enable when we are ready to publish windows preview releases - if: false && ${{ !(contains(github.event.pull_request.labels.*.name, 'run-bundling')) && env.RELEASE_CHANNEL == 'preview' }} # upload only preview + if: ${{ !(contains(github.event.pull_request.labels.*.name, 'run-bundling')) && env.RELEASE_CHANNEL == 'preview' }} # upload only preview with: draft: true prerelease: ${{ env.RELEASE_CHANNEL == 'preview' }} diff --git a/Cargo.lock b/Cargo.lock index 3950871688..119940fe87 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2148,7 +2148,7 @@ dependencies = [ [[package]] name = "blade-graphics" version = "0.6.0" -source = "git+https://github.com/kvark/blade?rev=416375211bb0b5826b3584dccdb6a43369e499ad#416375211bb0b5826b3584dccdb6a43369e499ad" +source = "git+https://github.com/kvark/blade?rev=e0ec4e720957edd51b945b64dd85605ea54bcfe5#e0ec4e720957edd51b945b64dd85605ea54bcfe5" dependencies = [ "ash", "ash-window", @@ -2181,7 +2181,7 @@ dependencies = [ [[package]] name = "blade-macros" version = "0.3.0" -source = "git+https://github.com/kvark/blade?rev=416375211bb0b5826b3584dccdb6a43369e499ad#416375211bb0b5826b3584dccdb6a43369e499ad" +source = "git+https://github.com/kvark/blade?rev=e0ec4e720957edd51b945b64dd85605ea54bcfe5#e0ec4e720957edd51b945b64dd85605ea54bcfe5" dependencies = [ "proc-macro2", "quote", @@ -2191,7 +2191,7 @@ dependencies = [ [[package]] name = "blade-util" version = "0.2.0" -source = "git+https://github.com/kvark/blade?rev=416375211bb0b5826b3584dccdb6a43369e499ad#416375211bb0b5826b3584dccdb6a43369e499ad" +source = "git+https://github.com/kvark/blade?rev=e0ec4e720957edd51b945b64dd85605ea54bcfe5#e0ec4e720957edd51b945b64dd85605ea54bcfe5" dependencies = [ "blade-graphics", "bytemuck", @@ -14709,6 +14709,7 @@ dependencies = [ "fs", "fuzzy", "gpui", + "itertools 0.14.0", "language", "log", "menu", @@ -14720,6 +14721,8 @@ dependencies = [ "serde", "serde_json", "settings", + "telemetry", + "tempfile", "theme", "tree-sitter-json", "tree-sitter-rust", @@ -16451,6 +16454,7 @@ dependencies = [ "schemars", "serde", "settings", + "settings_ui", "smallvec", "story", "telemetry", @@ -20095,7 +20099,7 @@ dependencies = [ [[package]] name = "zed" -version = "0.196.0" +version = "0.196.7" dependencies = [ "activity_indicator", "agent", diff --git a/Cargo.toml b/Cargo.toml index afb47c006e..a1bc1bed5d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -434,9 +434,9 @@ aws-smithy-runtime-api = { version = "1.7.4", features = ["http-1x", "client"] } aws-smithy-types = { version = "1.3.0", features = ["http-body-1-x"] } base64 = "0.22" bitflags = "2.6.0" -blade-graphics = { git = "https://github.com/kvark/blade", rev = "416375211bb0b5826b3584dccdb6a43369e499ad" } -blade-macros = { git = "https://github.com/kvark/blade", rev = "416375211bb0b5826b3584dccdb6a43369e499ad" } -blade-util = { git = "https://github.com/kvark/blade", rev = "416375211bb0b5826b3584dccdb6a43369e499ad" } +blade-graphics = { git = "https://github.com/kvark/blade", rev = "e0ec4e720957edd51b945b64dd85605ea54bcfe5" } +blade-macros = { git = "https://github.com/kvark/blade", rev = "e0ec4e720957edd51b945b64dd85605ea54bcfe5" } +blade-util = { git = "https://github.com/kvark/blade", rev = "e0ec4e720957edd51b945b64dd85605ea54bcfe5" } blake3 = "1.5.3" bytes = "1.0" cargo_metadata = "0.19" @@ -489,7 +489,7 @@ json_dotpath = "1.1" jsonschema = "0.30.0" jsonwebtoken = "9.3" jupyter-protocol = { git = "https://github.com/ConradIrwin/runtimed", rev = "7130c804216b6914355d15d0b91ea91f6babd734" } -jupyter-websocket-client = { git = "https://github.com/ConradIrwin/runtimed", rev = "7130c804216b6914355d15d0b91ea91f6babd734" } +jupyter-websocket-client = { git = "https://github.com/ConradIrwin/runtimed" ,rev = "7130c804216b6914355d15d0b91ea91f6babd734" } libc = "0.2" libsqlite3-sys = { version = "0.30.1", features = ["bundled"] } linkify = "0.10.0" @@ -500,7 +500,7 @@ metal = "0.29" moka = { version = "0.12.10", features = ["sync"] } naga = { version = "25.0", features = ["wgsl-in"] } nanoid = "0.4" -nbformat = { git = "https://github.com/ConradIrwin/runtimed", rev = "7130c804216b6914355d15d0b91ea91f6babd734" } +nbformat = { git = "https://github.com/ConradIrwin/runtimed", rev = "7130c804216b6914355d15d0b91ea91f6babd734" } nix = "0.29" num-format = "0.4.4" objc = "0.2" @@ -541,7 +541,7 @@ reqwest = { git = "https://github.com/zed-industries/reqwest.git", rev = "951c77 "stream", ] } rsa = "0.9.6" -runtimelib = { git = "https://github.com/ConradIrwin/runtimed", rev = "7130c804216b6914355d15d0b91ea91f6babd734", default-features = false, features = [ +runtimelib = { git = "https://github.com/ConradIrwin/runtimed", rev = "7130c804216b6914355d15d0b91ea91f6babd734", default-features = false, features = [ "async-dispatcher-runtime", ] } rust-embed = { version = "8.4", features = ["include-exclude"] } diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index da4d79eca1..567cd70cbe 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1118,7 +1118,12 @@ "ctrl-f": "search::FocusSearch", "alt-find": "keymap_editor::ToggleKeystrokeSearch", "alt-ctrl-f": "keymap_editor::ToggleKeystrokeSearch", - "alt-c": "keymap_editor::ToggleConflictFilter" + "alt-c": "keymap_editor::ToggleConflictFilter", + "enter": "keymap_editor::EditBinding", + "alt-enter": "keymap_editor::CreateBinding", + "ctrl-c": "keymap_editor::CopyAction", + "ctrl-shift-c": "keymap_editor::CopyContext", + "ctrl-t": "keymap_editor::ShowMatchingKeybinds" } }, { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 962760098b..eaa1810f94 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1216,8 +1216,14 @@ "context": "KeymapEditor", "use_key_equivalents": true, "bindings": { + "cmd-f": "search::FocusSearch", "cmd-alt-f": "keymap_editor::ToggleKeystrokeSearch", - "cmd-alt-c": "keymap_editor::ToggleConflictFilter" + "cmd-alt-c": "keymap_editor::ToggleConflictFilter", + "enter": "keymap_editor::EditBinding", + "alt-enter": "keymap_editor::CreateBinding", + "cmd-c": "keymap_editor::CopyAction", + "cmd-shift-c": "keymap_editor::CopyContext", + "cmd-t": "keymap_editor::ShowMatchingKeybinds" } }, { diff --git a/assets/settings/default.json b/assets/settings/default.json index 32d4c496c1..f8402c03f4 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -817,7 +817,7 @@ "edit_file": true, "fetch": true, "list_directory": true, - "project_notifications": true, + "project_notifications": false, "move_path": true, "now": true, "find_path": true, @@ -837,7 +837,7 @@ "diagnostics": true, "fetch": true, "list_directory": true, - "project_notifications": true, + "project_notifications": false, "now": true, "find_path": true, "read_file": true, diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index d46dada270..d90a450364 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -51,7 +51,7 @@ use util::{ResultExt as _, debug_panic, post_inc}; use uuid::Uuid; use zed_llm_client::{CompletionIntent, CompletionRequestStatus, UsageLimit}; -const MAX_RETRY_ATTEMPTS: u8 = 3; +const MAX_RETRY_ATTEMPTS: u8 = 4; const BASE_RETRY_DELAY: Duration = Duration::from_secs(5); #[derive(Debug, Clone)] @@ -396,6 +396,7 @@ pub struct Thread { remaining_turns: u32, configured_model: Option, profile: AgentProfile, + last_error_context: Option<(Arc, CompletionIntent)>, } #[derive(Clone, Debug)] @@ -489,10 +490,11 @@ impl Thread { retry_state: None, message_feedback: HashMap::default(), last_auto_capture_at: None, + last_error_context: None, last_received_chunk_at: None, request_callback: None, remaining_turns: u32::MAX, - configured_model, + configured_model: configured_model.clone(), profile: AgentProfile::new(profile_id, tools), } } @@ -613,6 +615,7 @@ impl Thread { feedback: None, message_feedback: HashMap::default(), last_auto_capture_at: None, + last_error_context: None, last_received_chunk_at: None, request_callback: None, remaining_turns: u32::MAX, @@ -1264,9 +1267,58 @@ impl Thread { self.flush_notifications(model.clone(), intent, cx); - let request = self.to_completion_request(model.clone(), intent, cx); + let _checkpoint = self.finalize_pending_checkpoint(cx); + self.stream_completion( + self.to_completion_request(model.clone(), intent, cx), + model, + intent, + window, + cx, + ); + } - self.stream_completion(request, model, intent, window, cx); + pub fn retry_last_completion( + &mut self, + window: Option, + cx: &mut Context, + ) { + // Clear any existing error state + self.retry_state = None; + + // Use the last error context if available, otherwise fall back to configured model + let (model, intent) = if let Some((model, intent)) = self.last_error_context.take() { + (model, intent) + } else if let Some(configured_model) = self.configured_model.as_ref() { + let model = configured_model.model.clone(); + let intent = if self.has_pending_tool_uses() { + CompletionIntent::ToolResults + } else { + CompletionIntent::UserPrompt + }; + (model, intent) + } else if let Some(configured_model) = self.get_or_init_configured_model(cx) { + let model = configured_model.model.clone(); + let intent = if self.has_pending_tool_uses() { + CompletionIntent::ToolResults + } else { + CompletionIntent::UserPrompt + }; + (model, intent) + } else { + return; + }; + + self.send_to_model(model, intent, window, cx); + } + + pub fn enable_burn_mode_and_retry( + &mut self, + window: Option, + cx: &mut Context, + ) { + self.completion_mode = CompletionMode::Burn; + cx.emit(ThreadEvent::ProfileChanged); + self.retry_last_completion(window, cx); } pub fn used_tools_since_last_user_message(&self) -> bool { @@ -1987,6 +2039,12 @@ impl Thread { if let Some(retry_strategy) = Thread::get_retry_strategy(completion_error) { + log::info!( + "Retrying with {:?} for language model completion error {:?}", + retry_strategy, + completion_error + ); + retry_scheduled = thread .handle_retryable_error_with_delay( &completion_error, @@ -2130,8 +2188,8 @@ impl Thread { // General strategy here: // - If retrying won't help (e.g. invalid API key or payload too large), return None so we don't retry at all. - // - If it's a time-based issue (e.g. server overloaded, rate limit exceeded), try multiple times with exponential backoff. - // - If it's an issue that *might* be fixed by retrying (e.g. internal server error), just retry once. + // - If it's a time-based issue (e.g. server overloaded, rate limit exceeded), retry up to 4 times with exponential backoff. + // - If it's an issue that *might* be fixed by retrying (e.g. internal server error), retry up to 3 times. match error { HttpResponseError { status_code: StatusCode::TOO_MANY_REQUESTS, @@ -2146,16 +2204,48 @@ impl Thread { max_attempts: MAX_RETRY_ATTEMPTS, }) } + UpstreamProviderError { + status, + retry_after, + .. + } => match *status { + StatusCode::TOO_MANY_REQUESTS | StatusCode::SERVICE_UNAVAILABLE => { + Some(RetryStrategy::Fixed { + delay: retry_after.unwrap_or(BASE_RETRY_DELAY), + max_attempts: MAX_RETRY_ATTEMPTS, + }) + } + StatusCode::INTERNAL_SERVER_ERROR => Some(RetryStrategy::Fixed { + delay: retry_after.unwrap_or(BASE_RETRY_DELAY), + // Internal Server Error could be anything, retry up to 3 times. + max_attempts: 3, + }), + status => { + // There is no StatusCode variant for the unofficial HTTP 529 ("The service is overloaded"), + // but we frequently get them in practice. See https://http.dev/529 + if status.as_u16() == 529 { + Some(RetryStrategy::Fixed { + delay: retry_after.unwrap_or(BASE_RETRY_DELAY), + max_attempts: MAX_RETRY_ATTEMPTS, + }) + } else { + Some(RetryStrategy::Fixed { + delay: retry_after.unwrap_or(BASE_RETRY_DELAY), + max_attempts: 2, + }) + } + } + }, ApiInternalServerError { .. } => Some(RetryStrategy::Fixed { delay: BASE_RETRY_DELAY, - max_attempts: 1, + max_attempts: 3, }), ApiReadResponseError { .. } | HttpSend { .. } | DeserializeResponse { .. } | BadRequestFormat { .. } => Some(RetryStrategy::Fixed { delay: BASE_RETRY_DELAY, - max_attempts: 1, + max_attempts: 3, }), // Retrying these errors definitely shouldn't help. HttpResponseError { @@ -2163,24 +2253,30 @@ impl Thread { StatusCode::PAYLOAD_TOO_LARGE | StatusCode::FORBIDDEN | StatusCode::UNAUTHORIZED, .. } - | SerializeRequest { .. } - | BuildRequestBody { .. } - | PromptTooLarge { .. } | AuthenticationError { .. } | PermissionError { .. } + | NoApiKey { .. } | ApiEndpointNotFound { .. } - | NoApiKey { .. } => None, + | PromptTooLarge { .. } => None, + // These errors might be transient, so retry them + SerializeRequest { .. } | BuildRequestBody { .. } => Some(RetryStrategy::Fixed { + delay: BASE_RETRY_DELAY, + max_attempts: 1, + }), // Retry all other 4xx and 5xx errors once. HttpResponseError { status_code, .. } if status_code.is_client_error() || status_code.is_server_error() => { Some(RetryStrategy::Fixed { delay: BASE_RETRY_DELAY, - max_attempts: 1, + max_attempts: 3, }) } // Conservatively assume that any other errors are non-retryable - HttpResponseError { .. } | Other(..) => None, + HttpResponseError { .. } | Other(..) => Some(RetryStrategy::Fixed { + delay: BASE_RETRY_DELAY, + max_attempts: 2, + }), } } @@ -2193,6 +2289,23 @@ impl Thread { window: Option, cx: &mut Context, ) -> bool { + // Store context for the Retry button + self.last_error_context = Some((model.clone(), intent)); + + // Only auto-retry if Burn Mode is enabled + if self.completion_mode != CompletionMode::Burn { + // Show error with retry options + cx.emit(ThreadEvent::ShowError(ThreadError::RetryableError { + message: format!( + "{}\n\nTo automatically retry when similar errors happen, enable Burn Mode.", + error + ) + .into(), + can_enable_burn_mode: true, + })); + return false; + } + let Some(strategy) = strategy.or_else(|| Self::get_retry_strategy(error)) else { return false; }; @@ -2273,6 +2386,13 @@ impl Thread { // Stop generating since we're giving up on retrying. self.pending_completions.clear(); + // Show error alongside a Retry button, but no + // Enable Burn Mode button (since it's already enabled) + cx.emit(ThreadEvent::ShowError(ThreadError::RetryableError { + message: format!("Failed after retrying: {}", error).into(), + can_enable_burn_mode: false, + })); + false } } @@ -3183,6 +3303,11 @@ pub enum ThreadError { header: SharedString, message: SharedString, }, + #[error("Retryable error: {message}")] + RetryableError { + message: SharedString, + can_enable_burn_mode: bool, + }, } #[derive(Debug, Clone)] @@ -3583,6 +3708,7 @@ fn main() {{ } #[gpui::test] + #[ignore] // turn this test on when project_notifications tool is re-enabled async fn test_stale_buffer_notification(cx: &mut TestAppContext) { init_test_settings(cx); @@ -4137,6 +4263,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create model that returns overloaded error let model = Arc::new(ErrorInjector::new(TestError::Overloaded)); @@ -4210,6 +4341,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create model that returns internal server error let model = Arc::new(ErrorInjector::new(TestError::InternalServerError)); @@ -4231,7 +4367,7 @@ fn main() {{ let retry_state = thread.retry_state.as_ref().unwrap(); assert_eq!(retry_state.attempt, 1, "Should be first retry attempt"); assert_eq!( - retry_state.max_attempts, 1, + retry_state.max_attempts, 3, "Should have correct max attempts" ); }); @@ -4247,8 +4383,9 @@ fn main() {{ if let MessageSegment::Text(text) = seg { text.contains("internal") && text.contains("Fake") - && text.contains("Retrying in") - && !text.contains("attempt") + && text.contains("Retrying") + && text.contains("attempt 1 of 3") + && text.contains("seconds") } else { false } @@ -4286,6 +4423,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create model that returns internal server error let model = Arc::new(ErrorInjector::new(TestError::InternalServerError)); @@ -4338,8 +4480,8 @@ fn main() {{ let retry_state = thread.retry_state.as_ref().unwrap(); assert_eq!(retry_state.attempt, 1, "Should be first retry attempt"); assert_eq!( - retry_state.max_attempts, 1, - "Internal server errors should only retry once" + retry_state.max_attempts, 3, + "Internal server errors should retry up to 3 times" ); }); @@ -4347,7 +4489,15 @@ fn main() {{ cx.executor().advance_clock(BASE_RETRY_DELAY); cx.run_until_parked(); - // Should have scheduled second retry - count retry messages + // Advance clock for second retry + cx.executor().advance_clock(BASE_RETRY_DELAY); + cx.run_until_parked(); + + // Advance clock for third retry + cx.executor().advance_clock(BASE_RETRY_DELAY); + cx.run_until_parked(); + + // Should have completed all retries - count retry messages let retry_count = thread.update(cx, |thread, _| { thread .messages @@ -4365,24 +4515,24 @@ fn main() {{ .count() }); assert_eq!( - retry_count, 1, - "Should have only one retry for internal server errors" + retry_count, 3, + "Should have 3 retries for internal server errors" ); - // For internal server errors, we only retry once and then give up - // Check that retry_state is cleared after the single retry + // For internal server errors, we retry 3 times and then give up + // Check that retry_state is cleared after all retries thread.read_with(cx, |thread, _| { assert!( thread.retry_state.is_none(), - "Retry state should be cleared after single retry" + "Retry state should be cleared after all retries" ); }); - // Verify total attempts (1 initial + 1 retry) + // Verify total attempts (1 initial + 3 retries) assert_eq!( *completion_count.lock(), - 2, - "Should have attempted once plus 1 retry" + 4, + "Should have attempted once plus 3 retries" ); } @@ -4393,6 +4543,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create model that returns overloaded error let model = Arc::new(ErrorInjector::new(TestError::Overloaded)); @@ -4479,6 +4634,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // We'll use a wrapper to switch behavior after first failure struct RetryTestModel { inner: Arc, @@ -4647,6 +4807,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create a model that fails once then succeeds struct FailOnceModel { inner: Arc, @@ -4808,6 +4973,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create a model that returns rate limit error with retry_after struct RateLimitModel { inner: Arc, @@ -5081,6 +5251,79 @@ fn main() {{ ); } + #[gpui::test] + async fn test_no_retry_without_burn_mode(cx: &mut TestAppContext) { + init_test_settings(cx); + + let project = create_test_project(cx, json!({})).await; + let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + + // Ensure we're in Normal mode (not Burn mode) + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Normal); + }); + + // Track error events + let error_events = Arc::new(Mutex::new(Vec::new())); + let error_events_clone = error_events.clone(); + + let _subscription = thread.update(cx, |_, cx| { + cx.subscribe(&thread, move |_, _, event: &ThreadEvent, _| { + if let ThreadEvent::ShowError(error) = event { + error_events_clone.lock().push(error.clone()); + } + }) + }); + + // Create model that returns overloaded error + let model = Arc::new(ErrorInjector::new(TestError::Overloaded)); + + // Insert a user message + thread.update(cx, |thread, cx| { + thread.insert_user_message("Hello!", ContextLoadResult::default(), None, vec![], cx); + }); + + // Start completion + thread.update(cx, |thread, cx| { + thread.send_to_model(model.clone(), CompletionIntent::UserPrompt, None, cx); + }); + + cx.run_until_parked(); + + // Verify no retry state was created + thread.read_with(cx, |thread, _| { + assert!( + thread.retry_state.is_none(), + "Should not have retry state in Normal mode" + ); + }); + + // Check that a retryable error was reported + let errors = error_events.lock(); + assert!(!errors.is_empty(), "Should have received an error event"); + + if let ThreadError::RetryableError { + message: _, + can_enable_burn_mode, + } = &errors[0] + { + assert!( + *can_enable_burn_mode, + "Error should indicate burn mode can be enabled" + ); + } else { + panic!("Expected RetryableError, got {:?}", errors[0]); + } + + // Verify the thread is no longer generating + thread.read_with(cx, |thread, _| { + assert!( + !thread.is_generating(), + "Should not be generating after error without retry" + ); + }); + } + #[gpui::test] async fn test_retry_cancelled_on_stop(cx: &mut TestAppContext) { init_test_settings(cx); @@ -5088,6 +5331,11 @@ fn main() {{ let project = create_test_project(cx, json!({})).await; let (_, _, thread, _, _base_model) = setup_test_environment(cx, project.clone()).await; + // Enable Burn Mode to allow retries + thread.update(cx, |thread, _| { + thread.set_completion_mode(CompletionMode::Burn); + }); + // Create model that returns overloaded error let model = Arc::new(ErrorInjector::new(TestError::Overloaded)); diff --git a/crates/agent_ui/src/active_thread.rs b/crates/agent_ui/src/active_thread.rs index 3cf68b887d..d97154cdbb 100644 --- a/crates/agent_ui/src/active_thread.rs +++ b/crates/agent_ui/src/active_thread.rs @@ -1036,7 +1036,7 @@ impl ActiveThread { .collect::>() .join("\n"); self.last_error = Some(ThreadError::Message { - header: "Error interacting with language model".into(), + header: "Error".into(), message: error_message.into(), }); } @@ -3722,8 +3722,11 @@ pub(crate) fn open_context( AgentContextHandle::Thread(thread_context) => workspace.update(cx, |workspace, cx| { if let Some(panel) = workspace.panel::(cx) { - panel.update(cx, |panel, cx| { - panel.open_thread(thread_context.thread.clone(), window, cx); + let thread = thread_context.thread.clone(); + window.defer(cx, move |window, cx| { + panel.update(cx, |panel, cx| { + panel.open_thread(thread, window, cx); + }); }); } }), @@ -3731,8 +3734,11 @@ pub(crate) fn open_context( AgentContextHandle::TextThread(text_thread_context) => { workspace.update(cx, |workspace, cx| { if let Some(panel) = workspace.panel::(cx) { - panel.update(cx, |panel, cx| { - panel.open_prompt_editor(text_thread_context.context.clone(), window, cx) + let context = text_thread_context.context.clone(); + window.defer(cx, move |window, cx| { + panel.update(cx, |panel, cx| { + panel.open_prompt_editor(context, window, cx) + }); }); } }) diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 2caa9dab42..3eca2fdb30 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -64,8 +64,9 @@ use theme::ThemeSettings; use time::UtcOffset; use ui::utils::WithRemSize; use ui::{ - Banner, Callout, CheckboxWithLabel, ContextMenu, ElevationIndex, KeyBinding, PopoverMenu, - PopoverMenuHandle, ProgressBar, Tab, Tooltip, Vector, VectorName, prelude::*, + Banner, Button, Callout, CheckboxWithLabel, ContextMenu, ElevationIndex, IconPosition, + KeyBinding, PopoverMenu, PopoverMenuHandle, ProgressBar, Tab, Tooltip, Vector, VectorName, + prelude::*, }; use util::ResultExt as _; use workspace::{ @@ -2913,6 +2914,21 @@ impl AgentPanel { .size(IconSize::Small) .color(Color::Error); + let retry_button = Button::new("retry", "Retry") + .icon(IconName::RotateCw) + .icon_position(IconPosition::Start) + .on_click({ + let thread = thread.clone(); + move |_, window, cx| { + thread.update(cx, |thread, cx| { + thread.clear_last_error(); + thread.thread().update(cx, |thread, cx| { + thread.retry_last_completion(Some(window.window_handle()), cx); + }); + }); + } + }); + div() .border_t_1() .border_color(cx.theme().colors().border) @@ -2921,13 +2937,72 @@ impl AgentPanel { .icon(icon) .title(header) .description(message.clone()) - .primary_action(self.dismiss_error_button(thread, cx)) - .secondary_action(self.create_copy_button(message_with_header)) + .primary_action(retry_button) + .secondary_action(self.dismiss_error_button(thread, cx)) + .tertiary_action(self.create_copy_button(message_with_header)) .bg_color(self.error_callout_bg(cx)), ) .into_any_element() } + fn render_retryable_error( + &self, + message: SharedString, + can_enable_burn_mode: bool, + thread: &Entity, + cx: &mut Context, + ) -> AnyElement { + let icon = Icon::new(IconName::XCircle) + .size(IconSize::Small) + .color(Color::Error); + + let retry_button = Button::new("retry", "Retry") + .icon(IconName::RotateCw) + .icon_position(IconPosition::Start) + .on_click({ + let thread = thread.clone(); + move |_, window, cx| { + thread.update(cx, |thread, cx| { + thread.clear_last_error(); + thread.thread().update(cx, |thread, cx| { + thread.retry_last_completion(Some(window.window_handle()), cx); + }); + }); + } + }); + + let mut callout = Callout::new() + .icon(icon) + .title("Error") + .description(message.clone()) + .bg_color(self.error_callout_bg(cx)) + .primary_action(retry_button); + + if can_enable_burn_mode { + let burn_mode_button = Button::new("enable_burn_retry", "Enable Burn Mode and Retry") + .icon(IconName::ZedBurnMode) + .icon_position(IconPosition::Start) + .on_click({ + let thread = thread.clone(); + move |_, window, cx| { + thread.update(cx, |thread, cx| { + thread.clear_last_error(); + thread.thread().update(cx, |thread, cx| { + thread.enable_burn_mode_and_retry(Some(window.window_handle()), cx); + }); + }); + } + }); + callout = callout.secondary_action(burn_mode_button); + } + + div() + .border_t_1() + .border_color(cx.theme().colors().border) + .child(callout) + .into_any_element() + } + fn render_prompt_editor( &self, context_editor: &Entity, @@ -3169,6 +3244,15 @@ impl Render for AgentPanel { ThreadError::Message { header, message } => { self.render_error_message(header, message, thread, cx) } + ThreadError::RetryableError { + message, + can_enable_burn_mode, + } => self.render_retryable_error( + message, + can_enable_burn_mode, + thread, + cx, + ), }) .into_any(), ) diff --git a/crates/assistant_tools/src/edit_agent/evals.rs b/crates/assistant_tools/src/edit_agent/evals.rs index c7af7dc64e..eda7eee0e3 100644 --- a/crates/assistant_tools/src/edit_agent/evals.rs +++ b/crates/assistant_tools/src/edit_agent/evals.rs @@ -12,6 +12,7 @@ use collections::HashMap; use fs::FakeFs; use futures::{FutureExt, future::LocalBoxFuture}; use gpui::{AppContext, TestAppContext, Timer}; +use http_client::StatusCode; use indoc::{formatdoc, indoc}; use language_model::{ LanguageModelRegistry, LanguageModelRequestTool, LanguageModelToolResult, @@ -1675,6 +1676,30 @@ async fn retry_on_rate_limit(mut request: impl AsyncFnMut() -> Result) -> Timer::after(retry_after + jitter).await; continue; } + LanguageModelCompletionError::UpstreamProviderError { + status, + retry_after, + .. + } => { + // Only retry for specific status codes + let should_retry = matches!( + *status, + StatusCode::TOO_MANY_REQUESTS | StatusCode::SERVICE_UNAVAILABLE + ) || status.as_u16() == 529; + + if !should_retry { + return Err(err.into()); + } + + // Use server-provided retry_after if available, otherwise use default + let retry_after = retry_after.unwrap_or(Duration::from_secs(5)); + let jitter = retry_after.mul_f64(rand::thread_rng().gen_range(0.0..1.0)); + eprintln!( + "Attempt #{attempt}: {err}. Retry after {retry_after:?} + jitter of {jitter:?}" + ); + Timer::after(retry_after + jitter).await; + continue; + } _ => return Err(err.into()), }, Err(err) => return Err(err), diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index c4211f72c8..afa23931ed 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -21,7 +21,7 @@ use futures::{ channel::oneshot, future::BoxFuture, }; use gpui::{App, AsyncApp, Entity, Global, Task, WeakEntity, actions}; -use http_client::{AsyncBody, HttpClient, HttpClientWithUrl}; +use http_client::{AsyncBody, HttpClient, HttpClientWithUrl, http}; use parking_lot::RwLock; use postage::watch; use proxy::connect_proxy_stream; @@ -1123,6 +1123,7 @@ impl Client { let http = self.http.clone(); let proxy = http.proxy().cloned(); + let user_agent = http.user_agent().cloned(); let credentials = credentials.clone(); let rpc_url = self.rpc_url(http, release_channel); let system_id = self.telemetry.system_id(); @@ -1174,7 +1175,7 @@ impl Client { // We then modify the request to add our desired headers. let request_headers = request.headers_mut(); request_headers.insert( - "Authorization", + http::header::AUTHORIZATION, HeaderValue::from_str(&credentials.authorization_header())?, ); request_headers.insert( @@ -1186,6 +1187,9 @@ impl Client { "x-zed-release-channel", HeaderValue::from_str(release_channel.map(|r| r.dev_name()).unwrap_or("unknown"))?, ); + if let Some(user_agent) = user_agent { + request_headers.insert(http::header::USER_AGENT, user_agent); + } if let Some(system_id) = system_id { request_headers.insert("x-zed-system-id", HeaderValue::from_str(&system_id)?); } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6ad4fc0318..96c8352a01 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1772,7 +1772,7 @@ impl Editor { ) -> Self { debug_assert!( display_map.is_none() || mode.is_minimap(), - "Providing a display map for a new editor is only intended for the minimap and might have unindended side effects otherwise!" + "Providing a display map for a new editor is only intended for the minimap and might have unintended side effects otherwise!" ); let full_mode = mode.is_full(); @@ -8193,8 +8193,7 @@ impl Editor { return; }; - // Try to find a closest, enclosing node using tree-sitter that has a - // task + // Try to find a closest, enclosing node using tree-sitter that has a task let Some((buffer, buffer_row, tasks)) = self .find_enclosing_node_task(cx) // Or find the task that's closest in row-distance. @@ -21732,11 +21731,11 @@ impl CodeActionProvider for Entity { cx: &mut App, ) -> Task>> { self.update(cx, |project, cx| { - let code_lens = project.code_lens(buffer, range.clone(), cx); + let code_lens_actions = project.code_lens_actions(buffer, range.clone(), cx); let code_actions = project.code_actions(buffer, range, None, cx); cx.background_spawn(async move { - let (code_lens, code_actions) = join(code_lens, code_actions).await; - Ok(code_lens + let (code_lens_actions, code_actions) = join(code_lens_actions, code_actions).await; + Ok(code_lens_actions .context("code lens fetch")? .into_iter() .chain(code_actions.context("code action fetch")?) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 43c9c0db65..b1588e419f 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9570,6 +9570,74 @@ async fn test_document_format_during_save(cx: &mut TestAppContext) { } } +#[gpui::test] +async fn test_redo_after_noop_format(cx: &mut TestAppContext) { + init_test(cx, |settings| { + settings.defaults.ensure_final_newline_on_save = Some(false); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.txt"), "foo".into()).await; + + let project = Project::test(fs, [path!("/file.txt").as_ref()], cx).await; + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.txt"), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([0..0]) + }); + }); + assert!(!cx.read(|cx| editor.is_dirty(cx))); + + editor.update_in(cx, |editor, window, cx| { + editor.handle_input("\n", window, cx) + }); + cx.run_until_parked(); + save(&editor, &project, cx).await; + assert_eq!("\nfoo", editor.read_with(cx, |editor, cx| editor.text(cx))); + + editor.update_in(cx, |editor, window, cx| { + editor.undo(&Default::default(), window, cx); + }); + save(&editor, &project, cx).await; + assert_eq!("foo", editor.read_with(cx, |editor, cx| editor.text(cx))); + + editor.update_in(cx, |editor, window, cx| { + editor.redo(&Default::default(), window, cx); + }); + cx.run_until_parked(); + assert_eq!("\nfoo", editor.read_with(cx, |editor, cx| editor.text(cx))); + + async fn save(editor: &Entity, project: &Entity, cx: &mut VisualTestContext) { + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + cx.executor().start_waiting(); + save.await; + assert!(!cx.read(|cx| editor.is_dirty(cx))); + } +} + #[gpui::test] async fn test_multibuffer_format_during_save(cx: &mut TestAppContext) { init_test(cx, |_| {}); @@ -9955,8 +10023,14 @@ async fn test_autosave_with_dirty_buffers(cx: &mut TestAppContext) { ); } -#[gpui::test] -async fn test_range_format_during_save(cx: &mut TestAppContext) { +async fn setup_range_format_test( + cx: &mut TestAppContext, +) -> ( + Entity, + Entity, + &mut gpui::VisualTestContext, + lsp::FakeLanguageServer, +) { init_test(cx, |_| {}); let fs = FakeFs::new(cx.executor()); @@ -9971,9 +10045,9 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) { FakeLspAdapter { capabilities: lsp::ServerCapabilities { document_range_formatting_provider: Some(lsp::OneOf::Left(true)), - ..Default::default() + ..lsp::ServerCapabilities::default() }, - ..Default::default() + ..FakeLspAdapter::default() }, ); @@ -9988,14 +10062,22 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) { let (editor, cx) = cx.add_window_view(|window, cx| { build_editor_with_project(project.clone(), buffer, window, cx) }); + + cx.executor().start_waiting(); + let fake_server = fake_servers.next().await.unwrap(); + + (project, editor, cx, fake_server) +} + +#[gpui::test] +async fn test_range_format_on_save_success(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test(cx).await; + editor.update_in(cx, |editor, window, cx| { editor.set_text("one\ntwo\nthree\n", window, cx) }); assert!(cx.read(|cx| editor.is_dirty(cx))); - cx.executor().start_waiting(); - let fake_server = fake_servers.next().await.unwrap(); - let save = editor .update_in(cx, |editor, window, cx| { editor.save( @@ -10030,13 +10112,18 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) { "one, two\nthree\n" ); assert!(!cx.read(|cx| editor.is_dirty(cx))); +} + +#[gpui::test] +async fn test_range_format_on_save_timeout(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test(cx).await; editor.update_in(cx, |editor, window, cx| { editor.set_text("one\ntwo\nthree\n", window, cx) }); assert!(cx.read(|cx| editor.is_dirty(cx))); - // Ensure we can still save even if formatting hangs. + // Test that save still works when formatting hangs fake_server.set_request_handler::( move |params, _| async move { assert_eq!( @@ -10068,8 +10155,13 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) { "one\ntwo\nthree\n" ); assert!(!cx.read(|cx| editor.is_dirty(cx))); +} - // For non-dirty buffer, no formatting request should be sent +#[gpui::test] +async fn test_range_format_not_called_for_clean_buffer(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test(cx).await; + + // Buffer starts clean, no formatting should be requested let save = editor .update_in(cx, |editor, window, cx| { editor.save( @@ -10090,6 +10182,12 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) { .next(); cx.executor().start_waiting(); save.await; + cx.run_until_parked(); +} + +#[gpui::test] +async fn test_range_format_respects_language_tab_size_override(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test(cx).await; // Set Rust language override and assert overridden tabsize is sent to language server update_test_language_settings(cx, |settings| { @@ -10103,7 +10201,7 @@ async fn test_range_format_during_save(cx: &mut TestAppContext) { }); editor.update_in(cx, |editor, window, cx| { - editor.set_text("somehting_new\n", window, cx) + editor.set_text("something_new\n", window, cx) }); assert!(cx.read(|cx| editor.is_dirty(cx))); let save = editor @@ -21188,16 +21286,32 @@ async fn test_apply_code_lens_actions_with_commands(cx: &mut gpui::TestAppContex }, ); - let (buffer, _handle) = project - .update(cx, |p, cx| { - p.open_local_buffer_with_lsp(path!("/dir/a.ts"), cx) + let editor = workspace + .update(cx, |workspace, window, cx| { + workspace.open_abs_path( + PathBuf::from(path!("/dir/a.ts")), + OpenOptions::default(), + window, + cx, + ) }) + .unwrap() .await + .unwrap() + .downcast::() .unwrap(); cx.executor().run_until_parked(); let fake_server = fake_language_servers.next().await.unwrap(); + let buffer = editor.update(cx, |editor, cx| { + editor + .buffer() + .read(cx) + .as_singleton() + .expect("have opened a single file by path") + }); + let buffer_snapshot = buffer.update(cx, |buffer, _| buffer.snapshot()); let anchor = buffer_snapshot.anchor_at(0, text::Bias::Left); drop(buffer_snapshot); @@ -21255,7 +21369,7 @@ async fn test_apply_code_lens_actions_with_commands(cx: &mut gpui::TestAppContex assert_eq!( actions.len(), 1, - "Should have only one valid action for the 0..0 range" + "Should have only one valid action for the 0..0 range, got: {actions:#?}" ); let action = actions[0].clone(); let apply = project.update(cx, |project, cx| { @@ -21301,7 +21415,7 @@ async fn test_apply_code_lens_actions_with_commands(cx: &mut gpui::TestAppContex .into_iter() .collect(), ), - ..Default::default() + ..lsp::WorkspaceEdit::default() }, }, ) @@ -21324,6 +21438,38 @@ async fn test_apply_code_lens_actions_with_commands(cx: &mut gpui::TestAppContex buffer.undo(cx); assert_eq!(buffer.text(), "a"); }); + + let actions_after_edits = cx + .update_window(*workspace, |_, window, cx| { + project.code_actions(&buffer, anchor..anchor, window, cx) + }) + .unwrap() + .await + .unwrap(); + assert_eq!( + actions, actions_after_edits, + "For the same selection, same code lens actions should be returned" + ); + + let _responses = + fake_server.set_request_handler::(|_, _| async move { + panic!("No more code lens requests are expected"); + }); + editor.update_in(cx, |editor, window, cx| { + editor.select_all(&SelectAll, window, cx); + }); + cx.executor().run_until_parked(); + let new_actions = cx + .update_window(*workspace, |_, window, cx| { + project.code_actions(&buffer, anchor..anchor, window, cx) + }) + .unwrap() + .await + .unwrap(); + assert_eq!( + actions, new_actions, + "Code lens are queried for the same range and should get the same set back, but without additional LSP queries now" + ); } #[gpui::test] @@ -22708,7 +22854,7 @@ pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsC workspace::init_settings(cx); crate::init(cx); }); - + zlog::init_test(); update_test_language_settings(cx, f); } diff --git a/crates/editor/src/lsp_colors.rs b/crates/editor/src/lsp_colors.rs index ce07dd43fe..08cf9078f2 100644 --- a/crates/editor/src/lsp_colors.rs +++ b/crates/editor/src/lsp_colors.rs @@ -6,7 +6,7 @@ use gpui::{Hsla, Rgba}; use itertools::Itertools; use language::point_from_lsp; use multi_buffer::Anchor; -use project::{DocumentColor, lsp_store::ColorFetchStrategy}; +use project::{DocumentColor, lsp_store::LspFetchStrategy}; use settings::Settings as _; use text::{Bias, BufferId, OffsetRangeExt as _}; use ui::{App, Context, Window}; @@ -180,9 +180,9 @@ impl Editor { .filter_map(|buffer| { let buffer_id = buffer.read(cx).remote_id(); let fetch_strategy = if ignore_cache { - ColorFetchStrategy::IgnoreCache + LspFetchStrategy::IgnoreCache } else { - ColorFetchStrategy::UseCache { + LspFetchStrategy::UseCache { known_cache_version: self.colors.as_ref().and_then(|colors| { Some(colors.buffer_colors.get(&buffer_id)?.cache_version_used) }), diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index 7310d6d3c0..ecaf7c11e4 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -12,7 +12,7 @@ use crate::{ }; pub use autoscroll::{Autoscroll, AutoscrollStrategy}; use core::fmt::Debug; -use gpui::{App, Axis, Context, Global, Pixels, Task, Window, point, px}; +use gpui::{Along, App, Axis, Context, Global, Pixels, Task, Window, point, px}; use language::language_settings::{AllLanguageSettings, SoftWrap}; use language::{Bias, Point}; pub use scroll_amount::ScrollAmount; @@ -49,14 +49,14 @@ impl ScrollAnchor { } pub fn scroll_position(&self, snapshot: &DisplaySnapshot) -> gpui::Point { - let mut scroll_position = self.offset; - if self.anchor == Anchor::min() { - scroll_position.y = 0.; - } else { - let scroll_top = self.anchor.to_display_point(snapshot).row().as_f32(); - scroll_position.y += scroll_top; - } - scroll_position + self.offset.apply_along(Axis::Vertical, |offset| { + if self.anchor == Anchor::min() { + 0. + } else { + let scroll_top = self.anchor.to_display_point(snapshot).row().as_f32(); + (offset + scroll_top).max(0.) + } + }) } pub fn top_row(&self, buffer: &MultiBufferSnapshot) -> u32 { diff --git a/crates/eval/src/example.rs b/crates/eval/src/example.rs index 09770364cb..7ce3b1fdf1 100644 --- a/crates/eval/src/example.rs +++ b/crates/eval/src/example.rs @@ -422,6 +422,13 @@ impl AppContext for ExampleContext { self.app.update_entity(handle, update) } + fn as_mut<'a, T>(&'a mut self, handle: &Entity) -> Self::Result> + where + T: 'static, + { + self.app.as_mut(handle) + } + fn read_entity( &self, handle: &Entity, diff --git a/crates/extension_host/src/wasm_host.rs b/crates/extension_host/src/wasm_host.rs index 3971fa4263..fd1633126a 100644 --- a/crates/extension_host/src/wasm_host.rs +++ b/crates/extension_host/src/wasm_host.rs @@ -102,7 +102,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn language_server_initialization_options( @@ -127,7 +127,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn language_server_workspace_configuration( @@ -150,7 +150,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn language_server_additional_initialization_options( @@ -175,7 +175,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn language_server_additional_workspace_configuration( @@ -200,7 +200,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn labels_for_completions( @@ -226,7 +226,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn labels_for_symbols( @@ -252,7 +252,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn complete_slash_command_argument( @@ -271,7 +271,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn run_slash_command( @@ -297,7 +297,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn context_server_command( @@ -316,7 +316,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn context_server_configuration( @@ -343,7 +343,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn suggest_docs_packages(&self, provider: Arc) -> Result> { @@ -358,7 +358,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn index_docs( @@ -384,7 +384,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn get_dap_binary( @@ -406,7 +406,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn dap_request_kind( &self, @@ -423,7 +423,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn dap_config_to_scenario(&self, config: ZedDebugConfig) -> Result { @@ -437,7 +437,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn dap_locator_create_scenario( @@ -461,7 +461,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } async fn run_dap_locator( &self, @@ -477,7 +477,7 @@ impl extension::Extension for WasmExtension { } .boxed() }) - .await + .await? } } @@ -739,7 +739,7 @@ impl WasmExtension { .with_context(|| format!("failed to load wasm extension {}", manifest.id)) } - pub async fn call(&self, f: Fn) -> T + pub async fn call(&self, f: Fn) -> Result where T: 'static + Send, Fn: 'static @@ -755,8 +755,19 @@ impl WasmExtension { } .boxed() })) - .expect("wasm extension channel should not be closed yet"); - return_rx.await.expect("wasm extension channel") + .map_err(|_| { + anyhow!( + "wasm extension channel should not be closed yet, extension {} (id {})", + self.manifest.name, + self.manifest.id, + ) + })?; + return_rx.await.with_context(|| { + format!( + "wasm extension channel, extension {} (id {})", + self.manifest.name, self.manifest.id, + ) + }) } } @@ -777,8 +788,19 @@ impl WasmState { } .boxed_local() })) - .expect("main thread message channel should not be closed yet"); - async move { return_rx.await.expect("main thread message channel") } + .unwrap_or_else(|_| { + panic!( + "main thread message channel should not be closed yet, extension {} (id {})", + self.manifest.name, self.manifest.id, + ) + }); + let name = self.manifest.name.clone(); + let id = self.manifest.id.clone(); + async move { + return_rx.await.unwrap_or_else(|_| { + panic!("main thread message channel, extension {name} (id {id})") + }) + } } fn work_dir(&self) -> PathBuf { diff --git a/crates/gpui/build.rs b/crates/gpui/build.rs index b9496cc014..aed4397440 100644 --- a/crates/gpui/build.rs +++ b/crates/gpui/build.rs @@ -126,7 +126,7 @@ mod macos { "ContentMask".into(), "Uniforms".into(), "AtlasTile".into(), - "PathInputIndex".into(), + "PathRasterizationInputIndex".into(), "PathVertex_ScaledPixels".into(), "ShadowInputIndex".into(), "Shadow".into(), diff --git a/crates/gpui/examples/painting.rs b/crates/gpui/examples/painting.rs index 9ab58cffc9..ff4b64cbda 100644 --- a/crates/gpui/examples/painting.rs +++ b/crates/gpui/examples/painting.rs @@ -1,13 +1,9 @@ use gpui::{ Application, Background, Bounds, ColorSpace, Context, MouseDownEvent, Path, PathBuilder, - PathStyle, Pixels, Point, Render, SharedString, StrokeOptions, Window, WindowBounds, - WindowOptions, canvas, div, linear_color_stop, linear_gradient, point, prelude::*, px, rgb, - size, + PathStyle, Pixels, Point, Render, SharedString, StrokeOptions, Window, WindowOptions, canvas, + div, linear_color_stop, linear_gradient, point, prelude::*, px, rgb, size, }; -const DEFAULT_WINDOW_WIDTH: Pixels = px(1024.0); -const DEFAULT_WINDOW_HEIGHT: Pixels = px(768.0); - struct PaintingViewer { default_lines: Vec<(Path, Background)>, lines: Vec>>, @@ -151,6 +147,8 @@ impl PaintingViewer { px(320.0 + (i as f32 * 10.0).sin() * 40.0), )); } + let path = builder.build().unwrap(); + lines.push((path, gpui::green().into())); Self { default_lines: lines.clone(), @@ -185,13 +183,9 @@ fn button( } impl Render for PaintingViewer { - fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - window.request_animation_frame(); - + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { let default_lines = self.default_lines.clone(); let lines = self.lines.clone(); - let window_size = window.bounds().size; - let scale = window_size.width / DEFAULT_WINDOW_WIDTH; let dashed = self.dashed; div() @@ -228,7 +222,7 @@ impl Render for PaintingViewer { move |_, _, _| {}, move |_, _, window, _| { for (path, color) in default_lines { - window.paint_path(path.clone().scale(scale), color); + window.paint_path(path, color); } for points in lines { @@ -304,11 +298,6 @@ fn main() { cx.open_window( WindowOptions { focus: true, - window_bounds: Some(WindowBounds::Windowed(Bounds::centered( - None, - size(DEFAULT_WINDOW_WIDTH, DEFAULT_WINDOW_HEIGHT), - cx, - ))), ..Default::default() }, |window, cx| cx.new(|cx| PaintingViewer::new(window, cx)), diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 957c7c4be6..a08ad3724f 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -448,15 +448,23 @@ impl App { } pub(crate) fn update(&mut self, update: impl FnOnce(&mut Self) -> R) -> R { - self.pending_updates += 1; + self.start_update(); let result = update(self); + self.finish_update(); + result + } + + pub(crate) fn start_update(&mut self) { + self.pending_updates += 1; + } + + pub(crate) fn finish_update(&mut self) { if !self.flushing_effects && self.pending_updates == 1 { self.flushing_effects = true; self.flush_effects(); self.flushing_effects = false; } self.pending_updates -= 1; - result } /// Arrange a callback to be invoked when the given entity calls `notify` on its respective context. @@ -868,7 +876,6 @@ impl App { loop { self.release_dropped_entities(); self.release_dropped_focus_handles(); - if let Some(effect) = self.pending_effects.pop_front() { match effect { Effect::Notify { emitter } => { @@ -1819,6 +1826,13 @@ impl AppContext for App { }) } + fn as_mut<'a, T>(&'a mut self, handle: &Entity) -> GpuiBorrow<'a, T> + where + T: 'static, + { + GpuiBorrow::new(handle.clone(), self) + } + fn read_entity( &self, handle: &Entity, @@ -2007,6 +2021,10 @@ impl HttpClient for NullHttpClient { .boxed() } + fn user_agent(&self) -> Option<&http_client::http::HeaderValue> { + None + } + fn proxy(&self) -> Option<&Url> { None } @@ -2015,3 +2033,79 @@ impl HttpClient for NullHttpClient { type_name::() } } + +/// A mutable reference to an entity owned by GPUI +pub struct GpuiBorrow<'a, T> { + inner: Option>, + app: &'a mut App, +} + +impl<'a, T: 'static> GpuiBorrow<'a, T> { + fn new(inner: Entity, app: &'a mut App) -> Self { + app.start_update(); + let lease = app.entities.lease(&inner); + Self { + inner: Some(lease), + app, + } + } +} + +impl<'a, T: 'static> std::borrow::Borrow for GpuiBorrow<'a, T> { + fn borrow(&self) -> &T { + self.inner.as_ref().unwrap().borrow() + } +} + +impl<'a, T: 'static> std::borrow::BorrowMut for GpuiBorrow<'a, T> { + fn borrow_mut(&mut self) -> &mut T { + self.inner.as_mut().unwrap().borrow_mut() + } +} + +impl<'a, T> Drop for GpuiBorrow<'a, T> { + fn drop(&mut self) { + let lease = self.inner.take().unwrap(); + self.app.notify(lease.id); + self.app.entities.end_lease(lease); + self.app.finish_update(); + } +} + +#[cfg(test)] +mod test { + use std::{cell::RefCell, rc::Rc}; + + use crate::{AppContext, TestAppContext}; + + #[test] + fn test_gpui_borrow() { + let cx = TestAppContext::single(); + let observation_count = Rc::new(RefCell::new(0)); + + let state = cx.update(|cx| { + let state = cx.new(|_| false); + cx.observe(&state, { + let observation_count = observation_count.clone(); + move |_, _| { + let mut count = observation_count.borrow_mut(); + *count += 1; + } + }) + .detach(); + + state + }); + + cx.update(|cx| { + // Calling this like this so that we don't clobber the borrow_mut above + *std::borrow::BorrowMut::borrow_mut(&mut state.as_mut(cx)) = true; + }); + + cx.update(|cx| { + state.write(cx, false); + }); + + assert_eq!(*observation_count.borrow(), 2); + } +} diff --git a/crates/gpui/src/app/async_context.rs b/crates/gpui/src/app/async_context.rs index c3b60dd580..d9d21c0244 100644 --- a/crates/gpui/src/app/async_context.rs +++ b/crates/gpui/src/app/async_context.rs @@ -3,7 +3,7 @@ use crate::{ Entity, EventEmitter, Focusable, ForegroundExecutor, Global, PromptButton, PromptLevel, Render, Reservation, Result, Subscription, Task, VisualContext, Window, WindowHandle, }; -use anyhow::Context as _; +use anyhow::{Context as _, anyhow}; use derive_more::{Deref, DerefMut}; use futures::channel::oneshot; use std::{future::Future, rc::Weak}; @@ -58,6 +58,15 @@ impl AppContext for AsyncApp { Ok(app.update_entity(handle, update)) } + fn as_mut<'a, T>(&'a mut self, _handle: &Entity) -> Self::Result> + where + T: 'static, + { + Err(anyhow!( + "Cannot as_mut with an async context. Try calling update() first" + )) + } + fn read_entity( &self, handle: &Entity, @@ -364,6 +373,15 @@ impl AppContext for AsyncWindowContext { .update(self, |_, _, cx| cx.update_entity(handle, update)) } + fn as_mut<'a, T>(&'a mut self, _: &Entity) -> Self::Result> + where + T: 'static, + { + Err(anyhow!( + "Cannot use as_mut() from an async context, call `update`" + )) + } + fn read_entity( &self, handle: &Entity, diff --git a/crates/gpui/src/app/context.rs b/crates/gpui/src/app/context.rs index 2d90ff35b1..392be2ffe9 100644 --- a/crates/gpui/src/app/context.rs +++ b/crates/gpui/src/app/context.rs @@ -726,6 +726,13 @@ impl AppContext for Context<'_, T> { self.app.update_entity(handle, update) } + fn as_mut<'a, E>(&'a mut self, handle: &Entity) -> Self::Result> + where + E: 'static, + { + self.app.as_mut(handle) + } + fn read_entity( &self, handle: &Entity, diff --git a/crates/gpui/src/app/entity_map.rs b/crates/gpui/src/app/entity_map.rs index f1aafa55e8..d4e5b2570e 100644 --- a/crates/gpui/src/app/entity_map.rs +++ b/crates/gpui/src/app/entity_map.rs @@ -1,4 +1,4 @@ -use crate::{App, AppContext, VisualContext, Window, seal::Sealed}; +use crate::{App, AppContext, GpuiBorrow, VisualContext, Window, seal::Sealed}; use anyhow::{Context as _, Result}; use collections::FxHashSet; use derive_more::{Deref, DerefMut}; @@ -105,7 +105,7 @@ impl EntityMap { /// Move an entity to the stack. #[track_caller] - pub fn lease<'a, T>(&mut self, pointer: &'a Entity) -> Lease<'a, T> { + pub fn lease(&mut self, pointer: &Entity) -> Lease { self.assert_valid_context(pointer); let mut accessed_entities = self.accessed_entities.borrow_mut(); accessed_entities.insert(pointer.entity_id); @@ -117,15 +117,14 @@ impl EntityMap { ); Lease { entity, - pointer, + id: pointer.entity_id, entity_type: PhantomData, } } /// Returns an entity after moving it to the stack. pub fn end_lease(&mut self, mut lease: Lease) { - self.entities - .insert(lease.pointer.entity_id, lease.entity.take().unwrap()); + self.entities.insert(lease.id, lease.entity.take().unwrap()); } pub fn read(&self, entity: &Entity) -> &T { @@ -187,13 +186,13 @@ fn double_lease_panic(operation: &str) -> ! { ) } -pub(crate) struct Lease<'a, T> { +pub(crate) struct Lease { entity: Option>, - pub pointer: &'a Entity, + pub id: EntityId, entity_type: PhantomData, } -impl core::ops::Deref for Lease<'_, T> { +impl core::ops::Deref for Lease { type Target = T; fn deref(&self) -> &Self::Target { @@ -201,13 +200,13 @@ impl core::ops::Deref for Lease<'_, T> { } } -impl core::ops::DerefMut for Lease<'_, T> { +impl core::ops::DerefMut for Lease { fn deref_mut(&mut self) -> &mut Self::Target { self.entity.as_mut().unwrap().downcast_mut().unwrap() } } -impl Drop for Lease<'_, T> { +impl Drop for Lease { fn drop(&mut self) { if self.entity.is_some() && !panicking() { panic!("Leases must be ended with EntityMap::end_lease") @@ -437,6 +436,19 @@ impl Entity { cx.update_entity(self, update) } + /// Updates the entity referenced by this handle with the given function. + pub fn as_mut<'a, C: AppContext>(&self, cx: &'a mut C) -> C::Result> { + cx.as_mut(self) + } + + /// Updates the entity referenced by this handle with the given function. + pub fn write(&self, cx: &mut C, value: T) -> C::Result<()> { + self.update(cx, |entity, cx| { + *entity = value; + cx.notify(); + }) + } + /// Updates the entity referenced by this handle with the given function if /// the referenced entity still exists, within a visual context that has a window. /// Returns an error if the entity has been released. diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index dfc7af0d9c..35e6032671 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -9,6 +9,7 @@ use crate::{ }; use anyhow::{anyhow, bail}; use futures::{Stream, StreamExt, channel::oneshot}; +use rand::{SeedableRng, rngs::StdRng}; use std::{cell::RefCell, future::Future, ops::Deref, rc::Rc, sync::Arc, time::Duration}; /// A TestAppContext is provided to tests created with `#[gpui::test]`, it provides @@ -63,6 +64,13 @@ impl AppContext for TestAppContext { app.update_entity(handle, update) } + fn as_mut<'a, T>(&'a mut self, _: &Entity) -> Self::Result> + where + T: 'static, + { + panic!("Cannot use as_mut with a test app context. Try calling update() first") + } + fn read_entity( &self, handle: &Entity, @@ -134,6 +142,12 @@ impl TestAppContext { } } + /// Create a single TestAppContext, for non-multi-client tests + pub fn single() -> Self { + let dispatcher = TestDispatcher::new(StdRng::from_entropy()); + Self::build(dispatcher, None) + } + /// The name of the test function that created this `TestAppContext` pub fn test_function_name(&self) -> Option<&'static str> { self.fn_name @@ -914,6 +928,13 @@ impl AppContext for VisualTestContext { self.cx.update_entity(handle, update) } + fn as_mut<'a, T>(&'a mut self, handle: &Entity) -> Self::Result> + where + T: 'static, + { + self.cx.as_mut(handle) + } + fn read_entity( &self, handle: &Entity, diff --git a/crates/gpui/src/element.rs b/crates/gpui/src/element.rs index 2852841b2c..e5f49c7be1 100644 --- a/crates/gpui/src/element.rs +++ b/crates/gpui/src/element.rs @@ -39,7 +39,7 @@ use crate::{ use derive_more::{Deref, DerefMut}; pub(crate) use smallvec::SmallVec; use std::{ - any::Any, + any::{Any, type_name}, fmt::{self, Debug, Display}, mem, panic, }; @@ -220,14 +220,17 @@ impl Element for Component { window: &mut Window, cx: &mut App, ) -> (LayoutId, Self::RequestLayoutState) { - let mut element = self - .component - .take() - .unwrap() - .render(window, cx) - .into_any_element(); - let layout_id = element.request_layout(window, cx); - (layout_id, element) + window.with_global_id(ElementId::Name(type_name::().into()), |_, window| { + let mut element = self + .component + .take() + .unwrap() + .render(window, cx) + .into_any_element(); + + let layout_id = element.request_layout(window, cx); + (layout_id, element) + }) } fn prepaint( @@ -239,7 +242,9 @@ impl Element for Component { window: &mut Window, cx: &mut App, ) { - element.prepaint(window, cx); + window.with_global_id(ElementId::Name(type_name::().into()), |_, window| { + element.prepaint(window, cx); + }) } fn paint( @@ -252,7 +257,9 @@ impl Element for Component { window: &mut Window, cx: &mut App, ) { - element.paint(window, cx); + window.with_global_id(ElementId::Name(type_name::().into()), |_, window| { + element.paint(window, cx); + }) } } diff --git a/crates/gpui/src/gpui.rs b/crates/gpui/src/gpui.rs index 91461a4d2c..4eb6fa8dab 100644 --- a/crates/gpui/src/gpui.rs +++ b/crates/gpui/src/gpui.rs @@ -197,6 +197,11 @@ pub trait AppContext { where T: 'static; + /// Update a entity in the app context. + fn as_mut<'a, T>(&'a mut self, handle: &Entity) -> Self::Result> + where + T: 'static; + /// Read a entity from the app context. fn read_entity( &self, diff --git a/crates/gpui/src/path_builder.rs b/crates/gpui/src/path_builder.rs index 13c168b0bb..6c8cfddd52 100644 --- a/crates/gpui/src/path_builder.rs +++ b/crates/gpui/src/path_builder.rs @@ -336,7 +336,10 @@ impl PathBuilder { let v1 = buf.vertices[i1]; let v2 = buf.vertices[i2]; - path.push_triangle((v0.into(), v1.into(), v2.into())); + path.push_triangle( + (v0.into(), v1.into(), v2.into()), + (point(0., 1.), point(0., 1.), point(0., 1.)), + ); } path diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 0250e59a9b..8918fdd28b 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -794,6 +794,7 @@ pub(crate) struct AtlasTextureId { pub(crate) enum AtlasTextureKind { Monochrome = 0, Polychrome = 1, + Path = 2, } #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] diff --git a/crates/gpui/src/platform/blade/blade_atlas.rs b/crates/gpui/src/platform/blade/blade_atlas.rs index 0b119c3910..78ba52056a 100644 --- a/crates/gpui/src/platform/blade/blade_atlas.rs +++ b/crates/gpui/src/platform/blade/blade_atlas.rs @@ -10,6 +10,8 @@ use etagere::BucketedAtlasAllocator; use parking_lot::Mutex; use std::{borrow::Cow, ops, sync::Arc}; +pub(crate) const PATH_TEXTURE_FORMAT: gpu::TextureFormat = gpu::TextureFormat::R16Float; + pub(crate) struct BladeAtlas(Mutex); struct PendingUpload { @@ -25,6 +27,7 @@ struct BladeAtlasState { tiles_by_key: FxHashMap, initializations: Vec, uploads: Vec, + path_sample_count: u32, } #[cfg(gles)] @@ -38,13 +41,13 @@ impl BladeAtlasState { } pub struct BladeTextureInfo { - #[allow(dead_code)] pub size: gpu::Extent, pub raw_view: gpu::TextureView, + pub msaa_view: Option, } impl BladeAtlas { - pub(crate) fn new(gpu: &Arc) -> Self { + pub(crate) fn new(gpu: &Arc, path_sample_count: u32) -> Self { BladeAtlas(Mutex::new(BladeAtlasState { gpu: Arc::clone(gpu), upload_belt: BufferBelt::new(BufferBeltDescriptor { @@ -56,6 +59,7 @@ impl BladeAtlas { tiles_by_key: Default::default(), initializations: Vec::new(), uploads: Vec::new(), + path_sample_count, })) } @@ -63,7 +67,6 @@ impl BladeAtlas { self.0.lock().destroy(); } - #[allow(dead_code)] pub(crate) fn clear_textures(&self, texture_kind: AtlasTextureKind) { let mut lock = self.0.lock(); let textures = &mut lock.storage[texture_kind]; @@ -72,6 +75,19 @@ impl BladeAtlas { } } + /// Allocate a rectangle and make it available for rendering immediately (without waiting for `before_frame`) + pub fn allocate_for_rendering( + &self, + size: Size, + texture_kind: AtlasTextureKind, + gpu_encoder: &mut gpu::CommandEncoder, + ) -> AtlasTile { + let mut lock = self.0.lock(); + let tile = lock.allocate(size, texture_kind); + lock.flush_initializations(gpu_encoder); + tile + } + pub fn before_frame(&self, gpu_encoder: &mut gpu::CommandEncoder) { let mut lock = self.0.lock(); lock.flush(gpu_encoder); @@ -93,6 +109,7 @@ impl BladeAtlas { depth: 1, }, raw_view: texture.raw_view, + msaa_view: texture.msaa_view, } } } @@ -183,8 +200,48 @@ impl BladeAtlasState { format = gpu::TextureFormat::Bgra8UnormSrgb; usage = gpu::TextureUsage::COPY | gpu::TextureUsage::RESOURCE; } + AtlasTextureKind::Path => { + format = PATH_TEXTURE_FORMAT; + usage = gpu::TextureUsage::COPY + | gpu::TextureUsage::RESOURCE + | gpu::TextureUsage::TARGET; + } } + // We currently only enable MSAA for path textures. + let (msaa, msaa_view) = if self.path_sample_count > 1 && kind == AtlasTextureKind::Path { + let msaa = self.gpu.create_texture(gpu::TextureDesc { + name: "msaa path texture", + format, + size: gpu::Extent { + width: size.width.into(), + height: size.height.into(), + depth: 1, + }, + array_layer_count: 1, + mip_level_count: 1, + sample_count: self.path_sample_count, + dimension: gpu::TextureDimension::D2, + usage: gpu::TextureUsage::TARGET, + external: None, + }); + + ( + Some(msaa), + Some(self.gpu.create_texture_view( + msaa, + gpu::TextureViewDesc { + name: "msaa texture view", + format, + dimension: gpu::ViewDimension::D2, + subresources: &Default::default(), + }, + )), + ) + } else { + (None, None) + }; + let raw = self.gpu.create_texture(gpu::TextureDesc { name: "atlas", format, @@ -222,6 +279,8 @@ impl BladeAtlasState { format, raw, raw_view, + msaa, + msaa_view, live_atlas_keys: 0, }; @@ -281,6 +340,7 @@ impl BladeAtlasState { struct BladeAtlasStorage { monochrome_textures: AtlasTextureList, polychrome_textures: AtlasTextureList, + path_textures: AtlasTextureList, } impl ops::Index for BladeAtlasStorage { @@ -289,6 +349,7 @@ impl ops::Index for BladeAtlasStorage { match kind { crate::AtlasTextureKind::Monochrome => &self.monochrome_textures, crate::AtlasTextureKind::Polychrome => &self.polychrome_textures, + crate::AtlasTextureKind::Path => &self.path_textures, } } } @@ -298,6 +359,7 @@ impl ops::IndexMut for BladeAtlasStorage { match kind { crate::AtlasTextureKind::Monochrome => &mut self.monochrome_textures, crate::AtlasTextureKind::Polychrome => &mut self.polychrome_textures, + crate::AtlasTextureKind::Path => &mut self.path_textures, } } } @@ -308,6 +370,7 @@ impl ops::Index for BladeAtlasStorage { let textures = match id.kind { crate::AtlasTextureKind::Monochrome => &self.monochrome_textures, crate::AtlasTextureKind::Polychrome => &self.polychrome_textures, + crate::AtlasTextureKind::Path => &self.path_textures, }; textures[id.index as usize].as_ref().unwrap() } @@ -321,6 +384,9 @@ impl BladeAtlasStorage { for mut texture in self.polychrome_textures.drain().flatten() { texture.destroy(gpu); } + for mut texture in self.path_textures.drain().flatten() { + texture.destroy(gpu); + } } } @@ -329,6 +395,8 @@ struct BladeAtlasTexture { allocator: BucketedAtlasAllocator, raw: gpu::Texture, raw_view: gpu::TextureView, + msaa: Option, + msaa_view: Option, format: gpu::TextureFormat, live_atlas_keys: u32, } @@ -356,6 +424,12 @@ impl BladeAtlasTexture { fn destroy(&mut self, gpu: &gpu::Context) { gpu.destroy_texture(self.raw); gpu.destroy_texture_view(self.raw_view); + if let Some(msaa) = self.msaa { + gpu.destroy_texture(msaa); + } + if let Some(msaa_view) = self.msaa_view { + gpu.destroy_texture_view(msaa_view); + } } fn bytes_per_pixel(&self) -> u8 { diff --git a/crates/gpui/src/platform/blade/blade_renderer.rs b/crates/gpui/src/platform/blade/blade_renderer.rs index 1b9f111b0d..cac47434ae 100644 --- a/crates/gpui/src/platform/blade/blade_renderer.rs +++ b/crates/gpui/src/platform/blade/blade_renderer.rs @@ -1,19 +1,24 @@ // Doing `if let` gives you nice scoping with passes/encoders #![allow(irrefutable_let_patterns)] -use super::{BladeAtlas, BladeContext}; +use super::{BladeAtlas, BladeContext, PATH_TEXTURE_FORMAT}; use crate::{ - Background, Bounds, ContentMask, DevicePixels, GpuSpecs, MonochromeSprite, PathVertex, - PolychromeSprite, PrimitiveBatch, Quad, ScaledPixels, Scene, Shadow, Size, Underline, + AtlasTextureKind, AtlasTile, Background, Bounds, ContentMask, DevicePixels, GpuSpecs, + MonochromeSprite, Path, PathId, PathVertex, PolychromeSprite, PrimitiveBatch, Quad, + ScaledPixels, Scene, Shadow, Size, Underline, }; -use blade_graphics::{self as gpu}; +use blade_graphics as gpu; use blade_util::{BufferBelt, BufferBeltDescriptor}; use bytemuck::{Pod, Zeroable}; +use collections::HashMap; #[cfg(target_os = "macos")] use media::core_video::CVMetalTextureCache; use std::{mem, sync::Arc}; const MAX_FRAME_TIME_MS: u32 = 10000; +// Use 4x MSAA, all devices support it. +// https://developer.apple.com/documentation/metal/mtldevice/1433355-supportstexturesamplecount +const DEFAULT_PATH_SAMPLE_COUNT: u32 = 4; #[repr(C)] #[derive(Clone, Copy, Pod, Zeroable)] @@ -61,9 +66,16 @@ struct ShaderShadowsData { } #[derive(blade_macros::ShaderData)] -struct ShaderPathsData { +struct ShaderPathRasterizationData { globals: GlobalParams, b_path_vertices: gpu::BufferPiece, +} + +#[derive(blade_macros::ShaderData)] +struct ShaderPathsData { + globals: GlobalParams, + t_sprite: gpu::TextureView, + s_sprite: gpu::Sampler, b_path_sprites: gpu::BufferPiece, } @@ -103,27 +115,13 @@ struct ShaderSurfacesData { struct PathSprite { bounds: Bounds, color: Background, -} - -/// Argument buffer layout for `draw_indirect` commands. -#[repr(C)] -#[derive(Copy, Clone, Debug, Default, Pod, Zeroable)] -pub struct DrawIndirectArgs { - /// The number of vertices to draw. - pub vertex_count: u32, - /// The number of instances to draw. - pub instance_count: u32, - /// The Index of the first vertex to draw. - pub first_vertex: u32, - /// The instance ID of the first instance to draw. - /// - /// Has to be 0, unless [`Features::INDIRECT_FIRST_INSTANCE`](crate::Features::INDIRECT_FIRST_INSTANCE) is enabled. - pub first_instance: u32, + tile: AtlasTile, } struct BladePipelines { quads: gpu::RenderPipeline, shadows: gpu::RenderPipeline, + path_rasterization: gpu::RenderPipeline, paths: gpu::RenderPipeline, underlines: gpu::RenderPipeline, mono_sprites: gpu::RenderPipeline, @@ -132,7 +130,7 @@ struct BladePipelines { } impl BladePipelines { - fn new(gpu: &gpu::Context, surface_info: gpu::SurfaceInfo, sample_count: u32) -> Self { + fn new(gpu: &gpu::Context, surface_info: gpu::SurfaceInfo, path_sample_count: u32) -> Self { use gpu::ShaderData as _; log::info!( @@ -180,10 +178,7 @@ impl BladePipelines { depth_stencil: None, fragment: Some(shader.at("fs_quad")), color_targets, - multisample_state: gpu::MultisampleState { - sample_count, - ..Default::default() - }, + multisample_state: gpu::MultisampleState::default(), }), shadows: gpu.create_render_pipeline(gpu::RenderPipelineDesc { name: "shadows", @@ -197,8 +192,26 @@ impl BladePipelines { depth_stencil: None, fragment: Some(shader.at("fs_shadow")), color_targets, + multisample_state: gpu::MultisampleState::default(), + }), + path_rasterization: gpu.create_render_pipeline(gpu::RenderPipelineDesc { + name: "path_rasterization", + data_layouts: &[&ShaderPathRasterizationData::layout()], + vertex: shader.at("vs_path_rasterization"), + vertex_fetches: &[], + primitive: gpu::PrimitiveState { + topology: gpu::PrimitiveTopology::TriangleList, + ..Default::default() + }, + depth_stencil: None, + fragment: Some(shader.at("fs_path_rasterization")), + color_targets: &[gpu::ColorTargetState { + format: PATH_TEXTURE_FORMAT, + blend: Some(gpu::BlendState::ADDITIVE), + write_mask: gpu::ColorWrites::default(), + }], multisample_state: gpu::MultisampleState { - sample_count, + sample_count: path_sample_count, ..Default::default() }, }), @@ -208,16 +221,13 @@ impl BladePipelines { vertex: shader.at("vs_path"), vertex_fetches: &[], primitive: gpu::PrimitiveState { - topology: gpu::PrimitiveTopology::TriangleList, + topology: gpu::PrimitiveTopology::TriangleStrip, ..Default::default() }, depth_stencil: None, fragment: Some(shader.at("fs_path")), color_targets, - multisample_state: gpu::MultisampleState { - sample_count, - ..Default::default() - }, + multisample_state: gpu::MultisampleState::default(), }), underlines: gpu.create_render_pipeline(gpu::RenderPipelineDesc { name: "underlines", @@ -231,10 +241,7 @@ impl BladePipelines { depth_stencil: None, fragment: Some(shader.at("fs_underline")), color_targets, - multisample_state: gpu::MultisampleState { - sample_count, - ..Default::default() - }, + multisample_state: gpu::MultisampleState::default(), }), mono_sprites: gpu.create_render_pipeline(gpu::RenderPipelineDesc { name: "mono-sprites", @@ -248,10 +255,7 @@ impl BladePipelines { depth_stencil: None, fragment: Some(shader.at("fs_mono_sprite")), color_targets, - multisample_state: gpu::MultisampleState { - sample_count, - ..Default::default() - }, + multisample_state: gpu::MultisampleState::default(), }), poly_sprites: gpu.create_render_pipeline(gpu::RenderPipelineDesc { name: "poly-sprites", @@ -265,10 +269,7 @@ impl BladePipelines { depth_stencil: None, fragment: Some(shader.at("fs_poly_sprite")), color_targets, - multisample_state: gpu::MultisampleState { - sample_count, - ..Default::default() - }, + multisample_state: gpu::MultisampleState::default(), }), surfaces: gpu.create_render_pipeline(gpu::RenderPipelineDesc { name: "surfaces", @@ -282,10 +283,7 @@ impl BladePipelines { depth_stencil: None, fragment: Some(shader.at("fs_surface")), color_targets, - multisample_state: gpu::MultisampleState { - sample_count, - ..Default::default() - }, + multisample_state: gpu::MultisampleState::default(), }), } } @@ -293,6 +291,7 @@ impl BladePipelines { fn destroy(&mut self, gpu: &gpu::Context) { gpu.destroy_render_pipeline(&mut self.quads); gpu.destroy_render_pipeline(&mut self.shadows); + gpu.destroy_render_pipeline(&mut self.path_rasterization); gpu.destroy_render_pipeline(&mut self.paths); gpu.destroy_render_pipeline(&mut self.underlines); gpu.destroy_render_pipeline(&mut self.mono_sprites); @@ -318,13 +317,12 @@ pub struct BladeRenderer { last_sync_point: Option, pipelines: BladePipelines, instance_belt: BufferBelt, + path_tiles: HashMap, atlas: Arc, atlas_sampler: gpu::Sampler, #[cfg(target_os = "macos")] core_video_texture_cache: CVMetalTextureCache, - sample_count: u32, - texture_msaa: Option, - texture_view_msaa: Option, + path_sample_count: u32, } impl BladeRenderer { @@ -333,18 +331,6 @@ impl BladeRenderer { window: &I, config: BladeSurfaceConfig, ) -> anyhow::Result { - // workaround for https://github.com/zed-industries/zed/issues/26143 - let sample_count = std::env::var("ZED_SAMPLE_COUNT") - .ok() - .or_else(|| std::env::var("ZED_PATH_SAMPLE_COUNT").ok()) - .and_then(|v| v.parse().ok()) - .or_else(|| { - [4, 2, 1] - .into_iter() - .find(|count| context.gpu.supports_texture_sample_count(*count)) - }) - .unwrap_or(1); - let surface_config = gpu::SurfaceConfig { size: config.size, usage: gpu::TextureUsage::TARGET, @@ -358,27 +344,22 @@ impl BladeRenderer { .create_surface_configured(window, surface_config) .map_err(|err| anyhow::anyhow!("Failed to create surface: {err:?}"))?; - let (texture_msaa, texture_view_msaa) = create_msaa_texture_if_needed( - &context.gpu, - surface.info().format, - config.size.width, - config.size.height, - sample_count, - ) - .unzip(); - let command_encoder = context.gpu.create_command_encoder(gpu::CommandEncoderDesc { name: "main", buffer_count: 2, }); - - let pipelines = BladePipelines::new(&context.gpu, surface.info(), sample_count); + // workaround for https://github.com/zed-industries/zed/issues/26143 + let path_sample_count = std::env::var("ZED_PATH_SAMPLE_COUNT") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(DEFAULT_PATH_SAMPLE_COUNT); + let pipelines = BladePipelines::new(&context.gpu, surface.info(), path_sample_count); let instance_belt = BufferBelt::new(BufferBeltDescriptor { memory: gpu::Memory::Shared, min_chunk_size: 0x1000, alignment: 0x40, // Vulkan `minStorageBufferOffsetAlignment` on Intel Xe }); - let atlas = Arc::new(BladeAtlas::new(&context.gpu)); + let atlas = Arc::new(BladeAtlas::new(&context.gpu, path_sample_count)); let atlas_sampler = context.gpu.create_sampler(gpu::SamplerDesc { name: "atlas", mag_filter: gpu::FilterMode::Linear, @@ -402,13 +383,12 @@ impl BladeRenderer { last_sync_point: None, pipelines, instance_belt, + path_tiles: HashMap::default(), atlas, atlas_sampler, #[cfg(target_os = "macos")] core_video_texture_cache, - sample_count, - texture_msaa, - texture_view_msaa, + path_sample_count, }) } @@ -461,24 +441,6 @@ impl BladeRenderer { self.surface_config.size = gpu_size; self.gpu .reconfigure_surface(&mut self.surface, self.surface_config); - - if let Some(texture_msaa) = self.texture_msaa { - self.gpu.destroy_texture(texture_msaa); - } - if let Some(texture_view_msaa) = self.texture_view_msaa { - self.gpu.destroy_texture_view(texture_view_msaa); - } - - let (texture_msaa, texture_view_msaa) = create_msaa_texture_if_needed( - &self.gpu, - self.surface.info().format, - gpu_size.width, - gpu_size.height, - self.sample_count, - ) - .unzip(); - self.texture_msaa = texture_msaa; - self.texture_view_msaa = texture_view_msaa; } } @@ -489,7 +451,8 @@ impl BladeRenderer { self.gpu .reconfigure_surface(&mut self.surface, self.surface_config); self.pipelines.destroy(&self.gpu); - self.pipelines = BladePipelines::new(&self.gpu, self.surface.info(), self.sample_count); + self.pipelines = + BladePipelines::new(&self.gpu, self.surface.info(), self.path_sample_count); } } @@ -527,6 +490,80 @@ impl BladeRenderer { objc2::rc::Retained::as_ptr(&self.surface.metal_layer()) as *mut _ } + #[profiling::function] + fn rasterize_paths(&mut self, paths: &[Path]) { + self.path_tiles.clear(); + let mut vertices_by_texture_id = HashMap::default(); + + for path in paths { + let clipped_bounds = path + .bounds + .intersect(&path.content_mask.bounds) + .map_origin(|origin| origin.floor()) + .map_size(|size| size.ceil()); + let tile = self.atlas.allocate_for_rendering( + clipped_bounds.size.map(Into::into), + AtlasTextureKind::Path, + &mut self.command_encoder, + ); + vertices_by_texture_id + .entry(tile.texture_id) + .or_insert(Vec::new()) + .extend(path.vertices.iter().map(|vertex| PathVertex { + xy_position: vertex.xy_position - clipped_bounds.origin + + tile.bounds.origin.map(Into::into), + st_position: vertex.st_position, + content_mask: ContentMask { + bounds: tile.bounds.map(Into::into), + }, + })); + self.path_tiles.insert(path.id, tile); + } + + for (texture_id, vertices) in vertices_by_texture_id { + let tex_info = self.atlas.get_texture_info(texture_id); + let globals = GlobalParams { + viewport_size: [tex_info.size.width as f32, tex_info.size.height as f32], + premultiplied_alpha: 0, + pad: 0, + }; + + let vertex_buf = unsafe { self.instance_belt.alloc_typed(&vertices, &self.gpu) }; + let frame_view = tex_info.raw_view; + let color_target = if let Some(msaa_view) = tex_info.msaa_view { + gpu::RenderTarget { + view: msaa_view, + init_op: gpu::InitOp::Clear(gpu::TextureColor::OpaqueBlack), + finish_op: gpu::FinishOp::ResolveTo(frame_view), + } + } else { + gpu::RenderTarget { + view: frame_view, + init_op: gpu::InitOp::Clear(gpu::TextureColor::OpaqueBlack), + finish_op: gpu::FinishOp::Store, + } + }; + + if let mut pass = self.command_encoder.render( + "paths", + gpu::RenderTargetSet { + colors: &[color_target], + depth_stencil: None, + }, + ) { + let mut encoder = pass.with(&self.pipelines.path_rasterization); + encoder.bind( + 0, + &ShaderPathRasterizationData { + globals, + b_path_vertices: vertex_buf, + }, + ); + encoder.draw(0, vertices.len() as u32, 0, 1); + } + } + } + pub fn destroy(&mut self) { self.wait_for_gpu(); self.atlas.destroy(); @@ -535,26 +572,17 @@ impl BladeRenderer { self.gpu.destroy_command_encoder(&mut self.command_encoder); self.pipelines.destroy(&self.gpu); self.gpu.destroy_surface(&mut self.surface); - if let Some(texture_msaa) = self.texture_msaa { - self.gpu.destroy_texture(texture_msaa); - } - if let Some(texture_view_msaa) = self.texture_view_msaa { - self.gpu.destroy_texture_view(texture_view_msaa); - } } pub fn draw(&mut self, scene: &Scene) { self.command_encoder.start(); self.atlas.before_frame(&mut self.command_encoder); + self.rasterize_paths(scene.paths()); let frame = { profiling::scope!("acquire frame"); self.surface.acquire_frame() }; - let frame_view = frame.texture_view(); - if let Some(texture_msaa) = self.texture_msaa { - self.command_encoder.init_texture(texture_msaa); - } self.command_encoder.init_texture(frame.texture()); let globals = GlobalParams { @@ -569,25 +597,14 @@ impl BladeRenderer { pad: 0, }; - let target = if let Some(texture_view_msaa) = self.texture_view_msaa { - gpu::RenderTarget { - view: texture_view_msaa, - init_op: gpu::InitOp::Clear(gpu::TextureColor::TransparentBlack), - finish_op: gpu::FinishOp::ResolveTo(frame_view), - } - } else { - gpu::RenderTarget { - view: frame_view, - init_op: gpu::InitOp::Clear(gpu::TextureColor::TransparentBlack), - finish_op: gpu::FinishOp::Store, - } - }; - - // draw to the target texture if let mut pass = self.command_encoder.render( "main", gpu::RenderTargetSet { - colors: &[target], + colors: &[gpu::RenderTarget { + view: frame.texture_view(), + init_op: gpu::InitOp::Clear(gpu::TextureColor::TransparentBlack), + finish_op: gpu::FinishOp::Store, + }], depth_stencil: None, }, ) { @@ -622,55 +639,32 @@ impl BladeRenderer { } PrimitiveBatch::Paths(paths) => { let mut encoder = pass.with(&self.pipelines.paths); - - let mut vertices = Vec::new(); - let mut sprites = Vec::with_capacity(paths.len()); - let mut draw_indirect_commands = Vec::with_capacity(paths.len()); - let mut first_vertex = 0; - - for (i, path) in paths.iter().enumerate() { - draw_indirect_commands.push(DrawIndirectArgs { - vertex_count: path.vertices.len() as u32, - instance_count: 1, - first_vertex, - first_instance: i as u32, - }); - first_vertex += path.vertices.len() as u32; - - vertices.extend(path.vertices.iter().map(|v| PathVertex { - xy_position: v.xy_position, - content_mask: ContentMask { - bounds: path.content_mask.bounds, + // todo(linux): group by texture ID + for path in paths { + let tile = &self.path_tiles[&path.id]; + let tex_info = self.atlas.get_texture_info(tile.texture_id); + let origin = path.bounds.intersect(&path.content_mask.bounds).origin; + let sprites = [PathSprite { + bounds: Bounds { + origin: origin.map(|p| p.floor()), + size: tile.bounds.size.map(Into::into), }, - })); - - sprites.push(PathSprite { - bounds: path.bounds, color: path.color, - }); - } + tile: (*tile).clone(), + }]; - let b_path_vertices = - unsafe { self.instance_belt.alloc_typed(&vertices, &self.gpu) }; - let instance_buf = - unsafe { self.instance_belt.alloc_typed(&sprites, &self.gpu) }; - let indirect_buf = unsafe { - self.instance_belt - .alloc_typed(&draw_indirect_commands, &self.gpu) - }; - - encoder.bind( - 0, - &ShaderPathsData { - globals, - b_path_vertices, - b_path_sprites: instance_buf, - }, - ); - - for i in 0..paths.len() { - encoder.draw_indirect(indirect_buf.buffer.at(indirect_buf.offset - + (i * mem::size_of::()) as u64)); + let instance_buf = + unsafe { self.instance_belt.alloc_typed(&sprites, &self.gpu) }; + encoder.bind( + 0, + &ShaderPathsData { + globals, + t_sprite: tex_info.raw_view, + s_sprite: self.atlas_sampler, + b_path_sprites: instance_buf, + }, + ); + encoder.draw(0, 4, 0, sprites.len() as u32); } } PrimitiveBatch::Underlines(underlines) => { @@ -823,47 +817,9 @@ impl BladeRenderer { profiling::scope!("finish"); self.instance_belt.flush(&sync_point); self.atlas.after_frame(&sync_point); + self.atlas.clear_textures(AtlasTextureKind::Path); self.wait_for_gpu(); self.last_sync_point = Some(sync_point); } } - -fn create_msaa_texture_if_needed( - gpu: &gpu::Context, - format: gpu::TextureFormat, - width: u32, - height: u32, - sample_count: u32, -) -> Option<(gpu::Texture, gpu::TextureView)> { - if sample_count <= 1 { - return None; - } - - let texture_msaa = gpu.create_texture(gpu::TextureDesc { - name: "msaa", - format, - size: gpu::Extent { - width, - height, - depth: 1, - }, - array_layer_count: 1, - mip_level_count: 1, - sample_count, - dimension: gpu::TextureDimension::D2, - usage: gpu::TextureUsage::TARGET, - external: None, - }); - let texture_view_msaa = gpu.create_texture_view( - texture_msaa, - gpu::TextureViewDesc { - name: "msaa view", - format, - dimension: gpu::ViewDimension::D2, - subresources: &Default::default(), - }, - ); - - Some((texture_msaa, texture_view_msaa)) -} diff --git a/crates/gpui/src/platform/blade/shaders.wgsl b/crates/gpui/src/platform/blade/shaders.wgsl index 00c9d07af7..0b34a0eea3 100644 --- a/crates/gpui/src/platform/blade/shaders.wgsl +++ b/crates/gpui/src/platform/blade/shaders.wgsl @@ -922,23 +922,59 @@ fn fs_shadow(input: ShadowVarying) -> @location(0) vec4 { return blend_color(input.color, alpha); } -// --- paths --- // +// --- path rasterization --- // struct PathVertex { xy_position: vec2, + st_position: vec2, content_mask: Bounds, } +var b_path_vertices: array; + +struct PathRasterizationVarying { + @builtin(position) position: vec4, + @location(0) st_position: vec2, + //TODO: use `clip_distance` once Naga supports it + @location(3) clip_distances: vec4, +} + +@vertex +fn vs_path_rasterization(@builtin(vertex_index) vertex_id: u32) -> PathRasterizationVarying { + let v = b_path_vertices[vertex_id]; + + var out = PathRasterizationVarying(); + out.position = to_device_position_impl(v.xy_position); + out.st_position = v.st_position; + out.clip_distances = distance_from_clip_rect_impl(v.xy_position, v.content_mask); + return out; +} + +@fragment +fn fs_path_rasterization(input: PathRasterizationVarying) -> @location(0) f32 { + let dx = dpdx(input.st_position); + let dy = dpdy(input.st_position); + if (any(input.clip_distances < vec4(0.0))) { + return 0.0; + } + + let gradient = 2.0 * input.st_position.xx * vec2(dx.x, dy.x) - vec2(dx.y, dy.y); + let f = input.st_position.x * input.st_position.x - input.st_position.y; + let distance = f / length(gradient); + return saturate(0.5 - distance); +} + +// --- paths --- // struct PathSprite { bounds: Bounds, color: Background, + tile: AtlasTile, } -var b_path_vertices: array; var b_path_sprites: array; struct PathVarying { @builtin(position) position: vec4, - @location(0) clip_distances: vec4, + @location(0) tile_position: vec2, @location(1) @interpolate(flat) instance_id: u32, @location(2) @interpolate(flat) color_solid: vec4, @location(3) @interpolate(flat) color0: vec4, @@ -947,12 +983,13 @@ struct PathVarying { @vertex fn vs_path(@builtin(vertex_index) vertex_id: u32, @builtin(instance_index) instance_id: u32) -> PathVarying { - let v = b_path_vertices[vertex_id]; + let unit_vertex = vec2(f32(vertex_id & 1u), 0.5 * f32(vertex_id & 2u)); let sprite = b_path_sprites[instance_id]; + // Don't apply content mask because it was already accounted for when rasterizing the path. var out = PathVarying(); - out.position = to_device_position_impl(v.xy_position); - out.clip_distances = distance_from_clip_rect_impl(v.xy_position, v.content_mask); + out.position = to_device_position(unit_vertex, sprite.bounds); + out.tile_position = to_tile_position(unit_vertex, sprite.tile); out.instance_id = instance_id; let gradient = prepare_gradient_color( @@ -969,15 +1006,13 @@ fn vs_path(@builtin(vertex_index) vertex_id: u32, @builtin(instance_index) insta @fragment fn fs_path(input: PathVarying) -> @location(0) vec4 { - if any(input.clip_distances < vec4(0.0)) { - return vec4(0.0); - } - + let sample = textureSample(t_sprite, s_sprite, input.tile_position).r; + let mask = 1.0 - abs(1.0 - sample % 2.0); let sprite = b_path_sprites[input.instance_id]; let background = sprite.color; let color = gradient_color(background, input.position.xy, sprite.bounds, input.color_solid, input.color0, input.color1); - return blend_color(color, 1.0); + return blend_color(color, mask); } // --- underlines --- // diff --git a/crates/gpui/src/platform/keystroke.rs b/crates/gpui/src/platform/keystroke.rs index 8b6e72d150..24601eefd6 100644 --- a/crates/gpui/src/platform/keystroke.rs +++ b/crates/gpui/src/platform/keystroke.rs @@ -417,17 +417,6 @@ impl Modifiers { self.control || self.alt || self.shift || self.platform || self.function } - /// Returns the XOR of two modifier sets - pub fn xor(&self, other: &Modifiers) -> Modifiers { - Modifiers { - control: self.control ^ other.control, - alt: self.alt ^ other.alt, - shift: self.shift ^ other.shift, - platform: self.platform ^ other.platform, - function: self.function ^ other.function, - } - } - /// Whether the semantically 'secondary' modifier key is pressed. /// /// On macOS, this is the command key. @@ -545,11 +534,62 @@ impl Modifiers { /// Checks if this [`Modifiers`] is a subset of another [`Modifiers`]. pub fn is_subset_of(&self, other: &Modifiers) -> bool { - (other.control || !self.control) - && (other.alt || !self.alt) - && (other.shift || !self.shift) - && (other.platform || !self.platform) - && (other.function || !self.function) + (*other & *self) == *self + } +} + +impl std::ops::BitOr for Modifiers { + type Output = Self; + + fn bitor(mut self, other: Self) -> Self::Output { + self |= other; + self + } +} + +impl std::ops::BitOrAssign for Modifiers { + fn bitor_assign(&mut self, other: Self) { + self.control |= other.control; + self.alt |= other.alt; + self.shift |= other.shift; + self.platform |= other.platform; + self.function |= other.function; + } +} + +impl std::ops::BitXor for Modifiers { + type Output = Self; + fn bitxor(mut self, rhs: Self) -> Self::Output { + self ^= rhs; + self + } +} + +impl std::ops::BitXorAssign for Modifiers { + fn bitxor_assign(&mut self, other: Self) { + self.control ^= other.control; + self.alt ^= other.alt; + self.shift ^= other.shift; + self.platform ^= other.platform; + self.function ^= other.function; + } +} + +impl std::ops::BitAnd for Modifiers { + type Output = Self; + fn bitand(mut self, rhs: Self) -> Self::Output { + self &= rhs; + self + } +} + +impl std::ops::BitAndAssign for Modifiers { + fn bitand_assign(&mut self, other: Self) { + self.control &= other.control; + self.alt &= other.alt; + self.shift &= other.shift; + self.platform &= other.platform; + self.function &= other.function; } } diff --git a/crates/gpui/src/platform/linux/platform.rs b/crates/gpui/src/platform/linux/platform.rs index bab44e0069..9efd7e9538 100644 --- a/crates/gpui/src/platform/linux/platform.rs +++ b/crates/gpui/src/platform/linux/platform.rs @@ -822,11 +822,41 @@ impl crate::Keystroke { Keysym::underscore => "_".to_owned(), Keysym::equal => "=".to_owned(), Keysym::plus => "+".to_owned(), + Keysym::space => "space".to_owned(), + Keysym::BackSpace => "backspace".to_owned(), + Keysym::Tab => "tab".to_owned(), + Keysym::Delete => "delete".to_owned(), + Keysym::Escape => "escape".to_owned(), + + Keysym::Left => "left".to_owned(), + Keysym::Right => "right".to_owned(), + Keysym::Up => "up".to_owned(), + Keysym::Down => "down".to_owned(), + Keysym::Home => "home".to_owned(), + Keysym::End => "end".to_owned(), _ => { let name = xkb::keysym_get_name(key_sym).to_lowercase(); if key_sym.is_keypad_key() { name.replace("kp_", "") + } else if let Some(key) = key_utf8.chars().next() + && key_utf8.len() == 1 + && key.is_ascii() + { + if key.is_ascii_graphic() { + key_utf8.to_lowercase() + // map ctrl-a to `a` + // ctrl-0..9 may emit control codes like ctrl-[, but + // we don't want to map them to `[` + } else if key_utf32 <= 0x1f + && !name.chars().next().is_some_and(|c| c.is_ascii_digit()) + { + ((key_utf32 as u8 + 0x40) as char) + .to_ascii_lowercase() + .to_string() + } else { + name + } } else if let Some(key_en) = guess_ascii(keycode, modifiers.shift) { String::from(key_en) } else { diff --git a/crates/gpui/src/platform/mac/metal_atlas.rs b/crates/gpui/src/platform/mac/metal_atlas.rs index 0c8e1d3703..366f2dcc3c 100644 --- a/crates/gpui/src/platform/mac/metal_atlas.rs +++ b/crates/gpui/src/platform/mac/metal_atlas.rs @@ -13,12 +13,14 @@ use std::borrow::Cow; pub(crate) struct MetalAtlas(Mutex); impl MetalAtlas { - pub(crate) fn new(device: Device) -> Self { + pub(crate) fn new(device: Device, path_sample_count: u32) -> Self { MetalAtlas(Mutex::new(MetalAtlasState { device: AssertSend(device), monochrome_textures: Default::default(), polychrome_textures: Default::default(), + path_textures: Default::default(), tiles_by_key: Default::default(), + path_sample_count, })) } @@ -26,7 +28,10 @@ impl MetalAtlas { self.0.lock().texture(id).metal_texture.clone() } - #[allow(dead_code)] + pub(crate) fn msaa_texture(&self, id: AtlasTextureId) -> Option { + self.0.lock().texture(id).msaa_texture.clone() + } + pub(crate) fn allocate( &self, size: Size, @@ -35,12 +40,12 @@ impl MetalAtlas { self.0.lock().allocate(size, texture_kind) } - #[allow(dead_code)] pub(crate) fn clear_textures(&self, texture_kind: AtlasTextureKind) { let mut lock = self.0.lock(); let textures = match texture_kind { AtlasTextureKind::Monochrome => &mut lock.monochrome_textures, AtlasTextureKind::Polychrome => &mut lock.polychrome_textures, + AtlasTextureKind::Path => &mut lock.path_textures, }; for texture in textures.iter_mut() { texture.clear(); @@ -52,7 +57,9 @@ struct MetalAtlasState { device: AssertSend, monochrome_textures: AtlasTextureList, polychrome_textures: AtlasTextureList, + path_textures: AtlasTextureList, tiles_by_key: FxHashMap, + path_sample_count: u32, } impl PlatformAtlas for MetalAtlas { @@ -87,6 +94,7 @@ impl PlatformAtlas for MetalAtlas { let textures = match id.kind { AtlasTextureKind::Monochrome => &mut lock.monochrome_textures, AtlasTextureKind::Polychrome => &mut lock.polychrome_textures, + AtlasTextureKind::Path => &mut lock.polychrome_textures, }; let Some(texture_slot) = textures @@ -120,6 +128,7 @@ impl MetalAtlasState { let textures = match texture_kind { AtlasTextureKind::Monochrome => &mut self.monochrome_textures, AtlasTextureKind::Polychrome => &mut self.polychrome_textures, + AtlasTextureKind::Path => &mut self.path_textures, }; if let Some(tile) = textures @@ -164,14 +173,31 @@ impl MetalAtlasState { pixel_format = metal::MTLPixelFormat::BGRA8Unorm; usage = metal::MTLTextureUsage::ShaderRead; } + AtlasTextureKind::Path => { + pixel_format = metal::MTLPixelFormat::R16Float; + usage = metal::MTLTextureUsage::RenderTarget | metal::MTLTextureUsage::ShaderRead; + } } texture_descriptor.set_pixel_format(pixel_format); texture_descriptor.set_usage(usage); let metal_texture = self.device.new_texture(&texture_descriptor); + // We currently only enable MSAA for path textures. + let msaa_texture = if self.path_sample_count > 1 && kind == AtlasTextureKind::Path { + let mut descriptor = texture_descriptor.clone(); + descriptor.set_texture_type(metal::MTLTextureType::D2Multisample); + descriptor.set_storage_mode(metal::MTLStorageMode::Private); + descriptor.set_sample_count(self.path_sample_count as _); + let msaa_texture = self.device.new_texture(&descriptor); + Some(msaa_texture) + } else { + None + }; + let texture_list = match kind { AtlasTextureKind::Monochrome => &mut self.monochrome_textures, AtlasTextureKind::Polychrome => &mut self.polychrome_textures, + AtlasTextureKind::Path => &mut self.path_textures, }; let index = texture_list.free_list.pop(); @@ -183,6 +209,7 @@ impl MetalAtlasState { }, allocator: etagere::BucketedAtlasAllocator::new(size.into()), metal_texture: AssertSend(metal_texture), + msaa_texture: AssertSend(msaa_texture), live_atlas_keys: 0, }; @@ -199,6 +226,7 @@ impl MetalAtlasState { let textures = match id.kind { crate::AtlasTextureKind::Monochrome => &self.monochrome_textures, crate::AtlasTextureKind::Polychrome => &self.polychrome_textures, + crate::AtlasTextureKind::Path => &self.path_textures, }; textures[id.index as usize].as_ref().unwrap() } @@ -208,6 +236,7 @@ struct MetalAtlasTexture { id: AtlasTextureId, allocator: BucketedAtlasAllocator, metal_texture: AssertSend, + msaa_texture: AssertSend>, live_atlas_keys: u32, } diff --git a/crates/gpui/src/platform/mac/metal_renderer.rs b/crates/gpui/src/platform/mac/metal_renderer.rs index 8936cf242c..3cdc2dd2cf 100644 --- a/crates/gpui/src/platform/mac/metal_renderer.rs +++ b/crates/gpui/src/platform/mac/metal_renderer.rs @@ -1,28 +1,27 @@ use super::metal_atlas::MetalAtlas; use crate::{ - AtlasTextureId, Background, Bounds, ContentMask, DevicePixels, MonochromeSprite, PaintSurface, - Path, PathVertex, PolychromeSprite, PrimitiveBatch, Quad, ScaledPixels, Scene, Shadow, Size, - Surface, Underline, point, size, + AtlasTextureId, AtlasTextureKind, AtlasTile, Background, Bounds, ContentMask, DevicePixels, + MonochromeSprite, PaintSurface, Path, PathId, PathVertex, PolychromeSprite, PrimitiveBatch, + Quad, ScaledPixels, Scene, Shadow, Size, Surface, Underline, point, size, }; -use anyhow::Result; +use anyhow::{Context as _, Result}; use block::ConcreteBlock; use cocoa::{ base::{NO, YES}, foundation::{NSSize, NSUInteger}, quartzcore::AutoresizingMask, }; +use collections::HashMap; use core_foundation::base::TCFType; use core_video::{ metal_texture::CVMetalTextureGetTexture, metal_texture_cache::CVMetalTextureCache, pixel_buffer::kCVPixelFormatType_420YpCbCr8BiPlanarFullRange, }; use foreign_types::{ForeignType, ForeignTypeRef}; -use metal::{ - CAMetalLayer, CommandQueue, MTLDrawPrimitivesIndirectArguments, MTLPixelFormat, - MTLResourceOptions, NSRange, -}; +use metal::{CAMetalLayer, CommandQueue, MTLPixelFormat, MTLResourceOptions, NSRange}; use objc::{self, msg_send, sel, sel_impl}; use parking_lot::Mutex; +use smallvec::SmallVec; use std::{cell::Cell, ffi::c_void, mem, ptr, sync::Arc}; // Exported to metal @@ -32,6 +31,9 @@ pub(crate) type PointF = crate::Point; const SHADERS_METALLIB: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/shaders.metallib")); #[cfg(feature = "runtime_shaders")] const SHADERS_SOURCE_FILE: &str = include_str!(concat!(env!("OUT_DIR"), "/stitched_shaders.metal")); +// Use 4x MSAA, all devices support it. +// https://developer.apple.com/documentation/metal/mtldevice/1433355-supportstexturesamplecount +const PATH_SAMPLE_COUNT: u32 = 4; pub type Context = Arc>; pub type Renderer = MetalRenderer; @@ -96,7 +98,8 @@ pub(crate) struct MetalRenderer { layer: metal::MetalLayer, presents_with_transaction: bool, command_queue: CommandQueue, - path_pipeline_state: metal::RenderPipelineState, + paths_rasterization_pipeline_state: metal::RenderPipelineState, + path_sprites_pipeline_state: metal::RenderPipelineState, shadows_pipeline_state: metal::RenderPipelineState, quads_pipeline_state: metal::RenderPipelineState, underlines_pipeline_state: metal::RenderPipelineState, @@ -108,8 +111,6 @@ pub(crate) struct MetalRenderer { instance_buffer_pool: Arc>, sprite_atlas: Arc, core_video_texture_cache: core_video::metal_texture_cache::CVMetalTextureCache, - sample_count: u64, - msaa_texture: Option, } impl MetalRenderer { @@ -168,19 +169,22 @@ impl MetalRenderer { MTLResourceOptions::StorageModeManaged, ); - let sample_count = [4, 2, 1] - .into_iter() - .find(|count| device.supports_texture_sample_count(*count)) - .unwrap_or(1); - - let path_pipeline_state = build_pipeline_state( + let paths_rasterization_pipeline_state = build_path_rasterization_pipeline_state( &device, &library, - "paths", - "path_vertex", - "path_fragment", + "paths_rasterization", + "path_rasterization_vertex", + "path_rasterization_fragment", + MTLPixelFormat::R16Float, + PATH_SAMPLE_COUNT, + ); + let path_sprites_pipeline_state = build_pipeline_state( + &device, + &library, + "path_sprites", + "path_sprite_vertex", + "path_sprite_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let shadows_pipeline_state = build_pipeline_state( &device, @@ -189,7 +193,6 @@ impl MetalRenderer { "shadow_vertex", "shadow_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let quads_pipeline_state = build_pipeline_state( &device, @@ -198,7 +201,6 @@ impl MetalRenderer { "quad_vertex", "quad_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let underlines_pipeline_state = build_pipeline_state( &device, @@ -207,7 +209,6 @@ impl MetalRenderer { "underline_vertex", "underline_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let monochrome_sprites_pipeline_state = build_pipeline_state( &device, @@ -216,7 +217,6 @@ impl MetalRenderer { "monochrome_sprite_vertex", "monochrome_sprite_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let polychrome_sprites_pipeline_state = build_pipeline_state( &device, @@ -225,7 +225,6 @@ impl MetalRenderer { "polychrome_sprite_vertex", "polychrome_sprite_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let surfaces_pipeline_state = build_pipeline_state( &device, @@ -234,21 +233,20 @@ impl MetalRenderer { "surface_vertex", "surface_fragment", MTLPixelFormat::BGRA8Unorm, - sample_count, ); let command_queue = device.new_command_queue(); - let sprite_atlas = Arc::new(MetalAtlas::new(device.clone())); + let sprite_atlas = Arc::new(MetalAtlas::new(device.clone(), PATH_SAMPLE_COUNT)); let core_video_texture_cache = CVMetalTextureCache::new(None, device.clone(), None).unwrap(); - let msaa_texture = create_msaa_texture(&device, &layer, sample_count); Self { device, layer, presents_with_transaction: false, command_queue, - path_pipeline_state, + paths_rasterization_pipeline_state, + path_sprites_pipeline_state, shadows_pipeline_state, quads_pipeline_state, underlines_pipeline_state, @@ -259,8 +257,6 @@ impl MetalRenderer { instance_buffer_pool, sprite_atlas, core_video_texture_cache, - sample_count, - msaa_texture, } } @@ -293,8 +289,6 @@ impl MetalRenderer { setDrawableSize: size ]; } - - self.msaa_texture = create_msaa_texture(&self.device, &self.layer, self.sample_count); } pub fn update_transparency(&self, _transparent: bool) { @@ -381,23 +375,25 @@ impl MetalRenderer { let command_queue = self.command_queue.clone(); let command_buffer = command_queue.new_command_buffer(); let mut instance_offset = 0; + + let path_tiles = self + .rasterize_paths( + scene.paths(), + instance_buffer, + &mut instance_offset, + command_buffer, + ) + .with_context(|| format!("rasterizing {} paths", scene.paths().len()))?; + let render_pass_descriptor = metal::RenderPassDescriptor::new(); let color_attachment = render_pass_descriptor .color_attachments() .object_at(0) .unwrap(); - if let Some(msaa_texture_ref) = self.msaa_texture.as_deref() { - color_attachment.set_texture(Some(msaa_texture_ref)); - color_attachment.set_load_action(metal::MTLLoadAction::Clear); - color_attachment.set_store_action(metal::MTLStoreAction::MultisampleResolve); - color_attachment.set_resolve_texture(Some(drawable.texture())); - } else { - color_attachment.set_load_action(metal::MTLLoadAction::Clear); - color_attachment.set_texture(Some(drawable.texture())); - color_attachment.set_store_action(metal::MTLStoreAction::Store); - } - + color_attachment.set_texture(Some(drawable.texture())); + color_attachment.set_load_action(metal::MTLLoadAction::Clear); + color_attachment.set_store_action(metal::MTLStoreAction::Store); let alpha = if self.layer.is_opaque() { 1. } else { 0. }; color_attachment.set_clear_color(metal::MTLClearColor::new(0., 0., 0., alpha)); let command_encoder = command_buffer.new_render_command_encoder(render_pass_descriptor); @@ -429,6 +425,7 @@ impl MetalRenderer { ), PrimitiveBatch::Paths(paths) => self.draw_paths( paths, + &path_tiles, instance_buffer, &mut instance_offset, viewport_size, @@ -496,6 +493,106 @@ impl MetalRenderer { Ok(command_buffer.to_owned()) } + fn rasterize_paths( + &self, + paths: &[Path], + instance_buffer: &mut InstanceBuffer, + instance_offset: &mut usize, + command_buffer: &metal::CommandBufferRef, + ) -> Option> { + self.sprite_atlas.clear_textures(AtlasTextureKind::Path); + + let mut tiles = HashMap::default(); + let mut vertices_by_texture_id = HashMap::default(); + for path in paths { + let clipped_bounds = path.bounds.intersect(&path.content_mask.bounds); + + let tile = self + .sprite_atlas + .allocate(clipped_bounds.size.map(Into::into), AtlasTextureKind::Path)?; + vertices_by_texture_id + .entry(tile.texture_id) + .or_insert(Vec::new()) + .extend(path.vertices.iter().map(|vertex| PathVertex { + xy_position: vertex.xy_position - clipped_bounds.origin + + tile.bounds.origin.map(Into::into), + st_position: vertex.st_position, + content_mask: ContentMask { + bounds: tile.bounds.map(Into::into), + }, + })); + tiles.insert(path.id, tile); + } + + for (texture_id, vertices) in vertices_by_texture_id { + align_offset(instance_offset); + let vertices_bytes_len = mem::size_of_val(vertices.as_slice()); + let next_offset = *instance_offset + vertices_bytes_len; + if next_offset > instance_buffer.size { + return None; + } + + let render_pass_descriptor = metal::RenderPassDescriptor::new(); + let color_attachment = render_pass_descriptor + .color_attachments() + .object_at(0) + .unwrap(); + + let texture = self.sprite_atlas.metal_texture(texture_id); + let msaa_texture = self.sprite_atlas.msaa_texture(texture_id); + + if let Some(msaa_texture) = msaa_texture { + color_attachment.set_texture(Some(&msaa_texture)); + color_attachment.set_resolve_texture(Some(&texture)); + color_attachment.set_load_action(metal::MTLLoadAction::Clear); + color_attachment.set_store_action(metal::MTLStoreAction::MultisampleResolve); + } else { + color_attachment.set_texture(Some(&texture)); + color_attachment.set_load_action(metal::MTLLoadAction::Clear); + color_attachment.set_store_action(metal::MTLStoreAction::Store); + } + color_attachment.set_clear_color(metal::MTLClearColor::new(0., 0., 0., 1.)); + + let command_encoder = command_buffer.new_render_command_encoder(render_pass_descriptor); + command_encoder.set_render_pipeline_state(&self.paths_rasterization_pipeline_state); + command_encoder.set_vertex_buffer( + PathRasterizationInputIndex::Vertices as u64, + Some(&instance_buffer.metal_buffer), + *instance_offset as u64, + ); + let texture_size = Size { + width: DevicePixels::from(texture.width()), + height: DevicePixels::from(texture.height()), + }; + command_encoder.set_vertex_bytes( + PathRasterizationInputIndex::AtlasTextureSize as u64, + mem::size_of_val(&texture_size) as u64, + &texture_size as *const Size as *const _, + ); + + let buffer_contents = unsafe { + (instance_buffer.metal_buffer.contents() as *mut u8).add(*instance_offset) + }; + unsafe { + ptr::copy_nonoverlapping( + vertices.as_ptr() as *const u8, + buffer_contents, + vertices_bytes_len, + ); + } + + command_encoder.draw_primitives( + metal::MTLPrimitiveType::Triangle, + 0, + vertices.len() as u64, + ); + command_encoder.end_encoding(); + *instance_offset = next_offset; + } + + Some(tiles) + } + fn draw_shadows( &self, shadows: &[Shadow], @@ -621,6 +718,7 @@ impl MetalRenderer { fn draw_paths( &self, paths: &[Path], + tiles_by_path_id: &HashMap, instance_buffer: &mut InstanceBuffer, instance_offset: &mut usize, viewport_size: Size, @@ -630,108 +728,100 @@ impl MetalRenderer { return true; } - command_encoder.set_render_pipeline_state(&self.path_pipeline_state); + command_encoder.set_render_pipeline_state(&self.path_sprites_pipeline_state); + command_encoder.set_vertex_buffer( + SpriteInputIndex::Vertices as u64, + Some(&self.unit_vertices), + 0, + ); + command_encoder.set_vertex_bytes( + SpriteInputIndex::ViewportSize as u64, + mem::size_of_val(&viewport_size) as u64, + &viewport_size as *const Size as *const _, + ); - unsafe { - let base_addr = instance_buffer.metal_buffer.contents(); - let mut p = (base_addr as *mut u8).add(*instance_offset); - let mut draw_indirect_commands = Vec::with_capacity(paths.len()); + let mut prev_texture_id = None; + let mut sprites = SmallVec::<[_; 1]>::new(); + let mut paths_and_tiles = paths + .iter() + .map(|path| (path, tiles_by_path_id.get(&path.id).unwrap())) + .peekable(); - // copy vertices - let vertices_offset = (p as usize) - (base_addr as usize); - let mut first_vertex = 0; - for (i, path) in paths.iter().enumerate() { - if (p as usize) - (base_addr as usize) - + (mem::size_of::>() * path.vertices.len()) - > instance_buffer.size - { + loop { + if let Some((path, tile)) = paths_and_tiles.peek() { + if prev_texture_id.map_or(true, |texture_id| texture_id == tile.texture_id) { + prev_texture_id = Some(tile.texture_id); + let origin = path.bounds.intersect(&path.content_mask.bounds).origin; + sprites.push(PathSprite { + bounds: Bounds { + origin: origin.map(|p| p.floor()), + size: tile.bounds.size.map(Into::into), + }, + color: path.color, + tile: (*tile).clone(), + }); + paths_and_tiles.next(); + continue; + } + } + + if sprites.is_empty() { + break; + } else { + align_offset(instance_offset); + let texture_id = prev_texture_id.take().unwrap(); + let texture: metal::Texture = self.sprite_atlas.metal_texture(texture_id); + let texture_size = size( + DevicePixels(texture.width() as i32), + DevicePixels(texture.height() as i32), + ); + + command_encoder.set_vertex_buffer( + SpriteInputIndex::Sprites as u64, + Some(&instance_buffer.metal_buffer), + *instance_offset as u64, + ); + command_encoder.set_vertex_bytes( + SpriteInputIndex::AtlasTextureSize as u64, + mem::size_of_val(&texture_size) as u64, + &texture_size as *const Size as *const _, + ); + command_encoder.set_fragment_buffer( + SpriteInputIndex::Sprites as u64, + Some(&instance_buffer.metal_buffer), + *instance_offset as u64, + ); + command_encoder + .set_fragment_texture(SpriteInputIndex::AtlasTexture as u64, Some(&texture)); + + let sprite_bytes_len = mem::size_of_val(sprites.as_slice()); + let next_offset = *instance_offset + sprite_bytes_len; + if next_offset > instance_buffer.size { return false; } - for v in &path.vertices { - *(p as *mut PathVertex) = PathVertex { - xy_position: v.xy_position, - content_mask: ContentMask { - bounds: path.content_mask.bounds, - }, - }; - p = p.add(mem::size_of::>()); + let buffer_contents = unsafe { + (instance_buffer.metal_buffer.contents() as *mut u8).add(*instance_offset) + }; + + unsafe { + ptr::copy_nonoverlapping( + sprites.as_ptr() as *const u8, + buffer_contents, + sprite_bytes_len, + ); } - draw_indirect_commands.push(MTLDrawPrimitivesIndirectArguments { - vertexCount: path.vertices.len() as u32, - instanceCount: 1, - vertexStart: first_vertex, - baseInstance: i as u32, - }); - first_vertex += path.vertices.len() as u32; - } - - // copy sprites - let sprites_offset = (p as u64) - (base_addr as u64); - if (p as usize) - (base_addr as usize) + (mem::size_of::() * paths.len()) - > instance_buffer.size - { - return false; - } - for path in paths { - *(p as *mut PathSprite) = PathSprite { - bounds: path.bounds, - color: path.color, - }; - p = p.add(mem::size_of::()); - } - - // copy indirect commands - let icb_bytes_len = mem::size_of_val(draw_indirect_commands.as_slice()); - let icb_offset = (p as u64) - (base_addr as u64); - if (p as usize) - (base_addr as usize) + icb_bytes_len > instance_buffer.size { - return false; - } - ptr::copy_nonoverlapping( - draw_indirect_commands.as_ptr() as *const u8, - p, - icb_bytes_len, - ); - p = p.add(icb_bytes_len); - - // draw path - command_encoder.set_vertex_buffer( - PathInputIndex::Vertices as u64, - Some(&instance_buffer.metal_buffer), - vertices_offset as u64, - ); - - command_encoder.set_vertex_bytes( - PathInputIndex::ViewportSize as u64, - mem::size_of_val(&viewport_size) as u64, - &viewport_size as *const Size as *const _, - ); - - command_encoder.set_vertex_buffer( - PathInputIndex::Sprites as u64, - Some(&instance_buffer.metal_buffer), - sprites_offset, - ); - - command_encoder.set_fragment_buffer( - PathInputIndex::Sprites as u64, - Some(&instance_buffer.metal_buffer), - sprites_offset, - ); - - for i in 0..paths.len() { - command_encoder.draw_primitives_indirect( + command_encoder.draw_primitives_instanced( metal::MTLPrimitiveType::Triangle, - &instance_buffer.metal_buffer, - icb_offset - + (i * std::mem::size_of::()) as u64, + 0, + 6, + sprites.len() as u64, ); + *instance_offset = next_offset; + sprites.clear(); } - - *instance_offset = (p as usize) - (base_addr as usize); } - true } @@ -1053,7 +1143,6 @@ fn build_pipeline_state( vertex_fn_name: &str, fragment_fn_name: &str, pixel_format: metal::MTLPixelFormat, - sample_count: u64, ) -> metal::RenderPipelineState { let vertex_fn = library .get_function(vertex_fn_name, None) @@ -1066,7 +1155,6 @@ fn build_pipeline_state( descriptor.set_label(label); descriptor.set_vertex_function(Some(vertex_fn.as_ref())); descriptor.set_fragment_function(Some(fragment_fn.as_ref())); - descriptor.set_sample_count(sample_count); let color_attachment = descriptor.color_attachments().object_at(0).unwrap(); color_attachment.set_pixel_format(pixel_format); color_attachment.set_blending_enabled(true); @@ -1082,45 +1170,50 @@ fn build_pipeline_state( .expect("could not create render pipeline state") } +fn build_path_rasterization_pipeline_state( + device: &metal::DeviceRef, + library: &metal::LibraryRef, + label: &str, + vertex_fn_name: &str, + fragment_fn_name: &str, + pixel_format: metal::MTLPixelFormat, + path_sample_count: u32, +) -> metal::RenderPipelineState { + let vertex_fn = library + .get_function(vertex_fn_name, None) + .expect("error locating vertex function"); + let fragment_fn = library + .get_function(fragment_fn_name, None) + .expect("error locating fragment function"); + + let descriptor = metal::RenderPipelineDescriptor::new(); + descriptor.set_label(label); + descriptor.set_vertex_function(Some(vertex_fn.as_ref())); + descriptor.set_fragment_function(Some(fragment_fn.as_ref())); + if path_sample_count > 1 { + descriptor.set_raster_sample_count(path_sample_count as _); + descriptor.set_alpha_to_coverage_enabled(true); + } + let color_attachment = descriptor.color_attachments().object_at(0).unwrap(); + color_attachment.set_pixel_format(pixel_format); + color_attachment.set_blending_enabled(true); + color_attachment.set_rgb_blend_operation(metal::MTLBlendOperation::Add); + color_attachment.set_alpha_blend_operation(metal::MTLBlendOperation::Add); + color_attachment.set_source_rgb_blend_factor(metal::MTLBlendFactor::One); + color_attachment.set_source_alpha_blend_factor(metal::MTLBlendFactor::One); + color_attachment.set_destination_rgb_blend_factor(metal::MTLBlendFactor::One); + color_attachment.set_destination_alpha_blend_factor(metal::MTLBlendFactor::One); + + device + .new_render_pipeline_state(&descriptor) + .expect("could not create render pipeline state") +} + // Align to multiples of 256 make Metal happy. fn align_offset(offset: &mut usize) { *offset = (*offset).div_ceil(256) * 256; } -fn create_msaa_texture( - device: &metal::Device, - layer: &metal::MetalLayer, - sample_count: u64, -) -> Option { - let viewport_size = layer.drawable_size(); - let width = viewport_size.width.ceil() as u64; - let height = viewport_size.height.ceil() as u64; - - if width == 0 || height == 0 { - return None; - } - - if sample_count <= 1 { - return None; - } - - let texture_descriptor = metal::TextureDescriptor::new(); - texture_descriptor.set_texture_type(metal::MTLTextureType::D2Multisample); - - // MTLStorageMode default is `shared` only for Apple silicon GPUs. Use `private` for Apple and Intel GPUs both. - // Reference: https://developer.apple.com/documentation/metal/choosing-a-resource-storage-mode-for-apple-gpus - texture_descriptor.set_storage_mode(metal::MTLStorageMode::Private); - - texture_descriptor.set_width(width); - texture_descriptor.set_height(height); - texture_descriptor.set_pixel_format(layer.pixel_format()); - texture_descriptor.set_usage(metal::MTLTextureUsage::RenderTarget); - texture_descriptor.set_sample_count(sample_count); - - let metal_texture = device.new_texture(&texture_descriptor); - Some(metal_texture) -} - #[repr(C)] enum ShadowInputIndex { Vertices = 0, @@ -1162,10 +1255,9 @@ enum SurfaceInputIndex { } #[repr(C)] -enum PathInputIndex { +enum PathRasterizationInputIndex { Vertices = 0, - ViewportSize = 1, - Sprites = 2, + AtlasTextureSize = 1, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -1173,6 +1265,7 @@ enum PathInputIndex { pub struct PathSprite { pub bounds: Bounds, pub color: Background, + pub tile: AtlasTile, } #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/crates/gpui/src/platform/mac/shaders.metal b/crates/gpui/src/platform/mac/shaders.metal index 5f0dc3323d..64ebb1e22b 100644 --- a/crates/gpui/src/platform/mac/shaders.metal +++ b/crates/gpui/src/platform/mac/shaders.metal @@ -698,27 +698,76 @@ fragment float4 polychrome_sprite_fragment( return color; } -struct PathVertexOutput { +struct PathRasterizationVertexOutput { float4 position [[position]]; + float2 st_position; + float clip_rect_distance [[clip_distance]][4]; +}; + +struct PathRasterizationFragmentInput { + float4 position [[position]]; + float2 st_position; +}; + +vertex PathRasterizationVertexOutput path_rasterization_vertex( + uint vertex_id [[vertex_id]], + constant PathVertex_ScaledPixels *vertices + [[buffer(PathRasterizationInputIndex_Vertices)]], + constant Size_DevicePixels *atlas_size + [[buffer(PathRasterizationInputIndex_AtlasTextureSize)]]) { + PathVertex_ScaledPixels v = vertices[vertex_id]; + float2 vertex_position = float2(v.xy_position.x, v.xy_position.y); + float2 viewport_size = float2(atlas_size->width, atlas_size->height); + return PathRasterizationVertexOutput{ + float4(vertex_position / viewport_size * float2(2., -2.) + + float2(-1., 1.), + 0., 1.), + float2(v.st_position.x, v.st_position.y), + {v.xy_position.x - v.content_mask.bounds.origin.x, + v.content_mask.bounds.origin.x + v.content_mask.bounds.size.width - + v.xy_position.x, + v.xy_position.y - v.content_mask.bounds.origin.y, + v.content_mask.bounds.origin.y + v.content_mask.bounds.size.height - + v.xy_position.y}}; +} + +fragment float4 path_rasterization_fragment(PathRasterizationFragmentInput input + [[stage_in]]) { + float2 dx = dfdx(input.st_position); + float2 dy = dfdy(input.st_position); + float2 gradient = float2((2. * input.st_position.x) * dx.x - dx.y, + (2. * input.st_position.x) * dy.x - dy.y); + float f = (input.st_position.x * input.st_position.x) - input.st_position.y; + float distance = f / length(gradient); + float alpha = saturate(0.5 - distance); + return float4(alpha, 0., 0., 1.); +} + +struct PathSpriteVertexOutput { + float4 position [[position]]; + float2 tile_position; uint sprite_id [[flat]]; float4 solid_color [[flat]]; float4 color0 [[flat]]; float4 color1 [[flat]]; - float4 clip_distance; }; -vertex PathVertexOutput path_vertex( - uint vertex_id [[vertex_id]], - constant PathVertex_ScaledPixels *vertices [[buffer(PathInputIndex_Vertices)]], - uint sprite_id [[instance_id]], - constant PathSprite *sprites [[buffer(PathInputIndex_Sprites)]], - constant Size_DevicePixels *input_viewport_size [[buffer(PathInputIndex_ViewportSize)]]) { - PathVertex_ScaledPixels v = vertices[vertex_id]; - float2 vertex_position = float2(v.xy_position.x, v.xy_position.y); - float2 viewport_size = float2((float)input_viewport_size->width, - (float)input_viewport_size->height); +vertex PathSpriteVertexOutput path_sprite_vertex( + uint unit_vertex_id [[vertex_id]], uint sprite_id [[instance_id]], + constant float2 *unit_vertices [[buffer(SpriteInputIndex_Vertices)]], + constant PathSprite *sprites [[buffer(SpriteInputIndex_Sprites)]], + constant Size_DevicePixels *viewport_size + [[buffer(SpriteInputIndex_ViewportSize)]], + constant Size_DevicePixels *atlas_size + [[buffer(SpriteInputIndex_AtlasTextureSize)]]) { + + float2 unit_vertex = unit_vertices[unit_vertex_id]; PathSprite sprite = sprites[sprite_id]; - float4 device_position = float4(vertex_position / viewport_size * float2(2., -2.) + float2(-1., 1.), 0., 1.); + // Don't apply content mask because it was already accounted for when + // rasterizing the path. + float4 device_position = + to_device_position(unit_vertex, sprite.bounds, viewport_size); + float2 tile_position = to_tile_position(unit_vertex, sprite.tile, atlas_size); GradientColor gradient = prepare_fill_color( sprite.color.tag, @@ -728,32 +777,30 @@ vertex PathVertexOutput path_vertex( sprite.color.colors[1].color ); - return PathVertexOutput{ + return PathSpriteVertexOutput{ device_position, + tile_position, sprite_id, gradient.solid, gradient.color0, - gradient.color1, - {v.xy_position.x - v.content_mask.bounds.origin.x, - v.content_mask.bounds.origin.x + v.content_mask.bounds.size.width - - v.xy_position.x, - v.xy_position.y - v.content_mask.bounds.origin.y, - v.content_mask.bounds.origin.y + v.content_mask.bounds.size.height - - v.xy_position.y} + gradient.color1 }; } -fragment float4 path_fragment( - PathVertexOutput input [[stage_in]], - constant PathSprite *sprites [[buffer(PathInputIndex_Sprites)]]) { - if (any(input.clip_distance < float4(0.0))) { - return float4(0.0); - } - +fragment float4 path_sprite_fragment( + PathSpriteVertexOutput input [[stage_in]], + constant PathSprite *sprites [[buffer(SpriteInputIndex_Sprites)]], + texture2d atlas_texture [[texture(SpriteInputIndex_AtlasTexture)]]) { + constexpr sampler atlas_texture_sampler(mag_filter::linear, + min_filter::linear); + float4 sample = + atlas_texture.sample(atlas_texture_sampler, input.tile_position); + float mask = 1. - abs(1. - fmod(sample.r, 2.)); PathSprite sprite = sprites[input.sprite_id]; Background background = sprite.color; float4 color = fill_color(background, input.position.xy, sprite.bounds, input.solid_color, input.color0, input.color1); + color.a *= mask; return color; } diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index 65ee10a13f..1b88415d3b 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -341,7 +341,7 @@ impl PlatformAtlas for TestAtlas { crate::AtlasTile { texture_id: AtlasTextureId { index: texture_id, - kind: crate::AtlasTextureKind::Polychrome, + kind: crate::AtlasTextureKind::Path, }, tile_id: TileId(tile_id), padding: 0, diff --git a/crates/gpui/src/scene.rs b/crates/gpui/src/scene.rs index 681444a473..4eaef64afa 100644 --- a/crates/gpui/src/scene.rs +++ b/crates/gpui/src/scene.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use crate::{ AtlasTextureId, AtlasTile, Background, Bounds, ContentMask, Corners, Edges, Hsla, Pixels, - Point, Radians, ScaledPixels, Size, bounds_tree::BoundsTree, + Point, Radians, ScaledPixels, Size, bounds_tree::BoundsTree, point, }; use std::{fmt::Debug, iter::Peekable, ops::Range, slice}; @@ -43,7 +43,13 @@ impl Scene { self.surfaces.clear(); } - #[allow(dead_code)] + #[cfg_attr( + all( + any(target_os = "linux", target_os = "freebsd"), + not(any(feature = "x11", feature = "wayland")) + ), + allow(dead_code) + )] pub fn paths(&self) -> &[Path] { &self.paths } @@ -683,7 +689,6 @@ pub struct Path { start: Point

, current: Point

, contour_count: usize, - base_scale: f32, } impl Path { @@ -702,35 +707,25 @@ impl Path { content_mask: Default::default(), color: Default::default(), contour_count: 0, - base_scale: 1.0, } } - /// Set the base scale of the path. - pub fn scale(mut self, factor: f32) -> Self { - self.base_scale = factor; - self - } - - /// Apply a scale to the path. - pub(crate) fn apply_scale(&self, factor: f32) -> Path { + /// Scale this path by the given factor. + pub fn scale(&self, factor: f32) -> Path { Path { id: self.id, order: self.order, - bounds: self.bounds.scale(self.base_scale * factor), - content_mask: self.content_mask.scale(self.base_scale * factor), + bounds: self.bounds.scale(factor), + content_mask: self.content_mask.scale(factor), vertices: self .vertices .iter() - .map(|vertex| vertex.scale(self.base_scale * factor)) + .map(|vertex| vertex.scale(factor)) .collect(), - start: self - .start - .map(|start| start.scale(self.base_scale * factor)), - current: self.current.scale(self.base_scale * factor), + start: self.start.map(|start| start.scale(factor)), + current: self.current.scale(factor), contour_count: self.contour_count, color: self.color, - base_scale: 1.0, } } @@ -745,7 +740,10 @@ impl Path { pub fn line_to(&mut self, to: Point) { self.contour_count += 1; if self.contour_count > 1 { - self.push_triangle((self.start, self.current, to)); + self.push_triangle( + (self.start, self.current, to), + (point(0., 1.), point(0., 1.), point(0., 1.)), + ); } self.current = to; } @@ -754,15 +752,25 @@ impl Path { pub fn curve_to(&mut self, to: Point, ctrl: Point) { self.contour_count += 1; if self.contour_count > 1 { - self.push_triangle((self.start, self.current, to)); + self.push_triangle( + (self.start, self.current, to), + (point(0., 1.), point(0., 1.), point(0., 1.)), + ); } - self.push_triangle((self.current, ctrl, to)); + self.push_triangle( + (self.current, ctrl, to), + (point(0., 0.), point(0.5, 0.), point(1., 1.)), + ); self.current = to; } /// Push a triangle to the Path. - pub fn push_triangle(&mut self, xy: (Point, Point, Point)) { + pub fn push_triangle( + &mut self, + xy: (Point, Point, Point), + st: (Point, Point, Point), + ) { self.bounds = self .bounds .union(&Bounds { @@ -780,14 +788,17 @@ impl Path { self.vertices.push(PathVertex { xy_position: xy.0, + st_position: st.0, content_mask: Default::default(), }); self.vertices.push(PathVertex { xy_position: xy.1, + st_position: st.1, content_mask: Default::default(), }); self.vertices.push(PathVertex { xy_position: xy.2, + st_position: st.2, content_mask: Default::default(), }); } @@ -803,6 +814,7 @@ impl From> for Primitive { #[repr(C)] pub(crate) struct PathVertex { pub(crate) xy_position: Point

, + pub(crate) st_position: Point, pub(crate) content_mask: ContentMask

, } @@ -810,6 +822,7 @@ impl PathVertex { pub fn scale(&self, factor: f32) -> PathVertex { PathVertex { xy_position: self.xy_position.scale(factor), + st_position: self.st_position, content_mask: self.content_mask.scale(factor), } } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index e9145bd9f5..b6601829c7 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -2424,6 +2424,53 @@ impl Window { result } + /// Use a piece of state that exists as long this element is being rendered in consecutive frames. + pub fn use_keyed_state( + &mut self, + key: impl Into, + cx: &mut App, + init: impl FnOnce(&mut Self, &mut App) -> S, + ) -> Entity { + let current_view = self.current_view(); + self.with_global_id(key.into(), |global_id, window| { + window.with_element_state(global_id, |state: Option>, window| { + if let Some(state) = state { + (state.clone(), state) + } else { + let new_state = cx.new(|cx| init(window, cx)); + cx.observe(&new_state, move |_, cx| { + cx.notify(current_view); + }) + .detach(); + (new_state.clone(), new_state) + } + }) + }) + } + + /// Immediately push an element ID onto the stack. Useful for simplifying IDs in lists + pub fn with_id(&mut self, id: impl Into, f: impl FnOnce(&mut Self) -> R) -> R { + self.with_global_id(id.into(), |_, window| f(window)) + } + + /// Use a piece of state that exists as long this element is being rendered in consecutive frames, without needing to specify a key + /// + /// NOTE: This method uses the location of the caller to generate an ID for this state. + /// If this is not sufficient to identify your state (e.g. you're rendering a list item), + /// you can provide a custom ElementID using the `use_keyed_state` method. + #[track_caller] + pub fn use_state( + &mut self, + cx: &mut App, + init: impl FnOnce(&mut Self, &mut App) -> S, + ) -> Entity { + self.use_keyed_state( + ElementId::CodeLocation(*core::panic::Location::caller()), + cx, + init, + ) + } + /// Updates or initializes state for an element with the given id that lives across multiple /// frames. If an element with this ID existed in the rendered frame, its state will be passed /// to the given closure. The state returned by the closure will be stored so it can be referenced @@ -2658,7 +2705,7 @@ impl Window { path.color = color.opacity(opacity); self.next_frame .scene - .insert_primitive(path.apply_scale(scale_factor)); + .insert_primitive(path.scale(scale_factor)); } /// Paint an underline into the scene for the next frame at the current z-index. @@ -4577,6 +4624,8 @@ pub enum ElementId { NamedInteger(SharedString, u64), /// A path. Path(Arc), + /// A code location. + CodeLocation(core::panic::Location<'static>), } impl ElementId { @@ -4596,6 +4645,7 @@ impl Display for ElementId { ElementId::NamedInteger(s, i) => write!(f, "{}-{}", s, i)?, ElementId::Uuid(uuid) => write!(f, "{}", uuid)?, ElementId::Path(path) => write!(f, "{}", path.display())?, + ElementId::CodeLocation(location) => write!(f, "{}", location)?, } Ok(()) diff --git a/crates/gpui_macros/src/derive_app_context.rs b/crates/gpui_macros/src/derive_app_context.rs index bca015b8dc..d2dc250d02 100644 --- a/crates/gpui_macros/src/derive_app_context.rs +++ b/crates/gpui_macros/src/derive_app_context.rs @@ -53,6 +53,16 @@ pub fn derive_app_context(input: TokenStream) -> TokenStream { self.#app_variable.update_entity(handle, update) } + fn as_mut<'y, 'z, T>( + &'y mut self, + handle: &'z gpui::Entity, + ) -> Self::Result> + where + T: 'static, + { + self.#app_variable.as_mut(handle) + } + fn read_entity( &self, handle: &gpui::Entity, diff --git a/crates/http_client/src/http_client.rs b/crates/http_client/src/http_client.rs index eebab86e21..434bd74fc8 100644 --- a/crates/http_client/src/http_client.rs +++ b/crates/http_client/src/http_client.rs @@ -4,6 +4,7 @@ pub mod github; pub use anyhow::{Result, anyhow}; pub use async_body::{AsyncBody, Inner}; use derive_more::Deref; +use http::HeaderValue; pub use http::{self, Method, Request, Response, StatusCode, Uri}; use futures::future::BoxFuture; @@ -39,6 +40,8 @@ impl HttpRequestExt for http::request::Builder { pub trait HttpClient: 'static + Send + Sync { fn type_name(&self) -> &'static str; + fn user_agent(&self) -> Option<&HeaderValue>; + fn send( &self, req: http::Request, @@ -118,6 +121,10 @@ impl HttpClient for HttpClientWithProxy { self.client.send(req) } + fn user_agent(&self) -> Option<&HeaderValue> { + self.client.user_agent() + } + fn proxy(&self) -> Option<&Url> { self.proxy.as_ref() } @@ -135,6 +142,10 @@ impl HttpClient for Arc { self.client.send(req) } + fn user_agent(&self) -> Option<&HeaderValue> { + self.client.user_agent() + } + fn proxy(&self) -> Option<&Url> { self.proxy.as_ref() } @@ -250,6 +261,10 @@ impl HttpClient for Arc { self.client.send(req) } + fn user_agent(&self) -> Option<&HeaderValue> { + self.client.user_agent() + } + fn proxy(&self) -> Option<&Url> { self.client.proxy.as_ref() } @@ -267,6 +282,10 @@ impl HttpClient for HttpClientWithUrl { self.client.send(req) } + fn user_agent(&self) -> Option<&HeaderValue> { + self.client.user_agent() + } + fn proxy(&self) -> Option<&Url> { self.client.proxy.as_ref() } @@ -314,6 +333,10 @@ impl HttpClient for BlockedHttpClient { }) } + fn user_agent(&self) -> Option<&HeaderValue> { + None + } + fn proxy(&self) -> Option<&Url> { None } @@ -334,6 +357,7 @@ type FakeHttpHandler = Box< #[cfg(feature = "test-support")] pub struct FakeHttpClient { handler: FakeHttpHandler, + user_agent: HeaderValue, } #[cfg(feature = "test-support")] @@ -348,6 +372,7 @@ impl FakeHttpClient { client: HttpClientWithProxy { client: Arc::new(Self { handler: Box::new(move |req| Box::pin(handler(req))), + user_agent: HeaderValue::from_static(type_name::()), }), proxy: None, }, @@ -390,6 +415,10 @@ impl HttpClient for FakeHttpClient { future } + fn user_agent(&self) -> Option<&HeaderValue> { + Some(&self.user_agent) + } + fn proxy(&self) -> Option<&Url> { None } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index ae0184b22a..4c3106ef37 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -2072,6 +2072,21 @@ impl Buffer { self.text.push_transaction(transaction, now); } + /// Differs from `push_transaction` in that it does not clear the redo + /// stack. Intended to be used to create a parent transaction to merge + /// potential child transactions into. + /// + /// The caller is responsible for removing it from the undo history using + /// `forget_transaction` if no edits are merged into it. Otherwise, if edits + /// are merged into this transaction, the caller is responsible for ensuring + /// the redo stack is cleared. The easiest way to ensure the redo stack is + /// cleared is to create transactions with the usual `start_transaction` and + /// `end_transaction` methods and merging the resulting transactions into + /// the transaction created by this method + pub fn push_empty_transaction(&mut self, now: Instant) -> TransactionId { + self.text.push_empty_transaction(now) + } + /// Prevent the last transaction from being grouped with any subsequent transactions, /// even if they occur with the buffer's undo grouping duration. pub fn finalize_last_transaction(&mut self) -> Option<&Transaction> { diff --git a/crates/language_model/src/language_model.rs b/crates/language_model/src/language_model.rs index 81a0f7d8a1..f29686b138 100644 --- a/crates/language_model/src/language_model.rs +++ b/crates/language_model/src/language_model.rs @@ -116,6 +116,12 @@ pub enum LanguageModelCompletionError { provider: LanguageModelProviderName, message: String, }, + #[error("{message}")] + UpstreamProviderError { + message: String, + status: StatusCode, + retry_after: Option, + }, #[error("HTTP response error from {provider}'s API: status {status_code} - {message:?}")] HttpResponseError { provider: LanguageModelProviderName, diff --git a/crates/language_models/src/provider/cloud.rs b/crates/language_models/src/provider/cloud.rs index c044a318b8..6aea576258 100644 --- a/crates/language_models/src/provider/cloud.rs +++ b/crates/language_models/src/provider/cloud.rs @@ -644,8 +644,62 @@ struct ApiError { headers: HeaderMap, } +/// Represents error responses from Zed's cloud API. +/// +/// Example JSON for an upstream HTTP error: +/// ```json +/// { +/// "code": "upstream_http_error", +/// "message": "Received an error from the Anthropic API: upstream connect error or disconnect/reset before headers, reset reason: connection timeout", +/// "upstream_status": 503 +/// } +/// ``` +#[derive(Debug, serde::Deserialize)] +struct CloudApiError { + code: String, + message: String, + #[serde(default)] + #[serde(deserialize_with = "deserialize_optional_status_code")] + upstream_status: Option, + #[serde(default)] + retry_after: Option, +} + +fn deserialize_optional_status_code<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let opt: Option = Option::deserialize(deserializer)?; + Ok(opt.and_then(|code| StatusCode::from_u16(code).ok())) +} + impl From for LanguageModelCompletionError { fn from(error: ApiError) -> Self { + if let Ok(cloud_error) = serde_json::from_str::(&error.body) { + if cloud_error.code.starts_with("upstream_http_") { + let status = if let Some(status) = cloud_error.upstream_status { + status + } else if cloud_error.code.ends_with("_error") { + error.status + } else { + // If there's a status code in the code string (e.g. "upstream_http_429") + // then use that; otherwise, see if the JSON contains a status code. + cloud_error + .code + .strip_prefix("upstream_http_") + .and_then(|code_str| code_str.parse::().ok()) + .and_then(|code| StatusCode::from_u16(code).ok()) + .unwrap_or(error.status) + }; + + return LanguageModelCompletionError::UpstreamProviderError { + message: cloud_error.message, + status, + retry_after: cloud_error.retry_after.map(Duration::from_secs_f64), + }; + } + } + let retry_after = None; LanguageModelCompletionError::from_http_status( PROVIDER_NAME, @@ -1279,3 +1333,155 @@ impl Component for ZedAiConfiguration { ) } } + +#[cfg(test)] +mod tests { + use super::*; + use http_client::http::{HeaderMap, StatusCode}; + use language_model::LanguageModelCompletionError; + + #[test] + fn test_api_error_conversion_with_upstream_http_error() { + // upstream_http_error with 503 status should become ServerOverloaded + let error_body = r#"{"code":"upstream_http_error","message":"Received an error from the Anthropic API: upstream connect error or disconnect/reset before headers, reset reason: connection timeout","upstream_status":503}"#; + + let api_error = ApiError { + status: StatusCode::INTERNAL_SERVER_ERROR, + body: error_body.to_string(), + headers: HeaderMap::new(), + }; + + let completion_error: LanguageModelCompletionError = api_error.into(); + + match completion_error { + LanguageModelCompletionError::UpstreamProviderError { message, .. } => { + assert_eq!( + message, + "Received an error from the Anthropic API: upstream connect error or disconnect/reset before headers, reset reason: connection timeout" + ); + } + _ => panic!( + "Expected UpstreamProviderError for upstream 503, got: {:?}", + completion_error + ), + } + + // upstream_http_error with 500 status should become ApiInternalServerError + let error_body = r#"{"code":"upstream_http_error","message":"Received an error from the OpenAI API: internal server error","upstream_status":500}"#; + + let api_error = ApiError { + status: StatusCode::INTERNAL_SERVER_ERROR, + body: error_body.to_string(), + headers: HeaderMap::new(), + }; + + let completion_error: LanguageModelCompletionError = api_error.into(); + + match completion_error { + LanguageModelCompletionError::UpstreamProviderError { message, .. } => { + assert_eq!( + message, + "Received an error from the OpenAI API: internal server error" + ); + } + _ => panic!( + "Expected UpstreamProviderError for upstream 500, got: {:?}", + completion_error + ), + } + + // upstream_http_error with 429 status should become RateLimitExceeded + let error_body = r#"{"code":"upstream_http_error","message":"Received an error from the Google API: rate limit exceeded","upstream_status":429}"#; + + let api_error = ApiError { + status: StatusCode::INTERNAL_SERVER_ERROR, + body: error_body.to_string(), + headers: HeaderMap::new(), + }; + + let completion_error: LanguageModelCompletionError = api_error.into(); + + match completion_error { + LanguageModelCompletionError::UpstreamProviderError { message, .. } => { + assert_eq!( + message, + "Received an error from the Google API: rate limit exceeded" + ); + } + _ => panic!( + "Expected UpstreamProviderError for upstream 429, got: {:?}", + completion_error + ), + } + + // Regular 500 error without upstream_http_error should remain ApiInternalServerError for Zed + let error_body = "Regular internal server error"; + + let api_error = ApiError { + status: StatusCode::INTERNAL_SERVER_ERROR, + body: error_body.to_string(), + headers: HeaderMap::new(), + }; + + let completion_error: LanguageModelCompletionError = api_error.into(); + + match completion_error { + LanguageModelCompletionError::ApiInternalServerError { provider, message } => { + assert_eq!(provider, PROVIDER_NAME); + assert_eq!(message, "Regular internal server error"); + } + _ => panic!( + "Expected ApiInternalServerError for regular 500, got: {:?}", + completion_error + ), + } + + // upstream_http_429 format should be converted to UpstreamProviderError + let error_body = r#"{"code":"upstream_http_429","message":"Upstream Anthropic rate limit exceeded.","retry_after":30.5}"#; + + let api_error = ApiError { + status: StatusCode::INTERNAL_SERVER_ERROR, + body: error_body.to_string(), + headers: HeaderMap::new(), + }; + + let completion_error: LanguageModelCompletionError = api_error.into(); + + match completion_error { + LanguageModelCompletionError::UpstreamProviderError { + message, + status, + retry_after, + } => { + assert_eq!(message, "Upstream Anthropic rate limit exceeded."); + assert_eq!(status, StatusCode::TOO_MANY_REQUESTS); + assert_eq!(retry_after, Some(Duration::from_secs_f64(30.5))); + } + _ => panic!( + "Expected UpstreamProviderError for upstream_http_429, got: {:?}", + completion_error + ), + } + + // Invalid JSON in error body should fall back to regular error handling + let error_body = "Not JSON at all"; + + let api_error = ApiError { + status: StatusCode::INTERNAL_SERVER_ERROR, + body: error_body.to_string(), + headers: HeaderMap::new(), + }; + + let completion_error: LanguageModelCompletionError = api_error.into(); + + match completion_error { + LanguageModelCompletionError::ApiInternalServerError { provider, .. } => { + assert_eq!(provider, PROVIDER_NAME); + } + _ => panic!( + "Expected ApiInternalServerError for invalid JSON, got: {:?}", + completion_error + ), + } + } +} diff --git a/crates/language_models/src/provider/mistral.rs b/crates/language_models/src/provider/mistral.rs index 11497fda35..fb385308fa 100644 --- a/crates/language_models/src/provider/mistral.rs +++ b/crates/language_models/src/provider/mistral.rs @@ -410,8 +410,20 @@ pub fn into_mistral( .push_part(mistral::MessagePart::Text { text: text.clone() }); } MessageContent::RedactedThinking(_) => {} - MessageContent::ToolUse(_) | MessageContent::ToolResult(_) => { - // Tool content is not supported in User messages for Mistral + MessageContent::ToolUse(_) => { + // Tool use is not supported in User messages for Mistral + } + MessageContent::ToolResult(tool_result) => { + let tool_content = match &tool_result.content { + LanguageModelToolResultContent::Text(text) => text.to_string(), + LanguageModelToolResultContent::Image(_) => { + "[Tool responded with an image, but Zed doesn't support these in Mistral models yet]".to_string() + } + }; + messages.push(mistral::RequestMessage::Tool { + content: tool_content, + tool_call_id: tool_result.tool_use_id.to_string(), + }); } } } @@ -482,24 +494,6 @@ pub fn into_mistral( } } - for message in &request.messages { - for content in &message.content { - if let MessageContent::ToolResult(tool_result) = content { - let content = match &tool_result.content { - LanguageModelToolResultContent::Text(text) => text.to_string(), - LanguageModelToolResultContent::Image(_) => { - "[Tool responded with an image, but Zed doesn't support these in Mistral models yet]".to_string() - } - }; - - messages.push(mistral::RequestMessage::Tool { - content, - tool_call_id: tool_result.tool_use_id.to_string(), - }); - } - } - } - // The Mistral API requires that tool messages be followed by assistant messages, // not user messages. When we have a tool->user sequence in the conversation, // we need to insert a placeholder assistant message to maintain proper conversation diff --git a/crates/languages/src/json.rs b/crates/languages/src/json.rs index 7a3300eb01..15818730b8 100644 --- a/crates/languages/src/json.rs +++ b/crates/languages/src/json.rs @@ -231,6 +231,13 @@ impl JsonLspAdapter { )) } + schemas + .as_array_mut() + .unwrap() + .extend(cx.all_action_names().into_iter().map(|&name| { + project::lsp_store::json_language_server_ext::url_schema_for_action(name) + })); + // This can be viewed via `dev: open language server logs` -> `json-language-server` -> // `Server Info` serde_json::json!({ diff --git a/crates/languages/src/lib.rs b/crates/languages/src/lib.rs index 431c051081..a224111002 100644 --- a/crates/languages/src/lib.rs +++ b/crates/languages/src/lib.rs @@ -273,6 +273,7 @@ pub fn init(languages: Arc, node: NodeRuntime, cx: &mut App) { "Astro", "CSS", "ERB", + "HTML/ERB", "HEEX", "HTML", "JavaScript", diff --git a/crates/languages/src/tailwind.rs b/crates/languages/src/tailwind.rs index 04f30b6246..cb4e939083 100644 --- a/crates/languages/src/tailwind.rs +++ b/crates/languages/src/tailwind.rs @@ -179,6 +179,7 @@ impl LspAdapter for TailwindLspAdapter { ("Elixir".to_string(), "phoenix-heex".to_string()), ("HEEX".to_string(), "phoenix-heex".to_string()), ("ERB".to_string(), "erb".to_string()), + ("HTML/ERB".to_string(), "erb".to_string()), ("PHP".to_string(), "php".to_string()), ("Vue.js".to_string(), "vue".to_string()), ]) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index e4078393ee..161b861dd0 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1,4 +1,5 @@ pub mod clangd_ext; +pub mod json_language_server_ext; pub mod lsp_ext_command; pub mod rust_analyzer_ext; @@ -1034,6 +1035,7 @@ impl LocalLspStore { }) .detach(); + json_language_server_ext::register_requests(this.clone(), language_server); rust_analyzer_ext::register_notifications(this.clone(), language_server); clangd_ext::register_notifications(this, language_server, adapter); } @@ -1272,15 +1274,11 @@ impl LocalLspStore { // grouped with the previous transaction in the history // based on the transaction group interval buffer.finalize_last_transaction(); - let transaction_id = buffer + buffer .start_transaction() .context("transaction already open")?; - let transaction = buffer - .get_transaction(transaction_id) - .expect("transaction started") - .clone(); buffer.end_transaction(cx); - buffer.push_transaction(transaction, cx.background_executor().now()); + let transaction_id = buffer.push_empty_transaction(cx.background_executor().now()); buffer.finalize_last_transaction(); anyhow::Ok(transaction_id) })??; @@ -3553,7 +3551,8 @@ pub struct LspStore { _maintain_buffer_languages: Task<()>, diagnostic_summaries: HashMap, HashMap>>, - lsp_data: HashMap, + lsp_document_colors: HashMap, + lsp_code_lens: HashMap, } #[derive(Debug, Default, Clone)] @@ -3563,6 +3562,7 @@ pub struct DocumentColors { } type DocumentColorTask = Shared>>>; +type CodeLensTask = Shared, Arc>>>; #[derive(Debug, Default)] struct DocumentColorData { @@ -3572,8 +3572,15 @@ struct DocumentColorData { colors_update: Option<(Global, DocumentColorTask)>, } +#[derive(Debug, Default)] +struct CodeLensData { + lens_for_version: Global, + lens: HashMap>, + update: Option<(Global, CodeLensTask)>, +} + #[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum ColorFetchStrategy { +pub enum LspFetchStrategy { IgnoreCache, UseCache { known_cache_version: Option }, } @@ -3806,7 +3813,8 @@ impl LspStore { language_server_statuses: Default::default(), nonce: StdRng::from_entropy().r#gen(), diagnostic_summaries: HashMap::default(), - lsp_data: HashMap::default(), + lsp_document_colors: HashMap::default(), + lsp_code_lens: HashMap::default(), active_entry: None, _maintain_workspace_config, _maintain_buffer_languages: Self::maintain_buffer_languages(languages, cx), @@ -3863,7 +3871,8 @@ impl LspStore { language_server_statuses: Default::default(), nonce: StdRng::from_entropy().r#gen(), diagnostic_summaries: HashMap::default(), - lsp_data: HashMap::default(), + lsp_document_colors: HashMap::default(), + lsp_code_lens: HashMap::default(), active_entry: None, toolchain_store, _maintain_workspace_config, @@ -4164,7 +4173,8 @@ impl LspStore { *refcount }; if refcount == 0 { - lsp_store.lsp_data.remove(&buffer_id); + lsp_store.lsp_document_colors.remove(&buffer_id); + lsp_store.lsp_code_lens.remove(&buffer_id); let local = lsp_store.as_local_mut().unwrap(); local.registered_buffers.remove(&buffer_id); local.buffers_opened_in_servers.remove(&buffer_id); @@ -5704,69 +5714,168 @@ impl LspStore { } } - pub fn code_lens( + pub fn code_lens_actions( &mut self, - buffer_handle: &Entity, + buffer: &Entity, cx: &mut Context, - ) -> Task>> { + ) -> CodeLensTask { + let version_queried_for = buffer.read(cx).version(); + let buffer_id = buffer.read(cx).remote_id(); + + if let Some(cached_data) = self.lsp_code_lens.get(&buffer_id) { + if !version_queried_for.changed_since(&cached_data.lens_for_version) { + let has_different_servers = self.as_local().is_some_and(|local| { + local + .buffers_opened_in_servers + .get(&buffer_id) + .cloned() + .unwrap_or_default() + != cached_data.lens.keys().copied().collect() + }); + if !has_different_servers { + return Task::ready(Ok(cached_data.lens.values().flatten().cloned().collect())) + .shared(); + } + } + } + + let lsp_data = self.lsp_code_lens.entry(buffer_id).or_default(); + if let Some((updating_for, running_update)) = &lsp_data.update { + if !version_queried_for.changed_since(&updating_for) { + return running_update.clone(); + } + } + let buffer = buffer.clone(); + let query_version_queried_for = version_queried_for.clone(); + let new_task = cx + .spawn(async move |lsp_store, cx| { + cx.background_executor() + .timer(Duration::from_millis(30)) + .await; + let fetched_lens = lsp_store + .update(cx, |lsp_store, cx| lsp_store.fetch_code_lens(&buffer, cx)) + .map_err(Arc::new)? + .await + .context("fetching code lens") + .map_err(Arc::new); + let fetched_lens = match fetched_lens { + Ok(fetched_lens) => fetched_lens, + Err(e) => { + lsp_store + .update(cx, |lsp_store, _| { + lsp_store.lsp_code_lens.entry(buffer_id).or_default().update = None; + }) + .ok(); + return Err(e); + } + }; + + lsp_store + .update(cx, |lsp_store, _| { + let lsp_data = lsp_store.lsp_code_lens.entry(buffer_id).or_default(); + if lsp_data.lens_for_version == query_version_queried_for { + lsp_data.lens.extend(fetched_lens.clone()); + } else if !lsp_data + .lens_for_version + .changed_since(&query_version_queried_for) + { + lsp_data.lens_for_version = query_version_queried_for; + lsp_data.lens = fetched_lens.clone(); + } + lsp_data.update = None; + lsp_data.lens.values().flatten().cloned().collect() + }) + .map_err(Arc::new) + }) + .shared(); + lsp_data.update = Some((version_queried_for, new_task.clone())); + new_task + } + + fn fetch_code_lens( + &mut self, + buffer: &Entity, + cx: &mut Context, + ) -> Task>>> { if let Some((upstream_client, project_id)) = self.upstream_client() { let request_task = upstream_client.request(proto::MultiLspQuery { - buffer_id: buffer_handle.read(cx).remote_id().into(), - version: serialize_version(&buffer_handle.read(cx).version()), + buffer_id: buffer.read(cx).remote_id().into(), + version: serialize_version(&buffer.read(cx).version()), project_id, strategy: Some(proto::multi_lsp_query::Strategy::All( proto::AllLanguageServers {}, )), request: Some(proto::multi_lsp_query::Request::GetCodeLens( - GetCodeLens.to_proto(project_id, buffer_handle.read(cx)), + GetCodeLens.to_proto(project_id, buffer.read(cx)), )), }); - let buffer = buffer_handle.clone(); - cx.spawn(async move |weak_project, cx| { - let Some(project) = weak_project.upgrade() else { - return Ok(Vec::new()); + let buffer = buffer.clone(); + cx.spawn(async move |weak_lsp_store, cx| { + let Some(lsp_store) = weak_lsp_store.upgrade() else { + return Ok(HashMap::default()); }; let responses = request_task.await?.responses; - let code_lens = join_all( + let code_lens_actions = join_all( responses .into_iter() - .filter_map(|lsp_response| match lsp_response.response? { - proto::lsp_response::Response::GetCodeLensResponse(response) => { - Some(response) - } - unexpected => { - debug_panic!("Unexpected response: {unexpected:?}"); - None - } + .filter_map(|lsp_response| { + let response = match lsp_response.response? { + proto::lsp_response::Response::GetCodeLensResponse(response) => { + Some(response) + } + unexpected => { + debug_panic!("Unexpected response: {unexpected:?}"); + None + } + }?; + let server_id = LanguageServerId::from_proto(lsp_response.server_id); + Some((server_id, response)) }) - .map(|code_lens_response| { - GetCodeLens.response_from_proto( - code_lens_response, - project.clone(), - buffer.clone(), - cx.clone(), - ) + .map(|(server_id, code_lens_response)| { + let lsp_store = lsp_store.clone(); + let buffer = buffer.clone(); + let cx = cx.clone(); + async move { + ( + server_id, + GetCodeLens + .response_from_proto( + code_lens_response, + lsp_store, + buffer, + cx, + ) + .await, + ) + } }), ) .await; - Ok(code_lens + let mut has_errors = false; + let code_lens_actions = code_lens_actions .into_iter() - .collect::>>>()? - .into_iter() - .flatten() - .collect()) + .filter_map(|(server_id, code_lens)| match code_lens { + Ok(code_lens) => Some((server_id, code_lens)), + Err(e) => { + has_errors = true; + log::error!("{e:#}"); + None + } + }) + .collect::>(); + anyhow::ensure!( + !has_errors || !code_lens_actions.is_empty(), + "Failed to fetch code lens" + ); + Ok(code_lens_actions) }) } else { - let code_lens_task = - self.request_multiple_lsp_locally(buffer_handle, None::, GetCodeLens, cx); - cx.spawn(async move |_, _| { - Ok(code_lens_task - .await - .into_iter() - .flat_map(|(_, code_lens)| code_lens) - .collect()) - }) + let code_lens_actions_task = + self.request_multiple_lsp_locally(buffer, None::, GetCodeLens, cx); + cx.background_spawn( + async move { Ok(code_lens_actions_task.await.into_iter().collect()) }, + ) } } @@ -6599,7 +6708,7 @@ impl LspStore { pub fn document_colors( &mut self, - fetch_strategy: ColorFetchStrategy, + fetch_strategy: LspFetchStrategy, buffer: Entity, cx: &mut Context, ) -> Option { @@ -6607,11 +6716,11 @@ impl LspStore { let buffer_id = buffer.read(cx).remote_id(); match fetch_strategy { - ColorFetchStrategy::IgnoreCache => {} - ColorFetchStrategy::UseCache { + LspFetchStrategy::IgnoreCache => {} + LspFetchStrategy::UseCache { known_cache_version, } => { - if let Some(cached_data) = self.lsp_data.get(&buffer_id) { + if let Some(cached_data) = self.lsp_document_colors.get(&buffer_id) { if !version_queried_for.changed_since(&cached_data.colors_for_version) { let has_different_servers = self.as_local().is_some_and(|local| { local @@ -6644,7 +6753,7 @@ impl LspStore { } } - let lsp_data = self.lsp_data.entry(buffer_id).or_default(); + let lsp_data = self.lsp_document_colors.entry(buffer_id).or_default(); if let Some((updating_for, running_update)) = &lsp_data.colors_update { if !version_queried_for.changed_since(&updating_for) { return Some(running_update.clone()); @@ -6658,14 +6767,14 @@ impl LspStore { .await; let fetched_colors = lsp_store .update(cx, |lsp_store, cx| { - lsp_store.fetch_document_colors_for_buffer(buffer.clone(), cx) + lsp_store.fetch_document_colors_for_buffer(&buffer, cx) })? .await .context("fetching document colors") .map_err(Arc::new); let fetched_colors = match fetched_colors { Ok(fetched_colors) => { - if fetch_strategy != ColorFetchStrategy::IgnoreCache + if fetch_strategy != LspFetchStrategy::IgnoreCache && Some(true) == buffer .update(cx, |buffer, _| { @@ -6681,7 +6790,7 @@ impl LspStore { lsp_store .update(cx, |lsp_store, _| { lsp_store - .lsp_data + .lsp_document_colors .entry(buffer_id) .or_default() .colors_update = None; @@ -6693,7 +6802,7 @@ impl LspStore { lsp_store .update(cx, |lsp_store, _| { - let lsp_data = lsp_store.lsp_data.entry(buffer_id).or_default(); + let lsp_data = lsp_store.lsp_document_colors.entry(buffer_id).or_default(); if lsp_data.colors_for_version == query_version_queried_for { lsp_data.colors.extend(fetched_colors.clone()); @@ -6727,7 +6836,7 @@ impl LspStore { fn fetch_document_colors_for_buffer( &mut self, - buffer: Entity, + buffer: &Entity, cx: &mut Context, ) -> Task>>> { if let Some((client, project_id)) = self.upstream_client() { @@ -6742,6 +6851,7 @@ impl LspStore { GetDocumentColor {}.to_proto(project_id, buffer.read(cx)), )), }); + let buffer = buffer.clone(); cx.spawn(async move |project, cx| { let Some(project) = project.upgrade() else { return Ok(HashMap::default()); @@ -6787,7 +6897,7 @@ impl LspStore { }) } else { let document_colors_task = - self.request_multiple_lsp_locally(&buffer, None::, GetDocumentColor, cx); + self.request_multiple_lsp_locally(buffer, None::, GetDocumentColor, cx); cx.spawn(async move |_, _| { Ok(document_colors_task .await @@ -7327,21 +7437,23 @@ impl LspStore { } pub(crate) async fn refresh_workspace_configurations( - this: &WeakEntity, + lsp_store: &WeakEntity, fs: Arc, cx: &mut AsyncApp, ) { maybe!(async move { - let servers = this - .update(cx, |this, cx| { - let Some(local) = this.as_local() else { + let mut refreshed_servers = HashSet::default(); + let servers = lsp_store + .update(cx, |lsp_store, cx| { + let toolchain_store = lsp_store.toolchain_store(cx); + let Some(local) = lsp_store.as_local() else { return Vec::default(); }; local .language_server_ids .iter() .flat_map(|((worktree_id, _), server_ids)| { - let worktree = this + let worktree = lsp_store .worktree_store .read(cx) .worktree_for_id(*worktree_id, cx); @@ -7357,43 +7469,54 @@ impl LspStore { ) }); - server_ids.iter().filter_map(move |server_id| { + let fs = fs.clone(); + let toolchain_store = toolchain_store.clone(); + server_ids.iter().filter_map(|server_id| { + let delegate = delegate.clone()? as Arc; let states = local.language_servers.get(server_id)?; match states { LanguageServerState::Starting { .. } => None, LanguageServerState::Running { adapter, server, .. - } => Some(( - adapter.adapter.clone(), - server.clone(), - delegate.clone()? as Arc, - )), + } => { + let fs = fs.clone(); + let toolchain_store = toolchain_store.clone(); + let adapter = adapter.clone(); + let server = server.clone(); + refreshed_servers.insert(server.name()); + Some(cx.spawn(async move |_, cx| { + let settings = + LocalLspStore::workspace_configuration_for_adapter( + adapter.adapter.clone(), + fs.as_ref(), + &delegate, + toolchain_store, + cx, + ) + .await + .ok()?; + server + .notify::( + &lsp::DidChangeConfigurationParams { settings }, + ) + .ok()?; + Some(()) + })) + } } - }) + }).collect::>() }) .collect::>() }) .ok()?; - let toolchain_store = this.update(cx, |this, cx| this.toolchain_store(cx)).ok()?; - for (adapter, server, delegate) in servers { - let settings = LocalLspStore::workspace_configuration_for_adapter( - adapter, - fs.as_ref(), - &delegate, - toolchain_store.clone(), - cx, - ) - .await - .ok()?; - - server - .notify::( - &lsp::DidChangeConfigurationParams { settings }, - ) - .ok(); - } + log::info!("Refreshing workspace configurations for servers {refreshed_servers:?}"); + // TODO this asynchronous job runs concurrently with extension (de)registration and may take enough time for a certain extension + // to stop and unregister its language server wrapper. + // This is racy : an extension might have already removed all `local.language_servers` state, but here we `.clone()` and hold onto it anyway. + // This now causes errors in the logs, we should find a way to remove such servers from the processing everywhere. + let _: Vec> = join_all(servers).await; Some(()) }) .await; @@ -11280,9 +11403,12 @@ impl LspStore { } fn cleanup_lsp_data(&mut self, for_server: LanguageServerId) { - for buffer_lsp_data in self.lsp_data.values_mut() { - buffer_lsp_data.colors.remove(&for_server); - buffer_lsp_data.cache_version += 1; + for buffer_colors in self.lsp_document_colors.values_mut() { + buffer_colors.colors.remove(&for_server); + buffer_colors.cache_version += 1; + } + for buffer_lens in self.lsp_code_lens.values_mut() { + buffer_lens.lens.remove(&for_server); } if let Some(local) = self.as_local_mut() { local.buffer_pull_diagnostics_result_ids.remove(&for_server); diff --git a/crates/project/src/lsp_store/json_language_server_ext.rs b/crates/project/src/lsp_store/json_language_server_ext.rs new file mode 100644 index 0000000000..3eb93386a9 --- /dev/null +++ b/crates/project/src/lsp_store/json_language_server_ext.rs @@ -0,0 +1,101 @@ +use anyhow::Context as _; +use collections::HashMap; +use gpui::WeakEntity; +use lsp::LanguageServer; + +use crate::LspStore; +/// https://github.com/Microsoft/vscode/blob/main/extensions/json-language-features/server/README.md#schema-content-request +/// +/// Represents a "JSON language server-specific, non-standardized, extension to the LSP" with which the vscode-json-language-server +/// can request the contents of a schema that is associated with a uri scheme it does not support. +/// In our case, we provide the uris for actions on server startup under the `zed://schemas/action/{normalize_action_name}` scheme. +/// We can then respond to this request with the schema content on demand, thereby greatly reducing the total size of the JSON we send to the server on startup +struct SchemaContentRequest {} + +impl lsp::request::Request for SchemaContentRequest { + type Params = Vec; + + type Result = String; + + const METHOD: &'static str = "vscode/content"; +} + +pub fn register_requests(_lsp_store: WeakEntity, language_server: &LanguageServer) { + language_server + .on_request::(|params, cx| { + // PERF: Use a cache (`OnceLock`?) to avoid recomputing the action schemas + let mut generator = settings::KeymapFile::action_schema_generator(); + let all_schemas = cx.update(|cx| HashMap::from_iter(cx.action_schemas(&mut generator))); + async move { + let all_schemas = all_schemas?; + let Some(uri) = params.get(0) else { + anyhow::bail!("No URI"); + }; + let normalized_action_name = uri + .strip_prefix("zed://schemas/action/") + .context("Invalid URI")?; + let action_name = denormalize_action_name(normalized_action_name); + let schema = root_schema_from_action_schema( + all_schemas + .get(action_name.as_str()) + .and_then(Option::as_ref), + &mut generator, + ) + .to_value(); + + serde_json::to_string(&schema).context("Failed to serialize schema") + } + }) + .detach(); +} + +pub fn normalize_action_name(action_name: &str) -> String { + action_name.replace("::", "__") +} + +pub fn denormalize_action_name(action_name: &str) -> String { + action_name.replace("__", "::") +} + +pub fn normalized_action_file_name(action_name: &str) -> String { + normalized_action_name_to_file_name(normalize_action_name(action_name)) +} + +pub fn normalized_action_name_to_file_name(mut normalized_action_name: String) -> String { + normalized_action_name.push_str(".json"); + normalized_action_name +} + +pub fn url_schema_for_action(action_name: &str) -> serde_json::Value { + let normalized_name = normalize_action_name(action_name); + let file_name = normalized_action_name_to_file_name(normalized_name.clone()); + serde_json::json!({ + "fileMatch": [file_name], + "url": format!("zed://schemas/action/{}", normalized_name) + }) +} + +fn root_schema_from_action_schema( + action_schema: Option<&schemars::Schema>, + generator: &mut schemars::SchemaGenerator, +) -> schemars::Schema { + let Some(action_schema) = action_schema else { + return schemars::json_schema!(false); + }; + let meta_schema = generator + .settings() + .meta_schema + .as_ref() + .expect("meta_schema should be present in schemars settings") + .to_string(); + let defs = generator.definitions(); + let mut schema = schemars::json_schema!({ + "$schema": meta_schema, + "allowTrailingCommas": true, + "$defs": defs, + }); + schema + .ensure_object() + .extend(std::mem::take(action_schema.clone().ensure_object())); + schema +} diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f9c59d2e95..a4e76ed475 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -113,7 +113,7 @@ use std::{ use task_store::TaskStore; use terminals::Terminals; -use text::{Anchor, BufferId, Point}; +use text::{Anchor, BufferId, OffsetRangeExt, Point}; use toolchain_store::EmptyToolchainStore; use util::{ ResultExt as _, @@ -590,7 +590,7 @@ pub(crate) struct CoreCompletion { } /// A code action provided by a language server. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct CodeAction { /// The id of the language server that produced this code action. pub server_id: LanguageServerId, @@ -604,7 +604,7 @@ pub struct CodeAction { } /// An action sent back by a language server. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum LspAction { /// An action with the full data, may have a command or may not. /// May require resolving. @@ -3607,20 +3607,29 @@ impl Project { }) } - pub fn code_lens( + pub fn code_lens_actions( &mut self, - buffer_handle: &Entity, + buffer: &Entity, range: Range, cx: &mut Context, ) -> Task>> { - let snapshot = buffer_handle.read(cx).snapshot(); - let range = snapshot.anchor_before(range.start)..snapshot.anchor_after(range.end); + let snapshot = buffer.read(cx).snapshot(); + let range = range.clone().to_owned().to_point(&snapshot); + let range_start = snapshot.anchor_before(range.start); + let range_end = if range.start == range.end { + range_start + } else { + snapshot.anchor_after(range.end) + }; + let range = range_start..range_end; let code_lens_actions = self .lsp_store - .update(cx, |lsp_store, cx| lsp_store.code_lens(buffer_handle, cx)); + .update(cx, |lsp_store, cx| lsp_store.code_lens_actions(buffer, cx)); cx.background_spawn(async move { - let mut code_lens_actions = code_lens_actions.await?; + let mut code_lens_actions = code_lens_actions + .await + .map_err(|e| anyhow!("code lens fetch failed: {e:#}"))?; code_lens_actions.retain(|code_lens_action| { range .start diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index b6fdcd6fa5..44f4e8985a 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -384,12 +384,20 @@ struct ItemColors { focused: Hsla, } -fn get_item_color(cx: &App) -> ItemColors { +fn get_item_color(is_sticky: bool, cx: &App) -> ItemColors { let colors = cx.theme().colors(); ItemColors { - default: colors.panel_background, - hover: colors.element_hover, + default: if is_sticky { + colors.panel_overlay_background + } else { + colors.panel_background + }, + hover: if is_sticky { + colors.panel_overlay_hover + } else { + colors.element_hover + }, marked: colors.element_selected, focused: colors.panel_focused_border, drag_over: colors.drop_target_background, @@ -3903,7 +3911,7 @@ impl ProjectPanel { let filename_text_color = details.filename_text_color; let diagnostic_severity = details.diagnostic_severity; - let item_colors = get_item_color(cx); + let item_colors = get_item_color(is_sticky, cx); let canonical_path = details .canonical_path diff --git a/crates/reqwest_client/src/reqwest_client.rs b/crates/reqwest_client/src/reqwest_client.rs index daff20ac4a..e02768876d 100644 --- a/crates/reqwest_client/src/reqwest_client.rs +++ b/crates/reqwest_client/src/reqwest_client.rs @@ -20,6 +20,7 @@ static REDACT_REGEX: LazyLock = LazyLock::new(|| Regex::new(r"key=[^&]+") pub struct ReqwestClient { client: reqwest::Client, proxy: Option, + user_agent: Option, handle: tokio::runtime::Handle, } @@ -44,9 +45,11 @@ impl ReqwestClient { Ok(client.into()) } - pub fn proxy_and_user_agent(proxy: Option, agent: &str) -> anyhow::Result { + pub fn proxy_and_user_agent(proxy: Option, user_agent: &str) -> anyhow::Result { + let user_agent = HeaderValue::from_str(user_agent)?; + let mut map = HeaderMap::new(); - map.insert(http::header::USER_AGENT, HeaderValue::from_str(agent)?); + map.insert(http::header::USER_AGENT, user_agent.clone()); let mut client = Self::builder().default_headers(map); let client_has_proxy; @@ -73,6 +76,7 @@ impl ReqwestClient { .build()?; let mut client: ReqwestClient = client.into(); client.proxy = client_has_proxy.then_some(proxy).flatten(); + client.user_agent = Some(user_agent); Ok(client) } } @@ -96,6 +100,7 @@ impl From for ReqwestClient { client, handle, proxy: None, + user_agent: None, } } } @@ -216,6 +221,10 @@ impl http_client::HttpClient for ReqwestClient { type_name::() } + fn user_agent(&self) -> Option<&HeaderValue> { + self.user_agent.as_ref() + } + fn send( &self, req: http::Request, diff --git a/crates/settings/src/keymap_file.rs b/crates/settings/src/keymap_file.rs index d738e30c4f..7802671fec 100644 --- a/crates/settings/src/keymap_file.rs +++ b/crates/settings/src/keymap_file.rs @@ -847,6 +847,7 @@ impl KeymapFile { } } +#[derive(Clone)] pub enum KeybindUpdateOperation<'a> { Replace { /// Describes the keybind to create @@ -865,6 +866,47 @@ pub enum KeybindUpdateOperation<'a> { }, } +impl KeybindUpdateOperation<'_> { + pub fn generate_telemetry( + &self, + ) -> ( + // The keybind that is created + String, + // The keybinding that was removed + String, + // The source of the keybinding + String, + ) { + let (new_binding, removed_binding, source) = match &self { + KeybindUpdateOperation::Replace { + source, + target, + target_keybind_source, + } => (Some(source), Some(target), Some(*target_keybind_source)), + KeybindUpdateOperation::Add { source, .. } => (Some(source), None, None), + KeybindUpdateOperation::Remove { + target, + target_keybind_source, + } => (None, Some(target), Some(*target_keybind_source)), + }; + + let new_binding = new_binding + .map(KeybindUpdateTarget::telemetry_string) + .unwrap_or("null".to_owned()); + let removed_binding = removed_binding + .map(KeybindUpdateTarget::telemetry_string) + .unwrap_or("null".to_owned()); + + let source = source + .as_ref() + .map(KeybindSource::name) + .map(ToOwned::to_owned) + .unwrap_or("null".to_owned()); + + (new_binding, removed_binding, source) + } +} + impl<'a> KeybindUpdateOperation<'a> { pub fn add(source: KeybindUpdateTarget<'a>) -> Self { Self::Add { source, from: None } @@ -905,21 +947,33 @@ impl<'a> KeybindUpdateTarget<'a> { keystrokes.pop(); keystrokes } + + fn telemetry_string(&self) -> String { + format!( + "action_name: {}, context: {}, action_arguments: {}, keystrokes: {}", + self.action_name, + self.context.unwrap_or("global"), + self.action_arguments.unwrap_or("none"), + self.keystrokes_unparsed() + ) + } } -#[derive(Clone, Copy, PartialEq, Eq)] +#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord)] pub enum KeybindSource { User, - Default, - Base, Vim, + Base, + #[default] + Default, + Unknown, } impl KeybindSource { - const BASE: KeyBindingMetaIndex = KeyBindingMetaIndex(0); - const DEFAULT: KeyBindingMetaIndex = KeyBindingMetaIndex(1); - const VIM: KeyBindingMetaIndex = KeyBindingMetaIndex(2); - const USER: KeyBindingMetaIndex = KeyBindingMetaIndex(3); + const BASE: KeyBindingMetaIndex = KeyBindingMetaIndex(KeybindSource::Base as u32); + const DEFAULT: KeyBindingMetaIndex = KeyBindingMetaIndex(KeybindSource::Default as u32); + const VIM: KeyBindingMetaIndex = KeyBindingMetaIndex(KeybindSource::Vim as u32); + const USER: KeyBindingMetaIndex = KeyBindingMetaIndex(KeybindSource::User as u32); pub fn name(&self) -> &'static str { match self { @@ -927,6 +981,7 @@ impl KeybindSource { KeybindSource::Default => "Default", KeybindSource::Base => "Base", KeybindSource::Vim => "Vim", + KeybindSource::Unknown => "Unknown", } } @@ -936,6 +991,7 @@ impl KeybindSource { KeybindSource::Default => Self::DEFAULT, KeybindSource::Base => Self::BASE, KeybindSource::Vim => Self::VIM, + KeybindSource::Unknown => KeyBindingMetaIndex(*self as u32), } } @@ -945,7 +1001,7 @@ impl KeybindSource { Self::BASE => KeybindSource::Base, Self::DEFAULT => KeybindSource::Default, Self::VIM => KeybindSource::Vim, - _ => unreachable!(), + _ => KeybindSource::Unknown, } } } @@ -958,7 +1014,7 @@ impl From for KeybindSource { impl From for KeyBindingMetaIndex { fn from(source: KeybindSource) -> Self { - return source.meta(); + source.meta() } } @@ -1567,4 +1623,44 @@ mod tests { .unindent(), ); } + + #[test] + fn test_keymap_remove() { + zlog::init_test(); + + check_keymap_update( + r#" + [ + { + "context": "Editor", + "bindings": { + "cmd-k cmd-u": "editor::ConvertToUpperCase", + "cmd-k cmd-l": "editor::ConvertToLowerCase", + "cmd-[": "pane::GoBack", + } + }, + ] + "#, + KeybindUpdateOperation::Remove { + target: KeybindUpdateTarget { + context: Some("Editor"), + keystrokes: &parse_keystrokes("cmd-k cmd-l"), + action_name: "editor::ConvertToLowerCase", + action_arguments: None, + }, + target_keybind_source: KeybindSource::User, + }, + r#" + [ + { + "context": "Editor", + "bindings": { + "cmd-k cmd-u": "editor::ConvertToUpperCase", + "cmd-[": "pane::GoBack", + } + }, + ] + "#, + ); + } } diff --git a/crates/settings/src/settings_json.rs b/crates/settings/src/settings_json.rs index a448eb2737..e6683857e7 100644 --- a/crates/settings/src/settings_json.rs +++ b/crates/settings/src/settings_json.rs @@ -190,6 +190,7 @@ fn replace_value_in_json_text( } } + let mut removed_comma = false; // Look backward for a preceding comma first let preceding_text = text.get(0..removal_start).unwrap_or(""); if let Some(comma_pos) = preceding_text.rfind(',') { @@ -197,10 +198,12 @@ fn replace_value_in_json_text( let between_comma_and_key = text.get(comma_pos + 1..removal_start).unwrap_or(""); if between_comma_and_key.trim().is_empty() { removal_start = comma_pos; + removed_comma = true; } } - - if let Some(remaining_text) = text.get(existing_value_range.end..) { + if let Some(remaining_text) = text.get(existing_value_range.end..) + && !removed_comma + { let mut chars = remaining_text.char_indices(); while let Some((offset, ch)) = chars.next() { if ch == ',' { diff --git a/crates/settings_ui/Cargo.toml b/crates/settings_ui/Cargo.toml index 4502d994e7..02327045fd 100644 --- a/crates/settings_ui/Cargo.toml +++ b/crates/settings_ui/Cargo.toml @@ -23,6 +23,7 @@ feature_flags.workspace = true fs.workspace = true fuzzy.workspace = true gpui.workspace = true +itertools.workspace = true language.workspace = true log.workspace = true menu.workspace = true @@ -34,6 +35,8 @@ search.workspace = true serde.workspace = true serde_json.workspace = true settings.workspace = true +telemetry.workspace = true +tempfile.workspace = true theme.workspace = true tree-sitter-json.workspace = true tree-sitter-rust.workspace = true diff --git a/crates/settings_ui/src/keybindings.rs b/crates/settings_ui/src/keybindings.rs index 78636f7023..c1852844c1 100644 --- a/crates/settings_ui/src/keybindings.rs +++ b/crates/settings_ui/src/keybindings.rs @@ -1,6 +1,8 @@ use std::{ + cmp::{self}, ops::{Not as _, Range}, sync::Arc, + time::Duration, }; use anyhow::{Context as _, anyhow}; @@ -12,20 +14,20 @@ use gpui::{ Action, Animation, AnimationExt, AppContext as _, AsyncApp, Axis, ClickEvent, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, Global, IsZero, KeyContext, Keystroke, Modifiers, ModifiersChangedEvent, MouseButton, Point, ScrollStrategy, - ScrollWheelEvent, StyledText, Subscription, WeakEntity, actions, anchored, deferred, div, + ScrollWheelEvent, Stateful, StyledText, Subscription, Task, TextStyleRefinement, WeakEntity, + actions, anchored, deferred, div, }; use language::{Language, LanguageConfig, ToOffset as _}; use notifications::status_toast::{StatusToast, ToastIcon}; -use settings::{BaseKeymap, KeybindSource, KeymapFile, SettingsAssets}; - -use util::ResultExt; - +use project::Project; +use settings::{BaseKeymap, KeybindSource, KeymapFile, Settings as _, SettingsAssets}; use ui::{ - ActiveTheme as _, App, Banner, BorrowAppContext, ContextMenu, IconButtonShape, Modal, - ModalFooter, ModalHeader, ParentElement as _, Render, Section, SharedString, Styled as _, - Tooltip, Window, prelude::*, + ActiveTheme as _, App, Banner, BorrowAppContext, ContextMenu, IconButtonShape, Indicator, + Modal, ModalFooter, ModalHeader, ParentElement as _, Render, Section, SharedString, + Styled as _, Tooltip, Window, prelude::*, }; use ui_input::SingleLineInput; +use util::ResultExt; use workspace::{ Item, ModalView, SerializableItem, Workspace, notifications::NotifyTaskExt as _, register_serializable_item, @@ -33,7 +35,7 @@ use workspace::{ use crate::{ keybindings::persistence::KEYBINDING_EDITORS, - ui_components::table::{Table, TableInteractionState}, + ui_components::table::{ColumnWidths, ResizeBehavior, Table, TableInteractionState}, }; const NO_ACTION_ARGUMENTS_TEXT: SharedString = SharedString::new_static(""); @@ -65,6 +67,8 @@ actions!( ToggleKeystrokeSearch, /// Toggles exact matching for keystroke search ToggleExactKeystrokeMatching, + /// Shows matching keystrokes for the currently selected binding + ShowMatchingKeybinds ] ); @@ -151,6 +155,13 @@ impl SearchMode { SearchMode::KeyStroke { .. } => SearchMode::Normal, } } + + fn exact_match(&self) -> bool { + match self { + SearchMode::Normal => false, + SearchMode::KeyStroke { exact_match } => *exact_match, + } + } } #[derive(Default, PartialEq, Copy, Clone)] @@ -171,73 +182,145 @@ impl FilterState { #[derive(Debug, Default, PartialEq, Eq, Clone, Hash)] struct ActionMapping { - keystroke_text: SharedString, + keystrokes: Vec, context: Option, } +#[derive(Debug)] +struct KeybindConflict { + first_conflict_index: usize, + remaining_conflict_amount: usize, +} + +impl KeybindConflict { + fn from_iter<'a>(mut indices: impl Iterator) -> Option { + indices.next().map(|origin| Self { + first_conflict_index: origin.index, + remaining_conflict_amount: indices.count(), + }) + } +} + +#[derive(Clone, Copy, PartialEq)] +struct ConflictOrigin { + override_source: KeybindSource, + overridden_source: Option, + index: usize, +} + +impl ConflictOrigin { + fn new(source: KeybindSource, index: usize) -> Self { + Self { + override_source: source, + index, + overridden_source: None, + } + } + + fn with_overridden_source(self, source: KeybindSource) -> Self { + Self { + overridden_source: Some(source), + ..self + } + } + + fn get_conflict_with(&self, other: &Self) -> Option { + if self.override_source == KeybindSource::User + && other.override_source == KeybindSource::User + { + Some( + Self::new(KeybindSource::User, other.index) + .with_overridden_source(self.override_source), + ) + } else if self.override_source > other.override_source { + Some(other.with_overridden_source(self.override_source)) + } else { + None + } + } + + fn is_user_keybind_conflict(&self) -> bool { + self.override_source == KeybindSource::User + && self.overridden_source == Some(KeybindSource::User) + } +} + #[derive(Default)] struct ConflictState { - conflicts: Vec, - action_keybind_mapping: HashMap>, + conflicts: Vec>, + keybind_mapping: HashMap>, + has_user_conflicts: bool, } impl ConflictState { - fn new(key_bindings: &[ProcessedKeybinding]) -> Self { - let mut action_keybind_mapping: HashMap<_, Vec> = HashMap::default(); + fn new(key_bindings: &[ProcessedBinding]) -> Self { + let mut action_keybind_mapping: HashMap<_, Vec> = HashMap::default(); - key_bindings + let mut largest_index = 0; + for (index, binding) in key_bindings .iter() .enumerate() - .filter(|(_, binding)| { - !binding.keystroke_text.is_empty() - && binding - .source - .as_ref() - .is_some_and(|source| matches!(source.0, KeybindSource::User)) - }) - .for_each(|(index, binding)| { - action_keybind_mapping - .entry(binding.get_action_mapping()) - .or_default() - .push(index); - }); + .flat_map(|(index, binding)| Some(index).zip(binding.keybind_information())) + { + action_keybind_mapping + .entry(binding.get_action_mapping()) + .or_default() + .push(ConflictOrigin::new(binding.source, index)); + largest_index = index; + } + + let mut conflicts = vec![None; largest_index + 1]; + let mut has_user_conflicts = false; + + for indices in action_keybind_mapping.values_mut() { + indices.sort_unstable_by_key(|origin| origin.override_source); + let Some((fst, snd)) = indices.get(0).zip(indices.get(1)) else { + continue; + }; + + for origin in indices.iter() { + conflicts[origin.index] = + origin.get_conflict_with(if origin == fst { &snd } else { &fst }) + } + + has_user_conflicts |= fst.override_source == KeybindSource::User + && snd.override_source == KeybindSource::User; + } Self { - conflicts: action_keybind_mapping - .values() - .filter(|indices| indices.len() > 1) - .flatten() - .copied() - .collect(), - action_keybind_mapping, + conflicts, + keybind_mapping: action_keybind_mapping, + has_user_conflicts, } } fn conflicting_indices_for_mapping( &self, - action_mapping: ActionMapping, - keybind_idx: usize, - ) -> Option> { - self.action_keybind_mapping - .get(&action_mapping) + action_mapping: &ActionMapping, + keybind_idx: Option, + ) -> Option { + self.keybind_mapping + .get(action_mapping) .and_then(|indices| { - let mut indices = indices.iter().filter(|&idx| *idx != keybind_idx).peekable(); - indices.peek().is_some().then(|| indices.copied().collect()) + KeybindConflict::from_iter( + indices + .iter() + .filter(|&conflict| Some(conflict.index) != keybind_idx), + ) }) } - fn will_conflict(&self, action_mapping: ActionMapping) -> Option> { - self.action_keybind_mapping - .get(&action_mapping) - .and_then(|indices| indices.is_empty().not().then_some(indices.clone())) + fn conflict_for_idx(&self, idx: usize) -> Option { + self.conflicts.get(idx).copied().flatten() } - fn has_conflict(&self, candidate_idx: &usize) -> bool { - self.conflicts.contains(candidate_idx) + fn has_user_conflict(&self, candidate_idx: usize) -> bool { + self.conflict_for_idx(candidate_idx) + .is_some_and(|conflict| conflict.is_user_keybind_conflict()) } - fn any_conflicts(&self) -> bool { - !self.conflicts.is_empty() + fn any_user_binding_conflicts(&self) -> bool { + self.has_user_conflicts } } @@ -245,10 +328,11 @@ struct KeymapEditor { workspace: WeakEntity, focus_handle: FocusHandle, _keymap_subscription: Subscription, - keybindings: Vec, + keybindings: Vec, keybinding_conflict_state: ConflictState, filter_state: FilterState, search_mode: SearchMode, + search_query_debounce: Option>, // corresponds 1 to 1 with keybindings string_match_candidates: Arc>, matches: Vec, @@ -258,7 +342,17 @@ struct KeymapEditor { selected_index: Option, context_menu: Option<(Entity, Point, Subscription)>, previous_edit: Option, - humanized_action_names: HashMap<&'static str, SharedString>, + humanized_action_names: HumanizedActionNameCache, + current_widths: Entity>, + show_hover_menus: bool, + /// In order for the JSON LSP to run in the actions arguments editor, we + /// require a backing file In order to avoid issues (primarily log spam) + /// with drop order between the buffer, file, worktree, etc, we create a + /// temporary directory for these backing files in the keymap editor struct + /// instead of here. This has the added benefit of only having to create a + /// worktree and directory once, although the perf improvement is negligible. + action_args_temp_dir_worktree: Option>, + action_args_temp_dir: Option, } enum PreviousEdit { @@ -283,18 +377,23 @@ impl EventEmitter<()> for KeymapEditor {} impl Focusable for KeymapEditor { fn focus_handle(&self, cx: &App) -> gpui::FocusHandle { - return self.filter_editor.focus_handle(cx); + if self.selected_index.is_some() { + self.focus_handle.clone() + } else { + self.filter_editor.focus_handle(cx) + } } } impl KeymapEditor { fn new(workspace: WeakEntity, window: &mut Window, cx: &mut Context) -> Self { - let _keymap_subscription = cx.observe_global::(Self::on_keymap_changed); + let _keymap_subscription = + cx.observe_global_in::(window, Self::on_keymap_changed); let table_interaction_state = TableInteractionState::new(window, cx); let keystroke_editor = cx.new(|cx| { let mut keystroke_editor = KeystrokeInput::new(None, window, cx); - keystroke_editor.highlight_on_focus = false; + keystroke_editor.search = true; keystroke_editor }); @@ -322,13 +421,23 @@ impl KeymapEditor { }) .detach(); - let humanized_action_names = - HashMap::from_iter(cx.all_action_names().into_iter().map(|&action_name| { - ( - action_name, - command_palette::humanize_action_name(action_name).into(), - ) - })); + cx.spawn({ + let workspace = workspace.clone(); + async move |this, cx| { + let temp_dir = tempfile::tempdir_in(paths::temp_dir())?; + let worktree = workspace + .update(cx, |ws, cx| { + ws.project() + .update(cx, |p, cx| p.create_worktree(temp_dir.path(), false, cx)) + })? + .await?; + this.update(cx, |this, _| { + this.action_args_temp_dir = Some(temp_dir); + this.action_args_temp_dir_worktree = Some(worktree); + }) + } + }) + .detach(); let mut this = Self { workspace, @@ -346,10 +455,15 @@ impl KeymapEditor { selected_index: None, context_menu: None, previous_edit: None, - humanized_action_names, + search_query_debounce: None, + humanized_action_names: HumanizedActionNameCache::new(cx), + show_hover_menus: true, + action_args_temp_dir: None, + action_args_temp_dir_worktree: None, + current_widths: cx.new(|cx| ColumnWidths::new(cx)), }; - this.on_keymap_changed(cx); + this.on_keymap_changed(window, cx); this } @@ -371,10 +485,32 @@ impl KeymapEditor { } } - fn on_query_changed(&self, cx: &mut Context) { + fn on_query_changed(&mut self, cx: &mut Context) { let action_query = self.current_action_query(cx); let keystroke_query = self.current_keystroke_query(cx); + let exact_match = self.search_mode.exact_match(); + let timer = cx.background_executor().timer(Duration::from_secs(1)); + self.search_query_debounce = Some(cx.background_spawn({ + let action_query = action_query.clone(); + let keystroke_query = keystroke_query.clone(); + async move { + timer.await; + + let keystroke_query = keystroke_query + .into_iter() + .map(|keystroke| keystroke.unparse()) + .collect::>() + .join(" "); + + telemetry::event!( + "Keystroke Search Completed", + action_query = action_query, + keystroke_query = keystroke_query, + keystroke_exact_match = exact_match + ) + } + })); cx.spawn(async move |this, cx| { Self::update_matches(this.clone(), action_query, keystroke_query, cx).await?; this.update(cx, |this, cx| { @@ -410,7 +546,7 @@ impl KeymapEditor { FilterState::Conflicts => { matches.retain(|candidate| { this.keybinding_conflict_state - .has_conflict(&candidate.candidate_id) + .has_user_conflict(candidate.candidate_id) }); } FilterState::All => {} @@ -430,24 +566,40 @@ impl KeymapEditor { && query.modifiers == keystroke.modifiers }, ) + } else if keystroke_query.len() > keystrokes.len() { + return false; } else { - let key_press_query = - KeyPressIterator::new(keystroke_query.as_slice()); - let mut last_match_idx = 0; + for keystroke_offset in 0..keystrokes.len() { + let mut found_count = 0; + let mut query_cursor = 0; + let mut keystroke_cursor = keystroke_offset; + while query_cursor < keystroke_query.len() + && keystroke_cursor < keystrokes.len() + { + let query = &keystroke_query[query_cursor]; + let keystroke = &keystrokes[keystroke_cursor]; + let matches = + query.modifiers.is_subset_of(&keystroke.modifiers) + && ((query.key.is_empty() + || query.key == keystroke.key) + && query + .key_char + .as_ref() + .map_or(true, |q_kc| { + q_kc == &keystroke.key + })); + if matches { + found_count += 1; + query_cursor += 1; + } + keystroke_cursor += 1; + } - key_press_query.into_iter().all(|key| { - let key_presses = KeyPressIterator::new(keystrokes); - key_presses.into_iter().enumerate().any( - |(index, keystroke)| { - if last_match_idx > index || keystroke != key { - return false; - } - - last_match_idx = index; - true - }, - ) - }) + if found_count == keystroke_query.len() { + return true; + } + } + return false; } }) }); @@ -456,40 +608,33 @@ impl KeymapEditor { } if action_query.is_empty() { - // apply default sort - // sorts by source precedence, and alphabetically by action name within each source - matches.sort_by_key(|match_item| { - let keybind = &this.keybindings[match_item.candidate_id]; - let source = keybind.source.as_ref().map(|s| s.0); - use KeybindSource::*; - let source_precedence = match source { - Some(User) => 0, - Some(Vim) => 1, - Some(Base) => 2, - Some(Default) => 3, - None => 4, - }; - return (source_precedence, keybind.action_name); + matches.sort_by(|item1, item2| { + let binding1 = &this.keybindings[item1.candidate_id]; + let binding2 = &this.keybindings[item2.candidate_id]; + + binding1.cmp(binding2) }); } this.selected_index.take(); this.matches = matches; + cx.notify(); }) } - fn has_conflict(&self, row_index: usize) -> bool { - self.matches - .get(row_index) - .map(|candidate| candidate.candidate_id) - .is_some_and(|id| self.keybinding_conflict_state.has_conflict(&id)) + fn get_conflict(&self, row_index: usize) -> Option { + self.matches.get(row_index).and_then(|candidate| { + self.keybinding_conflict_state + .conflict_for_idx(candidate.candidate_id) + }) } fn process_bindings( json_language: Arc, zed_keybind_context_language: Arc, + humanized_action_names: &HumanizedActionNameCache, cx: &mut App, - ) -> (Vec, Vec) { + ) -> (Vec, Vec) { let key_bindings_ptr = cx.key_bindings(); let lock = key_bindings_ptr.borrow(); let key_bindings = lock.bindings(); @@ -497,23 +642,24 @@ impl KeymapEditor { HashSet::from_iter(cx.all_action_names().into_iter().copied()); let action_documentation = cx.action_documentation(); let mut generator = KeymapFile::action_schema_generator(); - let action_schema = HashMap::from_iter( + let actions_with_schemas = HashSet::from_iter( cx.action_schemas(&mut generator) .into_iter() - .filter_map(|(name, schema)| schema.map(|schema| (name, schema))), + .filter_map(|(name, schema)| schema.is_some().then_some(name)), ); let mut processed_bindings = Vec::new(); let mut string_match_candidates = Vec::new(); for key_binding in key_bindings { - let source = key_binding.meta().map(settings::KeybindSource::from_meta); + let source = key_binding + .meta() + .map(KeybindSource::from_meta) + .unwrap_or(KeybindSource::Unknown); let keystroke_text = ui::text_for_keystrokes(key_binding.keystrokes(), cx); - let ui_key_binding = Some( - ui::KeyBinding::new_from_gpui(key_binding.clone(), cx) - .vim_mode(source == Some(settings::KeybindSource::Vim)), - ); + let ui_key_binding = ui::KeyBinding::new_from_gpui(key_binding.clone(), cx) + .vim_mode(source == KeybindSource::Vim); let context = key_binding .predicate() @@ -525,67 +671,69 @@ impl KeymapEditor { }) .unwrap_or(KeybindContextString::Global); - let source = source.map(|source| (source, source.name().into())); - let action_name = key_binding.action().name(); unmapped_action_names.remove(&action_name); + let action_arguments = key_binding .action_input() .map(|arguments| SyntaxHighlightedText::new(arguments, json_language.clone())); - let action_docs = action_documentation.get(action_name).copied(); - - let index = processed_bindings.len(); - let string_match_candidate = StringMatchCandidate::new(index, &action_name); - processed_bindings.push(ProcessedKeybinding { - keystroke_text: keystroke_text.into(), - ui_key_binding, + let action_information = ActionInformation::new( action_name, action_arguments, - action_docs, - action_schema: action_schema.get(action_name).cloned(), - context: Some(context), + &actions_with_schemas, + &action_documentation, + &humanized_action_names, + ); + + let index = processed_bindings.len(); + let string_match_candidate = + StringMatchCandidate::new(index, &action_information.humanized_name); + processed_bindings.push(ProcessedBinding::new_mapped( + keystroke_text, + ui_key_binding, + context, source, - }); + action_information, + )); string_match_candidates.push(string_match_candidate); } - let empty = SharedString::new_static(""); for action_name in unmapped_action_names.into_iter() { let index = processed_bindings.len(); - let string_match_candidate = StringMatchCandidate::new(index, &action_name); - processed_bindings.push(ProcessedKeybinding { - keystroke_text: empty.clone(), - ui_key_binding: None, + let action_information = ActionInformation::new( action_name, - action_arguments: None, - action_docs: action_documentation.get(action_name).copied(), - action_schema: action_schema.get(action_name).cloned(), - context: None, - source: None, - }); + None, + &actions_with_schemas, + &action_documentation, + &humanized_action_names, + ); + let string_match_candidate = + StringMatchCandidate::new(index, &action_information.humanized_name); + + processed_bindings.push(ProcessedBinding::Unmapped(action_information)); string_match_candidates.push(string_match_candidate); } (processed_bindings, string_match_candidates) } - fn on_keymap_changed(&mut self, cx: &mut Context) { + fn on_keymap_changed(&mut self, window: &mut Window, cx: &mut Context) { let workspace = self.workspace.clone(); - cx.spawn(async move |this, cx| { + cx.spawn_in(window, async move |this, cx| { let json_language = load_json_language(workspace.clone(), cx).await; let zed_keybind_context_language = load_keybind_context_language(workspace.clone(), cx).await; let (action_query, keystroke_query) = this.update(cx, |this, cx| { - let (key_bindings, string_match_candidates) = - Self::process_bindings(json_language, zed_keybind_context_language, cx); + let (key_bindings, string_match_candidates) = Self::process_bindings( + json_language, + zed_keybind_context_language, + &this.humanized_action_names, + cx, + ); this.keybinding_conflict_state = ConflictState::new(&key_bindings); - if !this.keybinding_conflict_state.any_conflicts() { - this.filter_state = FilterState::All; - } - this.keybindings = key_bindings; this.string_match_candidates = Arc::new(string_match_candidates); this.matches = this @@ -606,7 +754,7 @@ impl KeymapEditor { })?; // calls cx.notify Self::update_matches(this.clone(), action_query, keystroke_query, cx).await?; - this.update(cx, |this, cx| { + this.update_in(cx, |this, window, cx| { if let Some(previous_edit) = this.previous_edit.take() { match previous_edit { // should remove scroll from process_query @@ -624,8 +772,9 @@ impl KeymapEditor { let scroll_position = this.matches.iter().enumerate().find_map(|(index, item)| { let binding = &this.keybindings[item.candidate_id]; - if binding.get_action_mapping() == action_mapping - && binding.action_name == action_name + if binding.get_action_mapping().is_some_and(|binding_mapping| { + binding_mapping == action_mapping + }) && binding.action().name == action_name { Some(index) } else { @@ -634,8 +783,12 @@ impl KeymapEditor { }); if let Some(scroll_position) = scroll_position { - this.scroll_to_item(scroll_position, ScrollStrategy::Top, cx); - this.selected_index = Some(scroll_position); + this.select_index( + scroll_position, + Some(ScrollStrategy::Top), + window, + cx, + ); } else { this.table_interaction_state.update(cx, |table, _| { table.set_scrollbar_offset(Axis::Vertical, fallback) @@ -650,7 +803,7 @@ impl KeymapEditor { .detach_and_log_err(cx); } - fn dispatch_context(&self, _window: &Window, _cx: &Context) -> KeyContext { + fn key_context(&self) -> KeyContext { let mut dispatch_context = KeyContext::new_with_defaults(); dispatch_context.add("KeymapEditor"); dispatch_context.add("menu"); @@ -685,20 +838,35 @@ impl KeymapEditor { self.selected_index.take(); } - fn selected_keybind_idx(&self) -> Option { + fn selected_keybind_index(&self) -> Option { self.selected_index .and_then(|match_index| self.matches.get(match_index)) .map(|r#match| r#match.candidate_id) } - fn selected_binding(&self) -> Option<&ProcessedKeybinding> { - self.selected_keybind_idx() + fn selected_keybind_and_index(&self) -> Option<(&ProcessedBinding, usize)> { + self.selected_keybind_index() + .map(|keybind_index| (&self.keybindings[keybind_index], keybind_index)) + } + + fn selected_binding(&self) -> Option<&ProcessedBinding> { + self.selected_keybind_index() .and_then(|keybind_index| self.keybindings.get(keybind_index)) } - fn select_index(&mut self, index: usize, cx: &mut Context) { + fn select_index( + &mut self, + index: usize, + scroll: Option, + window: &mut Window, + cx: &mut Context, + ) { if self.selected_index != Some(index) { self.selected_index = Some(index); + if let Some(scroll_strategy) = scroll { + self.scroll_to_item(index, scroll_strategy, cx); + } + window.focus(&self.focus_handle); cx.notify(); } } @@ -709,55 +877,40 @@ impl KeymapEditor { window: &mut Window, cx: &mut Context, ) { - let weak = cx.weak_entity(); self.context_menu = self.selected_binding().map(|selected_binding| { - let key_strokes = selected_binding - .keystrokes() - .map(Vec::from) - .unwrap_or_default(); let selected_binding_has_no_context = selected_binding - .context - .as_ref() + .context() .and_then(KeybindContextString::local) .is_none(); - let selected_binding_is_unbound = selected_binding.keystrokes().is_none(); + let selected_binding_is_unbound = selected_binding.is_unbound(); let context_menu = ContextMenu::build(window, cx, |menu, _window, _cx| { - menu.action_disabled_when( - selected_binding_is_unbound, - "Edit", - Box::new(EditBinding), - ) - .action("Create", Box::new(CreateBinding)) - .action_disabled_when( - selected_binding_is_unbound, - "Delete", - Box::new(DeleteBinding), - ) - .separator() - .action("Copy Action", Box::new(CopyAction)) - .action_disabled_when( - selected_binding_has_no_context, - "Copy Context", - Box::new(CopyContext), - ) - .entry("Show matching keybindings", None, { - let weak = weak.clone(); - let key_strokes = key_strokes.clone(); - - move |_, cx| { - weak.update(cx, |this, cx| { - this.filter_state = FilterState::All; - this.search_mode = SearchMode::KeyStroke { exact_match: true }; - - this.keystroke_editor.update(cx, |editor, cx| { - editor.set_keystrokes(key_strokes.clone(), cx); - }); - }) - .ok(); - } - }) + menu.context(self.focus_handle.clone()) + .action_disabled_when( + selected_binding_is_unbound, + "Edit", + Box::new(EditBinding), + ) + .action("Create", Box::new(CreateBinding)) + .action_disabled_when( + selected_binding_is_unbound, + "Delete", + Box::new(DeleteBinding), + ) + .separator() + .action("Copy Action", Box::new(CopyAction)) + .action_disabled_when( + selected_binding_has_no_context, + "Copy Context", + Box::new(CopyContext), + ) + .separator() + .action_disabled_when( + selected_binding_has_no_context, + "Show Matching Keybindings", + Box::new(ShowMatchingKeybinds), + ) }); let context_menu_handle = context_menu.focus_handle(cx); @@ -785,15 +938,120 @@ impl KeymapEditor { self.context_menu.is_some() } + fn create_row_button( + &self, + index: usize, + conflict: Option, + cx: &mut Context, + ) -> IconButton { + if self.filter_state != FilterState::Conflicts + && let Some(conflict) = conflict + { + if conflict.is_user_keybind_conflict() { + base_button_style(index, IconName::Warning) + .icon_color(Color::Warning) + .tooltip(|window, cx| { + Tooltip::with_meta( + "View conflicts", + Some(&ToggleConflictFilter), + "Use alt+click to show all conflicts", + window, + cx, + ) + }) + .on_click(cx.listener(move |this, click: &ClickEvent, window, cx| { + if click.modifiers().alt { + this.set_filter_state(FilterState::Conflicts, cx); + } else { + this.select_index(index, None, window, cx); + this.open_edit_keybinding_modal(false, window, cx); + cx.stop_propagation(); + } + })) + } else if self.search_mode.exact_match() { + base_button_style(index, IconName::Info) + .tooltip(|window, cx| { + Tooltip::with_meta( + "Edit this binding", + Some(&ShowMatchingKeybinds), + "This binding is overridden by other bindings.", + window, + cx, + ) + }) + .on_click(cx.listener(move |this, _: &ClickEvent, window, cx| { + this.select_index(index, None, window, cx); + this.open_edit_keybinding_modal(false, window, cx); + cx.stop_propagation(); + })) + } else { + base_button_style(index, IconName::Info) + .tooltip(|window, cx| { + Tooltip::with_meta( + "Show matching keybinds", + Some(&ShowMatchingKeybinds), + "This binding is overridden by other bindings.\nUse alt+click to edit this binding", + window, + cx, + ) + }) + .on_click(cx.listener(move |this, click: &ClickEvent, window, cx| { + if click.modifiers().alt { + this.select_index(index, None, window, cx); + this.open_edit_keybinding_modal(false, window, cx); + cx.stop_propagation(); + } else { + this.show_matching_keystrokes(&Default::default(), window, cx); + } + })) + } + } else { + base_button_style(index, IconName::Pencil) + .visible_on_hover(if self.selected_index == Some(index) { + "".into() + } else if self.show_hover_menus { + row_group_id(index) + } else { + "never-show".into() + }) + .when( + self.show_hover_menus && !self.context_menu_deployed(), + |this| this.tooltip(Tooltip::for_action_title("Edit Keybinding", &EditBinding)), + ) + .on_click(cx.listener(move |this, _, window, cx| { + this.select_index(index, None, window, cx); + this.open_edit_keybinding_modal(false, window, cx); + cx.stop_propagation(); + })) + } + } + + fn render_no_matches_hint(&self, _window: &mut Window, _cx: &App) -> AnyElement { + let hint = match (self.filter_state, &self.search_mode) { + (FilterState::Conflicts, _) => { + if self.keybinding_conflict_state.any_user_binding_conflicts() { + "No conflicting keybinds found that match the provided query" + } else { + "No conflicting keybinds found" + } + } + (FilterState::All, SearchMode::KeyStroke { .. }) => { + "No keybinds found matching the entered keystrokes" + } + (FilterState::All, SearchMode::Normal) => "No matches found for the provided query", + }; + + Label::new(hint).color(Color::Muted).into_any_element() + } + fn select_next(&mut self, _: &menu::SelectNext, window: &mut Window, cx: &mut Context) { + self.show_hover_menus = false; if let Some(selected) = self.selected_index { let selected = selected + 1; if selected >= self.matches.len() { self.select_last(&Default::default(), window, cx); } else { - self.selected_index = Some(selected); - self.scroll_to_item(selected, ScrollStrategy::Center, cx); - cx.notify(); + self.select_index(selected, Some(ScrollStrategy::Center), window, cx); } } else { self.select_first(&Default::default(), window, cx); @@ -806,6 +1064,7 @@ impl KeymapEditor { window: &mut Window, cx: &mut Context, ) { + self.show_hover_menus = false; if let Some(selected) = self.selected_index { if selected == 0 { return; @@ -816,54 +1075,64 @@ impl KeymapEditor { if selected >= self.matches.len() { self.select_last(&Default::default(), window, cx); } else { - self.selected_index = Some(selected); - self.scroll_to_item(selected, ScrollStrategy::Center, cx); - cx.notify(); + self.select_index(selected, Some(ScrollStrategy::Center), window, cx); } } else { self.select_last(&Default::default(), window, cx); } } - fn select_first( - &mut self, - _: &menu::SelectFirst, - _window: &mut Window, - cx: &mut Context, - ) { + fn select_first(&mut self, _: &menu::SelectFirst, window: &mut Window, cx: &mut Context) { + self.show_hover_menus = false; if self.matches.get(0).is_some() { - self.selected_index = Some(0); - self.scroll_to_item(0, ScrollStrategy::Center, cx); - cx.notify(); + self.select_index(0, Some(ScrollStrategy::Center), window, cx); } } - fn select_last(&mut self, _: &menu::SelectLast, _window: &mut Window, cx: &mut Context) { + fn select_last(&mut self, _: &menu::SelectLast, window: &mut Window, cx: &mut Context) { + self.show_hover_menus = false; if self.matches.last().is_some() { let index = self.matches.len() - 1; - self.selected_index = Some(index); - self.scroll_to_item(index, ScrollStrategy::Center, cx); - cx.notify(); + self.select_index(index, Some(ScrollStrategy::Center), window, cx); } } - fn confirm(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context) { - self.open_edit_keybinding_modal(false, window, cx); - } - fn open_edit_keybinding_modal( &mut self, create: bool, window: &mut Window, cx: &mut Context, ) { - let Some((keybind_idx, keybind)) = self - .selected_keybind_idx() - .zip(self.selected_binding().cloned()) - else { + self.show_hover_menus = false; + let Some((keybind, keybind_index)) = self.selected_keybind_and_index() else { return; }; + let keybind = keybind.clone(); let keymap_editor = cx.entity(); + + let keystroke = keybind.keystroke_text().cloned().unwrap_or_default(); + let arguments = keybind + .action() + .arguments + .as_ref() + .map(|arguments| arguments.text.clone()); + let context = keybind + .context() + .map(|context| context.local_str().unwrap_or("global")); + let action = keybind.action().name; + let source = keybind.keybind_source().map(|source| source.name()); + + telemetry::event!( + "Edit Keybinding Modal Opened", + keystroke = keystroke, + action = action, + source = source, + context = context, + arguments = arguments, + ); + + let temp_dir = self.action_args_temp_dir.as_ref().map(|dir| dir.path()); + self.workspace .update(cx, |workspace, cx| { let fs = workspace.app_state().fs.clone(); @@ -872,8 +1141,9 @@ impl KeymapEditor { let modal = KeybindingEditorModal::new( create, keybind, - keybind_idx, + keybind_index, keymap_editor, + temp_dir, workspace_weak, fs, window, @@ -899,7 +1169,7 @@ impl KeymapEditor { return; }; - let Ok(fs) = self + let std::result::Result::Ok(fs) = self .workspace .read_with(cx, |workspace, _| workspace.app_state().fs.clone()) else { @@ -923,12 +1193,14 @@ impl KeymapEditor { ) { let context = self .selected_binding() - .and_then(|binding| binding.context.as_ref()) + .and_then(|binding| binding.context()) .and_then(KeybindContextString::local_str) .map(|context| context.to_string()); let Some(context) = context else { return; }; + + telemetry::event!("Keybinding Context Copied", context = context.clone()); cx.write_to_clipboard(gpui::ClipboardItem::new_string(context.clone())); } @@ -940,10 +1212,12 @@ impl KeymapEditor { ) { let action = self .selected_binding() - .map(|binding| binding.action_name.to_string()); + .map(|binding| binding.action().name.to_string()); let Some(action) = action else { return; }; + + telemetry::event!("Keybinding Action Copied", action = action.clone()); cx.write_to_clipboard(gpui::ClipboardItem::new_string(action.clone())); } @@ -972,18 +1246,19 @@ impl KeymapEditor { self.search_mode = self.search_mode.invert(); self.on_query_changed(cx); - // Update the keystroke editor to turn the `search` bool on - self.keystroke_editor.update(cx, |keystroke_editor, cx| { - keystroke_editor - .set_search_mode(matches!(self.search_mode, SearchMode::KeyStroke { .. })); - cx.notify(); - }); - match self.search_mode { SearchMode::KeyStroke { .. } => { - window.focus(&self.keystroke_editor.read(cx).recording_focus_handle(cx)); + self.keystroke_editor.update(cx, |editor, cx| { + editor.start_recording(&StartRecording, window, cx); + }); + } + SearchMode::Normal => { + self.keystroke_editor.update(cx, |editor, cx| { + editor.stop_recording(&StopRecording, window, cx); + editor.clear_keystrokes(&ClearKeystrokes, window, cx); + }); + window.focus(&self.filter_editor.focus_handle(cx)); } - SearchMode::Normal => {} } } @@ -1000,37 +1275,183 @@ impl KeymapEditor { *exact_match = !(*exact_match); self.on_query_changed(cx); } + + fn show_matching_keystrokes( + &mut self, + _: &ShowMatchingKeybinds, + _: &mut Window, + cx: &mut Context, + ) { + let Some(selected_binding) = self.selected_binding() else { + return; + }; + + let keystrokes = selected_binding + .keystrokes() + .map(Vec::from) + .unwrap_or_default(); + + self.filter_state = FilterState::All; + self.search_mode = SearchMode::KeyStroke { exact_match: true }; + + self.keystroke_editor.update(cx, |editor, cx| { + editor.set_keystrokes(keystrokes, cx); + }); + } +} + +struct HumanizedActionNameCache { + cache: HashMap<&'static str, SharedString>, +} + +impl HumanizedActionNameCache { + fn new(cx: &App) -> Self { + let cache = HashMap::from_iter(cx.all_action_names().into_iter().map(|&action_name| { + ( + action_name, + command_palette::humanize_action_name(action_name).into(), + ) + })); + Self { cache } + } + + fn get(&self, action_name: &'static str) -> SharedString { + match self.cache.get(action_name) { + Some(name) => name.clone(), + None => action_name.into(), + } + } } #[derive(Clone)] -struct ProcessedKeybinding { +struct KeybindInformation { keystroke_text: SharedString, - ui_key_binding: Option, - action_name: &'static str, - action_arguments: Option, - action_docs: Option<&'static str>, - action_schema: Option, - context: Option, - source: Option<(KeybindSource, SharedString)>, + ui_binding: ui::KeyBinding, + context: KeybindContextString, + source: KeybindSource, } -impl ProcessedKeybinding { +impl KeybindInformation { fn get_action_mapping(&self) -> ActionMapping { ActionMapping { - keystroke_text: self.keystroke_text.clone(), - context: self - .context - .as_ref() - .and_then(|context| context.local()) - .cloned(), + keystrokes: self.ui_binding.keystrokes.clone(), + context: self.context.local().cloned(), } } +} + +#[derive(Clone)] +struct ActionInformation { + name: &'static str, + humanized_name: SharedString, + arguments: Option, + documentation: Option<&'static str>, + has_schema: bool, +} + +impl ActionInformation { + fn new( + action_name: &'static str, + action_arguments: Option, + actions_with_schemas: &HashSet<&'static str>, + action_documentation: &HashMap<&'static str, &'static str>, + action_name_cache: &HumanizedActionNameCache, + ) -> Self { + Self { + humanized_name: action_name_cache.get(action_name), + has_schema: actions_with_schemas.contains(action_name), + arguments: action_arguments, + documentation: action_documentation.get(action_name).copied(), + name: action_name, + } + } +} + +#[derive(Clone)] +enum ProcessedBinding { + Mapped(KeybindInformation, ActionInformation), + Unmapped(ActionInformation), +} + +impl ProcessedBinding { + fn new_mapped( + keystroke_text: impl Into, + ui_key_binding: ui::KeyBinding, + context: KeybindContextString, + source: KeybindSource, + action_information: ActionInformation, + ) -> Self { + Self::Mapped( + KeybindInformation { + keystroke_text: keystroke_text.into(), + ui_binding: ui_key_binding, + context, + source, + }, + action_information, + ) + } + + fn is_unbound(&self) -> bool { + matches!(self, Self::Unmapped(_)) + } + + fn get_action_mapping(&self) -> Option { + self.keybind_information() + .map(|keybind| keybind.get_action_mapping()) + } fn keystrokes(&self) -> Option<&[Keystroke]> { - self.ui_key_binding - .as_ref() + self.ui_key_binding() .map(|binding| binding.keystrokes.as_slice()) } + + fn keybind_information(&self) -> Option<&KeybindInformation> { + match self { + Self::Mapped(keybind_information, _) => Some(keybind_information), + Self::Unmapped(_) => None, + } + } + + fn keybind_source(&self) -> Option { + self.keybind_information().map(|keybind| keybind.source) + } + + fn context(&self) -> Option<&KeybindContextString> { + self.keybind_information().map(|keybind| &keybind.context) + } + + fn ui_key_binding(&self) -> Option<&ui::KeyBinding> { + self.keybind_information() + .map(|keybind| &keybind.ui_binding) + } + + fn keystroke_text(&self) -> Option<&SharedString> { + self.keybind_information() + .map(|binding| &binding.keystroke_text) + } + + fn action(&self) -> &ActionInformation { + match self { + Self::Mapped(_, action) | Self::Unmapped(action) => action, + } + } + + fn cmp(&self, other: &Self) -> cmp::Ordering { + match (self, other) { + (Self::Mapped(keybind1, action1), Self::Mapped(keybind2, action2)) => { + match keybind1.source.cmp(&keybind2.source) { + cmp::Ordering::Equal => action1.humanized_name.cmp(&action2.humanized_name), + ordering => ordering, + } + } + (Self::Mapped(_, _), Self::Unmapped(_)) => cmp::Ordering::Less, + (Self::Unmapped(_), Self::Mapped(_, _)) => cmp::Ordering::Greater, + (Self::Unmapped(action1), Self::Unmapped(action2)) => { + action1.humanized_name.cmp(&action2.humanized_name) + } + } + } } #[derive(Clone, Debug, IntoElement, PartialEq, Eq, Hash)] @@ -1087,20 +1508,20 @@ impl Item for KeymapEditor { } impl Render for KeymapEditor { - fn render(&mut self, window: &mut Window, cx: &mut ui::Context) -> impl ui::IntoElement { + fn render(&mut self, _window: &mut Window, cx: &mut ui::Context) -> impl ui::IntoElement { let row_count = self.matches.len(); let theme = cx.theme(); + let focus_handle = &self.focus_handle; v_flex() .id("keymap-editor") - .track_focus(&self.focus_handle) - .key_context(self.dispatch_context(window, cx)) + .track_focus(focus_handle) + .key_context(self.key_context()) .on_action(cx.listener(Self::select_next)) .on_action(cx.listener(Self::select_previous)) .on_action(cx.listener(Self::select_first)) .on_action(cx.listener(Self::select_last)) .on_action(cx.listener(Self::focus_search)) - .on_action(cx.listener(Self::confirm)) .on_action(cx.listener(Self::edit_binding)) .on_action(cx.listener(Self::create_binding)) .on_action(cx.listener(Self::delete_binding)) @@ -1109,13 +1530,16 @@ impl Render for KeymapEditor { .on_action(cx.listener(Self::toggle_conflict_filter)) .on_action(cx.listener(Self::toggle_keystroke_search)) .on_action(cx.listener(Self::toggle_exact_keystroke_matching)) + .on_action(cx.listener(Self::show_matching_keystrokes)) + .on_mouse_move(cx.listener(|this, _, _window, _cx| { + this.show_hover_menus = true; + })) .size_full() .p_2() .gap_1() .bg(theme.colors().editor_background) .child( v_flex() - .p_2() .gap_2() .child( h_flex() @@ -1143,13 +1567,18 @@ impl Render for KeymapEditor { IconName::Keyboard, ) .shape(ui::IconButtonShape::Square) - .tooltip(|window, cx| { - Tooltip::for_action( - "Search by Keystroke", - &ToggleKeystrokeSearch, - window, - cx, - ) + .tooltip({ + let focus_handle = focus_handle.clone(); + + move |window, cx| { + Tooltip::for_action_in( + "Search by Keystroke", + &ToggleKeystrokeSearch, + &focus_handle.clone(), + window, + cx, + ) + } }) .toggle_state(matches!( self.search_mode, @@ -1159,38 +1588,44 @@ impl Render for KeymapEditor { window.dispatch_action(ToggleKeystrokeSearch.boxed_clone(), cx); }), ) - .when(self.keybinding_conflict_state.any_conflicts(), |this| { - this.child( - IconButton::new("KeymapEditorConflictIcon", IconName::Warning) - .shape(ui::IconButtonShape::Square) - .tooltip({ - let filter_state = self.filter_state; + .child( + IconButton::new("KeymapEditorConflictIcon", IconName::Warning) + .shape(ui::IconButtonShape::Square) + .when( + self.keybinding_conflict_state.any_user_binding_conflicts(), + |this| { + this.indicator(Indicator::dot().color(Color::Warning)) + }, + ) + .tooltip({ + let filter_state = self.filter_state; + let focus_handle = focus_handle.clone(); - move |window, cx| { - Tooltip::for_action( - match filter_state { - FilterState::All => "Show Conflicts", - FilterState::Conflicts => "Hide Conflicts", - }, - &ToggleConflictFilter, - window, - cx, - ) - } - }) - .selected_icon_color(Color::Warning) - .toggle_state(matches!( - self.filter_state, - FilterState::Conflicts - )) - .on_click(|_, window, cx| { - window.dispatch_action( - ToggleConflictFilter.boxed_clone(), + move |window, cx| { + Tooltip::for_action_in( + match filter_state { + FilterState::All => "Show Conflicts", + FilterState::Conflicts => "Hide Conflicts", + }, + &ToggleConflictFilter, + &focus_handle.clone(), + window, cx, - ); - }), - ) - }), + ) + } + }) + .selected_icon_color(Color::Warning) + .toggle_state(matches!( + self.filter_state, + FilterState::Conflicts + )) + .on_click(|_, window, cx| { + window.dispatch_action( + ToggleConflictFilter.boxed_clone(), + cx, + ); + }), + ), ) .when_some( match self.search_mode { @@ -1201,29 +1636,45 @@ impl Render for KeymapEditor { this.child( h_flex() .map(|this| { - if self.keybinding_conflict_state.any_conflicts() { + if self + .keybinding_conflict_state + .any_user_binding_conflicts() + { this.pr(rems_from_px(54.)) } else { this.pr_7() } }) + .gap_2() .child(self.keystroke_editor.clone()) .child( - div().p_1().child( - IconButton::new( - "keystrokes-exact-match", - IconName::Equal, - ) - .shape(IconButtonShape::Square) - .toggle_state(exact_match) - .on_click( - cx.listener(|_, _, window, cx| { - window.dispatch_action( - ToggleExactKeystrokeMatching.boxed_clone(), - cx, - ); - }), - ), + IconButton::new( + "keystrokes-exact-match", + IconName::CaseSensitive, + ) + .tooltip({ + let keystroke_focus_handle = + self.keystroke_editor.read(cx).focus_handle(cx); + + move |window, cx| { + Tooltip::for_action_in( + "Toggle Exact Match Mode", + &ToggleExactKeystrokeMatching, + &keystroke_focus_handle, + window, + cx, + ) + } + }) + .shape(IconButtonShape::Square) + .toggle_state(exact_match) + .on_click( + cx.listener(|_, _, window, cx| { + window.dispatch_action( + ToggleExactKeystrokeMatching.boxed_clone(), + cx, + ); + }), ), ), ) @@ -1234,14 +1685,30 @@ impl Render for KeymapEditor { Table::new() .interactable(&self.table_interaction_state) .striped() + .empty_table_callback({ + let this = cx.entity(); + move |window, cx| this.read(cx).render_no_matches_hint(window, cx) + }) .column_widths([ - rems(2.5), - rems(16.), - rems(16.), - rems(16.), - rems(32.), - rems(8.), + DefiniteLength::Absolute(AbsoluteLength::Pixels(px(40.))), + DefiniteLength::Fraction(0.25), + DefiniteLength::Fraction(0.20), + DefiniteLength::Fraction(0.14), + DefiniteLength::Fraction(0.45), + DefiniteLength::Fraction(0.08), ]) + .resizable_columns( + [ + ResizeBehavior::None, + ResizeBehavior::Resizable, + ResizeBehavior::Resizable, + ResizeBehavior::Resizable, + ResizeBehavior::Resizable, + ResizeBehavior::Resizable, // this column doesn't matter + ], + &self.current_widths, + cx, + ) .header(["", "Action", "Arguments", "Keystrokes", "Context", "Source"]) .uniform_list( "keymap-editor-table", @@ -1252,59 +1719,22 @@ impl Render for KeymapEditor { .filter_map(|index| { let candidate_id = this.matches.get(index)?.candidate_id; let binding = &this.keybindings[candidate_id]; - let action_name = binding.action_name; + let action_name = binding.action().name; + let conflict = this.get_conflict(index); + let is_overridden = conflict.is_some_and(|conflict| { + !conflict.is_user_keybind_conflict() + }); - let icon = (this.filter_state != FilterState::Conflicts - && this.has_conflict(index)) - .then(|| { - base_button_style(index, IconName::Warning) - .icon_color(Color::Warning) - .tooltip(|window, cx| { - Tooltip::with_meta( - "Edit Keybinding", - None, - "Use alt+click to show conflicts", - window, - cx, - ) - }) - .on_click(cx.listener( - move |this, click: &ClickEvent, window, cx| { - if click.modifiers().alt { - this.set_filter_state( - FilterState::Conflicts, - cx, - ); - } else { - this.select_index(index, cx); - this.open_edit_keybinding_modal( - false, window, cx, - ); - cx.stop_propagation(); - } - }, - )) - }) - .unwrap_or_else(|| { - base_button_style(index, IconName::Pencil) - .visible_on_hover(row_group_id(index)) - .tooltip(Tooltip::text("Edit Keybinding")) - .on_click(cx.listener(move |this, _, window, cx| { - this.select_index(index, cx); - this.open_edit_keybinding_modal(false, window, cx); - cx.stop_propagation(); - })) - }) - .into_any_element(); + let icon = this.create_row_button(index, conflict, cx); let action = div() .id(("keymap action", index)) .child({ if action_name != gpui::NoAction.name() { - this.humanized_action_names - .get(action_name) - .cloned() - .unwrap_or(action_name.into()) + binding + .action() + .humanized_name + .clone() .into_any_element() } else { const NULL: SharedString = @@ -1313,29 +1743,41 @@ impl Render for KeymapEditor { .into_any_element() } }) - .when(!context_menu_deployed, |this| { - this.tooltip({ - let action_name = binding.action_name; - let action_docs = binding.action_docs; - move |_, cx| { - let action_tooltip = Tooltip::new(action_name); - let action_tooltip = match action_docs { - Some(docs) => action_tooltip.meta(docs), - None => action_tooltip, - }; - cx.new(|_| action_tooltip).into() - } - }) - }) + .when( + !context_menu_deployed + && this.show_hover_menus + && !is_overridden, + |this| { + this.tooltip({ + let action_name = binding.action().name; + let action_docs = + binding.action().documentation; + move |_, cx| { + let action_tooltip = + Tooltip::new(action_name); + let action_tooltip = match action_docs { + Some(docs) => action_tooltip.meta(docs), + None => action_tooltip, + }; + cx.new(|_| action_tooltip).into() + } + }) + }, + ) .into_any_element(); - let keystrokes = binding.ui_key_binding.clone().map_or( - binding.keystroke_text.clone().into_any_element(), + let keystrokes = binding.ui_key_binding().cloned().map_or( + binding + .keystroke_text() + .cloned() + .unwrap_or_default() + .into_any_element(), IntoElement::into_any_element, ); - let action_arguments = match binding.action_arguments.clone() { + let action_arguments = match binding.action().arguments.clone() + { Some(arguments) => arguments.into_any_element(), None => { - if binding.action_schema.is_some() { + if binding.action().has_schema { muted_styled_text(NO_ACTION_ARGUMENTS_TEXT, cx) .into_any_element() } else { @@ -1343,7 +1785,7 @@ impl Render for KeymapEditor { } } }; - let context = binding.context.clone().map_or( + let context = binding.context().cloned().map_or( gpui::Empty.into_any_element(), |context| { let is_local = context.local().is_some(); @@ -1351,24 +1793,29 @@ impl Render for KeymapEditor { div() .id(("keymap context", index)) .child(context.clone()) - .when(is_local && !context_menu_deployed, |this| { - this.tooltip(Tooltip::element({ - move |_, _| { - context.clone().into_any_element() - } - })) - }) + .when( + is_local + && !context_menu_deployed + && !is_overridden + && this.show_hover_menus, + |this| { + this.tooltip(Tooltip::element({ + move |_, _| { + context.clone().into_any_element() + } + })) + }, + ) .into_any_element() }, ); let source = binding - .source - .clone() - .map(|(_source, name)| name) + .keybind_source() + .map(|source| source.name()) .unwrap_or_default() .into_any_element(); Some([ - icon, + icon.into_any_element(), action, action_arguments, keystrokes, @@ -1379,51 +1826,90 @@ impl Render for KeymapEditor { .collect() }), ) - .map_row( - cx.processor(|this, (row_index, row): (usize, Div), _window, cx| { - let is_conflict = this.has_conflict(row_index); + .map_row(cx.processor( + |this, (row_index, row): (usize, Stateful

), _window, cx| { + let conflict = this.get_conflict(row_index); let is_selected = this.selected_index == Some(row_index); let row_id = row_group_id(row_index); - let row = row - .id(row_id.clone()) - .on_any_mouse_down(cx.listener( - move |this, - mouse_down_event: &gpui::MouseDownEvent, - window, - cx| { - match mouse_down_event.button { - MouseButton::Right => { - this.select_index(row_index, cx); - this.create_context_menu( - mouse_down_event.position, - window, - cx, - ); - } - _ => {} - } - }, - )) - .on_click(cx.listener( - move |this, event: &ClickEvent, window, cx| { - this.select_index(row_index, cx); - if event.up.click_count == 2 { - this.open_edit_keybinding_modal(false, window, cx); - } - }, - )) - .group(row_id) + div() + .id(("keymap-row-wrapper", row_index)) + .child( + row.id(row_id.clone()) + .on_any_mouse_down(cx.listener( + move |this, + mouse_down_event: &gpui::MouseDownEvent, + window, + cx| { + match mouse_down_event.button { + MouseButton::Right => { + this.select_index( + row_index, None, window, cx, + ); + this.create_context_menu( + mouse_down_event.position, + window, + cx, + ); + } + _ => {} + } + }, + )) + .on_click(cx.listener( + move |this, event: &ClickEvent, window, cx| { + this.select_index(row_index, None, window, cx); + if event.up.click_count == 2 { + this.open_edit_keybinding_modal( + false, window, cx, + ); + } + }, + )) + .group(row_id) + .when( + conflict.is_some_and(|conflict| { + !conflict.is_user_keybind_conflict() + }), + |row| { + const OVERRIDDEN_OPACITY: f32 = 0.5; + row.opacity(OVERRIDDEN_OPACITY) + }, + ) + .when_some( + conflict.filter(|conflict| { + !this.context_menu_deployed() && + !conflict.is_user_keybind_conflict() + }), + |row, conflict| { + let overriding_binding = this.keybindings.get(conflict.index); + let context = overriding_binding.and_then(|binding| { + match conflict.override_source { + KeybindSource::User => Some("your keymap"), + KeybindSource::Vim => Some("the vim keymap"), + KeybindSource::Base => Some("your base keymap"), + _ => { + log::error!("Unexpected override from the {} keymap", conflict.override_source.name()); + None + } + }.map(|source| format!("This keybinding is overridden by the '{}' binding from {}.", binding.action().humanized_name, source)) + }).unwrap_or_else(|| "This binding is overridden.".to_string()); + + row.tooltip(Tooltip::text(context))}, + ), + ) .border_2() - .when(is_conflict, |row| { - row.bg(cx.theme().status().error_background) - }) + .when( + conflict.is_some_and(|conflict| { + conflict.is_user_keybind_conflict() + }), + |row| row.bg(cx.theme().status().error_background), + ) .when(is_selected, |row| { row.border_color(cx.theme().colors().panel_focused_border) - }); - - row.into_any_element() + }) + .into_any_element() }), ), ) @@ -1505,43 +1991,39 @@ impl RenderOnce for SyntaxHighlightedText { runs.push(text_style.to_run(text.len() - offset)); } - return StyledText::new(text).with_runs(runs); + StyledText::new(text).with_runs(runs) } } #[derive(PartialEq)] -enum InputError { - Warning(SharedString), - Error(SharedString), +struct InputError { + severity: ui::Severity, + content: SharedString, } impl InputError { fn warning(message: impl Into) -> Self { - Self::Warning(message.into()) - } - - fn error(message: impl Into) -> Self { - Self::Error(message.into()) - } - - fn content(&self) -> &SharedString { - match self { - InputError::Warning(content) | InputError::Error(content) => content, + Self { + severity: ui::Severity::Warning, + content: message.into(), } } - fn is_warning(&self) -> bool { - matches!(self, InputError::Warning(_)) + fn error(message: anyhow::Error) -> Self { + Self { + severity: ui::Severity::Error, + content: message.to_string().into(), + } } } struct KeybindingEditorModal { creating: bool, - editing_keybind: ProcessedKeybinding, + editing_keybind: ProcessedBinding, editing_keybind_idx: usize, keybind_editor: Entity, context_editor: Entity, - action_arguments_editor: Option>, + action_arguments_editor: Option>, fs: Arc, error: Option, keymap_editor: Entity, @@ -1562,9 +2044,10 @@ impl Focusable for KeybindingEditorModal { impl KeybindingEditorModal { pub fn new( create: bool, - editing_keybind: ProcessedKeybinding, + editing_keybind: ProcessedBinding, editing_keybind_idx: usize, keymap_editor: Entity, + action_args_temp_dir: Option<&std::path::Path>, workspace: WeakEntity, fs: Arc, window: &mut Window, @@ -1579,8 +2062,7 @@ impl KeybindingEditorModal { .label_size(LabelSize::Default); if let Some(context) = editing_keybind - .context - .as_ref() + .context() .and_then(KeybindContextString::local) { input.editor().update(cx, |editor, cx| { @@ -1614,40 +2096,30 @@ impl KeybindingEditorModal { input }); - let action_arguments_editor = editing_keybind.action_schema.clone().map(|_schema| { + let action_arguments_editor = editing_keybind.action().has_schema.then(|| { + let arguments = editing_keybind + .action() + .arguments + .as_ref() + .map(|args| args.text.clone()); cx.new(|cx| { - let mut editor = Editor::auto_height_unbounded(1, window, cx); - let workspace = workspace.clone(); - - if let Some(arguments) = editing_keybind.action_arguments.clone() { - editor.set_text(arguments.text, window, cx); - } else { - // TODO: default value from schema? - editor.set_placeholder_text("Action Arguments", cx); - } - cx.spawn(async |editor, cx| { - let json_language = load_json_language(workspace, cx).await; - editor - .update(cx, |editor, cx| { - if let Some(buffer) = editor.buffer().read(cx).as_singleton() { - buffer.update(cx, |buffer, cx| { - buffer.set_language(Some(json_language), cx) - }); - } - }) - .context("Failed to load JSON language for editing keybinding action arguments input") - }) - .detach_and_log_err(cx); - editor + ActionArgumentsEditor::new( + editing_keybind.action().name, + arguments, + action_args_temp_dir, + workspace.clone(), + window, + cx, + ) }) }); let focus_state = KeybindingEditorModalFocusState::new( - keybind_editor.read_with(cx, |keybind_editor, cx| keybind_editor.focus_handle(cx)), - action_arguments_editor.as_ref().map(|args_editor| { - args_editor.read_with(cx, |args_editor, cx| args_editor.focus_handle(cx)) - }), - context_editor.read_with(cx, |context_editor, cx| context_editor.focus_handle(cx)), + keybind_editor.focus_handle(cx), + action_arguments_editor + .as_ref() + .map(|args_editor| args_editor.focus_handle(cx)), + context_editor.focus_handle(cx), ); Self { @@ -1666,11 +2138,9 @@ impl KeybindingEditorModal { } fn set_error(&mut self, error: InputError, cx: &mut Context) -> bool { - if self - .error - .as_ref() - .is_some_and(|old_error| old_error.is_warning() && *old_error == error) - { + if self.error.as_ref().is_some_and(|old_error| { + old_error.severity == ui::Severity::Warning && *old_error == error + }) { false } else { self.error = Some(error); @@ -1683,7 +2153,7 @@ impl KeybindingEditorModal { let action_arguments = self .action_arguments_editor .as_ref() - .map(|editor| editor.read(cx).text(cx)); + .map(|editor| editor.read(cx).editor.read(cx).text(cx)); let value = action_arguments .as_ref() @@ -1692,7 +2162,7 @@ impl KeybindingEditorModal { }) .transpose()?; - cx.build_action(&self.editing_keybind.action_name, value) + cx.build_action(&self.editing_keybind.action().name, value) .context("Failed to validate action arguments")?; Ok(action_arguments) } @@ -1717,66 +2187,59 @@ impl KeybindingEditorModal { Ok(Some(context)) } - fn save(&mut self, cx: &mut Context) { + fn save_or_display_error(&mut self, cx: &mut Context) { + self.save(cx).map_err(|err| self.set_error(err, cx)).ok(); + } + + fn save(&mut self, cx: &mut Context) -> Result<(), InputError> { let existing_keybind = self.editing_keybind.clone(); let fs = self.fs.clone(); let tab_size = cx.global::().json_tab_size(); - let new_keystrokes = match self.validate_keystrokes(cx) { - Err(err) => { - self.set_error(InputError::error(err.to_string()), cx); - return; - } - Ok(keystrokes) => keystrokes, - }; - let new_context = match self.validate_context(cx) { - Err(err) => { - self.set_error(InputError::error(err.to_string()), cx); - return; - } - Ok(context) => context, - }; + let new_keystrokes = self + .validate_keystrokes(cx) + .map_err(InputError::error)? + .into_iter() + .map(remove_key_char) + .collect::>(); - let new_action_args = match self.validate_action_arguments(cx) { - Err(input_err) => { - self.set_error(InputError::error(input_err.to_string()), cx); - return; - } - Ok(input) => input, - }; + let new_context = self.validate_context(cx).map_err(InputError::error)?; + let new_action_args = self + .validate_action_arguments(cx) + .map_err(InputError::error)?; let action_mapping = ActionMapping { - keystroke_text: ui::text_for_keystrokes(&new_keystrokes, cx).into(), - context: new_context.as_ref().map(Into::into), + keystrokes: new_keystrokes, + context: new_context.map(SharedString::from), }; - let conflicting_indices = if self.creating { - self.keymap_editor - .read(cx) - .keybinding_conflict_state - .will_conflict(action_mapping) - } else { - self.keymap_editor - .read(cx) - .keybinding_conflict_state - .conflicting_indices_for_mapping(action_mapping, self.editing_keybind_idx) - }; - if let Some(conflicting_indices) = conflicting_indices { - let first_conflicting_index = conflicting_indices[0]; + let conflicting_indices = self + .keymap_editor + .read(cx) + .keybinding_conflict_state + .conflicting_indices_for_mapping( + &action_mapping, + self.creating.not().then_some(self.editing_keybind_idx), + ); + + conflicting_indices.map(|KeybindConflict { + first_conflict_index, + remaining_conflict_amount, + }| + { let conflicting_action_name = self .keymap_editor .read(cx) .keybindings - .get(first_conflicting_index) - .map(|keybind| keybind.action_name); + .get(first_conflict_index) + .map(|keybind| keybind.action().name); let warning_message = match conflicting_action_name { Some(name) => { - let confliction_action_amount = conflicting_indices.len() - 1; - if confliction_action_amount > 0 { + if remaining_conflict_amount > 0 { format!( "Your keybind would conflict with the \"{}\" action and {} other bindings", - name, confliction_action_amount + name, remaining_conflict_amount ) } else { format!("Your keybind would conflict with the \"{}\" action", name) @@ -1785,23 +2248,26 @@ impl KeybindingEditorModal { None => { log::info!( "Could not find action in keybindings with index {}", - first_conflicting_index + first_conflict_index ); "Your keybind would conflict with other actions".to_string() } }; - if self.set_error(InputError::warning(warning_message), cx) { - return; + let warning = InputError::warning(warning_message); + if self.error.as_ref().is_some_and(|old_error| *old_error == warning) { + Ok(()) + } else { + Err(warning) } - } + }).unwrap_or(Ok(()))?; let create = self.creating; let status_toast = StatusToast::new( format!( "Saved edits to the {} action.", - command_palette::humanize_action_name(&self.editing_keybind.action_name) + &self.editing_keybind.action().humanized_name ), cx, move |this, _cx| { @@ -1818,13 +2284,12 @@ impl KeybindingEditorModal { .log_err(); cx.spawn(async move |this, cx| { - let action_name = existing_keybind.action_name; + let action_name = existing_keybind.action().name; if let Err(err) = save_keybinding_update( create, existing_keybind, - &new_keystrokes, - new_context.as_deref(), + &action_mapping, new_action_args.as_deref(), &fs, tab_size, @@ -1832,17 +2297,11 @@ impl KeybindingEditorModal { .await { this.update(cx, |this, cx| { - this.set_error(InputError::error(err.to_string()), cx); + this.set_error(InputError::error(err), cx); }) .log_err(); } else { this.update(cx, |this, cx| { - let action_mapping = ActionMapping { - keystroke_text: ui::text_for_keystrokes(new_keystrokes.as_slice(), cx) - .into(), - context: new_context.map(SharedString::from), - }; - this.keymap_editor.update(cx, |keymap, cx| { keymap.previous_edit = Some(PreviousEdit::Keybinding { action_mapping, @@ -1859,6 +2318,8 @@ impl KeybindingEditorModal { } }) .detach(); + + Ok(()) } fn key_context(&self) -> KeyContext { @@ -1881,7 +2342,7 @@ impl KeybindingEditorModal { } fn confirm(&mut self, _: &menu::Confirm, _window: &mut Window, cx: &mut Context) { - self.save(cx); + self.save_or_display_error(cx); } fn cancel(&mut self, _: &menu::Cancel, _window: &mut Window, cx: &mut Context) { @@ -1889,11 +2350,17 @@ impl KeybindingEditorModal { } } +fn remove_key_char(Keystroke { modifiers, key, .. }: Keystroke) -> Keystroke { + Keystroke { + modifiers, + key, + ..Default::default() + } +} + impl Render for KeybindingEditorModal { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let theme = cx.theme().colors(); - let action_name = - command_palette::humanize_action_name(&self.editing_keybind.action_name).to_string(); v_flex() .w(rems(34.)) @@ -1913,12 +2380,19 @@ impl Render for KeybindingEditorModal { .gap_0p5() .border_b_1() .border_color(theme.border_variant) - .child(Label::new(action_name)) - .when_some(self.editing_keybind.action_docs, |this, docs| { - this.child( - Label::new(docs).size(LabelSize::Small).color(Color::Muted), - ) - }), + .child(Label::new( + self.editing_keybind.action().humanized_name.clone(), + )) + .when_some( + self.editing_keybind.action().documentation, + |this, docs| { + this.child( + Label::new(docs) + .size(LabelSize::Small) + .color(Color::Muted), + ) + }, + ), ), ) .section( @@ -1937,38 +2411,21 @@ impl Render for KeybindingEditorModal { .mt_1p5() .gap_1() .child(Label::new("Edit Arguments")) - .child( - div() - .w_full() - .py_1() - .px_1p5() - .rounded_lg() - .bg(theme.editor_background) - .border_1() - .border_color(theme.border_variant) - .child(editor), - ), + .child(editor), ) }) .child(self.context_editor.clone()) .when_some(self.error.as_ref(), |this, error| { this.child( Banner::new() - .map(|banner| match error { - InputError::Error(_) => { - banner.severity(ui::Severity::Error) - } - InputError::Warning(_) => { - banner.severity(ui::Severity::Warning) - } - }) + .severity(error.severity) // For some reason, the div overflows its container to the //right. The padding accounts for that. .child( div() .size_full() .pr_2() - .child(Label::new(error.content())), + .child(Label::new(error.content.clone())), ), ) }), @@ -1984,7 +2441,7 @@ impl Render for KeybindingEditorModal { ) .child(Button::new("save-btn", "Save").on_click(cx.listener( |this, _event, _window, cx| { - this.save(cx); + this.save_or_display_error(cx); }, ))), ), @@ -2048,6 +2505,240 @@ impl KeybindingEditorModalFocusState { } } +struct ActionArgumentsEditor { + editor: Entity, + focus_handle: FocusHandle, + is_loading: bool, + /// See documentation in `KeymapEditor` for why a temp dir is needed. + /// This field exists because the keymap editor temp dir creation may fail, + /// and rather than implement a complicated retry mechanism, we simply + /// fallback to trying to create a temporary directory in this editor on + /// demand. Of note is that the TempDir struct will remove the directory + /// when dropped. + backup_temp_dir: Option, +} + +impl Focusable for ActionArgumentsEditor { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl ActionArgumentsEditor { + fn new( + action_name: &'static str, + arguments: Option, + temp_dir: Option<&std::path::Path>, + workspace: WeakEntity, + window: &mut Window, + cx: &mut Context, + ) -> Self { + let focus_handle = cx.focus_handle(); + cx.on_focus_in(&focus_handle, window, |this, window, cx| { + this.editor.focus_handle(cx).focus(window); + }) + .detach(); + let editor = cx.new(|cx| { + let mut editor = Editor::auto_height_unbounded(1, window, cx); + Self::set_editor_text(&mut editor, arguments.clone(), window, cx); + editor.set_read_only(true); + editor + }); + + let temp_dir = temp_dir.map(|path| path.to_owned()); + cx.spawn_in(window, async move |this, cx| { + let result = async { + let (project, fs) = workspace.read_with(cx, |workspace, _cx| { + ( + workspace.project().downgrade(), + workspace.app_state().fs.clone(), + ) + })?; + + let file_name = + project::lsp_store::json_language_server_ext::normalized_action_file_name( + action_name, + ); + + let (buffer, backup_temp_dir) = + Self::create_temp_buffer(temp_dir, file_name.clone(), project.clone(), fs, cx) + .await + .context(concat!( + "Failed to create temporary buffer for action arguments. ", + "Auto-complete will not work" + ))?; + + let editor = cx.new_window_entity(|window, cx| { + let multi_buffer = cx.new(|cx| editor::MultiBuffer::singleton(buffer, cx)); + let mut editor = Editor::new( + editor::EditorMode::Full { + scale_ui_elements_with_buffer_font_size: true, + show_active_line_background: false, + sized_by_content: true, + }, + multi_buffer, + project.upgrade(), + window, + cx, + ); + editor.set_searchable(false); + editor.disable_scrollbars_and_minimap(window, cx); + editor.set_show_edit_predictions(Some(false), window, cx); + editor.set_show_gutter(false, cx); + Self::set_editor_text(&mut editor, arguments, window, cx); + editor + })?; + + this.update_in(cx, |this, window, cx| { + if this.editor.focus_handle(cx).is_focused(window) { + editor.focus_handle(cx).focus(window); + } + this.editor = editor; + this.backup_temp_dir = backup_temp_dir; + this.is_loading = false; + })?; + + anyhow::Ok(()) + } + .await; + if result.is_err() { + let json_language = load_json_language(workspace.clone(), cx).await; + this.update(cx, |this, cx| { + this.editor.update(cx, |editor, cx| { + if let Some(buffer) = editor.buffer().read(cx).as_singleton() { + buffer.update(cx, |buffer, cx| { + buffer.set_language(Some(json_language.clone()), cx) + }); + } + }) + // .context("Failed to load JSON language for editing keybinding action arguments input") + }) + .ok(); + this.update(cx, |this, _cx| { + this.is_loading = false; + }) + .ok(); + } + return result; + }) + .detach_and_log_err(cx); + Self { + editor, + focus_handle, + is_loading: true, + backup_temp_dir: None, + } + } + + fn set_editor_text( + editor: &mut Editor, + arguments: Option, + window: &mut Window, + cx: &mut Context, + ) { + if let Some(arguments) = arguments { + editor.set_text(arguments, window, cx); + } else { + // TODO: default value from schema? + editor.set_placeholder_text("Action Arguments", cx); + } + } + + async fn create_temp_buffer( + temp_dir: Option, + file_name: String, + project: WeakEntity, + fs: Arc, + cx: &mut AsyncApp, + ) -> anyhow::Result<(Entity, Option)> { + let (temp_file_path, temp_dir) = { + let file_name = file_name.clone(); + async move { + let temp_dir_backup = match temp_dir.as_ref() { + Some(_) => None, + None => { + let temp_dir = paths::temp_dir(); + let sub_temp_dir = tempfile::Builder::new() + .tempdir_in(temp_dir) + .context("Failed to create temporary directory")?; + Some(sub_temp_dir) + } + }; + let dir_path = temp_dir.as_deref().unwrap_or_else(|| { + temp_dir_backup + .as_ref() + .expect("created backup tempdir") + .path() + }); + let path = dir_path.join(file_name); + fs.create_file( + &path, + fs::CreateOptions { + ignore_if_exists: true, + overwrite: true, + }, + ) + .await + .context("Failed to create temporary file")?; + anyhow::Ok((path, temp_dir_backup)) + } + } + .await + .context("Failed to create backing file")?; + + project + .update(cx, |project, cx| { + project.open_local_buffer(temp_file_path, cx) + })? + .await + .context("Failed to create buffer") + .map(|buffer| (buffer, temp_dir)) + } +} + +impl Render for ActionArgumentsEditor { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let background_color; + let border_color; + let text_style = { + let colors = cx.theme().colors(); + let settings = theme::ThemeSettings::get_global(cx); + background_color = colors.editor_background; + border_color = if self.is_loading { + colors.border_disabled + } else { + colors.border_variant + }; + TextStyleRefinement { + font_size: Some(rems(0.875).into()), + font_weight: Some(settings.buffer_font.weight), + line_height: Some(relative(1.2)), + font_style: Some(gpui::FontStyle::Normal), + color: self.is_loading.then_some(colors.text_disabled), + ..Default::default() + } + }; + + self.editor + .update(cx, |editor, _| editor.set_text_style_refinement(text_style)); + + return v_flex().w_full().child( + h_flex() + .min_h_8() + .min_w_48() + .px_2() + .py_1p5() + .flex_grow() + .rounded_lg() + .bg(background_color) + .border_1() + .border_color(border_color) + .track_focus(&self.focus_handle) + .child(self.editor.clone()), + ); + } +} + struct KeyContextCompletionProvider { contexts: Vec, } @@ -2171,9 +2862,8 @@ async fn load_keybind_context_language( async fn save_keybinding_update( create: bool, - existing: ProcessedKeybinding, - new_keystrokes: &[Keystroke], - new_context: Option<&str>, + existing: ProcessedBinding, + action_mapping: &ActionMapping, new_args: Option<&str>, fs: &Arc, tab_size: usize, @@ -2183,37 +2873,31 @@ async fn save_keybinding_update( .context("Failed to load keymap file")?; let existing_keystrokes = existing.keystrokes().unwrap_or_default(); - let existing_context = existing - .context - .as_ref() - .and_then(KeybindContextString::local_str); + let existing_context = existing.context().and_then(KeybindContextString::local_str); let existing_args = existing - .action_arguments + .action() + .arguments .as_ref() .map(|args| args.text.as_ref()); let target = settings::KeybindUpdateTarget { context: existing_context, keystrokes: existing_keystrokes, - action_name: &existing.action_name, + action_name: &existing.action().name, action_arguments: existing_args, }; let source = settings::KeybindUpdateTarget { - context: new_context, - keystrokes: new_keystrokes, - action_name: &existing.action_name, + context: action_mapping.context.as_ref().map(|a| &***a), + keystrokes: &action_mapping.keystrokes, + action_name: &existing.action().name, action_arguments: new_args, }; let operation = if !create { settings::KeybindUpdateOperation::Replace { target, - target_keybind_source: existing - .source - .as_ref() - .map(|(source, _name)| *source) - .unwrap_or(KeybindSource::User), + target_keybind_source: existing.keybind_source().unwrap_or(KeybindSource::User), source, } } else { @@ -2222,6 +2906,9 @@ async fn save_keybinding_update( from: Some(target), } }; + + let (new_keybinding, removed_keybinding, source) = operation.generate_telemetry(); + let updated_keymap_contents = settings::KeymapFile::update_keybinding(operation, keymap_contents, tab_size) .context("Failed to update keybinding")?; @@ -2231,11 +2918,18 @@ async fn save_keybinding_update( ) .await .context("Failed to write keymap file")?; + + telemetry::event!( + "Keybinding Updated", + new_keybinding = new_keybinding, + removed_keybinding = removed_keybinding, + source = source + ); Ok(()) } async fn remove_keybinding( - existing: ProcessedKeybinding, + existing: ProcessedBinding, fs: &Arc, tab_size: usize, ) -> anyhow::Result<()> { @@ -2248,24 +2942,19 @@ async fn remove_keybinding( let operation = settings::KeybindUpdateOperation::Remove { target: settings::KeybindUpdateTarget { - context: existing - .context - .as_ref() - .and_then(KeybindContextString::local_str), + context: existing.context().and_then(KeybindContextString::local_str), keystrokes, - action_name: &existing.action_name, + action_name: &existing.action().name, action_arguments: existing - .action_arguments + .action() + .arguments .as_ref() .map(|arguments| arguments.text.as_ref()), }, - target_keybind_source: existing - .source - .as_ref() - .map(|(source, _name)| *source) - .unwrap_or(KeybindSource::User), + target_keybind_source: existing.keybind_source().unwrap_or(KeybindSource::User), }; + let (new_keybinding, removed_keybinding, source) = operation.generate_telemetry(); let updated_keymap_contents = settings::KeymapFile::update_keybinding(operation, keymap_contents, tab_size) .context("Failed to update keybinding")?; @@ -2275,6 +2964,13 @@ async fn remove_keybinding( ) .await .context("Failed to write keymap file")?; + + telemetry::event!( + "Keybinding Removed", + new_keybinding = new_keybinding, + removed_keybinding = removed_keybinding, + source = source + ); Ok(()) } @@ -2285,20 +2981,9 @@ enum CloseKeystrokeResult { None, } -#[derive(PartialEq, Eq, Debug, Clone)] -enum KeyPress<'a> { - Alt, - Control, - Function, - Shift, - Platform, - Key(&'a String), -} - struct KeystrokeInput { keystrokes: Vec, placeholder_keystrokes: Option>, - highlight_on_focus: bool, outer_focus_handle: FocusHandle, inner_focus_handle: FocusHandle, intercept_subscription: Option, @@ -2307,6 +2992,7 @@ struct KeystrokeInput { /// Handles tripe escape to stop recording close_keystrokes: Option>, close_keystrokes_start: Option, + previous_modifiers: Modifiers, } impl KeystrokeInput { @@ -2326,7 +3012,6 @@ impl KeystrokeInput { Self { keystrokes: Vec::new(), placeholder_keystrokes, - highlight_on_focus: true, inner_focus_handle, outer_focus_handle, intercept_subscription: None, @@ -2334,6 +3019,7 @@ impl KeystrokeInput { search: false, close_keystrokes: None, close_keystrokes_start: None, + previous_modifiers: Modifiers::default(), } } @@ -2356,7 +3042,7 @@ impl KeystrokeInput { } fn key_context() -> KeyContext { - let mut key_context = KeyContext::new_with_defaults(); + let mut key_context = KeyContext::default(); key_context.add("KeystrokeInput"); key_context } @@ -2372,6 +3058,7 @@ impl KeystrokeInput { else { log::trace!("No keybinding to stop recording keystrokes in keystroke input"); self.close_keystrokes.take(); + self.close_keystrokes_start.take(); return CloseKeystrokeResult::None; }; let action_keystrokes = keybind_for_close_action.keystrokes(); @@ -2422,12 +3109,26 @@ impl KeystrokeInput { ) { let keystrokes_len = self.keystrokes.len(); + if self.previous_modifiers.modified() + && event.modifiers.is_subset_of(&self.previous_modifiers) + { + self.previous_modifiers &= event.modifiers; + cx.stop_propagation(); + return; + } + if let Some(last) = self.keystrokes.last_mut() && last.key.is_empty() && keystrokes_len <= Self::KEYSTROKE_COUNT_MAX { if self.search { - last.modifiers = last.modifiers.xor(&event.modifiers); + if self.previous_modifiers.modified() { + last.modifiers |= event.modifiers; + self.previous_modifiers |= event.modifiers; + } else { + self.keystrokes.push(Self::dummy(event.modifiers)); + self.previous_modifiers |= event.modifiers; + } } else if !event.modifiers.modified() { self.keystrokes.pop(); } else { @@ -2437,6 +3138,9 @@ impl KeystrokeInput { self.keystrokes_changed(cx); } else if keystrokes_len < Self::KEYSTROKE_COUNT_MAX { self.keystrokes.push(Self::dummy(event.modifiers)); + if self.search { + self.previous_modifiers |= event.modifiers; + } self.keystrokes_changed(cx); } cx.stop_propagation(); @@ -2457,6 +3161,14 @@ impl KeystrokeInput { { if self.search { last.key = keystroke.key.clone(); + if close_keystroke_result == CloseKeystrokeResult::Partial + && self.close_keystrokes_start.is_none() + { + self.close_keystrokes_start = Some(self.keystrokes.len() - 1); + } + if self.search { + self.previous_modifiers = keystroke.modifiers; + } self.keystrokes_changed(cx); cx.stop_propagation(); return; @@ -2471,9 +3183,13 @@ impl KeystrokeInput { self.close_keystrokes_start = Some(self.keystrokes.len()); } self.keystrokes.push(keystroke.clone()); - if self.keystrokes.len() < Self::KEYSTROKE_COUNT_MAX { + if self.search { + self.previous_modifiers = keystroke.modifiers; + } else if self.keystrokes.len() < Self::KEYSTROKE_COUNT_MAX { self.keystrokes.push(Self::dummy(keystroke.modifiers)); } + } else if close_keystroke_result != CloseKeystrokeResult::Partial { + self.clear_keystrokes(&ClearKeystrokes, window, cx); } } self.keystrokes_changed(cx); @@ -2539,21 +3255,11 @@ impl KeystrokeInput { }) } - fn recording_focus_handle(&self, _cx: &App) -> FocusHandle { - self.inner_focus_handle.clone() - } - - fn set_search_mode(&mut self, search: bool) { - self.search = search; - } - fn start_recording(&mut self, _: &StartRecording, window: &mut Window, cx: &mut Context) { - if !self.outer_focus_handle.is_focused(window) { - return; - } - self.clear_keystrokes(&ClearKeystrokes, window, cx); window.focus(&self.inner_focus_handle); - cx.notify(); + self.clear_keystrokes(&ClearKeystrokes, window, cx); + self.previous_modifiers = window.modifiers(); + cx.stop_propagation(); } fn stop_recording(&mut self, _: &StopRecording, window: &mut Window, cx: &mut Context) { @@ -2561,7 +3267,9 @@ impl KeystrokeInput { return; } window.focus(&self.outer_focus_handle); - if let Some(close_keystrokes_start) = self.close_keystrokes_start.take() { + if let Some(close_keystrokes_start) = self.close_keystrokes_start.take() + && close_keystrokes_start < self.keystrokes.len() + { self.keystrokes.drain(close_keystrokes_start..); } self.close_keystrokes.take(); @@ -2599,7 +3307,7 @@ impl Render for KeystrokeInput { .editor_background .blend(colors.text_accent.opacity(0.1)); - let recording_pulse = || { + let recording_pulse = |color: Color| { Icon::new(IconName::Circle) .size(IconSize::Small) .color(Color::Error) @@ -2609,7 +3317,7 @@ impl Render for KeystrokeInput { .repeat() .with_easing(gpui::pulsating_between(0.4, 0.8)), { - let color = Color::Error.color(cx); + let color = color.color(cx); move |this, delta| this.color(Color::Custom(color.opacity(delta))) }, ) @@ -2625,7 +3333,7 @@ impl Render for KeystrokeInput { .editor_background .blend(colors.text_accent.opacity(0.1))) .rounded_sm() - .child(recording_pulse()) + .child(recording_pulse(Color::Error)) .child( Label::new("REC") .size(LabelSize::XSmall) @@ -2643,7 +3351,7 @@ impl Render for KeystrokeInput { .editor_background .blend(colors.text_accent.opacity(0.1))) .rounded_sm() - .child(recording_pulse()) + .child(recording_pulse(Color::Accent)) .child( Label::new("SEARCH") .size(LabelSize::XSmall) @@ -2657,7 +3365,7 @@ impl Render for KeystrokeInput { IconName::PlayFilled }; - return h_flex() + h_flex() .id("keystroke-input") .track_focus(&self.outer_focus_handle) .py_2() @@ -2683,7 +3391,7 @@ impl Render for KeystrokeInput { }) .key_context(Self::key_context()) .on_action(cx.listener(Self::start_recording)) - .on_action(cx.listener(Self::stop_recording)) + .on_action(cx.listener(Self::clear_keystrokes)) .child( h_flex() .w(horizontal_padding) @@ -2706,7 +3414,7 @@ impl Render for KeystrokeInput { .track_focus(&self.inner_focus_handle) .on_modifiers_changed(cx.listener(Self::on_modifiers_changed)) .size_full() - .when(self.highlight_on_focus, |this| { + .when(!self.search, |this| { this.focus(|mut style| { style.border_color = Some(colors.border_focused); style @@ -2780,7 +3488,7 @@ impl Render for KeystrokeInput { this.clear_keystrokes(&ClearKeystrokes, window, cx); })), ), - ); + ) } } @@ -2952,72 +3660,3 @@ mod persistence { } } } - -/// Iterator that yields KeyPress values from a slice of Keystrokes -struct KeyPressIterator<'a> { - keystrokes: &'a [Keystroke], - current_keystroke_index: usize, - current_key_press_index: usize, -} - -impl<'a> KeyPressIterator<'a> { - fn new(keystrokes: &'a [Keystroke]) -> Self { - Self { - keystrokes, - current_keystroke_index: 0, - current_key_press_index: 0, - } - } -} - -impl<'a> Iterator for KeyPressIterator<'a> { - type Item = KeyPress<'a>; - - fn next(&mut self) -> Option { - loop { - let keystroke = self.keystrokes.get(self.current_keystroke_index)?; - - match self.current_key_press_index { - 0 => { - self.current_key_press_index = 1; - if keystroke.modifiers.platform { - return Some(KeyPress::Platform); - } - } - 1 => { - self.current_key_press_index = 2; - if keystroke.modifiers.alt { - return Some(KeyPress::Alt); - } - } - 2 => { - self.current_key_press_index = 3; - if keystroke.modifiers.control { - return Some(KeyPress::Control); - } - } - 3 => { - self.current_key_press_index = 4; - if keystroke.modifiers.shift { - return Some(KeyPress::Shift); - } - } - 4 => { - self.current_key_press_index = 5; - if keystroke.modifiers.function { - return Some(KeyPress::Function); - } - } - _ => { - self.current_keystroke_index += 1; - self.current_key_press_index = 0; - - if keystroke.key.is_empty() { - continue; - } - return Some(KeyPress::Key(&keystroke.key)); - } - } - } - } -} diff --git a/crates/settings_ui/src/ui_components/table.rs b/crates/settings_ui/src/ui_components/table.rs index 98dd738765..69207f559b 100644 --- a/crates/settings_ui/src/ui_components/table.rs +++ b/crates/settings_ui/src/ui_components/table.rs @@ -2,19 +2,24 @@ use std::{ops::Range, rc::Rc, time::Duration}; use editor::{EditorSettings, ShowScrollbar, scroll::ScrollbarAutoHide}; use gpui::{ - AppContext, Axis, Context, Entity, FocusHandle, Length, ListHorizontalSizingBehavior, - ListSizingBehavior, MouseButton, Point, Task, UniformListScrollHandle, WeakEntity, - transparent_black, uniform_list, + AbsoluteLength, AppContext, Axis, Context, DefiniteLength, DragMoveEvent, Entity, FocusHandle, + Length, ListHorizontalSizingBehavior, ListSizingBehavior, MouseButton, Point, Stateful, Task, + UniformListScrollHandle, WeakEntity, transparent_black, uniform_list, }; + +use itertools::intersperse_with; use settings::Settings as _; use ui::{ ActiveTheme as _, AnyElement, App, Button, ButtonCommon as _, ButtonStyle, Color, Component, ComponentScope, Div, ElementId, FixedWidth as _, FluentBuilder as _, Indicator, - InteractiveElement as _, IntoElement, ParentElement, Pixels, RegisterComponent, RenderOnce, - Scrollbar, ScrollbarState, StatefulInteractiveElement as _, Styled, StyledExt as _, + InteractiveElement, IntoElement, ParentElement, Pixels, RegisterComponent, RenderOnce, + Scrollbar, ScrollbarState, StatefulInteractiveElement, Styled, StyledExt as _, StyledTypography, Window, div, example_group_with_title, h_flex, px, single_example, v_flex, }; +#[derive(Debug)] +struct DraggedColumn(usize); + struct UniformListData { render_item_fn: Box, &mut Window, &mut App) -> Vec<[AnyElement; COLS]>>, element_id: ElementId, @@ -40,6 +45,10 @@ impl TableContents { TableContents::UniformList(data) => data.row_count, } } + + fn is_empty(&self) -> bool { + self.len() == 0 + } } pub struct TableInteractionState { @@ -187,6 +196,87 @@ impl TableInteractionState { } } + fn render_resize_handles( + &self, + column_widths: &[Length; COLS], + resizable_columns: &[ResizeBehavior; COLS], + initial_sizes: [DefiniteLength; COLS], + columns: Option>>, + window: &mut Window, + cx: &mut App, + ) -> AnyElement { + let spacers = column_widths + .iter() + .map(|width| base_cell_style(Some(*width)).into_any_element()); + + let mut column_ix = 0; + let resizable_columns_slice = *resizable_columns; + let mut resizable_columns = resizable_columns.into_iter(); + let dividers = intersperse_with(spacers, || { + window.with_id(column_ix, |window| { + let mut resize_divider = div() + // This is required because this is evaluated at a different time than the use_state call above + .id(column_ix) + .relative() + .top_0() + .w_0p5() + .h_full() + .bg(cx.theme().colors().border.opacity(0.5)); + + let mut resize_handle = div() + .id("column-resize-handle") + .absolute() + .left_neg_0p5() + .w(px(5.0)) + .h_full(); + + if resizable_columns + .next() + .is_some_and(ResizeBehavior::is_resizable) + { + let hovered = window.use_state(cx, |_window, _cx| false); + resize_divider = resize_divider.when(*hovered.read(cx), |div| { + div.bg(cx.theme().colors().border_focused) + }); + resize_handle = resize_handle + .on_hover(move |&was_hovered, _, cx| hovered.write(cx, was_hovered)) + .cursor_col_resize() + .when_some(columns.clone(), |this, columns| { + this.on_click(move |event, window, cx| { + if event.down.click_count >= 2 { + columns.update(cx, |columns, _| { + columns.on_double_click( + column_ix, + &initial_sizes, + &resizable_columns_slice, + window, + ); + }) + } + + cx.stop_propagation(); + }) + }) + .on_drag(DraggedColumn(column_ix), |_, _offset, _window, cx| { + cx.new(|_cx| gpui::Empty) + }) + } + + column_ix += 1; + resize_divider.child(resize_handle).into_any_element() + }) + }); + + div() + .id("resize-handles") + .h_flex() + .absolute() + .w_full() + .inset_0() + .children(dividers) + .into_any_element() + } + fn render_vertical_scrollbar_track( this: &Entity, parent: Div, @@ -365,6 +455,242 @@ impl TableInteractionState { } } +#[derive(Debug, Copy, Clone, PartialEq)] +pub enum ResizeBehavior { + None, + Resizable, + MinSize(f32), +} + +impl ResizeBehavior { + pub fn is_resizable(&self) -> bool { + *self != ResizeBehavior::None + } + + pub fn min_size(&self) -> Option { + match self { + ResizeBehavior::None => None, + ResizeBehavior::Resizable => Some(0.05), + ResizeBehavior::MinSize(min_size) => Some(*min_size), + } + } +} + +pub struct ColumnWidths { + widths: [DefiniteLength; COLS], + cached_bounds_width: Pixels, + initialized: bool, +} + +impl ColumnWidths { + pub fn new(_: &mut App) -> Self { + Self { + widths: [DefiniteLength::default(); COLS], + cached_bounds_width: Default::default(), + initialized: false, + } + } + + fn get_fraction(length: &DefiniteLength, bounds_width: Pixels, rem_size: Pixels) -> f32 { + match length { + DefiniteLength::Absolute(AbsoluteLength::Pixels(pixels)) => *pixels / bounds_width, + DefiniteLength::Absolute(AbsoluteLength::Rems(rems_width)) => { + rems_width.to_pixels(rem_size) / bounds_width + } + DefiniteLength::Fraction(fraction) => *fraction, + } + } + + fn on_double_click( + &mut self, + double_click_position: usize, + initial_sizes: &[DefiniteLength; COLS], + resize_behavior: &[ResizeBehavior; COLS], + window: &mut Window, + ) { + let bounds_width = self.cached_bounds_width; + let rem_size = window.rem_size(); + let initial_sizes = + initial_sizes.map(|length| Self::get_fraction(&length, bounds_width, rem_size)); + let mut widths = self + .widths + .map(|length| Self::get_fraction(&length, bounds_width, rem_size)); + + let diff = initial_sizes[double_click_position] - widths[double_click_position]; + + if diff > 0.0 { + let diff_remaining = self.propagate_resize_diff_right( + diff, + double_click_position, + &mut widths, + resize_behavior, + ); + + if diff_remaining > 0.0 && double_click_position > 0 { + self.propagate_resize_diff_left( + -diff_remaining, + double_click_position - 1, + &mut widths, + resize_behavior, + ); + } + } else if double_click_position > 0 { + let diff_remaining = self.propagate_resize_diff_left( + diff, + double_click_position, + &mut widths, + resize_behavior, + ); + + if diff_remaining < 0.0 { + self.propagate_resize_diff_right( + -diff_remaining, + double_click_position, + &mut widths, + resize_behavior, + ); + } + } + self.widths = widths.map(DefiniteLength::Fraction); + } + + fn on_drag_move( + &mut self, + drag_event: &DragMoveEvent, + resize_behavior: &[ResizeBehavior; COLS], + window: &mut Window, + cx: &mut Context, + ) { + let drag_position = drag_event.event.position; + let bounds = drag_event.bounds; + + let mut col_position = 0.0; + let rem_size = window.rem_size(); + let bounds_width = bounds.right() - bounds.left(); + let col_idx = drag_event.drag(cx).0; + + let mut widths = self + .widths + .map(|length| Self::get_fraction(&length, bounds_width, rem_size)); + + for length in widths[0..=col_idx].iter() { + col_position += length; + } + + let mut total_length_ratio = col_position; + for length in widths[col_idx + 1..].iter() { + total_length_ratio += length; + } + + let drag_fraction = (drag_position.x - bounds.left()) / bounds_width; + let drag_fraction = drag_fraction * total_length_ratio; + let diff = drag_fraction - col_position; + + let is_dragging_right = diff > 0.0; + + if is_dragging_right { + self.propagate_resize_diff_right(diff, col_idx, &mut widths, resize_behavior); + } else { + // Resize behavior should be improved in the future by also seeking to the right column when there's not enough space + self.propagate_resize_diff_left(diff, col_idx, &mut widths, resize_behavior); + } + self.widths = widths.map(DefiniteLength::Fraction); + } + + fn propagate_resize_diff_right( + &self, + diff: f32, + col_idx: usize, + widths: &mut [f32; COLS], + resize_behavior: &[ResizeBehavior; COLS], + ) -> f32 { + let mut diff_remaining = diff; + let mut curr_column = col_idx + 1; + + while diff_remaining > 0.0 && curr_column < COLS { + let Some(min_size) = resize_behavior[curr_column - 1].min_size() else { + curr_column += 1; + continue; + }; + + let mut curr_width = widths[curr_column] - diff_remaining; + + diff_remaining = 0.0; + if min_size > curr_width { + diff_remaining += min_size - curr_width; + curr_width = min_size; + } + widths[curr_column] = curr_width; + curr_column += 1; + } + + widths[col_idx] = widths[col_idx] + (diff - diff_remaining); + return diff_remaining; + } + + fn propagate_resize_diff_left( + &mut self, + diff: f32, + mut curr_column: usize, + widths: &mut [f32; COLS], + resize_behavior: &[ResizeBehavior; COLS], + ) -> f32 { + let mut diff_remaining = diff; + let col_idx = curr_column; + while diff_remaining < 0.0 { + let Some(min_size) = resize_behavior[curr_column].min_size() else { + if curr_column == 0 { + break; + } + curr_column -= 1; + continue; + }; + + let mut curr_width = widths[curr_column] + diff_remaining; + + diff_remaining = 0.0; + if curr_width < min_size { + diff_remaining = curr_width - min_size; + curr_width = min_size + } + + widths[curr_column] = curr_width; + if curr_column == 0 { + break; + } + curr_column -= 1; + } + widths[col_idx + 1] = widths[col_idx + 1] - (diff - diff_remaining); + + return diff_remaining; + } +} + +pub struct TableWidths { + initial: [DefiniteLength; COLS], + current: Option>>, + resizable: [ResizeBehavior; COLS], +} + +impl TableWidths { + pub fn new(widths: [impl Into; COLS]) -> Self { + let widths = widths.map(Into::into); + + TableWidths { + initial: widths, + current: None, + resizable: [ResizeBehavior::None; COLS], + } + } + + fn lengths(&self, cx: &App) -> [Length; COLS] { + self.current + .as_ref() + .map(|entity| entity.read(cx).widths.map(Length::Definite)) + .unwrap_or(self.initial.map(Length::Definite)) + } +} + /// A table component #[derive(RegisterComponent, IntoElement)] pub struct Table { @@ -373,21 +699,23 @@ pub struct Table { headers: Option<[AnyElement; COLS]>, rows: TableContents, interaction_state: Option>, - column_widths: Option<[Length; COLS]>, - map_row: Option AnyElement>>, + col_widths: Option>, + map_row: Option), &mut Window, &mut App) -> AnyElement>>, + empty_table_callback: Option AnyElement>>, } impl Table { /// number of headers provided. pub fn new() -> Self { - Table { + Self { striped: false, width: None, headers: None, rows: TableContents::Vec(Vec::new()), interaction_state: None, - column_widths: None, map_row: None, + empty_table_callback: None, + col_widths: None, } } @@ -448,32 +776,68 @@ impl Table { self } - pub fn column_widths(mut self, widths: [impl Into; COLS]) -> Self { - self.column_widths = Some(widths.map(Into::into)); + pub fn column_widths(mut self, widths: [impl Into; COLS]) -> Self { + if self.col_widths.is_none() { + self.col_widths = Some(TableWidths::new(widths)); + } + self + } + + pub fn resizable_columns( + mut self, + resizable: [ResizeBehavior; COLS], + column_widths: &Entity>, + cx: &mut App, + ) -> Self { + if let Some(table_widths) = self.col_widths.as_mut() { + table_widths.resizable = resizable; + let column_widths = table_widths + .current + .get_or_insert_with(|| column_widths.clone()); + + column_widths.update(cx, |widths, _| { + if !widths.initialized { + widths.initialized = true; + widths.widths = table_widths.initial; + } + }) + } self } pub fn map_row( mut self, - callback: impl Fn((usize, Div), &mut Window, &mut App) -> AnyElement + 'static, + callback: impl Fn((usize, Stateful
), &mut Window, &mut App) -> AnyElement + 'static, ) -> Self { self.map_row = Some(Rc::new(callback)); self } + + /// Provide a callback that is invoked when the table is rendered without any rows + pub fn empty_table_callback( + mut self, + callback: impl Fn(&mut Window, &mut App) -> AnyElement + 'static, + ) -> Self { + self.empty_table_callback = Some(Rc::new(callback)); + self + } } -fn base_cell_style(width: Option, cx: &App) -> Div { +fn base_cell_style(width: Option) -> Div { div() .px_1p5() .when_some(width, |this, width| this.w(width)) .when(width.is_none(), |this| this.flex_1()) .justify_start() - .text_ui(cx) .whitespace_nowrap() .text_ellipsis() .overflow_hidden() } +fn base_cell_style_text(width: Option, cx: &App) -> Div { + base_cell_style(width).text_ui(cx) +} + pub fn render_row( row_index: usize, items: [impl IntoElement; COLS], @@ -492,33 +856,33 @@ pub fn render_row( .column_widths .map_or([None; COLS], |widths| widths.map(Some)); - let row = div().w_full().child( - h_flex() - .id("table_row") - .w_full() - .justify_between() - .px_1p5() - .py_1() - .when_some(bg, |row, bg| row.bg(bg)) - .when(!is_striped, |row| { - row.border_b_1() - .border_color(transparent_black()) - .when(!is_last, |row| row.border_color(cx.theme().colors().border)) - }) - .children( - items - .map(IntoElement::into_any_element) - .into_iter() - .zip(column_widths) - .map(|(cell, width)| base_cell_style(width, cx).child(cell)), - ), + let mut row = h_flex() + .h_full() + .id(("table_row", row_index)) + .w_full() + .justify_between() + .when_some(bg, |row, bg| row.bg(bg)) + .when(!is_striped, |row| { + row.border_b_1() + .border_color(transparent_black()) + .when(!is_last, |row| row.border_color(cx.theme().colors().border)) + }); + + row = row.children( + items + .map(IntoElement::into_any_element) + .into_iter() + .zip(column_widths) + .map(|(cell, width)| base_cell_style_text(width, cx).px_1p5().py_1().child(cell)), ); - if let Some(map_row) = table_context.map_row { + let row = if let Some(map_row) = table_context.map_row { map_row((row_index, row), window, cx) } else { row.into_any_element() - } + }; + + div().h_full().w_full().child(row).into_any_element() } pub fn render_header( @@ -542,7 +906,7 @@ pub fn render_header( headers .into_iter() .zip(column_widths) - .map(|(h, width)| base_cell_style(width, cx).child(h)), + .map(|(h, width)| base_cell_style_text(width, cx).child(h)), ) } @@ -551,15 +915,15 @@ pub struct TableRenderContext { pub striped: bool, pub total_row_count: usize, pub column_widths: Option<[Length; COLS]>, - pub map_row: Option AnyElement>>, + pub map_row: Option), &mut Window, &mut App) -> AnyElement>>, } impl TableRenderContext { - fn new(table: &Table) -> Self { + fn new(table: &Table, cx: &App) -> Self { Self { striped: table.striped, total_row_count: table.rows.len(), - column_widths: table.column_widths, + column_widths: table.col_widths.as_ref().map(|widths| widths.lengths(cx)), map_row: table.map_row.clone(), } } @@ -567,8 +931,13 @@ impl TableRenderContext { impl RenderOnce for Table { fn render(mut self, window: &mut Window, cx: &mut App) -> impl IntoElement { - let table_context = TableRenderContext::new(&self); + let table_context = TableRenderContext::new(&self, cx); let interaction_state = self.interaction_state.and_then(|state| state.upgrade()); + let current_widths = self + .col_widths + .as_ref() + .and_then(|widths| Some((widths.current.as_ref()?, widths.resizable))) + .map(|(curr, resize_behavior)| (curr.downgrade(), resize_behavior)); let scroll_track_size = px(16.); let h_scroll_offset = if interaction_state @@ -582,6 +951,7 @@ impl RenderOnce for Table { }; let width = self.width; + let no_rows_rendered = self.rows.is_empty(); let table = div() .when_some(width, |this, width| this.w(width)) @@ -590,6 +960,31 @@ impl RenderOnce for Table { .when_some(self.headers.take(), |this, headers| { this.child(render_header(headers, table_context.clone(), cx)) }) + .when_some(current_widths, { + |this, (widths, resize_behavior)| { + this.on_drag_move::({ + let widths = widths.clone(); + move |e, window, cx| { + widths + .update(cx, |widths, cx| { + widths.on_drag_move(e, &resize_behavior, window, cx); + }) + .ok(); + } + }) + .on_children_prepainted(move |bounds, _, cx| { + widths + .update(cx, |widths, _| { + // This works because all children x axis bounds are the same + widths.cached_bounds_width = bounds[0].right() - bounds[0].left(); + }) + .ok(); + }) + } + }) + .on_drop::(|_, _, _| { + // Finish the resize operation + }) .child( div() .flex_grow() @@ -644,6 +1039,25 @@ impl RenderOnce for Table { ), ), }) + .when_some( + self.col_widths.as_ref().zip(interaction_state.as_ref()), + |parent, (table_widths, state)| { + parent.child(state.update(cx, |state, cx| { + let resizable_columns = table_widths.resizable; + let column_widths = table_widths.lengths(cx); + let columns = table_widths.current.clone(); + let initial_sizes = table_widths.initial; + state.render_resize_handles( + &column_widths, + &resizable_columns, + initial_sizes, + columns, + window, + cx, + ) + })) + }, + ) .when_some(interaction_state.as_ref(), |this, interaction_state| { this.map(|this| { TableInteractionState::render_vertical_scrollbar_track( @@ -662,6 +1076,21 @@ impl RenderOnce for Table { }) }), ) + .when_some( + no_rows_rendered + .then_some(self.empty_table_callback) + .flatten(), + |this, callback| { + this.child( + h_flex() + .size_full() + .p_3() + .items_start() + .justify_center() + .child(callback(window, cx)), + ) + }, + ) .when_some( width.and(interaction_state.as_ref()), |this, interaction_state| { diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index a2742081f4..aa9682029e 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -320,7 +320,39 @@ impl History { last_edit_at: now, suppress_grouping: false, }); - self.redo_stack.clear(); + } + + /// Differs from `push_transaction` in that it does not clear the redo + /// stack. Intended to be used to create a parent transaction to merge + /// potential child transactions into. + /// + /// The caller is responsible for removing it from the undo history using + /// `forget_transaction` if no edits are merged into it. Otherwise, if edits + /// are merged into this transaction, the caller is responsible for ensuring + /// the redo stack is cleared. The easiest way to ensure the redo stack is + /// cleared is to create transactions with the usual `start_transaction` and + /// `end_transaction` methods and merging the resulting transactions into + /// the transaction created by this method + fn push_empty_transaction( + &mut self, + start: clock::Global, + now: Instant, + clock: &mut clock::Lamport, + ) -> TransactionId { + assert_eq!(self.transaction_depth, 0); + let id = clock.tick(); + let transaction = Transaction { + id, + start, + edit_ids: Vec::new(), + }; + self.undo_stack.push(HistoryEntry { + transaction, + first_edit_at: now, + last_edit_at: now, + suppress_grouping: false, + }); + id } fn push_undo(&mut self, op_id: clock::Lamport) { @@ -1495,6 +1527,24 @@ impl Buffer { self.history.push_transaction(transaction, now); } + /// Differs from `push_transaction` in that it does not clear the redo stack. + /// The caller responsible for + /// Differs from `push_transaction` in that it does not clear the redo + /// stack. Intended to be used to create a parent transaction to merge + /// potential child transactions into. + /// + /// The caller is responsible for removing it from the undo history using + /// `forget_transaction` if no edits are merged into it. Otherwise, if edits + /// are merged into this transaction, the caller is responsible for ensuring + /// the redo stack is cleared. The easiest way to ensure the redo stack is + /// cleared is to create transactions with the usual `start_transaction` and + /// `end_transaction` methods and merging the resulting transactions into + /// the transaction created by this method + pub fn push_empty_transaction(&mut self, now: Instant) -> TransactionId { + self.history + .push_empty_transaction(self.version.clone(), now, &mut self.lamport_clock) + } + pub fn edited_ranges_for_transaction_id( &self, transaction_id: TransactionId, diff --git a/crates/theme/src/default_colors.rs b/crates/theme/src/default_colors.rs index 3424e0fe04..1c3f48b548 100644 --- a/crates/theme/src/default_colors.rs +++ b/crates/theme/src/default_colors.rs @@ -83,6 +83,8 @@ impl ThemeColors { panel_indent_guide: neutral().light_alpha().step_5(), panel_indent_guide_hover: neutral().light_alpha().step_6(), panel_indent_guide_active: neutral().light_alpha().step_6(), + panel_overlay_background: neutral().light().step_2(), + panel_overlay_hover: neutral().light_alpha().step_4(), pane_focused_border: blue().light().step_5(), pane_group_border: neutral().light().step_6(), scrollbar_thumb_background: neutral().light_alpha().step_3(), @@ -206,6 +208,8 @@ impl ThemeColors { panel_indent_guide: neutral().dark_alpha().step_4(), panel_indent_guide_hover: neutral().dark_alpha().step_6(), panel_indent_guide_active: neutral().dark_alpha().step_6(), + panel_overlay_background: neutral().dark().step_2(), + panel_overlay_hover: neutral().dark_alpha().step_4(), pane_focused_border: blue().dark().step_5(), pane_group_border: neutral().dark().step_6(), scrollbar_thumb_background: neutral().dark_alpha().step_3(), diff --git a/crates/theme/src/fallback_themes.rs b/crates/theme/src/fallback_themes.rs index 5e9967d460..4d77dd5d81 100644 --- a/crates/theme/src/fallback_themes.rs +++ b/crates/theme/src/fallback_themes.rs @@ -59,6 +59,7 @@ pub(crate) fn zed_default_dark() -> Theme { let bg = hsla(215. / 360., 12. / 100., 15. / 100., 1.); let editor = hsla(220. / 360., 12. / 100., 18. / 100., 1.); let elevated_surface = hsla(225. / 360., 12. / 100., 17. / 100., 1.); + let hover = hsla(225.0 / 360., 11.8 / 100., 26.7 / 100., 1.0); let blue = hsla(207.8 / 360., 81. / 100., 66. / 100., 1.0); let gray = hsla(218.8 / 360., 10. / 100., 40. / 100., 1.0); @@ -108,14 +109,14 @@ pub(crate) fn zed_default_dark() -> Theme { surface_background: bg, background: bg, element_background: hsla(223.0 / 360., 13. / 100., 21. / 100., 1.0), - element_hover: hsla(225.0 / 360., 11.8 / 100., 26.7 / 100., 1.0), + element_hover: hover, element_active: hsla(220.0 / 360., 11.8 / 100., 20.0 / 100., 1.0), element_selected: hsla(224.0 / 360., 11.3 / 100., 26.1 / 100., 1.0), element_disabled: SystemColors::default().transparent, element_selection_background: player.local().selection.alpha(0.25), drop_target_background: hsla(220.0 / 360., 8.3 / 100., 21.4 / 100., 1.0), ghost_element_background: SystemColors::default().transparent, - ghost_element_hover: hsla(225.0 / 360., 11.8 / 100., 26.7 / 100., 1.0), + ghost_element_hover: hover, ghost_element_active: hsla(220.0 / 360., 11.8 / 100., 20.0 / 100., 1.0), ghost_element_selected: hsla(224.0 / 360., 11.3 / 100., 26.1 / 100., 1.0), ghost_element_disabled: SystemColors::default().transparent, @@ -202,10 +203,12 @@ pub(crate) fn zed_default_dark() -> Theme { panel_indent_guide: hsla(228. / 360., 8. / 100., 25. / 100., 1.), panel_indent_guide_hover: hsla(225. / 360., 13. / 100., 12. / 100., 1.), panel_indent_guide_active: hsla(225. / 360., 13. / 100., 12. / 100., 1.), + panel_overlay_background: bg, + panel_overlay_hover: hover, pane_focused_border: blue, pane_group_border: hsla(225. / 360., 13. / 100., 12. / 100., 1.), scrollbar_thumb_background: gpui::transparent_black(), - scrollbar_thumb_hover_background: hsla(225.0 / 360., 11.8 / 100., 26.7 / 100., 1.0), + scrollbar_thumb_hover_background: hover, scrollbar_thumb_active_background: hsla( 225.0 / 360., 11.8 / 100., diff --git a/crates/theme/src/schema.rs b/crates/theme/src/schema.rs index b2a13b54b6..718aad042e 100644 --- a/crates/theme/src/schema.rs +++ b/crates/theme/src/schema.rs @@ -352,6 +352,12 @@ pub struct ThemeColorsContent { #[serde(rename = "panel.indent_guide_active")] pub panel_indent_guide_active: Option, + #[serde(rename = "panel.overlay_background")] + pub panel_overlay_background: Option, + + #[serde(rename = "panel.overlay_hover")] + pub panel_overlay_hover: Option, + #[serde(rename = "pane.focused_border")] pub pane_focused_border: Option, @@ -675,6 +681,14 @@ impl ThemeColorsContent { .scrollbar_thumb_border .as_ref() .and_then(|color| try_parse_color(color).ok()); + let element_hover = self + .element_hover + .as_ref() + .and_then(|color| try_parse_color(color).ok()); + let panel_background = self + .panel_background + .as_ref() + .and_then(|color| try_parse_color(color).ok()); ThemeColorsRefinement { border, border_variant: self @@ -713,10 +727,7 @@ impl ThemeColorsContent { .element_background .as_ref() .and_then(|color| try_parse_color(color).ok()), - element_hover: self - .element_hover - .as_ref() - .and_then(|color| try_parse_color(color).ok()), + element_hover, element_active: self .element_active .as_ref() @@ -833,10 +844,7 @@ impl ThemeColorsContent { .search_match_background .as_ref() .and_then(|color| try_parse_color(color).ok()), - panel_background: self - .panel_background - .as_ref() - .and_then(|color| try_parse_color(color).ok()), + panel_background, panel_focused_border: self .panel_focused_border .as_ref() @@ -853,6 +861,16 @@ impl ThemeColorsContent { .panel_indent_guide_active .as_ref() .and_then(|color| try_parse_color(color).ok()), + panel_overlay_background: self + .panel_overlay_background + .as_ref() + .and_then(|color| try_parse_color(color).ok()) + .or(panel_background), + panel_overlay_hover: self + .panel_overlay_hover + .as_ref() + .and_then(|color| try_parse_color(color).ok()) + .or(element_hover), pane_focused_border: self .pane_focused_border .as_ref() diff --git a/crates/theme/src/styles/colors.rs b/crates/theme/src/styles/colors.rs index 7c5270e361..aab11803f4 100644 --- a/crates/theme/src/styles/colors.rs +++ b/crates/theme/src/styles/colors.rs @@ -131,6 +131,12 @@ pub struct ThemeColors { pub panel_indent_guide: Hsla, pub panel_indent_guide_hover: Hsla, pub panel_indent_guide_active: Hsla, + + /// The color of the overlay surface on top of panel. + pub panel_overlay_background: Hsla, + /// The color of the overlay surface on top of panel when hovered over. + pub panel_overlay_hover: Hsla, + pub pane_focused_border: Hsla, pub pane_group_border: Hsla, /// The color of the scrollbar thumb. @@ -326,6 +332,8 @@ pub enum ThemeColorField { PanelIndentGuide, PanelIndentGuideHover, PanelIndentGuideActive, + PanelOverlayBackground, + PanelOverlayHover, PaneFocusedBorder, PaneGroupBorder, ScrollbarThumbBackground, @@ -438,6 +446,8 @@ impl ThemeColors { ThemeColorField::PanelIndentGuide => self.panel_indent_guide, ThemeColorField::PanelIndentGuideHover => self.panel_indent_guide_hover, ThemeColorField::PanelIndentGuideActive => self.panel_indent_guide_active, + ThemeColorField::PanelOverlayBackground => self.panel_overlay_background, + ThemeColorField::PanelOverlayHover => self.panel_overlay_hover, ThemeColorField::PaneFocusedBorder => self.pane_focused_border, ThemeColorField::PaneGroupBorder => self.pane_group_border, ThemeColorField::ScrollbarThumbBackground => self.scrollbar_thumb_background, diff --git a/crates/title_bar/Cargo.toml b/crates/title_bar/Cargo.toml index 123d0468ac..3c39e6b946 100644 --- a/crates/title_bar/Cargo.toml +++ b/crates/title_bar/Cargo.toml @@ -40,6 +40,7 @@ rpc.workspace = true schemars.workspace = true serde.workspace = true settings.workspace = true +settings_ui.workspace = true smallvec.workspace = true story = { workspace = true, optional = true } telemetry.workspace = true diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index 4b8902d14e..453bb54db8 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -30,6 +30,7 @@ use onboarding_banner::OnboardingBanner; use project::Project; use rpc::proto; use settings::Settings as _; +use settings_ui::keybindings; use std::sync::Arc; use theme::ActiveTheme; use title_bar_settings::TitleBarSettings; @@ -683,7 +684,7 @@ impl TitleBar { ) .separator() .action("Settings", zed_actions::OpenSettings.boxed_clone()) - .action("Key Bindings", Box::new(zed_actions::OpenKeymap)) + .action("Key Bindings", Box::new(keybindings::OpenKeymapEditor)) .action( "Themes…", zed_actions::theme_selector::Toggle::default().boxed_clone(), @@ -727,7 +728,7 @@ impl TitleBar { .menu(|window, cx| { ContextMenu::build(window, cx, |menu, _, _| { menu.action("Settings", zed_actions::OpenSettings.boxed_clone()) - .action("Key Bindings", Box::new(zed_actions::OpenKeymap)) + .action("Key Bindings", Box::new(keybindings::OpenKeymapEditor)) .action( "Themes…", zed_actions::theme_selector::Toggle::default().boxed_clone(), diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 3ba73f6dff..467dd226fb 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -972,12 +972,10 @@ impl ContextMenu { .children(action.as_ref().and_then(|action| { self.action_context .as_ref() - .map(|focus| { + .and_then(|focus| { KeyBinding::for_action_in(&**action, focus, window, cx) }) - .unwrap_or_else(|| { - KeyBinding::for_action(&**action, window, cx) - }) + .or_else(|| KeyBinding::for_action(&**action, window, cx)) .map(|binding| { div().ml_4().child(binding.disabled(*disabled)).when( *disabled && documentation_aside.is_some(), diff --git a/crates/workspace/src/pane_group.rs b/crates/workspace/src/pane_group.rs index 4565cef347..5c87206e9e 100644 --- a/crates/workspace/src/pane_group.rs +++ b/crates/workspace/src/pane_group.rs @@ -943,6 +943,8 @@ mod element { pub struct PaneAxisElement { axis: Axis, basis: usize, + /// Equivalent to ColumnWidths (but in terms of flexes instead of percentages) + /// For example, flexes "1.33, 1, 1", instead of "40%, 30%, 30%" flexes: Arc>>, bounding_boxes: Arc>>>>, children: SmallVec<[AnyElement; 2]>, @@ -998,6 +1000,7 @@ mod element { let mut flexes = flexes.lock(); debug_assert!(flex_values_in_bounds(flexes.as_slice())); + // Math to convert a flex value to a pixel value let size = move |ix, flexes: &[f32]| { container_size.along(axis) * (flexes[ix] / flexes.len() as f32) }; @@ -1007,9 +1010,13 @@ mod element { return; } + // This is basically a "bucket" of pixel changes that need to be applied in response to this + // mouse event. Probably a small, fractional number like 0.5 or 1.5 pixels let mut proposed_current_pixel_change = (e.position - child_start).along(axis) - size(ix, flexes.as_slice()); + // This takes a pixel change, and computes the flex changes that correspond to this pixel change + // as well as the next one, for some reason let flex_changes = |pixel_dx, target_ix, next: isize, flexes: &[f32]| { let flex_change = pixel_dx / container_size.along(axis); let current_target_flex = flexes[target_ix] + flex_change; @@ -1017,6 +1024,9 @@ mod element { (current_target_flex, next_target_flex) }; + // Generate the list of flex successors, from the current index. + // If you're dragging column 3 forward, out of 6 columns, then this code will produce [4, 5, 6] + // If you're dragging column 3 backward, out of 6 columns, then this code will produce [2, 1, 0] let mut successors = iter::from_fn({ let forward = proposed_current_pixel_change > px(0.); let mut ix_offset = 0; @@ -1034,6 +1044,7 @@ mod element { } }); + // Now actually loop over these, and empty our bucket of pixel changes while proposed_current_pixel_change.abs() > px(0.) { let Some(current_ix) = successors.next() else { break; diff --git a/crates/workspace/src/tasks.rs b/crates/workspace/src/tasks.rs index 26edbd8d03..32d066c7eb 100644 --- a/crates/workspace/src/tasks.rs +++ b/crates/workspace/src/tasks.rs @@ -73,7 +73,7 @@ impl Workspace { if let Some(terminal_provider) = self.terminal_provider.as_ref() { let task_status = terminal_provider.spawn(spawn_in_terminal, window, cx); - cx.background_spawn(async move { + let task = cx.background_spawn(async move { match task_status.await { Some(Ok(status)) => { if status.success() { @@ -82,11 +82,11 @@ impl Workspace { log::debug!("Task spawn failed, code: {:?}", status.code()); } } - Some(Err(e)) => log::error!("Task spawn failed: {e}"), + Some(Err(e)) => log::error!("Task spawn failed: {e:#}"), None => log::debug!("Task spawn got cancelled"), } - }) - .detach(); + }); + self.scheduled_tasks.push(task); } } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index be5d693d35..4201058f43 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1088,6 +1088,7 @@ pub struct Workspace { serialized_ssh_project: Option, _items_serializer: Task>, session_id: Option, + scheduled_tasks: Vec>, } impl EventEmitter for Workspace {} @@ -1420,6 +1421,7 @@ impl Workspace { _items_serializer, session_id: Some(session_id), serialized_ssh_project: None, + scheduled_tasks: Vec::new(), } } diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 3af1709b74..d87731910d 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -2,7 +2,7 @@ description = "The fast, collaborative code editor." edition.workspace = true name = "zed" -version = "0.196.0" +version = "0.196.7" publish.workspace = true license = "GPL-3.0-or-later" authors = ["Zed Team "] diff --git a/crates/zed/RELEASE_CHANNEL b/crates/zed/RELEASE_CHANNEL index 38f8e886e1..870bbe4e50 100644 --- a/crates/zed/RELEASE_CHANNEL +++ b/crates/zed/RELEASE_CHANNEL @@ -1 +1 @@ -dev +stable \ No newline at end of file diff --git a/crates/zed/src/zed/app_menus.rs b/crates/zed/src/zed/app_menus.rs index ddab724f4a..c4131dbee9 100644 --- a/crates/zed/src/zed/app_menus.rs +++ b/crates/zed/src/zed/app_menus.rs @@ -1,5 +1,6 @@ use collab_ui::collab_panel; use gpui::{Menu, MenuItem, OsAction}; +use settings_ui::keybindings; use terminal_view::terminal_panel; pub fn app_menus() -> Vec { @@ -16,7 +17,7 @@ pub fn app_menus() -> Vec { name: "Settings".into(), items: vec![ MenuItem::action("Open Settings", super::OpenSettings), - MenuItem::action("Open Key Bindings", zed_actions::OpenKeymap), + MenuItem::action("Open Key Bindings", keybindings::OpenKeymapEditor), MenuItem::action("Open Default Settings", super::OpenDefaultSettings), MenuItem::action( "Open Default Key Bindings", diff --git a/docs/src/linux.md b/docs/src/linux.md index 896bfdaf3f..ca65da2969 100644 --- a/docs/src/linux.md +++ b/docs/src/linux.md @@ -148,7 +148,7 @@ On some systems the file `/etc/prime-discrete` can be used to enforce the use of On others, you may be able to the environment variable `DRI_PRIME=1` when running Zed to force the use of the discrete GPU. -If you're using an AMD GPU and Zed crashes when selecting long lines, try setting the `ZED_SAMPLE_COUNT=0` environment variable. (See [#26143](https://github.com/zed-industries/zed/issues/26143)) +If you're using an AMD GPU and Zed crashes when selecting long lines, try setting the `ZED_PATH_SAMPLE_COUNT=0` environment variable. (See [#26143](https://github.com/zed-industries/zed/issues/26143)) If you're using an AMD GPU, you might get a 'Broken Pipe' error. Try using the RADV or Mesa drivers. (See [#13880](https://github.com/zed-industries/zed/issues/13880))