Clean up language model selector (#26134)

Marshall Bowers created 10 months ago

This PR does some cleanup for the language model selector after
https://github.com/zed-industries/zed/pull/26090.

Release Notes:

- N/A

Change summary

crates/assistant/src/inline_assistant.rs                      | 16 +-
crates/assistant/src/terminal_inline_assistant.rs             | 17 +-
crates/assistant2/src/assistant_model_selector.rs             | 16 +-
crates/assistant_context_editor/src/context_editor.rs         | 18 ++-
crates/language_model_selector/src/language_model_selector.rs | 23 ++--
5 files changed, 48 insertions(+), 42 deletions(-)

Detailed changes

crates/assistant/src/inline_assistant.rs

@@ -1567,7 +1567,6 @@ impl Render for PromptEditor {
                 ]
             }
         });
-        let fs_clone = self.fs.clone();
 
         h_flex()
             .key_context("PromptEditor")
@@ -1590,12 +1589,15 @@ impl Render for PromptEditor {
                     .w(gutter_dimensions.full_width() + (gutter_dimensions.margin / 2.0))
                     .justify_center()
                     .gap_2()
-                    .child(inline_language_model_selector(move |model, cx| {
-                        update_settings_file::<AssistantSettings>(
-                            fs_clone.clone(),
-                            cx,
-                            move |settings, _| settings.set_model(model.clone()),
-                        );
+                    .child(inline_language_model_selector({
+                        let fs = self.fs.clone();
+                        move |model, cx| {
+                            update_settings_file::<AssistantSettings>(
+                                fs.clone(),
+                                cx,
+                                move |settings, _| settings.set_model(model.clone()),
+                            );
+                        }
                     }))
                     .map(|el| {
                         let CodegenStatus::Error(error) = self.codegen.read(cx).status(cx) else {

crates/assistant/src/terminal_inline_assistant.rs

@@ -624,8 +624,6 @@ impl Render for PromptEditor {
             }
         };
 
-        let fs_clone = self.fs.clone();
-
         h_flex()
             .bg(cx.theme().colors().editor_background)
             .border_y_1()
@@ -643,12 +641,15 @@ impl Render for PromptEditor {
                     .w_12()
                     .justify_center()
                     .gap_2()
-                    .child(inline_language_model_selector(move |model, cx| {
-                        update_settings_file::<AssistantSettings>(
-                            fs_clone.clone(),
-                            cx,
-                            move |settings, _| settings.set_model(model.clone()),
-                        );
+                    .child(inline_language_model_selector({
+                        let fs = self.fs.clone();
+                        move |model, cx| {
+                            update_settings_file::<AssistantSettings>(
+                                fs.clone(),
+                                cx,
+                                move |settings, _| settings.set_model(model.clone()),
+                            );
+                        }
                     }))
                     .children(
                         if let CodegenStatus::Error(error) = &self.codegen.read(cx).status {

crates/assistant2/src/assistant_model_selector.rs

@@ -33,17 +33,19 @@ impl AssistantModelSelector {
 
 impl Render for AssistantModelSelector {
     fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
-        let fs_clone = self.fs.clone();
         assistant_language_model_selector(
             self.focus_handle.clone(),
             Some(self.menu_handle.clone()),
             cx,
-            move |model, cx| {
-                update_settings_file::<AssistantSettings>(
-                    fs_clone.clone(),
-                    cx,
-                    move |settings, _| settings.set_model(model.clone()),
-                );
+            {
+                let fs = self.fs.clone();
+                move |model, cx| {
+                    update_settings_file::<AssistantSettings>(
+                        fs.clone(),
+                        cx,
+                        move |settings, _| settings.set_model(model.clone()),
+                    );
+                }
             },
         )
     }

crates/assistant_context_editor/src/context_editor.rs

@@ -2818,7 +2818,6 @@ impl Render for ContextEditor {
         } else {
             None
         };
-        let fs_clone = self.fs.clone();
 
         let language_model_selector = self.language_model_selector.clone();
         v_flex()
@@ -2877,12 +2876,17 @@ impl Render for ContextEditor {
                                     self.editor().focus_handle(cx),
                                     Some(self.language_model_selector.clone()),
                                     cx,
-                                    move |model, cx| {
-                                        update_settings_file::<AssistantSettings>(
-                                            fs_clone.clone(),
-                                            cx,
-                                            move |settings, _| settings.set_model(model.clone()),
-                                        );
+                                    {
+                                        let fs = self.fs.clone();
+                                        move |model, cx| {
+                                            update_settings_file::<AssistantSettings>(
+                                                fs.clone(),
+                                                cx,
+                                                move |settings, _| {
+                                                    settings.set_model(model.clone())
+                                                },
+                                            );
+                                        }
                                     },
                                 ))),
                         )

crates/language_model_selector/src/language_model_selector.rs

@@ -484,17 +484,17 @@ impl PickerDelegate for LanguageModelPickerDelegate {
 }
 
 pub fn inline_language_model_selector(
-    f: impl Fn(Arc<dyn LanguageModel>, &App) + 'static,
-) -> AnyElement {
-    let f = Rc::new(f);
+    on_model_changed: impl Fn(Arc<dyn LanguageModel>, &App) + 'static,
+) -> PopoverMenu<LanguageModelSelector> {
+    let on_model_changed = Rc::new(on_model_changed);
     PopoverMenu::new("popover-button")
         .menu(move |window, cx| {
             Some(cx.new(|cx| {
                 LanguageModelSelector::new(
                     {
-                        let f = f.clone();
+                        let on_model_changed = on_model_changed.clone();
                         move |model, cx| {
-                            f(model, cx);
+                            on_model_changed(model, cx);
                         }
                     },
                     window,
@@ -524,36 +524,34 @@ pub fn inline_language_model_selector(
             },
         )
         .anchor(gpui::Corner::TopRight)
-        // .when_some(menu_handle, |el, handle| el.with_handle(handle))
         .offset(gpui::Point {
             x: px(0.0),
             y: px(-2.0),
         })
-        .into_any_element()
 }
 
 pub fn assistant_language_model_selector(
     keybinding_target: FocusHandle,
     menu_handle: Option<PopoverMenuHandle<LanguageModelSelector>>,
     cx: &App,
-    f: impl Fn(Arc<dyn LanguageModel>, &App) + 'static,
-) -> AnyElement {
+    on_model_changed: impl Fn(Arc<dyn LanguageModel>, &App) + 'static,
+) -> PopoverMenu<LanguageModelSelector> {
     let active_model = LanguageModelRegistry::read_global(cx).active_model();
     let model_name = match active_model {
         Some(model) => model.name().0,
         _ => SharedString::from("No model selected"),
     };
 
-    let f = Rc::new(f);
+    let on_model_changed = Rc::new(on_model_changed);
 
     PopoverMenu::new("popover-button")
         .menu(move |window, cx| {
             Some(cx.new(|cx| {
                 LanguageModelSelector::new(
                     {
-                        let f = f.clone();
+                        let on_model_changed = on_model_changed.clone();
                         move |model, cx| {
-                            f(model, cx);
+                            on_model_changed(model, cx);
                         }
                     },
                     window,
@@ -594,5 +592,4 @@ pub fn assistant_language_model_selector(
             x: px(0.0),
             y: px(-2.0),
         })
-        .into_any_element()
 }