From 2551ec9948e09bc9db9b2f47650c178166cab3b0 Mon Sep 17 00:00:00 2001
From: Michael Sloan <michael@zed.dev>
Date: Tue, 4 Feb 2025 19:32:53 -0700
Subject: [PATCH] Update `editor::Cancel` to require fewer uses to dismiss
 things

Before this change one thing would be dismissed at a time, in a somwhat arbitrary order.  Now there are two groups - it prioritizes dismissing everything that typically appears without a specific action.  If there are none of those, then it dismisses everything that is typically requested via a specific action.

This also dismisses everything except expanded diff hunks and pending selections when switching to vim normal mode. For example, before this change if the LSP completion is open then the edit prediction would not get dismissed.
---
 crates/editor/src/editor.rs | 82 ++++++++++++++++++-------------------
 crates/vim/src/insert.rs    |  8 +++-
 2 files changed, 46 insertions(+), 44 deletions(-)

diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs
index 05337a0f5a..ee6563bbf2 100644
--- a/crates/editor/src/editor.rs
+++ b/crates/editor/src/editor.rs
@@ -996,6 +996,12 @@ pub enum GotoDefinitionKind {
     Implementation,
 }
 
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum DismissMenusAndPopupsReason {
+    VimModeChange,
+    Cancel,
+}
+
 #[derive(Debug, Clone)]
 enum InlayHintRefreshReason {
     Toggle(bool),
@@ -2569,63 +2575,55 @@ impl Editor {
     pub fn cancel(&mut self, _: &Cancel, window: &mut Window, cx: &mut Context<Self>) {
         self.selection_mark_mode = false;
 
-        if self.clear_expanded_diff_hunks(cx) {
-            cx.notify();
-            return;
+        let dismissed =
+            self.dismiss_menus_and_popups(DismissMenusAndPopupsReason::Cancel, window, cx);
+        if !dismissed {
+            cx.propagate();
         }
-        if self.dismiss_menus_and_popups(true, window, cx) {
-            return;
-        }
-
-        if self.mode == EditorMode::Full
-            && self.change_selections(Some(Autoscroll::fit()), window, cx, |s| s.try_cancel())
-        {
-            return;
-        }
-
-        cx.propagate();
     }
 
     pub fn dismiss_menus_and_popups(
         &mut self,
-        should_report_inline_completion_event: bool,
+        reason: DismissMenusAndPopupsReason,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> bool {
-        if self.take_rename(false, window, cx).is_some() {
-            return true;
-        }
-
-        if hide_hover(self, cx) {
-            return true;
-        }
-
-        if self.hide_signature_help(cx, SignatureHelpHiddenBy::Escape) {
-            return true;
-        }
-
-        if self.hide_context_menu(window, cx).is_some() {
-            return true;
-        }
-
-        if self.mouse_context_menu.take().is_some() {
-            return true;
-        }
-
-        if self.discard_inline_completion(should_report_inline_completion_event, cx) {
-            return true;
-        }
-
-        if self.snippet_stack.pop().is_some() {
+        use DismissMenusAndPopupsReason::*;
+
+        let mut dismissed = false;
+
+        // First cancel hides anything that opens without a specific action.
+        dismissed = dismissed || hide_hover(self, cx);
+        dismissed = dismissed || self.hide_signature_help(cx, SignatureHelpHiddenBy::Escape);
+        dismissed = dismissed || self.discard_inline_completion(reason == Cancel, cx);
+        if dismissed && reason == Cancel {
             return true;
         }
 
+        // Otherwise, hide anything that opens via specific action.
+        dismissed = dismissed || self.take_rename(false, window, cx).is_some();
+        dismissed = dismissed || self.hide_context_menu(window, cx).is_some();
+        dismissed = dismissed || self.mouse_context_menu.take().is_some();
+        dismissed = dismissed || self.snippet_stack.pop().is_some();
+        // todo! Should this really be dismissed on entering normal mode?
         if self.mode == EditorMode::Full && self.active_diagnostics.is_some() {
             self.dismiss_diagnostics(cx);
-            return true;
+            dismissed = true;
+        }
+        if reason != VimModeChange && self.clear_expanded_diff_hunks(cx) {
+            if !dismissed {
+                cx.notify()
+            }
+            dismissed = true;
         }
 
-        false
+        // todo! Should this be grouped with the "specific action" group?
+        if reason != VimModeChange && self.mode == EditorMode::Full {
+            dismissed = dismissed
+                || self.change_selections(Some(Autoscroll::fit()), window, cx, |s| s.try_cancel());
+        }
+
+        dismissed
     }
 
     fn linked_editing_ranges_for(
diff --git a/crates/vim/src/insert.rs b/crates/vim/src/insert.rs
index 9f4f9e9944..9de54eef83 100644
--- a/crates/vim/src/insert.rs
+++ b/crates/vim/src/insert.rs
@@ -1,5 +1,5 @@
 use crate::{state::Mode, Vim};
-use editor::{scroll::Autoscroll, Bias, Editor};
+use editor::{scroll::Autoscroll, Bias, DismissMenusAndPopupsReason, Editor};
 use gpui::{actions, Action, Context, Window};
 use language::SelectionGoal;
 
@@ -27,7 +27,11 @@ impl Vim {
         if count <= 1 || Vim::globals(cx).dot_replaying {
             self.create_mark("^".into(), false, window, cx);
             self.update_editor(window, cx, |_, editor, window, cx| {
-                editor.dismiss_menus_and_popups(false, window, cx);
+                editor.dismiss_menus_and_popups(
+                    DismissMenusAndPopupsReason::VimModeChange,
+                    window,
+                    cx,
+                );
                 editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
                     s.move_cursors_with(|map, mut cursor, _| {
                         *cursor.column_mut() = cursor.column().saturating_sub(1);