From ca9fb2399eb4fec9ae85e0f851bef8c711cac2b7 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Thu, 27 Mar 2025 20:19:37 +0100 Subject: [PATCH] Prevent `toggle_dock` from opening assistant panel when it is disabled via settings (#27215) Part of #27171 Follows-up the change in https://github.com/zed-industries/zed/pull/22346 to consider the case where the assistant-panel is disabled via settings (as also noted in [this comment](https://github.com/zed-industries/zed/pull/22346#issuecomment-2558372412), Notably, only the explicit case is considered here. Can extend this change to also cover the implicit case where the button is disabled if requested.). Currently, if the user toggles the right dock, the assistant panel will be shown even if it is disabled via settings, because it has the highest priority (see https://github.com/zed-industries/zed/pull/22346#issuecomment-2564890493). With this change, the assistant panel is no longer activated when disabled and the dock with the next highest activation order is activated instead. I did not opt in to make the priority configurabe, as I agree with https://github.com/zed-industries/zed/pull/22346#issuecomment-2564890493 that this will most likely rarely be used (the active panel is only none on the first toggle of the dock, afterwards it remains set for the remainder of the session). Release Notes: - `workspace::ToggleRightDock` will no longer open the assistant panel when it is disabled via settings. --- crates/assistant/src/assistant.rs | 6 +++- crates/assistant/src/assistant_panel.rs | 36 +++++++++++------------- crates/assistant/src/inline_assistant.rs | 5 ++-- crates/assistant2/src/assistant_panel.rs | 22 +++++++-------- crates/collab_ui/src/chat_panel.rs | 30 +++++++++++--------- crates/workspace/src/dock.rs | 21 ++++++++++++++ crates/workspace/src/workspace.rs | 10 +++++-- 7 files changed, 81 insertions(+), 49 deletions(-) diff --git a/crates/assistant/src/assistant.rs b/crates/assistant/src/assistant.rs index 4e53f225f38a5fb4328ef3821618cc3114e351a8..a62564c55d7af59cab237da08e9a12ecf4e0c714 100644 --- a/crates/assistant/src/assistant.rs +++ b/crates/assistant/src/assistant.rs @@ -15,7 +15,7 @@ use client::Client; use command_palette_hooks::CommandPaletteFilter; use feature_flags::FeatureFlagAppExt; use fs::Fs; -use gpui::{actions, App, Global, UpdateGlobal}; +use gpui::{actions, App, Global, ReadGlobal, UpdateGlobal}; use language_model::{ LanguageModelId, LanguageModelProviderId, LanguageModelRegistry, LanguageModelResponseMessage, }; @@ -86,6 +86,10 @@ impl Assistant { filter.show_namespace(Self::NAMESPACE); }); } + + pub fn enabled(cx: &App) -> bool { + Self::global(cx).enabled + } } pub fn init( diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index 6621899b7c401a23702b860135bea1c839ee0cb0..520d6f850ab65f19e9983ee4b0311e3d8dca6e78 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -1,4 +1,5 @@ use crate::assistant_configuration::{ConfigurationView, ConfigurationViewEvent}; +use crate::Assistant; use crate::{ terminal_inline_assistant::TerminalInlineAssistant, DeployHistory, InlineAssistant, NewChat, }; @@ -58,8 +59,7 @@ pub fn init(cx: &mut App) { cx.observe_new( |terminal_panel: &mut TerminalPanel, _, cx: &mut Context| { - let settings = AssistantSettings::get_global(cx); - terminal_panel.set_assistant_enabled(settings.enabled, cx); + terminal_panel.set_assistant_enabled(Assistant::enabled(cx), cx); }, ) .detach(); @@ -342,12 +342,12 @@ impl AssistantPanel { window: &mut Window, cx: &mut Context, ) { - let settings = AssistantSettings::get_global(cx); - if !settings.enabled { - return; + if workspace + .panel::(cx) + .is_some_and(|panel| panel.read(cx).enabled(cx)) + { + workspace.toggle_panel_focus::(window, cx); } - - workspace.toggle_panel_focus::(window, cx); } fn watch_client_status( @@ -595,12 +595,10 @@ impl AssistantPanel { window: &mut Window, cx: &mut Context, ) { - let settings = AssistantSettings::get_global(cx); - if !settings.enabled { - return; - } - - let Some(assistant_panel) = workspace.panel::(cx) else { + let Some(assistant_panel) = workspace + .panel::(cx) + .filter(|panel| panel.read(cx).enabled(cx)) + else { return; }; @@ -1298,12 +1296,8 @@ impl Panel for AssistantPanel { } fn icon(&self, _: &Window, cx: &App) -> Option { - let settings = AssistantSettings::get_global(cx); - if !settings.enabled || !settings.button { - return None; - } - - Some(IconName::ZedAssistant) + (self.enabled(cx) && AssistantSettings::get_global(cx).button) + .then_some(IconName::ZedAssistant) } fn icon_tooltip(&self, _: &Window, _: &App) -> Option<&'static str> { @@ -1317,6 +1311,10 @@ impl Panel for AssistantPanel { fn activation_priority(&self) -> u32 { 4 } + + fn enabled(&self, cx: &App) -> bool { + Assistant::enabled(cx) + } } impl EventEmitter for AssistantPanel {} diff --git a/crates/assistant/src/inline_assistant.rs b/crates/assistant/src/inline_assistant.rs index db48b4e3f212c50a3922a985a9330883e3b9e9df..da6fd986cd523056700725540492fec22f8169d3 100644 --- a/crates/assistant/src/inline_assistant.rs +++ b/crates/assistant/src/inline_assistant.rs @@ -1,5 +1,6 @@ use crate::{ - AssistantPanel, AssistantPanelEvent, CycleNextInlineAssist, CyclePreviousInlineAssist, + Assistant, AssistantPanel, AssistantPanelEvent, CycleNextInlineAssist, + CyclePreviousInlineAssist, }; use anyhow::{anyhow, Context as _, Result}; use assistant_context_editor::{humanize_token_count, RequestType}; @@ -3555,7 +3556,7 @@ impl CodeActionProvider for AssistantCodeActionProvider { _: &mut Window, cx: &mut App, ) -> Task>> { - if !AssistantSettings::get_global(cx).enabled { + if !Assistant::enabled(cx) { return Task::ready(Ok(Vec::new())); } diff --git a/crates/assistant2/src/assistant_panel.rs b/crates/assistant2/src/assistant_panel.rs index 3c08bb60b4d5ed260df2a0ad8132c44eb29ad80e..74b5d47cf331026d07f7929b65e5448a3bc879d5 100644 --- a/crates/assistant2/src/assistant_panel.rs +++ b/crates/assistant2/src/assistant_panel.rs @@ -225,12 +225,12 @@ impl AssistantPanel { window: &mut Window, cx: &mut Context, ) { - let settings = AssistantSettings::get_global(cx); - if !settings.enabled { - return; + if workspace + .panel::(cx) + .is_some_and(|panel| panel.read(cx).enabled(cx)) + { + workspace.toggle_panel_focus::(window, cx); } - - workspace.toggle_panel_focus::(window, cx); } pub(crate) fn local_timezone(&self) -> UtcOffset { @@ -637,12 +637,8 @@ impl Panel for AssistantPanel { } fn icon(&self, _window: &Window, cx: &App) -> Option { - let settings = AssistantSettings::get_global(cx); - if !settings.enabled || !settings.button { - return None; - } - - Some(IconName::ZedAssistant) + (self.enabled(cx) && AssistantSettings::get_global(cx).button) + .then_some(IconName::ZedAssistant) } fn icon_tooltip(&self, _window: &Window, _cx: &App) -> Option<&'static str> { @@ -656,6 +652,10 @@ impl Panel for AssistantPanel { fn activation_priority(&self) -> u32 { 3 } + + fn enabled(&self, cx: &App) -> bool { + AssistantSettings::get_global(cx).enabled + } } impl AssistantPanel { diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index c480fe55a6b8766add45f1830810727e07260b36..627fa5524929cb67473ac3dc85c1ad852d086ee5 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -1156,20 +1156,7 @@ impl Panel for ChatPanel { } fn icon(&self, _window: &Window, cx: &App) -> Option { - let show_icon = match ChatPanelSettings::get_global(cx).button { - ChatPanelButton::Never => false, - ChatPanelButton::Always => true, - ChatPanelButton::WhenInCall => { - let is_in_call = ActiveCall::global(cx) - .read(cx) - .room() - .map_or(false, |room| room.read(cx).contains_guests()); - - self.active || is_in_call - } - }; - - show_icon.then(|| ui::IconName::MessageBubbles) + self.enabled(cx).then(|| ui::IconName::MessageBubbles) } fn icon_tooltip(&self, _: &Window, _: &App) -> Option<&'static str> { @@ -1190,6 +1177,21 @@ impl Panel for ChatPanel { fn activation_priority(&self) -> u32 { 7 } + + fn enabled(&self, cx: &App) -> bool { + match ChatPanelSettings::get_global(cx).button { + ChatPanelButton::Never => false, + ChatPanelButton::Always => true, + ChatPanelButton::WhenInCall => { + let is_in_call = ActiveCall::global(cx) + .read(cx) + .room() + .map_or(false, |room| room.read(cx).contains_guests()); + + self.active || is_in_call + } + } + } } impl EventEmitter for ChatPanel {} diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 77d47ec5624a73c2aa996fe7908aef7e963cefc4..a7696777ebacd95a064d73782aeed6f1e0ab7e51 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -1,6 +1,7 @@ use crate::persistence::model::DockData; use crate::{status_bar::StatusItemView, Workspace}; use crate::{DraggedDock, Event, ModalLayer, Pane}; +use anyhow::Context as _; use client::proto; use gpui::{ deferred, div, px, Action, AnyView, App, Axis, Context, Corner, Entity, EntityId, EventEmitter, @@ -53,6 +54,9 @@ pub trait Panel: Focusable + EventEmitter + Render + Sized { None } fn activation_priority(&self) -> u32; + fn enabled(&self, _cx: &App) -> bool { + true + } } pub trait PanelHandle: Send + Sync { @@ -75,6 +79,7 @@ pub trait PanelHandle: Send + Sync { fn panel_focus_handle(&self, cx: &App) -> FocusHandle; fn to_any(&self) -> AnyView; fn activation_priority(&self, cx: &App) -> u32; + fn enabled(&self, cx: &App) -> bool; fn move_to_next_position(&self, window: &mut Window, cx: &mut App) { let current_position = self.position(window, cx); let next_position = [ @@ -171,6 +176,10 @@ where fn activation_priority(&self, cx: &App) -> u32 { self.read(cx).activation_priority() } + + fn enabled(&self, cx: &App) -> bool { + self.read(cx).enabled(cx) + } } impl From<&dyn PanelHandle> for AnyView { @@ -351,6 +360,18 @@ impl Dock { .position(|entry| entry.panel.remote_id() == Some(panel_id)) } + pub fn first_enabled_panel_idx(&mut self, cx: &mut Context) -> anyhow::Result { + self.panel_entries + .iter() + .position(|entry| entry.panel.enabled(cx)) + .with_context(|| { + format!( + "Couldn't find any enabled panel for the {} dock.", + self.position.label() + ) + }) + } + fn active_panel_entry(&self) -> Option<&PanelEntry> { self.active_panel_index .and_then(|index| self.panel_entries.get(index)) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 80c76efed36b53a24dc9d1699a21a00d751a4c4e..b4ef0ea1f11253bcf16ac92f2cebe98b4f26ded1 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2522,8 +2522,14 @@ impl Workspace { let was_visible = dock.is_open() && !other_is_zoomed; dock.set_open(!was_visible, window, cx); - if dock.active_panel().is_none() && dock.panels_len() > 0 { - dock.activate_panel(0, window, cx); + if dock.active_panel().is_none() { + let Some(panel_ix) = dock + .first_enabled_panel_idx(cx) + .log_with_level(log::Level::Info) + else { + return; + }; + dock.activate_panel(panel_ix, window, cx); } if let Some(active_panel) = dock.active_panel() {