From 4c2addf11e1248f638d3b7c8b93b7f92a7834289 Mon Sep 17 00:00:00 2001 From: cossonleo Date: Tue, 31 Aug 2021 15:28:02 +0800 Subject: [PATCH 1/9] optimize completion doc's render --- helix-term/src/ui/completion.rs | 47 +++++++++++++++++++-------------- helix-term/src/ui/popup.rs | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+), 20 deletions(-) diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 906577643ffe..4047c696ba54 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -251,18 +251,12 @@ impl Component for Completion { // --- // option.documentation - let (view, doc) = current!(cx.editor); + // let (_view, doc) = current!(cx.editor); + let doc = doc_mut!(cx.editor); let language = doc .language() .and_then(|scope| scope.strip_prefix("source.")) .unwrap_or(""); - let cursor_pos = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); - let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row - - view.offset.row) as u16; - let mut doc = match &option.documentation { Some(lsp::Documentation::String(contents)) | Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { @@ -311,19 +305,32 @@ impl Component for Completion { None => return, }; - let half = area.height / 2; - let height = 15.min(half); - // we want to make sure the cursor is visible (not hidden behind the documentation) - let y = if cursor_pos + area.y - >= (cx.editor.tree.area().height - height - 2/* statusline + commandline */) - { - 0 - } else { - // -2 to subtract command line + statusline. a bit of a hack, because of splits. - area.height.saturating_sub(height).saturating_sub(2) - }; + let (popup_x, popup_y) = self.popup.get_rel_position(area, cx); + let (popup_width, _popup_height) = self.popup.get_size(); + let mut width = area.width - popup_x - popup_width; + let mut height = area.height - popup_y; + let x = popup_x + popup_width; + let y = popup_y; - let area = Rect::new(0, y, area.width, height); + if let Some((rel_width, rel_height)) = doc.required_size((width, height)) { + width = rel_width; + height = rel_height; + } + let area = Rect::new(x, y, width, height); + + // let half = area.height / 2; + // let height = 15.min(half); + // // we want to make sure the cursor is visible (not hidden behind the documentation) + // let y = if cursor_pos + area.y + // >= (cx.editor.tree.area().height - height - 2/* statusline + commandline */) + // { + // 0 + // } else { + // // -2 to subtract command line + statusline. a bit of a hack, because of splits. + // area.height.saturating_sub(height).saturating_sub(2) + // }; + + // Rect::new(0, y, area.width, height) // clear area let background = cx.editor.theme.get("ui.popup"); diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs index e126c84506cf..b1f31c5afc92 100644 --- a/helix-term/src/ui/popup.rs +++ b/helix-term/src/ui/popup.rs @@ -31,6 +31,33 @@ impl Popup { self.position = pos; } + pub fn get_rel_position(&mut self, viewport: Rect, cx: &Context) -> (u16, u16) { + let position = self + .position + .get_or_insert_with(|| cx.editor.cursor().0.unwrap_or_default()); + + let (width, height) = self.size; + + // -- make sure frame doesn't stick out of bounds + let mut rel_x = position.col as u16; + let 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)); + }; + + // TODO: be able to specify orientation preference. We want above for most popups, below + // for menus/autocomplete. + if height <= rel_y { + (rel_x, rel_y.saturating_sub(height)) // position above point + } else { + (rel_x, rel_y + 1) // position below point + } + } + + pub fn get_size(&self) -> (u16, u16) { + (self.size.0, self.size.1) + } + pub fn scroll(&mut self, offset: usize, direction: bool) { if direction { self.scroll += offset; From af9c5d1d680310632b0dc85b0d7115fb35149bac Mon Sep 17 00:00:00 2001 From: cossonleo Date: Tue, 31 Aug 2021 16:58:13 +0800 Subject: [PATCH 2/9] optimize completion doc's render --- helix-term/src/ui/markdown.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 28542cdcc22c..81cea09c82b5 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -217,8 +217,24 @@ impl Component for Markdown { fn required_size(&mut self, viewport: (u16, u16)) -> Option<(u16, u16)> { let contents = parse(&self.contents, None, &self.config_loader); let padding = 2; - let width = std::cmp::min(contents.width() as u16 + padding, viewport.0); - let height = std::cmp::min(contents.height() as u16 + padding, viewport.1); + let area_width = viewport.0 - padding; + let mut width = 0; + let mut height = 0; + for content in contents { + let cw = content.width() as u16; + height += 1; + if cw > area_width { + width = area_width; + height += cw / area_width; + continue; + } + if cw > width { + width = cw; + } + } + // let width = std::cmp::min(contents.width() as u16 + padding, viewport.0); + // let height = std::cmp::min(contents.height() as u16 + padding, viewport.1); + let height = std::cmp::min(height + padding, viewport.1); Some((width, height)) } } From 6ae88ba59e65c413d0d346060ae113252700e45b Mon Sep 17 00:00:00 2001 From: cossonleo Date: Wed, 1 Sep 2021 12:22:39 +0800 Subject: [PATCH 3/9] optimize completion doc position --- helix-term/src/ui/completion.rs | 59 ++++++++++++++++++--------------- helix-term/src/ui/markdown.rs | 21 ++++++------ helix-term/src/ui/popup.rs | 27 +++------------ 3 files changed, 48 insertions(+), 59 deletions(-) diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 4047c696ba54..c17274c8952c 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -251,13 +251,18 @@ impl Component for Completion { // --- // option.documentation - // let (_view, doc) = current!(cx.editor); - let doc = doc_mut!(cx.editor); + let (view, doc) = current!(cx.editor); let language = doc .language() .and_then(|scope| scope.strip_prefix("source.")) .unwrap_or(""); - let mut doc = match &option.documentation { + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); + let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row + - view.offset.row) as u16; + let mut markdown_doc = match &option.documentation { Some(lsp::Documentation::String(contents)) | Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { kind: lsp::MarkupKind::PlainText, @@ -308,34 +313,36 @@ impl Component for Completion { let (popup_x, popup_y) = self.popup.get_rel_position(area, cx); let (popup_width, _popup_height) = self.popup.get_size(); let mut width = area.width - popup_x - popup_width; - let mut height = area.height - popup_y; - let x = popup_x + popup_width; - let y = popup_y; + let area = if width > 30 { + let mut height = area.height - popup_y; + let x = popup_x + popup_width; + let y = popup_y; + + if let Some((rel_width, rel_height)) = markdown_doc.required_size((width, height)) { + width = rel_width; + height = rel_height; + } + Rect::new(x, y, width, height) + } else { + let half = area.height / 2; + let height = 15.min(half); + // we want to make sure the cursor is visible (not hidden behind the documentation) + let y = if cursor_pos + area.y + >= (cx.editor.tree.area().height - height - 2/* statusline + commandline */) + { + 0 + } else { + // -2 to subtract command line + statusline. a bit of a hack, because of splits. + area.height.saturating_sub(height).saturating_sub(2) + }; - if let Some((rel_width, rel_height)) = doc.required_size((width, height)) { - width = rel_width; - height = rel_height; - } - let area = Rect::new(x, y, width, height); - - // let half = area.height / 2; - // let height = 15.min(half); - // // we want to make sure the cursor is visible (not hidden behind the documentation) - // let y = if cursor_pos + area.y - // >= (cx.editor.tree.area().height - height - 2/* statusline + commandline */) - // { - // 0 - // } else { - // // -2 to subtract command line + statusline. a bit of a hack, because of splits. - // area.height.saturating_sub(height).saturating_sub(2) - // }; - - // Rect::new(0, y, area.width, height) + Rect::new(0, y, area.width, height) + }; // clear area let background = cx.editor.theme.get("ui.popup"); surface.clear_with(area, background); - doc.render(area, surface, cx); + markdown_doc.render(area, surface, cx); } } } diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 81cea09c82b5..70f7c090d4f1 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -219,22 +219,23 @@ impl Component for Markdown { let padding = 2; let area_width = viewport.0 - padding; let mut width = 0; - let mut height = 0; + let mut height = padding; for content in contents { - let cw = content.width() as u16; + if height >= viewport.1 { + break; + } + let mut content_width = content.width() as u16; height += 1; - if cw > area_width { - width = area_width; - height += cw / area_width; + if content_width > area_width { + width = viewport.0; + height += content_width / area_width; continue; } - if cw > width { - width = cw; + content_width += padding; + if content_width > width { + width = content_width } } - // let width = std::cmp::min(contents.width() as u16 + padding, viewport.0); - // let height = std::cmp::min(contents.height() as u16 + padding, viewport.1); - let height = std::cmp::min(height + padding, viewport.1); Some((width, height)) } } diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs index b1f31c5afc92..7f39238e1e07 100644 --- a/helix-term/src/ui/popup.rs +++ b/helix-term/src/ui/popup.rs @@ -134,30 +134,11 @@ impl Component for Popup { fn render(&mut self, viewport: Rect, surface: &mut Surface, cx: &mut Context) { cx.scroll = Some(self.scroll); - - let position = self - .position - .get_or_insert_with(|| cx.editor.cursor().0.unwrap_or_default()); - - let (width, height) = self.size; - - // -- 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)); - }; - - // TODO: be able to specify orientation preference. We want above for most popups, below - // for menus/autocomplete. - if height <= rel_y { - rel_y = rel_y.saturating_sub(height) // position above point - } else { - rel_y += 1 // position below point - } - + + let (rel_x, rel_y) = self.get_rel_position(viewport, cx); + // clip to viewport - let area = viewport.intersection(Rect::new(rel_x, rel_y, width, height)); + 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"); From 4b1987d0321eec6c3b274674a61da31ffcca3907 Mon Sep 17 00:00:00 2001 From: cossonleo Date: Wed, 1 Sep 2021 12:52:36 +0800 Subject: [PATCH 4/9] cargo fmt --- helix-term/src/ui/popup.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs index 7f39238e1e07..846cefb8a7cd 100644 --- a/helix-term/src/ui/popup.rs +++ b/helix-term/src/ui/popup.rs @@ -134,9 +134,9 @@ impl Component for Popup { fn render(&mut self, viewport: Rect, surface: &mut Surface, cx: &mut Context) { 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)); From eb02cd4d705ff184ab7e5541459ecdfd7ab65e0b Mon Sep 17 00:00:00 2001 From: cossonleo Date: Wed, 1 Sep 2021 17:42:02 +0800 Subject: [PATCH 5/9] fix panic --- helix-term/src/ui/completion.rs | 2 +- helix-term/src/ui/markdown.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index c17274c8952c..279cecc8f65a 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -314,7 +314,7 @@ impl Component for Completion { let (popup_width, _popup_height) = self.popup.get_size(); let mut width = area.width - popup_x - popup_width; let area = if width > 30 { - let mut height = area.height - popup_y; + let mut height = area.height.saturating_sub(popup_y); let x = popup_x + popup_width; let y = popup_y; diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 70f7c090d4f1..79eaee4e351d 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -222,6 +222,7 @@ impl Component for Markdown { let mut height = padding; for content in contents { if height >= viewport.1 { + height = viewport.1; break; } let mut content_width = content.width() as u16; @@ -236,6 +237,7 @@ impl Component for Markdown { width = content_width } } + Some((width, height)) } } From 25df4d670f77623c31b273d599bf198bab3b6731 Mon Sep 17 00:00:00 2001 From: cossonleo Date: Thu, 2 Sep 2021 17:24:47 +0800 Subject: [PATCH 6/9] use saturating_sub --- helix-term/src/ui/completion.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 279cecc8f65a..6c9e3a808c15 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -312,7 +312,10 @@ impl Component for Completion { let (popup_x, popup_y) = self.popup.get_rel_position(area, cx); let (popup_width, _popup_height) = self.popup.get_size(); - let mut width = area.width - popup_x - popup_width; + let mut width = area + .width + .saturating_sub(popup_x) + .saturating_sub(popup_width); let area = if width > 30 { let mut height = area.height.saturating_sub(popup_y); let x = popup_x + popup_width; From 650348074f81948a472f45531052467805ca2db9 Mon Sep 17 00:00:00 2001 From: cossonleo Date: Sat, 4 Sep 2021 12:29:14 +0800 Subject: [PATCH 7/9] fixs --- helix-term/src/ui/markdown.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 79eaee4e351d..ee5185c5bac4 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -215,27 +215,30 @@ impl Component for Markdown { } fn required_size(&mut self, viewport: (u16, u16)) -> Option<(u16, u16)> { - let contents = parse(&self.contents, None, &self.config_loader); let padding = 2; - let area_width = viewport.0 - padding; + if padding >= viewport.1 || padding >= viewport.0 { + return None; + } + let contents = parse(&self.contents, None, &self.config_loader); + let max_text_width = viewport.0 - padding; let mut width = 0; let mut height = padding; for content in contents { - if height >= viewport.1 { - height = viewport.1; - break; - } let mut content_width = content.width() as u16; height += 1; - if content_width > area_width { + if content_width > max_text_width { width = viewport.0; - height += content_width / area_width; - continue; - } - content_width += padding; - if content_width > width { + height += content_width / max_text_width; + } else if { + content_width += padding; + content_width > width + } { width = content_width } + if height >= viewport.1 { + height = viewport.1; + break; + } } Some((width, height)) From 0f2bea3f4135c9d8e50fbe77fced771c2efcd420 Mon Sep 17 00:00:00 2001 From: cossonleo Date: Sat, 4 Sep 2021 14:40:15 +0800 Subject: [PATCH 8/9] fix clippy --- helix-term/src/ui/markdown.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index ee5185c5bac4..5a17a6b064b7 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -229,11 +229,11 @@ impl Component for Markdown { if content_width > max_text_width { width = viewport.0; height += content_width / max_text_width; - } else if { + } else { content_width += padding; - content_width > width - } { - width = content_width + if content_width > width { + width = content_width; + } } if height >= viewport.1 { height = viewport.1; From e066bd44b25e9ab68afcfb7686d13fcd47c7de9b Mon Sep 17 00:00:00 2001 From: cossonleo Date: Mon, 6 Sep 2021 10:33:50 +0800 Subject: [PATCH 9/9] limit completion doc max width 120 --- helix-term/src/ui/markdown.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/helix-term/src/ui/markdown.rs b/helix-term/src/ui/markdown.rs index 5a17a6b064b7..87b35a2db2d0 100644 --- a/helix-term/src/ui/markdown.rs +++ b/helix-term/src/ui/markdown.rs @@ -220,27 +220,25 @@ impl Component for Markdown { return None; } let contents = parse(&self.contents, None, &self.config_loader); - let max_text_width = viewport.0 - padding; - let mut width = 0; + let max_text_width = (viewport.0 - padding).min(120); + let mut text_width = 0; let mut height = padding; for content in contents { - let mut content_width = content.width() as u16; height += 1; + let content_width = content.width() as u16; if content_width > max_text_width { - width = viewport.0; + text_width = max_text_width; height += content_width / max_text_width; - } else { - content_width += padding; - if content_width > width { - width = content_width; - } + } else if content_width > text_width { + text_width = content_width; } + if height >= viewport.1 { height = viewport.1; break; } } - Some((width, height)) + Some((text_width + padding, height)) } }