Skip to content

Commit

Permalink
Hide signature help if it overlays completion menu (#5523)
Browse files Browse the repository at this point in the history
  • Loading branch information
pascalkuthe authored and archseer committed Mar 31, 2023
1 parent ab819d8 commit 7a69c40
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 24 deletions.
15 changes: 12 additions & 3 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};

Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
},
);
}
Expand Down
17 changes: 16 additions & 1 deletion helix-term/src/commands/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
);
Expand Down
6 changes: 5 additions & 1 deletion helix-term/src/ui/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,10 @@ impl Completion {

true
}

pub fn area(&mut self, viewport: Rect, editor: &Editor) -> Rect {
self.popup.area(viewport, editor)
}
}

impl Component for Completion {
Expand Down Expand Up @@ -481,7 +485,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)
};
Expand Down
26 changes: 18 additions & 8 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ impl EditorView {
start_offset: usize,
trigger_offset: usize,
size: Rect,
) {
) -> Option<Rect> {
let mut completion = Completion::new(
editor,
savepoint,
Expand All @@ -964,15 +964,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) {
Expand Down Expand Up @@ -1256,20 +1258,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,
);
}
}
}
Expand Down
28 changes: 17 additions & 11 deletions helix-term/src/ui/popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7a69c40

Please sign in to comment.