fix modal exchange (#3620)

- Fix opening GoToLine from Command
- Extend test to cover go_to_line behavior too

Release Notes:

- N/A

Cargo.lock

@@ -2046,8 +2046,10 @@ dependencies = [
  "editor2",
  "env_logger 0.9.3",
  "fuzzy2",
+ "go_to_line2",
  "gpui2",
  "language2",
+ "menu2",
  "picker2",
  "project2",
  "serde",

crates/command_palette2/Cargo.toml

@@ -28,6 +28,8 @@ gpui = { package = "gpui2", path = "../gpui2", features = ["test-support"] }
 editor = { package = "editor2", path = "../editor2", features = ["test-support"] }
 language = { package="language2", path = "../language2", features = ["test-support"] }
 project = { package="project2", path = "../project2", features = ["test-support"] }
+menu = { package = "menu2", path = "../menu2" }
+go_to_line = { package = "go_to_line2", path = "../go_to_line2" }
 serde_json.workspace = true
 workspace = { package="workspace2", path = "../workspace2", features = ["test-support"] }
 ctor.workspace = true

crates/command_palette2/src/command_palette.rs

@@ -362,7 +362,9 @@ mod tests {
 
     use super::*;
     use editor::Editor;
+    use go_to_line::GoToLine;
     use gpui::TestAppContext;
+    use language::Point;
     use project::Project;
     use workspace::{AppState, Workspace};
 
@@ -385,7 +387,6 @@ mod tests {
     #[gpui::test]
     async fn test_command_palette(cx: &mut TestAppContext) {
         let app_state = init_test(cx);
-
         let project = Project::test(app_state.fs.clone(), [], cx).await;
         let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project.clone(), cx));
 
@@ -455,12 +456,46 @@ mod tests {
         });
     }
 
+    #[gpui::test]
+    async fn test_go_to_line(cx: &mut TestAppContext) {
+        let app_state = init_test(cx);
+        let project = Project::test(app_state.fs.clone(), [], cx).await;
+        let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project.clone(), cx));
+
+        cx.simulate_keystrokes("cmd-n");
+
+        let editor = workspace.update(cx, |workspace, cx| {
+            workspace.active_item_as::<Editor>(cx).unwrap()
+        });
+        editor.update(cx, |editor, cx| editor.set_text("1\n2\n3\n4\n5\n6\n", cx));
+
+        cx.simulate_keystrokes("cmd-shift-p");
+        cx.simulate_input("go to line: Toggle");
+        cx.simulate_keystrokes("enter");
+
+        workspace.update(cx, |workspace, cx| {
+            assert!(workspace.active_modal::<GoToLine>(cx).is_some())
+        });
+
+        cx.simulate_keystrokes("3 enter");
+
+        editor.update(cx, |editor, cx| {
+            assert!(editor.focus_handle(cx).is_focused(cx));
+            assert_eq!(
+                editor.selections.last::<Point>(cx).range().start,
+                Point::new(2, 0)
+            );
+        });
+    }
+
     fn init_test(cx: &mut TestAppContext) -> Arc<AppState> {
         cx.update(|cx| {
             let app_state = AppState::test(cx);
             theme::init(theme::LoadThemes::JustBase, cx);
             language::init(cx);
             editor::init(cx);
+            menu::init();
+            go_to_line::init(cx);
             workspace::init(app_state.clone(), cx);
             init(cx);
             Project::init_settings(cx);

crates/feedback2/src/feedback_modal.rs

@@ -53,6 +53,7 @@ pub struct FeedbackModal {
     email_address_editor: View<Editor>,
     awaiting_submission: bool,
     user_submitted: bool,
+    discarded: bool,
     character_count: i32,
 }
 
@@ -64,21 +65,35 @@ impl FocusableView for FeedbackModal {
 impl EventEmitter<DismissEvent> for FeedbackModal {}
 
 impl ModalView for FeedbackModal {
-    fn dismiss(&mut self, cx: &mut ViewContext<Self>) -> Task<bool> {
+    fn on_before_dismiss(&mut self, cx: &mut ViewContext<Self>) -> bool {
         if self.user_submitted {
             self.set_user_submitted(false, cx);
-            return cx.spawn(|_, _| async { true });
+            return true;
+        }
+
+        if self.discarded {
+            return true;
         }
 
         let has_feedback = self.feedback_editor.read(cx).text_option(cx).is_some();
-
         if !has_feedback {
-            return cx.spawn(|_, _| async { true });
+            return true;
         }
 
         let answer = cx.prompt(PromptLevel::Info, "Discard feedback?", &["Yes", "No"]);
 
-        cx.spawn(|_, _| async { answer.await.ok() == Some(0) })
+        cx.spawn(move |this, mut cx| async move {
+            if answer.await.ok() == Some(0) {
+                this.update(&mut cx, |this, cx| {
+                    this.discarded = true;
+                    cx.emit(DismissEvent)
+                })
+                .log_err();
+            }
+        })
+        .detach();
+
+        false
     }
 }
 
@@ -169,6 +184,7 @@ impl FeedbackModal {
             email_address_editor,
             awaiting_submission: false,
             user_submitted: false,
+            discarded: false,
             character_count: 0,
         }
     }

crates/go_to_line2/src/go_to_line.rs

@@ -92,7 +92,6 @@ impl GoToLine {
         cx: &mut ViewContext<Self>,
     ) {
         match event {
-            // todo!() this isn't working...
             editor::EditorEvent::Blurred => cx.emit(DismissEvent),
             editor::EditorEvent::BufferEdited { .. } => self.highlight_current_line(cx),
             _ => {}

crates/workspace2/src/modal_layer.rs

@@ -1,23 +1,23 @@
 use gpui::{
-    div, prelude::*, px, AnyView, Div, FocusHandle, ManagedView, Render, Subscription, Task, View,
-    ViewContext, WindowContext,
+    div, prelude::*, px, AnyView, DismissEvent, Div, FocusHandle, ManagedView, Render,
+    Subscription, View, ViewContext, WindowContext,
 };
 use ui::{h_stack, v_stack};
 
 pub trait ModalView: ManagedView {
-    fn dismiss(&mut self, cx: &mut ViewContext<Self>) -> Task<bool> {
-        Task::ready(true)
+    fn on_before_dismiss(&mut self, cx: &mut ViewContext<Self>) -> bool {
+        true
     }
 }
 
 trait ModalViewHandle {
-    fn should_dismiss(&mut self, cx: &mut WindowContext) -> Task<bool>;
+    fn on_before_dismiss(&mut self, cx: &mut WindowContext) -> bool;
     fn view(&self) -> AnyView;
 }
 
 impl<V: ModalView> ModalViewHandle for View<V> {
-    fn should_dismiss(&mut self, cx: &mut WindowContext) -> Task<bool> {
-        self.update(cx, |this, cx| this.dismiss(cx))
+    fn on_before_dismiss(&mut self, cx: &mut WindowContext) -> bool {
+        self.update(cx, |this, cx| this.on_before_dismiss(cx))
     }
 
     fn view(&self) -> AnyView {
@@ -48,8 +48,8 @@ impl ModalLayer {
     {
         if let Some(active_modal) = &self.active_modal {
             let is_close = active_modal.modal.view().downcast::<V>().is_ok();
-            self.hide_modal(cx);
-            if is_close {
+            let did_close = self.hide_modal(cx);
+            if is_close || !did_close {
                 return;
             }
         }
@@ -57,13 +57,15 @@ impl ModalLayer {
         self.show_modal(new_modal, cx);
     }
 
-    pub fn show_modal<V>(&mut self, new_modal: View<V>, cx: &mut ViewContext<Self>)
+    fn show_modal<V>(&mut self, new_modal: View<V>, cx: &mut ViewContext<Self>)
     where
         V: ModalView,
     {
         self.active_modal = Some(ActiveModal {
             modal: Box::new(new_modal.clone()),
-            subscription: cx.subscribe(&new_modal, |this, modal, e, cx| this.hide_modal(cx)),
+            subscription: cx.subscribe(&new_modal, |this, modal, _: &DismissEvent, cx| {
+                this.hide_modal(cx);
+            }),
             previous_focus_handle: cx.focused(),
             focus_handle: cx.focus_handle(),
         });
@@ -71,29 +73,25 @@ impl ModalLayer {
         cx.notify();
     }
 
-    pub fn hide_modal(&mut self, cx: &mut ViewContext<Self>) {
+    fn hide_modal(&mut self, cx: &mut ViewContext<Self>) -> bool {
         let Some(active_modal) = self.active_modal.as_mut() else {
-            return;
+            return false;
         };
 
-        let dismiss = active_modal.modal.should_dismiss(cx);
+        let dismiss = active_modal.modal.on_before_dismiss(cx);
+        if !dismiss {
+            return false;
+        }
 
-        cx.spawn(|this, mut cx| async move {
-            if dismiss.await {
-                this.update(&mut cx, |this, cx| {
-                    if let Some(active_modal) = this.active_modal.take() {
-                        if let Some(previous_focus) = active_modal.previous_focus_handle {
-                            if active_modal.focus_handle.contains_focused(cx) {
-                                previous_focus.focus(cx);
-                            }
-                        }
-                        cx.notify();
-                    }
-                })
-                .ok();
+        if let Some(active_modal) = self.active_modal.take() {
+            if let Some(previous_focus) = active_modal.previous_focus_handle {
+                if active_modal.focus_handle.contains_focused(cx) {
+                    previous_focus.focus(cx);
+                }
             }
-        })
-        .detach();
+            cx.notify();
+        }
+        true
     }
 
     pub fn active_modal<V>(&self) -> Option<View<V>>