copilot: Decouple copilot sign in from edit prediction settings (#26689)

Closes #25883

This PR allows you to use copilot chat for assistant without setting
copilot as the edit prediction provider.


[copilot.webm](https://github.com/user-attachments/assets/fecfbde1-d72c-4c0c-b080-a07671fb846e)

Todos:
- [x] Remove redudant "copilot" key from settings
- [x] Do not disable copilot LSP when `edit_prediction_provider` is not
set to `copilot`
- [x] Start copilot LSP when:
  - [x]  `edit_prediction_provider` is set to `copilot`
  - [x] Copilot sign in clicked from assistant settings
- [x] Handle flicker for frame after starting LSP, but before signing in
caused due to signed out status
- [x] Fixed this by adding intermediate state for awaiting signing in in
sign out enum
- [x] Handle cancel button should sign out from `copilot` (existing bug)
- [x] Handle modal dismissal should sign out if not in signed in state
(existing bug)

Release Notes:

- You can now sign into Copilot from assistant settings without making
it your edit prediction provider. This is useful if you want to use
Copilot chat while keeping a different provider, like Zed, for
predictions.
- Removed the `copilot` key from `features` in settings. Use
`edit_prediction_provider` instead.
This commit is contained in:
Smit Barmase 2025-03-14 09:40:56 +00:00 committed by GitHub
parent 8d7b021f92
commit 6a95ec6a64
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 114 additions and 61 deletions

View file

@ -170,7 +170,9 @@ enum SignInStatus {
prompt: Option<request::PromptUserDeviceFlow>, prompt: Option<request::PromptUserDeviceFlow>,
task: Shared<Task<Result<(), Arc<anyhow::Error>>>>, task: Shared<Task<Result<(), Arc<anyhow::Error>>>>,
}, },
SignedOut, SignedOut {
awaiting_signing_in: bool,
},
} }
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
@ -180,7 +182,9 @@ pub enum Status {
}, },
Error(Arc<str>), Error(Arc<str>),
Disabled, Disabled,
SignedOut, SignedOut {
awaiting_signing_in: bool,
},
SigningIn { SigningIn {
prompt: Option<request::PromptUserDeviceFlow>, prompt: Option<request::PromptUserDeviceFlow>,
}, },
@ -345,8 +349,8 @@ impl Copilot {
buffers: Default::default(), buffers: Default::default(),
_subscription: cx.on_app_quit(Self::shutdown_language_server), _subscription: cx.on_app_quit(Self::shutdown_language_server),
}; };
this.enable_or_disable_copilot(cx); this.start_copilot(true, false, cx);
cx.observe_global::<SettingsStore>(move |this, cx| this.enable_or_disable_copilot(cx)) cx.observe_global::<SettingsStore>(move |this, cx| this.start_copilot(true, false, cx))
.detach(); .detach();
this this
} }
@ -364,26 +368,40 @@ impl Copilot {
} }
} }
fn enable_or_disable_copilot(&mut self, cx: &mut Context<Self>) { fn start_copilot(
&mut self,
check_edit_prediction_provider: bool,
awaiting_sign_in_after_start: bool,
cx: &mut Context<Self>,
) {
if !matches!(self.server, CopilotServer::Disabled) {
return;
}
let language_settings = all_language_settings(None, cx);
if check_edit_prediction_provider
&& language_settings.edit_predictions.provider != EditPredictionProvider::Copilot
{
return;
}
let server_id = self.server_id; let server_id = self.server_id;
let http = self.http.clone(); let http = self.http.clone();
let node_runtime = self.node_runtime.clone(); let node_runtime = self.node_runtime.clone();
let language_settings = all_language_settings(None, cx); let env = self.build_env(&language_settings.edit_predictions.copilot);
if language_settings.edit_predictions.provider == EditPredictionProvider::Copilot { let start_task = cx
if matches!(self.server, CopilotServer::Disabled) { .spawn(move |this, cx| {
let env = self.build_env(&language_settings.edit_predictions.copilot); Self::start_language_server(
let start_task = cx server_id,
.spawn(move |this, cx| { http,
Self::start_language_server(server_id, http, node_runtime, env, this, cx) node_runtime,
}) env,
.shared(); this,
self.server = CopilotServer::Starting { task: start_task }; awaiting_sign_in_after_start,
cx.notify(); cx,
} )
} else { })
self.server = CopilotServer::Disabled; .shared();
cx.notify(); self.server = CopilotServer::Starting { task: start_task };
} cx.notify();
} }
fn build_env(&self, copilot_settings: &CopilotSettings) -> Option<HashMap<String, String>> { fn build_env(&self, copilot_settings: &CopilotSettings) -> Option<HashMap<String, String>> {
@ -449,6 +467,7 @@ impl Copilot {
node_runtime: NodeRuntime, node_runtime: NodeRuntime,
env: Option<HashMap<String, String>>, env: Option<HashMap<String, String>>,
this: WeakEntity<Self>, this: WeakEntity<Self>,
awaiting_sign_in_after_start: bool,
mut cx: AsyncApp, mut cx: AsyncApp,
) { ) {
let start_language_server = async { let start_language_server = async {
@ -522,7 +541,9 @@ impl Copilot {
Ok((server, status)) => { Ok((server, status)) => {
this.server = CopilotServer::Running(RunningCopilotServer { this.server = CopilotServer::Running(RunningCopilotServer {
lsp: server, lsp: server,
sign_in_status: SignInStatus::SignedOut, sign_in_status: SignInStatus::SignedOut {
awaiting_signing_in: awaiting_sign_in_after_start,
},
registered_buffers: Default::default(), registered_buffers: Default::default(),
}); });
cx.emit(Event::CopilotLanguageServerStarted); cx.emit(Event::CopilotLanguageServerStarted);
@ -545,7 +566,7 @@ impl Copilot {
cx.notify(); cx.notify();
task.clone() task.clone()
} }
SignInStatus::SignedOut | SignInStatus::Unauthorized { .. } => { SignInStatus::SignedOut { .. } | SignInStatus::Unauthorized { .. } => {
let lsp = server.lsp.clone(); let lsp = server.lsp.clone();
let task = cx let task = cx
.spawn(|this, mut cx| async move { .spawn(|this, mut cx| async move {
@ -633,10 +654,6 @@ impl Copilot {
anyhow::Ok(()) anyhow::Ok(())
}) })
} }
CopilotServer::Disabled => cx.background_spawn(async move {
clear_copilot_config_dir().await;
anyhow::Ok(())
}),
_ => Task::ready(Err(anyhow!("copilot hasn't started yet"))), _ => Task::ready(Err(anyhow!("copilot hasn't started yet"))),
} }
} }
@ -651,7 +668,8 @@ impl Copilot {
let server_id = self.server_id; let server_id = self.server_id;
move |this, cx| async move { move |this, cx| async move {
clear_copilot_dir().await; clear_copilot_dir().await;
Self::start_language_server(server_id, http, node_runtime, env, this, cx).await Self::start_language_server(server_id, http, node_runtime, env, this, false, cx)
.await
} }
}) })
.shared(); .shared();
@ -961,7 +979,11 @@ impl Copilot {
SignInStatus::SigningIn { prompt, .. } => Status::SigningIn { SignInStatus::SigningIn { prompt, .. } => Status::SigningIn {
prompt: prompt.clone(), prompt: prompt.clone(),
}, },
SignInStatus::SignedOut => Status::SignedOut, SignInStatus::SignedOut {
awaiting_signing_in,
} => Status::SignedOut {
awaiting_signing_in: *awaiting_signing_in,
},
} }
} }
} }
@ -990,7 +1012,11 @@ impl Copilot {
} }
} }
request::SignInStatus::Ok { user: None } | request::SignInStatus::NotSignedIn => { request::SignInStatus::Ok { user: None } | request::SignInStatus::NotSignedIn => {
server.sign_in_status = SignInStatus::SignedOut; if !matches!(server.sign_in_status, SignInStatus::SignedOut { .. }) {
server.sign_in_status = SignInStatus::SignedOut {
awaiting_signing_in: false,
};
}
cx.emit(Event::CopilotAuthSignedOut); cx.emit(Event::CopilotAuthSignedOut);
for buffer in self.buffers.iter().cloned().collect::<Vec<_>>() { for buffer in self.buffers.iter().cloned().collect::<Vec<_>>() {
self.unregister_buffer(&buffer); self.unregister_buffer(&buffer);
@ -1021,10 +1047,6 @@ async fn clear_copilot_dir() {
remove_matching(paths::copilot_dir(), |_| true).await remove_matching(paths::copilot_dir(), |_| true).await
} }
async fn clear_copilot_config_dir() {
remove_matching(copilot_chat::copilot_chat_config_dir(), |_| true).await
}
async fn get_copilot_lsp(http: Arc<dyn HttpClient>) -> anyhow::Result<PathBuf> { async fn get_copilot_lsp(http: Arc<dyn HttpClient>) -> anyhow::Result<PathBuf> {
const SERVER_PATH: &str = "dist/language-server.js"; const SERVER_PATH: &str = "dist/language-server.js";

View file

@ -1,9 +1,10 @@
use crate::{request::PromptUserDeviceFlow, Copilot, Status}; use crate::{request::PromptUserDeviceFlow, Copilot, Status};
use gpui::{ use gpui::{
div, App, ClipboardItem, Context, DismissEvent, Element, Entity, EventEmitter, FocusHandle, div, percentage, svg, Animation, AnimationExt, App, ClipboardItem, Context, DismissEvent,
Focusable, InteractiveElement, IntoElement, MouseDownEvent, ParentElement, Render, Styled, Element, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement,
Subscription, Window, MouseDownEvent, ParentElement, Render, Styled, Subscription, Transformation, Window,
}; };
use std::time::Duration;
use ui::{prelude::*, Button, Label, Vector, VectorName}; use ui::{prelude::*, Button, Label, Vector, VectorName};
use util::ResultExt as _; use util::ResultExt as _;
use workspace::notifications::NotificationId; use workspace::notifications::NotificationId;
@ -17,11 +18,13 @@ pub fn initiate_sign_in(window: &mut Window, cx: &mut App) {
let Some(copilot) = Copilot::global(cx) else { let Some(copilot) = Copilot::global(cx) else {
return; return;
}; };
let status = copilot.read(cx).status();
let Some(workspace) = window.root::<Workspace>().flatten() else { let Some(workspace) = window.root::<Workspace>().flatten() else {
return; return;
}; };
match status { if matches!(copilot.read(cx).status(), Status::Disabled) {
copilot.update(cx, |this, cx| this.start_copilot(false, true, cx));
}
match copilot.read(cx).status() {
Status::Starting { task } => { Status::Starting { task } => {
workspace.update(cx, |workspace, cx| { workspace.update(cx, |workspace, cx| {
workspace.show_toast( workspace.show_toast(
@ -54,6 +57,15 @@ pub fn initiate_sign_in(window: &mut Window, cx: &mut App) {
copilot copilot
.update(cx, |copilot, cx| copilot.sign_in(cx)) .update(cx, |copilot, cx| copilot.sign_in(cx))
.detach_and_log_err(cx); .detach_and_log_err(cx);
if let Some(window_handle) = cx.active_window() {
window_handle
.update(cx, |_, window, cx| {
workspace.toggle_modal(window, cx, |_, cx| {
CopilotCodeVerification::new(&copilot, cx)
});
})
.log_err();
}
} }
}) })
.log_err(); .log_err();
@ -76,6 +88,7 @@ pub struct CopilotCodeVerification {
status: Status, status: Status,
connect_clicked: bool, connect_clicked: bool,
focus_handle: FocusHandle, focus_handle: FocusHandle,
copilot: Entity<Copilot>,
_subscription: Subscription, _subscription: Subscription,
} }
@ -86,7 +99,20 @@ impl Focusable for CopilotCodeVerification {
} }
impl EventEmitter<DismissEvent> for CopilotCodeVerification {} impl EventEmitter<DismissEvent> for CopilotCodeVerification {}
impl ModalView for CopilotCodeVerification {} impl ModalView for CopilotCodeVerification {
fn on_before_dismiss(
&mut self,
_: &mut Window,
cx: &mut Context<Self>,
) -> workspace::DismissDecision {
self.copilot.update(cx, |copilot, cx| {
if matches!(copilot.status(), Status::SigningIn { .. }) {
copilot.sign_out(cx).detach_and_log_err(cx);
}
});
workspace::DismissDecision::Dismiss(true)
}
}
impl CopilotCodeVerification { impl CopilotCodeVerification {
pub fn new(copilot: &Entity<Copilot>, cx: &mut Context<Self>) -> Self { pub fn new(copilot: &Entity<Copilot>, cx: &mut Context<Self>) -> Self {
@ -95,6 +121,7 @@ impl CopilotCodeVerification {
status, status,
connect_clicked: false, connect_clicked: false,
focus_handle: cx.focus_handle(), focus_handle: cx.focus_handle(),
copilot: copilot.clone(),
_subscription: cx.observe(copilot, |this, copilot, cx| { _subscription: cx.observe(copilot, |this, copilot, cx| {
let status = copilot.read(cx).status(); let status = copilot.read(cx).status();
match status { match status {
@ -180,9 +207,12 @@ impl CopilotCodeVerification {
.child( .child(
Button::new("copilot-enable-cancel-button", "Cancel") Button::new("copilot-enable-cancel-button", "Cancel")
.full_width() .full_width()
.on_click(cx.listener(|_, _, _, cx| cx.emit(DismissEvent))), .on_click(cx.listener(|_, _, _, cx| {
cx.emit(DismissEvent);
})),
) )
} }
fn render_enabled_modal(cx: &mut Context<Self>) -> impl Element { fn render_enabled_modal(cx: &mut Context<Self>) -> impl Element {
v_flex() v_flex()
.gap_2() .gap_2()
@ -216,16 +246,27 @@ impl CopilotCodeVerification {
) )
} }
fn render_disabled_modal() -> impl Element { fn render_loading(window: &mut Window, _: &mut Context<Self>) -> impl Element {
v_flex() let loading_icon = svg()
.child(Headline::new("Copilot is disabled").size(HeadlineSize::Large)) .size_8()
.child(Label::new("You can enable Copilot in your settings.")) .path(IconName::ArrowCircle.path())
.text_color(window.text_style().color)
.with_animation(
"icon_circle_arrow",
Animation::new(Duration::from_secs(2)).repeat(),
|svg, delta| svg.with_transformation(Transformation::rotate(percentage(delta))),
);
h_flex().justify_center().child(loading_icon)
} }
} }
impl Render for CopilotCodeVerification { impl Render for CopilotCodeVerification {
fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement { fn render(&mut self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
let prompt = match &self.status { let prompt = match &self.status {
Status::SigningIn { prompt: None } => {
Self::render_loading(window, cx).into_any_element()
}
Status::SigningIn { Status::SigningIn {
prompt: Some(prompt), prompt: Some(prompt),
} => Self::render_prompting_modal(self.connect_clicked, prompt, cx).into_any_element(), } => Self::render_prompting_modal(self.connect_clicked, prompt, cx).into_any_element(),
@ -237,10 +278,6 @@ impl Render for CopilotCodeVerification {
self.connect_clicked = false; self.connect_clicked = false;
Self::render_enabled_modal(cx).into_any_element() Self::render_enabled_modal(cx).into_any_element()
} }
Status::Disabled => {
self.connect_clicked = false;
Self::render_disabled_modal().into_any_element()
}
_ => div().into_any_element(), _ => div().into_any_element(),
}; };

View file

@ -580,8 +580,6 @@ pub struct CopilotSettingsContent {
#[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, JsonSchema)] #[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")] #[serde(rename_all = "snake_case")]
pub struct FeaturesContent { pub struct FeaturesContent {
/// Whether the GitHub Copilot feature is enabled.
pub copilot: Option<bool>,
/// Determines which edit prediction provider to use. /// Determines which edit prediction provider to use.
pub edit_prediction_provider: Option<EditPredictionProvider>, pub edit_prediction_provider: Option<EditPredictionProvider>,
} }
@ -1158,7 +1156,6 @@ impl settings::Settings for AllLanguageSettings {
languages.insert(language_name.clone(), language_settings); languages.insert(language_name.clone(), language_settings);
} }
let mut copilot_enabled = default_value.features.as_ref().and_then(|f| f.copilot);
let mut edit_prediction_provider = default_value let mut edit_prediction_provider = default_value
.features .features
.as_ref() .as_ref()
@ -1205,9 +1202,6 @@ impl settings::Settings for AllLanguageSettings {
} }
for user_settings in sources.customizations() { for user_settings in sources.customizations() {
if let Some(copilot) = user_settings.features.as_ref().and_then(|f| f.copilot) {
copilot_enabled = Some(copilot);
}
if let Some(provider) = user_settings if let Some(provider) = user_settings
.features .features
.as_ref() .as_ref()
@ -1282,8 +1276,6 @@ impl settings::Settings for AllLanguageSettings {
edit_predictions: EditPredictionSettings { edit_predictions: EditPredictionSettings {
provider: if let Some(provider) = edit_prediction_provider { provider: if let Some(provider) = edit_prediction_provider {
provider provider
} else if copilot_enabled.unwrap_or(true) {
EditPredictionProvider::Copilot
} else { } else {
EditPredictionProvider::None EditPredictionProvider::None
}, },

View file

@ -129,7 +129,7 @@ impl LanguageModelProvider for CopilotChatLanguageModelProvider {
Status::Error(err) => anyhow!(format!("Received the following error while signing into Copilot: {err}")), Status::Error(err) => anyhow!(format!("Received the following error while signing into Copilot: {err}")),
Status::Starting { task: _ } => anyhow!("Copilot is still starting, please wait for Copilot to start then try again"), Status::Starting { task: _ } => anyhow!("Copilot is still starting, please wait for Copilot to start then try again"),
Status::Unauthorized => anyhow!("Unable to authorize with Copilot. Please make sure that you have an active Copilot and Copilot Chat subscription."), Status::Unauthorized => anyhow!("Unable to authorize with Copilot. Please make sure that you have an active Copilot and Copilot Chat subscription."),
Status::SignedOut => anyhow!("You have signed out of Copilot. Please sign in to Copilot and try again."), Status::SignedOut {..} => anyhow!("You have signed out of Copilot. Please sign in to Copilot and try again."),
Status::SigningIn { prompt: _ } => anyhow!("Still signing into Copilot..."), Status::SigningIn { prompt: _ } => anyhow!("Still signing into Copilot..."),
}; };
@ -366,7 +366,6 @@ impl Render for ConfigurationView {
match &self.copilot_status { match &self.copilot_status {
Some(status) => match status { Some(status) => match status {
Status::Disabled => v_flex().gap_6().p_4().child(Label::new(ERROR_LABEL)),
Status::Starting { task: _ } => { Status::Starting { task: _ } => {
const LABEL: &str = "Starting Copilot..."; const LABEL: &str = "Starting Copilot...";
v_flex() v_flex()
@ -376,7 +375,10 @@ impl Render for ConfigurationView {
.child(Label::new(LABEL)) .child(Label::new(LABEL))
.child(loading_icon) .child(loading_icon)
} }
Status::SigningIn { prompt: _ } => { Status::SigningIn { prompt: _ }
| Status::SignedOut {
awaiting_signing_in: true,
} => {
const LABEL: &str = "Signing in to Copilot..."; const LABEL: &str = "Signing in to Copilot...";
v_flex() v_flex()
.gap_6() .gap_6()