Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only render the auto-complete menu if it intersects with signature help #5523

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,10 @@ use crate::{
filter_picker_entry,
job::Callback,
keymap::ReverseKeymap,
ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent},
ui::{
self, lsp::SignatureHelp, overlay::overlayed, FilePicker, Picker, Popup, Prompt,
PromptEvent,
},
};

use crate::job::{self, Jobs};
Expand Down Expand Up @@ -4215,15 +4218,27 @@ pub fn completion(cx: &mut Context) {
return;
}
let size = compositor.size();
let signature_help_area = compositor
.find_id::<Popup<SignatureHelp>>(SignatureHelp::ID)
.map(|signature_help| signature_help.area(size, editor));
let ui = compositor.find::<ui::EditorView>().unwrap();
ui.set_completion(
editor,
items,
offset_encoding,
start_offset,
trigger_offset,
size,
);

// Delete the signature help popup if they intersect.
if ui
.set_completion(
editor,
items,
offset_encoding,
start_offset,
trigger_offset,
size,
)
.zip(signature_help_area)
.filter(|(a, b)| a.intersects(*b))
.is_some()
{
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 @@ -1164,10 +1164,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()
kirawi marked this conversation as resolved.
Show resolved Hide resolved
.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 @@ -359,6 +359,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 @@ -433,7 +437,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
27 changes: 19 additions & 8 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,15 +948,17 @@ impl EditorView {
start_offset: usize,
trigger_offset: usize,
size: Rect,
) {
) -> Option<Rect> {
let mut completion =
Completion::new(editor, items, offset_encoding, start_offset, trigger_offset);

if completion.is_empty() {
// skip if we got no completion results
return;
return None;
}

let area = completion.area(size, editor);

// Immediately initialize a savepoint
doc_mut!(editor).savepoint();

Expand All @@ -966,6 +968,7 @@ impl EditorView {
// 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 @@ -1240,20 +1243,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