Hide signature help if it overlays completion menu (helix-editor#5523)

Author: pascalkuthe

helix-term/src/commands.rs

@@ -54,8 +54,8 @@ use crate::{
     job::Callback,
     keymap::ReverseKeymap,
     ui::{
-        self, editor::InsertEvent, overlay::overlayed, FilePicker, Picker, Popup, Prompt,
-        PromptEvent,
+        self, editor::InsertEvent, lsp::SignatureHelp, overlay::overlayed, FilePicker, Picker,
+        Popup, Prompt, PromptEvent,
     },
 };
 
@@ -4261,7 +4261,7 @@ pub fn completion(cx: &mut Context) {
             }
             let size = compositor.size();
             let ui = compositor.find::<ui::EditorView>().unwrap();
-            ui.set_completion(
+            let completion_area = ui.set_completion(
                 editor,
                 savepoint,
                 items,
@@ -4270,6 +4270,15 @@ pub fn completion(cx: &mut Context) {
                 trigger_offset,
                 size,
             );
+            let size = compositor.size();
+            let signature_help_area = compositor
+                .find_id::<Popup<SignatureHelp>>(SignatureHelp::ID)
+                .map(|signature_help| signature_help.area(size, editor));
+            // Delete the signature help popup if they intersect.
+            if matches!((completion_area, signature_help_area),(Some(a), Some(b)) if a.intersects(b))
+            {
+                compositor.remove(SignatureHelp::ID);
+            }
         },
     );
 }

helix-term/src/commands/lsp.rs

@@ -1221,10 +1221,25 @@ pub fn signature_help_impl(cx: &mut Context, invoked: SignatureHelpInvoked) {
             contents.set_active_param_range(active_param_range());
 
             let old_popup = compositor.find_id::<Popup<SignatureHelp>>(SignatureHelp::ID);
-            let popup = Popup::new(SignatureHelp::ID, contents)
+            let mut popup = Popup::new(SignatureHelp::ID, contents)
                 .position(old_popup.and_then(|p| p.get_position()))
                 .position_bias(Open::Above)
                 .ignore_escape_key(true);
+
+            // Don't create a popup if it intersects the auto-complete menu.
+            let size = compositor.size();
+            if compositor
+                .find::<ui::EditorView>()
+                .unwrap()
+                .completion
+                .as_mut()
+                .map(|completion| completion.area(size, editor))
+                .filter(|area| area.intersects(popup.area(size, editor)))
+                .is_some()
+            {
+                return;
+            }
+
             compositor.replace_or_push(SignatureHelp::ID, popup);
         },
     );

helix-term/src/ui/completion.rs

@@ -416,6 +416,10 @@ impl Completion {
 
         true
     }
+
+    pub fn area(&mut self, viewport: Rect, editor: &Editor) -> Rect {
+        self.popup.area(viewport, editor)
+    }
 }
 
 impl Component for Completion {
@@ -483,7 +487,7 @@ impl Component for Completion {
         };
 
         let popup_area = {
-            let (popup_x, popup_y) = self.popup.get_rel_position(area, cx);
+            let (popup_x, popup_y) = self.popup.get_rel_position(area, cx.editor);
             let (popup_width, popup_height) = self.popup.get_size();
             Rect::new(popup_x, popup_y, popup_width, popup_height)
         };

helix-term/src/ui/editor.rs

@@ -952,7 +952,7 @@ impl EditorView {
         start_offset: usize,
         trigger_offset: usize,
         size: Rect,
-    ) {
+    ) -> Option<Rect> {
         let mut completion = Completion::new(
             editor,
             savepoint,
@@ -965,15 +965,17 @@ impl EditorView {
 
         if completion.is_empty() {
             // skip if we got no completion results
-            return;
+            return None;
         }
 
+        let area = completion.area(size, editor);
         editor.last_completion = None;
         self.last_insert.1.push(InsertEvent::TriggerCompletion);
 
         // TODO : propagate required size on resize to completion too
         completion.required_size((size.width, size.height));
         self.completion = Some(completion);
+        Some(area)
     }
 
     pub fn clear_completion(&mut self, editor: &mut Editor) {
@@ -1257,20 +1259,28 @@ impl Component for EditorView {
                             // let completion swallow the event if necessary
                             let mut consumed = false;
                             if let Some(completion) = &mut self.completion {
-                                // use a fake context here
-                                let mut cx = Context {
-                                    editor: cx.editor,
-                                    jobs: cx.jobs,
-                                    scroll: None,
+                                let res = {
+                                    // use a fake context here
+                                    let mut cx = Context {
+                                        editor: cx.editor,
+                                        jobs: cx.jobs,
+                                        scroll: None,
+                                    };
+                                    completion.handle_event(event, &mut cx)
                                 };
-                                let res = completion.handle_event(event, &mut cx);
 
                                 if let EventResult::Consumed(callback) = res {
                                     consumed = true;
 
                                     if callback.is_some() {
                                         // assume close_fn
                                         self.clear_completion(cx.editor);
+
+                                        // In case the popup was deleted because of an intersection w/ the auto-complete menu.
+                                        commands::signature_help_impl(
+                                            &mut cx,
+                                            commands::SignatureHelpInvoked::Automatic,
+                                        );
                                     }
                                 }
                             }

helix-term/src/ui/popup.rs

@@ -6,7 +6,10 @@ use crate::{
 use tui::buffer::Buffer as Surface;
 
 use helix_core::Position;
-use helix_view::graphics::{Margin, Rect};
+use helix_view::{
+    graphics::{Margin, Rect},
+    Editor,
+};
 
 // TODO: share logic with Menu, it's essentially Popup(render_fn), but render fn needs to return
 // a width/height hint. maybe Popup(Box<Component>)
@@ -88,10 +91,10 @@ impl<T: Component> Popup<T> {
 
     /// Calculate the position where the popup should be rendered and return the coordinates of the
     /// top left corner.
-    pub fn get_rel_position(&mut self, viewport: Rect, cx: &Context) -> (u16, u16) {
+    pub fn get_rel_position(&mut self, viewport: Rect, editor: &Editor) -> (u16, u16) {
         let position = self
             .position
-            .get_or_insert_with(|| cx.editor.cursor().0.unwrap_or_default());
+            .get_or_insert_with(|| editor.cursor().0.unwrap_or_default());
 
         let (width, height) = self.size;
 
@@ -155,6 +158,16 @@ impl<T: Component> Popup<T> {
     pub fn contents_mut(&mut self) -> &mut T {
         &mut self.contents
     }
+
+    pub fn area(&mut self, viewport: Rect, editor: &Editor) -> Rect {
+        // trigger required_size so we recalculate if the child changed
+        self.required_size((viewport.width, viewport.height));
+
+        let (rel_x, rel_y) = self.get_rel_position(viewport, editor);
+
+        // clip to viewport
+        viewport.intersection(Rect::new(rel_x, rel_y, self.size.0, self.size.1))
+    }
 }
 
 impl<T: Component> Component for Popup<T> {
@@ -232,16 +245,9 @@ impl<T: Component> Component for Popup<T> {
     }
 
     fn render(&mut self, viewport: Rect, surface: &mut Surface, cx: &mut Context) {
-        // trigger required_size so we recalculate if the child changed
-        self.required_size((viewport.width, viewport.height));
-
+        let area = self.area(viewport, cx.editor);
         cx.scroll = Some(self.scroll);
 
-        let (rel_x, rel_y) = self.get_rel_position(viewport, cx);
-
-        // clip to viewport
-        let area = viewport.intersection(Rect::new(rel_x, rel_y, self.size.0, self.size.1));
-
         // clear area
         let background = cx.editor.theme.get("ui.popup");
         surface.clear_with(area, background);