Merge pull request #2348 from zed-industries/copilot-feedback

Fix issues from copilot feedback

assets/keymaps/default.json

@@ -177,9 +177,9 @@
                     "focus": false
                 }
             ],
+            "alt-\\": "copilot::NextSuggestion",
             "alt-]": "copilot::NextSuggestion",
-            "alt-[": "copilot::PreviousSuggestion",
-            "alt-\\": "copilot::Toggle"
+            "alt-[": "copilot::PreviousSuggestion"
         }
     },
     {

assets/settings/default.json

@@ -205,9 +205,7 @@
     // Different settings for specific languages.
     "languages": {
         "Plain Text": {
-            "soft_wrap": "preferred_line_length",
-            // Copilot can be a little strange on non-code files
-            "copilot": "off"
+            "soft_wrap": "preferred_line_length"
         },
         "Elixir": {
             "tab_size": 2
@@ -217,9 +215,7 @@
             "hard_tabs": true
         },
         "Markdown": {
-            "soft_wrap": "preferred_line_length",
-            // Copilot can be a little strange on non-code files
-            "copilot": "off"
+            "soft_wrap": "preferred_line_length"
         },
         "JavaScript": {
             "tab_size": 2
@@ -232,9 +228,6 @@
         },
         "YAML": {
             "tab_size": 2
-        },
-        "JSON": {
-            "copilot": "off"
         }
     },
     // LSP Specific settings.

crates/copilot/src/copilot.rs

@@ -32,10 +32,7 @@ const COPILOT_AUTH_NAMESPACE: &'static str = "copilot_auth";
 actions!(copilot_auth, [SignIn, SignOut]);
 
 const COPILOT_NAMESPACE: &'static str = "copilot";
-actions!(
-    copilot,
-    [NextSuggestion, PreviousSuggestion, Toggle, Reinstall]
-);
+actions!(copilot, [NextSuggestion, PreviousSuggestion, Reinstall]);
 
 pub fn init(client: Arc<Client>, node_runtime: Arc<NodeRuntime>, cx: &mut MutableAppContext) {
     let copilot = cx.add_model(|cx| Copilot::start(client.http_client(), node_runtime, cx));

crates/copilot/src/sign_in.rs

@@ -23,28 +23,17 @@ pub fn init(cx: &mut MutableAppContext) {
 
         match &status {
             crate::Status::SigningIn { prompt } => {
-                if let Some(code_verification) = code_verification.as_ref() {
-                    code_verification.update(cx, |code_verification, cx| {
-                        code_verification.set_status(status, cx)
-                    });
-                    cx.activate_window(code_verification.window_id());
+                if let Some(code_verification_handle) = code_verification.as_mut() {
+                    if cx.has_window(code_verification_handle.window_id()) {
+                        code_verification_handle.update(cx, |code_verification_view, cx| {
+                            code_verification_view.set_status(status, cx)
+                        });
+                        cx.activate_window(code_verification_handle.window_id());
+                    } else {
+                        create_copilot_auth_window(cx, &status, &mut code_verification);
+                    }
                 } else if let Some(_prompt) = prompt {
-                    let window_size = cx.global::<Settings>().theme.copilot.modal.dimensions();
-                    let window_options = WindowOptions {
-                        bounds: gpui::WindowBounds::Fixed(RectF::new(
-                            Default::default(),
-                            window_size,
-                        )),
-                        titlebar: None,
-                        center: true,
-                        focus: true,
-                        kind: WindowKind::Normal,
-                        is_movable: true,
-                        screen: None,
-                    };
-                    let (_, view) =
-                        cx.add_window(window_options, |_cx| CopilotCodeVerification::new(status));
-                    code_verification = Some(view);
+                    create_copilot_auth_window(cx, &status, &mut code_verification);
                 }
             }
             Status::Authorized | Status::Unauthorized => {
@@ -67,6 +56,27 @@ pub fn init(cx: &mut MutableAppContext) {
     .detach();
 }
 
+fn create_copilot_auth_window(
+    cx: &mut MutableAppContext,
+    status: &Status,
+    code_verification: &mut Option<ViewHandle<CopilotCodeVerification>>,
+) {
+    let window_size = cx.global::<Settings>().theme.copilot.modal.dimensions();
+    let window_options = WindowOptions {
+        bounds: gpui::WindowBounds::Fixed(RectF::new(Default::default(), window_size)),
+        titlebar: None,
+        center: true,
+        focus: true,
+        kind: WindowKind::Normal,
+        is_movable: true,
+        screen: None,
+    };
+    let (_, view) = cx.add_window(window_options, |_cx| {
+        CopilotCodeVerification::new(status.clone())
+    });
+    *code_verification = Some(view);
+}
+
 pub struct CopilotCodeVerification {
     status: Status,
 }

crates/copilot_button/src/copilot_button.rs

@@ -249,19 +249,6 @@ impl CopilotButton {
 
         let mut menu_options = Vec::with_capacity(6);
 
-        if let Some((_, view_id)) = self.editor_subscription.as_ref() {
-            let locally_enabled = self.editor_enabled.unwrap_or(settings.copilot_on(None));
-            menu_options.push(ContextMenuItem::item_for_view(
-                if locally_enabled {
-                    "Pause Copilot for this file"
-                } else {
-                    "Resume Copilot for this file"
-                },
-                *view_id,
-                copilot::Toggle,
-            ));
-        }
-
         if let Some(language) = &self.language {
             let language_enabled = settings.copilot_on(Some(language.as_ref()));
 
@@ -334,11 +321,7 @@ impl CopilotButton {
 
         self.language = language_name.clone();
 
-        if let Some(enabled) = editor.copilot_state.user_enabled {
-            self.editor_enabled = Some(enabled);
-        } else {
-            self.editor_enabled = Some(settings.copilot_on(language_name.as_deref()));
-        }
+        self.editor_enabled = Some(settings.copilot_on(language_name.as_deref()));
 
         cx.notify()
     }

crates/editor/src/editor.rs

@@ -391,7 +391,6 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_async_action(Editor::find_all_references);
     cx.add_action(Editor::next_copilot_suggestion);
     cx.add_action(Editor::previous_copilot_suggestion);
-    cx.add_action(Editor::toggle_copilot_suggestions);
 
     hover_popover::init(cx);
     link_go_to_definition::init(cx);
@@ -1013,7 +1012,6 @@ pub struct CopilotState {
     pending_refresh: Task<Option<()>>,
     completions: Vec<copilot::Completion>,
     active_completion_index: usize,
-    pub user_enabled: Option<bool>,
 }
 
 impl Default for CopilotState {
@@ -1023,7 +1021,6 @@ impl Default for CopilotState {
             pending_refresh: Task::ready(Some(())),
             completions: Default::default(),
             active_completion_index: 0,
-            user_enabled: None,
         }
     }
 }
@@ -2780,36 +2777,25 @@ impl Editor {
             return None;
         }
 
-        let settings = cx.global::<Settings>();
-
-        if !self
-            .copilot_state
-            .user_enabled
-            .unwrap_or_else(|| settings.copilot_on(None))
-        {
-            return None;
-        }
-
-        let snapshot = self.buffer.read(cx).snapshot(cx);
         let selection = self.selections.newest_anchor();
-
-        if !self.copilot_state.user_enabled.is_some() {
-            let language_name = snapshot
-                .language_at(selection.start)
-                .map(|language| language.name());
-
-            let copilot_enabled = settings.copilot_on(language_name.as_deref());
-
-            if !copilot_enabled {
-                return None;
-            }
-        }
+        let snapshot = self.buffer.read(cx).snapshot(cx);
 
         let cursor = if selection.start == selection.end {
             selection.start.bias_left(&snapshot)
         } else {
             return None;
         };
+
+        let language_name = snapshot
+            .language_at(selection.start)
+            .map(|language| language.name());
+
+        let copilot_enabled = cx.global::<Settings>().copilot_on(language_name.as_deref());
+
+        if !copilot_enabled {
+            return None;
+        }
+
         self.refresh_active_copilot_suggestion(cx);
 
         if !copilot.read(cx).status().is_authorized() {
@@ -2849,12 +2835,6 @@ impl Editor {
     }
 
     fn next_copilot_suggestion(&mut self, _: &copilot::NextSuggestion, cx: &mut ViewContext<Self>) {
-        // Auto re-enable copilot if you're asking for a suggestion
-        if self.copilot_state.user_enabled == Some(false) {
-            cx.notify();
-            self.copilot_state.user_enabled = Some(true);
-        }
-
         if self.copilot_state.completions.is_empty() {
             self.refresh_copilot_suggestions(cx);
             return;
@@ -2871,12 +2851,6 @@ impl Editor {
         _: &copilot::PreviousSuggestion,
         cx: &mut ViewContext<Self>,
     ) {
-        // Auto re-enable copilot if you're asking for a suggestion
-        if self.copilot_state.user_enabled == Some(false) {
-            cx.notify();
-            self.copilot_state.user_enabled = Some(true);
-        }
-
         if self.copilot_state.completions.is_empty() {
             self.refresh_copilot_suggestions(cx);
             return;
@@ -2892,33 +2866,6 @@ impl Editor {
         self.refresh_active_copilot_suggestion(cx);
     }
 
-    fn toggle_copilot_suggestions(&mut self, _: &copilot::Toggle, cx: &mut ViewContext<Self>) {
-        self.copilot_state.user_enabled = match self.copilot_state.user_enabled {
-            Some(enabled) => Some(!enabled),
-            None => {
-                let selection = self.selections.newest_anchor().start;
-
-                let language_name = self
-                    .snapshot(cx)
-                    .language_at(selection)
-                    .map(|language| language.name());
-
-                let copilot_enabled = cx.global::<Settings>().copilot_on(language_name.as_deref());
-
-                Some(!copilot_enabled)
-            }
-        };
-
-        // We know this can't be None, as we just set it to Some above
-        if self.copilot_state.user_enabled == Some(true) {
-            self.refresh_copilot_suggestions(cx);
-        } else {
-            self.clear_copilot_suggestions(cx);
-        }
-
-        cx.notify();
-    }
-
     fn refresh_active_copilot_suggestion(&mut self, cx: &mut ViewContext<Self>) {
         let snapshot = self.buffer.read(cx).snapshot(cx);
         let cursor = self.selections.newest_anchor().head();