helix

diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs
index d62b0a4e..eda94c44 100644
--- a/helix-term/src/commands/dap.rs
+++ b/helix-term/src/commands/dap.rs

@@ -8,7 +8,7 @@ use dap::{StackFrame, Thread, ThreadStates};

use helix_core::syntax::{DebugArgumentValue, DebugConfigCompletion, DebugTemplate};

use helix_dap::{self as dap, Client};

use helix_lsp::block_on;

-use helix_view::{editor::Breakpoint, graphics::Margin};

+use helix_view::editor::Breakpoint;

use serde_json::{to_value, Value};

use tokio_stream::wrappers::UnboundedReceiverStream;

@@ -581,12 +581,7 @@ pub fn dap_variables(cx: &mut Context) {

}

let contents = Text::from(tui::text::Text::from(variables));

- let margin = if cx.editor.popup_border() {

- Margin::all(1)

- } else {

- Margin::none()

- };

- let popup = Popup::new("dap-variables", contents).margin(margin);

+ let popup = Popup::new("dap-variables", contents);

cx.replace_or_push_layer("dap-variables", popup);

}

diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index 88bd0791..e7ba9f0f 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs

@@ -21,7 +21,6 @@ use helix_stdx::path;

use helix_view::{

document::{DocumentInlayHints, DocumentInlayHintsId},

editor::Action,

- graphics::Margin,

handlers::lsp::SignatureHelpInvoked,

theme::Style,

Document, View,

@@ -733,15 +732,7 @@ pub fn code_action(cx: &mut Context) {

});

picker.move_down(); // pre-select the first item

- let margin = if editor.menu_border() {

- Margin::vertical(1)

- } else {

- Margin::none()

- };

- let popup = Popup::new("code-action", picker)

- .with_scrollbar(false)

- .margin(margin);

+ let popup = Popup::new("code-action", picker).with_scrollbar(false);

compositor.replace_or_push("code-action", popup);

};

diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs
index 7a08c90c..f793dd48 100644
--- a/helix-term/src/ui/completion.rs
+++ b/helix-term/src/ui/completion.rs

@@ -5,7 +5,6 @@ use crate::{

use helix_view::{

document::SavePoint,

editor::CompleteAction,

- graphics::Margin,

handlers::lsp::SignatureHelpInvoked,

theme::{Modifier, Style},

ViewId,

@@ -334,16 +333,9 @@ impl Completion {

}

});

- let margin = if editor.menu_border() {

- Margin::vertical(1)

- } else {

- Margin::none()

- };

let popup = Popup::new(Self::ID, menu)

.with_scrollbar(false)

- .ignore_escape_key(true)

- .margin(margin);

+ .ignore_escape_key(true);

let (view, doc) = current_ref!(editor);

let text = doc.text().slice(..);

diff --git a/helix-term/src/ui/menu.rs b/helix-term/src/ui/menu.rs
index f9f038e7..c5006f95 100644
--- a/helix-term/src/ui/menu.rs
+++ b/helix-term/src/ui/menu.rs

@@ -7,18 +7,11 @@ use crate::{

use helix_core::fuzzy::MATCHER;

use nucleo::pattern::{Atom, AtomKind, CaseMatching};

use nucleo::{Config, Utf32Str};

-use tui::{

- buffer::Buffer as Surface,

- widgets::{Block, Borders, Table, Widget},

-};

+use tui::{buffer::Buffer as Surface, widgets::Table};

pub use tui::widgets::{Cell, Row};

-use helix_view::{

- editor::SmartTabConfig,

- graphics::{Margin, Rect},

- Editor,

-};

+use helix_view::{editor::SmartTabConfig, graphics::Rect, Editor};

use tui::layout::Constraint;

pub trait Item: Sync + Send + 'static {

@@ -341,16 +334,8 @@ impl<T: Item + 'static> Component for Menu<T> {

.try_get("ui.menu")

.unwrap_or_else(|| theme.get("ui.text"));

let selected = theme.get("ui.menu.selected");

- surface.clear_with(area, style);

- let render_borders = cx.editor.menu_border();

- let area = if render_borders {

- Widget::render(Block::default().borders(Borders::ALL), area, surface);

- area.inner(&Margin::vertical(1))

- } else {

- area

- };

+ surface.clear_with(area, style);

let scroll = self.scroll;

diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs
index 8d436fda..7ee65db5 100644
--- a/helix-term/src/ui/popup.rs
+++ b/helix-term/src/ui/popup.rs

@@ -15,7 +15,16 @@ use helix_view::{

Editor,

};

-const MIN_HEIGHT: u16 = 4;

+const MIN_HEIGHT: u16 = 6;

+const MAX_HEIGHT: u16 = 26;

+const MAX_WIDTH: u16 = 120;

+struct RenderInfo {

+ area: Rect,

+ child_height: u16,

+ render_borders: bool,

+ is_menu: bool,

// 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>)

@@ -23,7 +32,6 @@ const MIN_HEIGHT: u16 = 4;

pub struct Popup<T: Component> {

contents: T,

position: Option<Position>,

- margin: Margin,

area: Rect,

position_bias: Open,

scroll_half_pages: usize,

@@ -38,7 +46,6 @@ impl<T: Component> Popup<T> {

Self {

contents,

position: None,

- margin: Margin::none(),

position_bias: Open::Below,

area: Rect::new(0, 0, 0, 0),

scroll_half_pages: 0,

@@ -71,11 +78,6 @@ impl<T: Component> Popup<T> {

self

}

- pub fn margin(mut self, margin: Margin) -> Self {

- self.margin = margin;

- self

- }

pub fn auto_close(mut self, auto_close: bool) -> Self {

self.auto_close = auto_close;

self

@@ -118,38 +120,32 @@ impl<T: Component> Popup<T> {

}

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

- let child_size = self

- .contents

- .required_size((viewport.width, viewport.height))

- .expect("Component needs required_size implemented in order to be embedded in a popup");

- self.area_internal(viewport, editor, child_size)

+ self.render_info(viewport, editor).area

}

- pub fn area_internal(

- &mut self,

- viewport: Rect,

- editor: &Editor,

- child_size: (u16, u16),

- ) -> Rect {

- let width = child_size.0.min(viewport.width);

- let height = child_size.1.min(viewport.height.saturating_sub(2)); // add some spacing in the viewport

+ fn render_info(&mut self, viewport: Rect, editor: &Editor) -> RenderInfo {

let position = self

.position

.get_or_insert_with(|| editor.cursor().0.unwrap_or_default());

- // if there's a orientation preference, use that

- // if we're on the top part of the screen, do below

- // if we're on the bottom part, do above

+ let is_menu = self

+ .contents

+ .type_name()

+ .starts_with("helix_term::ui::menu::Menu");

+ let mut render_borders = if is_menu {

+ editor.menu_border()

+ } else {

+ editor.popup_border()

+ };

// -- make sure frame doesn't stick out of bounds

let mut rel_x = position.col as u16;

let mut rel_y = position.row as u16;

- if viewport.width <= rel_x + width {

- rel_x = rel_x.saturating_sub((rel_x + width).saturating_sub(viewport.width));

- }

+ // if there's a orientation preference, use that

+ // if we're on the top part of the screen, do below

+ // if we're on the bottom part, do above

let can_put_below = viewport.height > rel_y + MIN_HEIGHT;

let can_put_above = rel_y.checked_sub(MIN_HEIGHT).is_some();

let final_pos = match self.position_bias {

@@ -163,7 +159,40 @@ impl<T: Component> Popup<T> {

};

- match final_pos {

+ // compute maximum space available for child

+ let mut max_height = match final_pos {

+ Open::Above => rel_y,

+ Open::Below => viewport.height.saturating_sub(1 + rel_y),

+ };

+ max_height = max_height.min(MAX_HEIGHT);

+ let mut max_width = viewport.width.saturating_sub(2).min(MAX_WIDTH);

+ render_borders = render_borders && max_height > 3 && max_width > 3;

+ if render_borders {

+ max_width -= 2;

+ max_height -= 2;

+ }

+ // compute required child size and reclamp

+ let (mut width, child_height) = self

+ .contents

+ .required_size((max_width, max_height))

+ .expect("Component needs required_size implemented in order to be embedded in a popup");

+ width = width.min(MAX_WIDTH);

+ let height = if render_borders {

+ (child_height + 2).min(MAX_HEIGHT)

+ } else {

+ child_height.min(MAX_HEIGHT)

+ };

+ if render_borders {

+ width += 2;

+ }

+ if viewport.width <= rel_x + width + 2 {

+ rel_x = viewport.width.saturating_sub(width + 2);

+ width = viewport.width.saturating_sub(rel_x + 2)

+ }

+ let area = match final_pos {

Open::Above => {

rel_y = rel_y.saturating_sub(height);

Rect::new(rel_x, rel_y, width, position.row as u16 - rel_y)

@@ -173,6 +202,12 @@ impl<T: Component> Popup<T> {

let y_max = viewport.bottom().min(height + rel_y);

Rect::new(rel_x, rel_y, width, y_max - rel_y)

}

+ };

+ RenderInfo {

+ area,

+ child_height,

+ render_borders,

+ is_menu,

}

@@ -259,35 +294,16 @@ impl<T: Component> Component for Popup<T> {

// tab/enter/ctrl-k or whatever will confirm the selection/ ctrl-n/ctrl-p for scroll.

}

- fn required_size(&mut self, _viewport: (u16, u16)) -> Option<(u16, u16)> {

- const MAX_WIDTH: u16 = 120;

- const MAX_HEIGHT: u16 = 26;

- let inner = Rect::new(0, 0, MAX_WIDTH, MAX_HEIGHT).inner(&self.margin);

- let (width, height) = self

- .contents

- .required_size((inner.width, inner.height))

- .expect("Component needs required_size implemented in order to be embedded in a popup");

- let size = (

- (width + self.margin.width()).min(MAX_WIDTH),

- (height + self.margin.height()).min(MAX_HEIGHT),

- );

- Some(size)

- }

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

- let child_size = self

- .contents

- .required_size((viewport.width, viewport.height))

- .expect("Component needs required_size implemented in order to be embedded in a popup");

- let area = self.area_internal(viewport, cx.editor, child_size);

+ let RenderInfo {

+ area,

+ child_height,

+ render_borders,

+ is_menu,

+ } = self.render_info(viewport, cx.editor);

self.area = area;

- let max_offset = child_size.1.saturating_sub(area.height) as usize;

+ let max_offset = child_height.saturating_sub(area.height) as usize;

let half_page_size = (area.height / 2) as usize;

let scroll = max_offset.min(self.scroll_half_pages * half_page_size);

if half_page_size > 0 {

@@ -296,32 +312,30 @@ impl<T: Component> Component for Popup<T> {

cx.scroll = Some(scroll);

// clear area

- let background = cx.editor.theme.get("ui.popup");

- surface.clear_with(area, background);

- let render_borders = cx.editor.popup_border();

- let inner = if self

- .contents

- .type_name()

- .starts_with("helix_term::ui::menu::Menu")

- {

- area

+ let background = if is_menu {

+ // TODO: consistently style menu

+ cx.editor

+ .theme

+ .try_get("ui.menu")

+ .unwrap_or_else(|| cx.editor.theme.get("ui.text"))

} else {

- area.inner(&self.margin)

+ cx.editor.theme.get("ui.popup")

};

+ surface.clear_with(area, background);

- let border = usize::from(render_borders);

+ let mut inner = area;

if render_borders {

+ inner = area.inner(&Margin::all(1));

Widget::render(Block::default().borders(Borders::ALL), area, surface);

}

+ let border = usize::from(render_borders);

self.contents.render(inner, surface, cx);

// render scrollbar if contents do not fit

if self.has_scrollbar {

let win_height = (inner.height as usize).saturating_sub(2 * border);

- let len = (child_size.1 as usize).saturating_sub(2 * border);

+ let len = (child_height as usize).saturating_sub(2 * border);

let fits = len <= win_height;

let scroll_style = cx.editor.theme.get("ui.menu.scroll");