Skip to content

Commit

Permalink
Handle PageUp and PageDown in the completion and popups
Browse files Browse the repository at this point in the history
This fixes three issues:

1) PageUp and PageDown during completion selection moved the
   viewport without cancelling the completion itself. It also
   left the last completion selected in the text.

   The change adds PageUp and PageDown handling to the existing
   movements (with the Ctrl-u and Ctrl-d aliases) to make
   the behavior of the editor and the menus consistent with
   each other.

2) Completion key actions were processed after the popup key
   actions. This prevented the PageUp / PageDown logic from
   being executed, because the popup swallowed them.

   The change reverses the logic, because the most active
   element should be handling the keys. Both code lens
   (space-k) and code completion now handle the page
   movements properly and consistently.

3) Popup Ctrl-u and Ctrl-d movements did not have the
   PageUp and PageDown aliases defined. This was confusing
   for new users as the editor itself recognizes those.

Signed-off-by: Martin Sivak <[email protected]>
  • Loading branch information
MarSik committed Mar 8, 2025
1 parent fab08c0 commit 06e9b5e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 25 deletions.
27 changes: 27 additions & 0 deletions helix-term/src/ui/menu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,28 @@ impl<T: Item> Menu<T> {
self.adjust_scroll();
}

pub fn move_half_page_up(&mut self) {
let len = self.matches.len();
let max_index = len.saturating_sub((self.size.1 as usize / 2).max(1));
let pos = self.cursor.map_or(max_index, |i| (i + max_index) % len) % len;
self.cursor = Some(pos);
self.adjust_scroll();
}

pub fn move_down(&mut self) {
let len = self.matches.len();
let pos = self.cursor.map_or(0, |i| i + 1) % len;
self.cursor = Some(pos);
self.adjust_scroll();
}

pub fn move_half_page_down(&mut self) {
let len = self.matches.len();
let pos = self.cursor.map_or(0, |i| i + (self.size.1 as usize / 2).max(1)) % len;
self.cursor = Some(pos);
self.adjust_scroll();
}

fn recalculate_size(&mut self, viewport: (u16, u16)) {
let n = self
.options
Expand Down Expand Up @@ -251,6 +266,18 @@ impl<T: Item + 'static> Component for Menu<T> {
(self.callback_fn)(cx.editor, self.selection(), MenuEvent::Update);
return EventResult::Consumed(None);
}
key!(PageUp) | ctrl!('u') => {
// page up moves back in the completion choice (including updating the doc)
self.move_half_page_up();
(self.callback_fn)(cx.editor, self.selection(), MenuEvent::Update);
return EventResult::Consumed(None);
}
key!(PageDown) | ctrl!('d') => {
// page down advances completion choice (including updating the doc)
self.move_half_page_down();
(self.callback_fn)(cx.editor, self.selection(), MenuEvent::Update);
return EventResult::Consumed(None);
}
key!(Enter) => {
if let Some(selection) = self.selection() {
(self.callback_fn)(cx.editor, Some(selection), MenuEvent::Validate);
Expand Down
55 changes: 30 additions & 25 deletions helix-term/src/ui/popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,34 +270,39 @@ impl<T: Component> Component for Popup<T> {
compositor.remove(self.id.as_ref());
});

match key {
// esc or ctrl-c aborts the completion and closes the menu
key!(Esc) | ctrl!('c') => {
let _ = self.contents.handle_event(event, cx);
EventResult::Consumed(Some(close_fn))
}
ctrl!('d') => {
self.scroll_half_page_down();
EventResult::Consumed(None)
}
ctrl!('u') => {
self.scroll_half_page_up();
EventResult::Consumed(None)
}
_ => {
let contents_event_result = self.contents.handle_event(event, cx);

if self.auto_close {
if let EventResult::Ignored(None) = contents_event_result {
return EventResult::Ignored(Some(close_fn));
// Code completion handles arrows and page up/down itself,
// but code lens does not. First check whether content knows
// about the key event. When not, check the default keys.
match self.contents.handle_event(event, cx) {
EventResult::Ignored(fn_once) => {
match key {
// esc or ctrl-c aborts the completion and closes the menu
key!(Esc) | ctrl!('c') => {
let _ = self.contents.handle_event(event, cx);
EventResult::Consumed(Some(close_fn))
}
key!(PageDown) | ctrl!('d') => {
self.scroll_half_page_down();
EventResult::Consumed(None)
}
key!(PageUp) | ctrl!('u') => {
self.scroll_half_page_up();
EventResult::Consumed(None)
}
_ => {
// for some events, we want to process them but send ignore, specifically all input except
// tab/enter/ctrl-k or whatever will confirm the selection/ ctrl-n/ctrl-p for scroll.

if self.auto_close {
EventResult::Ignored(Some(close_fn))
} else {
EventResult::Ignored(fn_once)
}
}
}

contents_event_result
}
},
ev => ev
}
// for some events, we want to process them but send ignore, specifically all input except
// tab/enter/ctrl-k or whatever will confirm the selection/ ctrl-n/ctrl-p for scroll.
}

fn render(&mut self, viewport: Rect, surface: &mut Surface, cx: &mut Context) {
Expand Down

0 comments on commit 06e9b5e

Please sign in to comment.