Auto close signature help

helix-term/src/commands.rs

@@ -4010,26 +4010,15 @@ pub mod insert {
         {
             // TODO: what if trigger is multiple chars long
             let is_trigger = triggers.iter().any(|trigger| trigger.contains(ch));
+            // lsp doesn't tell us when to close the signature help, so we request
+            // the help information again after common close triggers which should
+            // return None, which in turn closes the popup.
+            let close_triggers = &[')', ';'];
 
-            if is_trigger {
+            if is_trigger || close_triggers.contains(&ch) {
                 super::signature_help(cx);
             }
         }
-
-        // SignatureHelp {
-        // signatures: [
-        //  SignatureInformation {
-        //      label: "fn open(&mut self, path: PathBuf, action: Action) -> Result<DocumentId, Error>",
-        //      documentation: None,
-        //      parameters: Some(
-        //          [ParameterInformation { label: Simple("path: PathBuf"), documentation: None },
-        //          ParameterInformation { label: Simple("action: Action"), documentation: None }]
-        //      ),
-        //      active_parameter: Some(0)
-        //  }
-        // ],
-        // active_signature: None, active_parameter: Some(0)
-        // }
     }
 
     // The default insert hook: simply insert the character

helix-term/src/commands/lsp.rs

@@ -582,7 +582,10 @@ pub fn signature_help(cx: &mut Context) {
         move |editor, compositor, response: Option<lsp::SignatureHelp>| {
             let response = match response {
                 Some(s) => s,
-                None => return,
+                None => {
+                    compositor.remove(SignatureHelp::ID);
+                    return;
+                }
             };
             let doc = doc!(editor);
             let language = doc
@@ -614,7 +617,7 @@ pub fn signature_help(cx: &mut Context) {
                 match &param.label {
                     lsp::ParameterLabel::Simple(string) => {
                         let start = signature.label.find(string.as_str())?;
-                        Some((start, string.len()))
+                        Some((start, start + string.len()))
                     }
                     lsp::ParameterLabel::LabelOffsets([start, end]) => {
                         Some((*start as usize, *end as usize))
@@ -623,10 +626,10 @@ pub fn signature_help(cx: &mut Context) {
             };
             contents.set_active_param_range(active_param_range());
 
-            let mut popup = Popup::new("signature-help", contents);
-            let old_popup = compositor.find_id::<Popup<SignatureHelp>>("signature-help");
+            let mut popup = Popup::new(SignatureHelp::ID, contents);
+            let old_popup = compositor.find_id::<Popup<SignatureHelp>>(SignatureHelp::ID);
             popup.set_position(old_popup.and_then(|p| p.position().copied()));
-            compositor.replace_or_push("signature-help", popup);
+            compositor.replace_or_push(SignatureHelp::ID, popup);
         },
     );
 }

helix-term/src/compositor.rs

@@ -140,6 +140,14 @@ impl Compositor {
         self.layers.pop()
     }
 
+    pub fn remove(&mut self, id: &'static str) -> Option<Box<dyn Component>> {
+        let idx = self
+            .layers
+            .iter()
+            .position(|layer| layer.id() == Some(id))?;
+        Some(self.layers.remove(idx))
+    }
+
     pub fn handle_event(&mut self, event: Event, cx: &mut Context) -> bool {
         // If it is a key event and a macro is being recorded, push the key event to the recording.
         if let (Event::Key(key), Some((_, keys))) = (event, &mut cx.editor.macro_recording) {

helix-term/src/ui/completion.rs

@@ -77,6 +77,8 @@ pub struct Completion {
 }
 
 impl Completion {
+    pub const ID: &'static str = "completion";
+
     pub fn new(
         editor: &Editor,
         items: Vec<CompletionItem>,
@@ -179,7 +181,7 @@ impl Completion {
                 }
             };
         });
-        let popup = Popup::new("completion", menu);
+        let popup = Popup::new(Self::ID, menu);
         let mut completion = Self {
             popup,
             start_offset,

helix-term/src/ui/editor.rs

@@ -1,7 +1,7 @@
 use crate::{
     commands,
     compositor::{Component, Context, EventResult},
-    key,
+    job, key,
     keymap::{KeymapResult, KeymapResultKind, Keymaps},
     ui::{Completion, ProgressSpinners},
 };
@@ -30,6 +30,8 @@ use std::borrow::Cow;
 use crossterm::event::{Event, MouseButton, MouseEvent, MouseEventKind};
 use tui::buffer::Buffer as Surface;
 
+use super::lsp::SignatureHelp;
+
 pub struct EditorView {
     pub keymaps: Keymaps,
     on_next_key: Option<Box<dyn FnOnce(&mut commands::Context, KeyEvent)>>,
@@ -1147,6 +1149,13 @@ impl Component for EditorView {
                     (Mode::Insert, Mode::Normal) => {
                         // if exiting insert mode, remove completion
                         self.completion = None;
+                        // TODO: Use an on_mode_change hook to remove signature help
+                        context.jobs.callback(async {
+                            let call: job::Callback = Box::new(|_editor, compositor| {
+                                compositor.remove(SignatureHelp::ID);
+                            });
+                            Ok(call)
+                        });
                     }
                     _ => (),
                 }

helix-term/src/ui/lsp.rs

@@ -20,6 +20,8 @@ pub struct SignatureHelp {
 }
 
 impl SignatureHelp {
+    pub const ID: &'static str = "signature-help";
+
     pub fn new(signature: String, language: String, config_loader: Arc<syntax::Loader>) -> Self {
         Self {
             signature,
@@ -73,7 +75,7 @@ impl Component for SignatureHelp {
         let sep_style = Style::default();
         let borders = BorderType::line_symbols(BorderType::Plain);
         for x in sig_text_area.left()..sig_text_area.right() {
-            if let Some(cell) = surface.get_mut(x, sig_text_area.y + 1) {
+            if let Some(cell) = surface.get_mut(x, sig_text_area.bottom()) {
                 cell.set_symbol(borders.horizontal).set_style(sep_style);
             }
         }

helix-term/src/ui/popup.rs

@@ -123,7 +123,7 @@ impl<T: Component> Component for Popup<T> {
 
         let close_fn: Callback = Box::new(|compositor, _| {
             // remove the layer
-            compositor.pop();
+            compositor.remove(self.id.as_ref());
         });
 
         match key.into() {