Fix panics when resizing (#1408)

* Change buffer.get & buffer.get_mut to return Option, Implement Trait Index & IndexMut to panic

* Prevent FilePicker from drawing outside buffer (rust panics)

* apply suggestion

* add function in_bounds to avoid useless calculations

Co-authored-by: mathis <mathis.brossier@universite-paris-saclay.fr>
This commit is contained in:
Mathis Brossier 2022-01-16 02:55:28 +01:00 committed by GitHub
parent 9da0abaa5d
commit f5b0821860
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 95 additions and 47 deletions

View file

@ -97,8 +97,7 @@ impl EditorView {
let x = area.right();
let border_style = theme.get("ui.window");
for y in area.top()..area.bottom() {
surface
.get_mut(x, y)
surface[(x, y)]
.set_symbol(tui::symbols::line::VERTICAL)
//.set_symbol(" ")
.set_style(border_style);
@ -393,8 +392,7 @@ impl EditorView {
.add_modifier(Modifier::DIM)
});
surface
.get_mut(viewport.x + pos.col as u16, viewport.y + pos.row as u16)
surface[(viewport.x + pos.col as u16, viewport.y + pos.row as u16)]
.set_style(style);
}
}

View file

@ -301,7 +301,7 @@ impl<T: Item + 'static> Component for Menu<T> {
let is_marked = i >= scroll_line && i < scroll_line + scroll_height;
if is_marked {
let cell = surface.get_mut(area.x + area.width - 2, area.y + i as u16);
let cell = &mut surface[(area.x + area.width - 2, area.y + i as u16)];
cell.set_symbol("");
// cell.set_style(selected);
// cell.set_style(if is_marked { selected } else { style });

View file

@ -159,6 +159,7 @@ impl<T: 'static> Component for FilePicker<T> {
// |picker | | |
// | | | |
// +---------+ +---------+
let render_preview = area.width > MIN_SCREEN_WIDTH_FOR_PREVIEW;
let area = inner_rect(area);
// -- Render the frame:
@ -492,10 +493,9 @@ impl<T: 'static> Component for Picker<T> {
let sep_style = Style::default().fg(Color::Rgb(90, 89, 119));
let borders = BorderType::line_symbols(BorderType::Plain);
for x in inner.left()..inner.right() {
surface
.get_mut(x, inner.y + 1)
.set_symbol(borders.horizontal)
.set_style(sep_style);
if let Some(cell) = surface.get_mut(x, inner.y + 1) {
cell.set_symbol(borders.horizontal).set_style(sep_style);
}
}
// -- Render the contents:
@ -505,7 +505,7 @@ impl<T: 'static> Component for Picker<T> {
let selected = cx.editor.theme.get("ui.text.focus");
let rows = inner.height;
let offset = self.cursor / (rows as usize) * (rows as usize);
let offset = self.cursor / std::cmp::max(1, (rows as usize) * (rows as usize));
let files = self.matches.iter().skip(offset).map(|(index, _score)| {
(index, self.options.get(*index).unwrap()) // get_unchecked
@ -513,7 +513,7 @@ impl<T: 'static> Component for Picker<T> {
for (i, (_index, option)) in files.take(rows as usize).enumerate() {
if i == (self.cursor - offset) {
surface.set_string(inner.x - 2, inner.y + i as u16, ">", selected);
surface.set_string(inner.x.saturating_sub(2), inner.y + i as u16, ">", selected);
}
surface.set_string_truncated(

View file

@ -330,7 +330,7 @@ impl Prompt {
.max(BASE_WIDTH);
let cols = std::cmp::max(1, area.width / max_len);
let col_width = (area.width - (cols)) / cols;
let col_width = (area.width.saturating_sub(cols)) / cols;
let height = ((self.completion.len() as u16 + cols - 1) / cols)
.min(10) // at most 10 rows (or less)

View file

@ -111,8 +111,7 @@ impl Backend for TestBackend {
I: Iterator<Item = (u16, u16, &'a Cell)>,
{
for (x, y, c) in content {
let cell = self.buffer.get_mut(x, y);
*cell = c.clone();
self.buffer[(x, y)] = c.clone();
}
Ok(())
}

View file

@ -90,17 +90,17 @@ impl Default for Cell {
/// use helix_view::graphics::{Rect, Color, Style, Modifier};
///
/// let mut buf = Buffer::empty(Rect{x: 0, y: 0, width: 10, height: 5});
/// buf.get_mut(0, 2).set_symbol("x");
/// assert_eq!(buf.get(0, 2).symbol, "x");
/// buf[(0, 2)].set_symbol("x");
/// assert_eq!(buf[(0, 2)].symbol, "x");
/// buf.set_string(3, 0, "string", Style::default().fg(Color::Red).bg(Color::White));
/// assert_eq!(buf.get(5, 0), &Cell{
/// assert_eq!(buf[(5, 0)], Cell{
/// symbol: String::from("r"),
/// fg: Color::Red,
/// bg: Color::White,
/// modifier: Modifier::empty()
/// });
/// buf.get_mut(5, 0).set_char('x');
/// assert_eq!(buf.get(5, 0).symbol, "x");
/// buf[(5, 0)].set_char('x');
/// assert_eq!(buf[(5, 0)].symbol, "x");
/// ```
#[derive(Debug, Default, Clone, PartialEq)]
pub struct Buffer {
@ -162,15 +162,38 @@ impl Buffer {
}
/// Returns a reference to Cell at the given coordinates
pub fn get(&self, x: u16, y: u16) -> &Cell {
let i = self.index_of(x, y);
&self.content[i]
pub fn get(&self, x: u16, y: u16) -> Option<&Cell> {
self.index_of_opt(x, y).map(|i| &self.content[i])
}
/// Returns a mutable reference to Cell at the given coordinates
pub fn get_mut(&mut self, x: u16, y: u16) -> &mut Cell {
let i = self.index_of(x, y);
&mut self.content[i]
pub fn get_mut(&mut self, x: u16, y: u16) -> Option<&mut Cell> {
self.index_of_opt(x, y).map(|i| &mut self.content[i])
}
/// Tells whether the global (x, y) coordinates are inside the Buffer's area.
///
/// Global coordinates are offset by the Buffer's area offset (`x`/`y`).
///
/// # Examples
///
/// ```
/// # use helix_tui::buffer::Buffer;
/// # use helix_view::graphics::Rect;
/// let rect = Rect::new(200, 100, 10, 10);
/// let buffer = Buffer::empty(rect);
/// // Global coordinates inside the Buffer's area
/// assert!(buffer.in_bounds(209, 100));
/// // Global coordinates outside the Buffer's area
/// assert!(!buffer.in_bounds(210, 100));
/// ```
///
/// Global coordinates are offset by the Buffer's area offset (`x`/`y`).
pub fn in_bounds(&self, x: u16, y: u16) -> bool {
x >= self.area.left()
&& x < self.area.right()
&& y >= self.area.top()
&& y < self.area.bottom()
}
/// Returns the index in the Vec<Cell> for the given global (x, y) coordinates.
@ -184,7 +207,7 @@ impl Buffer {
/// # use helix_view::graphics::Rect;
/// let rect = Rect::new(200, 100, 10, 10);
/// let buffer = Buffer::empty(rect);
/// // Global coordinates to the top corner of this buffer's area
/// // Global coordinates to the top corner of this Buffer's area
/// assert_eq!(buffer.index_of(200, 100), 0);
/// ```
///
@ -193,10 +216,7 @@ impl Buffer {
/// Panics when given an coordinate that is outside of this Buffer's area.
pub fn index_of(&self, x: u16, y: u16) -> usize {
debug_assert!(
x >= self.area.left()
&& x < self.area.right()
&& y >= self.area.top()
&& y < self.area.bottom(),
self.in_bounds(x, y),
"Trying to access position outside the buffer: x={}, y={}, area={:?}",
x,
y,
@ -205,6 +225,16 @@ impl Buffer {
((y - self.area.y) * self.area.width + (x - self.area.x)) as usize
}
/// Returns the index in the Vec<Cell> for the given global (x, y) coordinates,
/// or `None` if the coordinates are outside the buffer's area.
fn index_of_opt(&self, x: u16, y: u16) -> Option<usize> {
if self.in_bounds(x, y) {
Some(self.index_of(x, y))
} else {
None
}
}
/// Returns the (global) coordinates of a cell given its index
///
/// Global coordinates are offset by the Buffer's area offset (`x`/`y`).
@ -278,6 +308,11 @@ impl Buffer {
where
S: AsRef<str>,
{
// prevent panic if out of range
if !self.in_bounds(x, y) || width == 0 {
return (x, y);
}
let mut index = self.index_of(x, y);
let mut x_offset = x as usize;
let width = if ellipsis { width - 1 } else { width };
@ -372,7 +407,7 @@ impl Buffer {
pub fn set_background(&mut self, area: Rect, color: Color) {
for y in area.top()..area.bottom() {
for x in area.left()..area.right() {
self.get_mut(x, y).set_bg(color);
self[(x, y)].set_bg(color);
}
}
}
@ -380,7 +415,7 @@ impl Buffer {
pub fn set_style(&mut self, area: Rect, style: Style) {
for y in area.top()..area.bottom() {
for x in area.left()..area.right() {
self.get_mut(x, y).set_style(style);
self[(x, y)].set_style(style);
}
}
}
@ -408,7 +443,7 @@ impl Buffer {
pub fn clear(&mut self, area: Rect) {
for x in area.left()..area.right() {
for y in area.top()..area.bottom() {
self.get_mut(x, y).reset();
self[(x, y)].reset();
}
}
}
@ -417,7 +452,7 @@ impl Buffer {
pub fn clear_with(&mut self, area: Rect, style: Style) {
for x in area.left()..area.right() {
for y in area.top()..area.bottom() {
let cell = self.get_mut(x, y);
let cell = &mut self[(x, y)];
cell.reset();
cell.set_style(style);
}
@ -509,6 +544,22 @@ impl Buffer {
}
}
impl std::ops::Index<(u16, u16)> for Buffer {
type Output = Cell;
fn index(&self, (x, y): (u16, u16)) -> &Self::Output {
let i = self.index_of(x, y);
&self.content[i]
}
}
impl std::ops::IndexMut<(u16, u16)> for Buffer {
fn index_mut(&mut self, (x, y): (u16, u16)) -> &mut Self::Output {
let i = self.index_of(x, y);
&mut self.content[i]
}
}
#[cfg(test)]
mod tests {
use super::*;

View file

@ -140,14 +140,14 @@ impl<'a> Widget for Block<'a> {
// Sides
if self.borders.intersects(Borders::LEFT) {
for y in area.top()..area.bottom() {
buf.get_mut(area.left(), y)
buf[(area.left(), y)]
.set_symbol(symbols.vertical)
.set_style(self.border_style);
}
}
if self.borders.intersects(Borders::TOP) {
for x in area.left()..area.right() {
buf.get_mut(x, area.top())
buf[(x, area.top())]
.set_symbol(symbols.horizontal)
.set_style(self.border_style);
}
@ -155,7 +155,7 @@ impl<'a> Widget for Block<'a> {
if self.borders.intersects(Borders::RIGHT) {
let x = area.right() - 1;
for y in area.top()..area.bottom() {
buf.get_mut(x, y)
buf[(x, y)]
.set_symbol(symbols.vertical)
.set_style(self.border_style);
}
@ -163,7 +163,7 @@ impl<'a> Widget for Block<'a> {
if self.borders.intersects(Borders::BOTTOM) {
let y = area.bottom() - 1;
for x in area.left()..area.right() {
buf.get_mut(x, y)
buf[(x, y)]
.set_symbol(symbols.horizontal)
.set_style(self.border_style);
}
@ -171,22 +171,22 @@ impl<'a> Widget for Block<'a> {
// Corners
if self.borders.contains(Borders::RIGHT | Borders::BOTTOM) {
buf.get_mut(area.right() - 1, area.bottom() - 1)
buf[(area.right() - 1, area.bottom() - 1)]
.set_symbol(symbols.bottom_right)
.set_style(self.border_style);
}
if self.borders.contains(Borders::RIGHT | Borders::TOP) {
buf.get_mut(area.right() - 1, area.top())
buf[(area.right() - 1, area.top())]
.set_symbol(symbols.top_right)
.set_style(self.border_style);
}
if self.borders.contains(Borders::LEFT | Borders::BOTTOM) {
buf.get_mut(area.left(), area.bottom() - 1)
buf[(area.left(), area.bottom() - 1)]
.set_symbol(symbols.bottom_left)
.set_style(self.border_style);
}
if self.borders.contains(Borders::LEFT | Borders::TOP) {
buf.get_mut(area.left(), area.top())
buf[(area.left(), area.top())]
.set_symbol(symbols.top_left)
.set_style(self.border_style);
}

View file

@ -176,7 +176,7 @@ impl<'a> Widget for Paragraph<'a> {
if y >= self.scroll.0 {
let mut x = get_line_offset(current_line_width, text_area.width, self.alignment);
for StyledGrapheme { symbol, style } in current_line {
buf.get_mut(text_area.left() + x, text_area.top() + y - self.scroll.0)
buf[(text_area.left() + x, text_area.top() + y - self.scroll.0)]
.set_symbol(if symbol.is_empty() {
// If the symbol is empty, the last char which rendered last time will
// leave on the line. It's a quick fix.

View file

@ -323,7 +323,7 @@ impl FromStr for Modifier {
/// ];
/// let mut buffer = Buffer::empty(Rect::new(0, 0, 1, 1));
/// for style in &styles {
/// buffer.get_mut(0, 0).set_style(*style);
/// buffer[(0, 0)].set_style(*style);
/// }
/// assert_eq!(
/// Style {
@ -332,7 +332,7 @@ impl FromStr for Modifier {
/// add_modifier: Modifier::BOLD,
/// sub_modifier: Modifier::empty(),
/// },
/// buffer.get(0, 0).style(),
/// buffer[(0, 0)].style(),
/// );
/// ```
///
@ -348,7 +348,7 @@ impl FromStr for Modifier {
/// ];
/// let mut buffer = Buffer::empty(Rect::new(0, 0, 1, 1));
/// for style in &styles {
/// buffer.get_mut(0, 0).set_style(*style);
/// buffer[(0, 0)].set_style(*style);
/// }
/// assert_eq!(
/// Style {
@ -357,7 +357,7 @@ impl FromStr for Modifier {
/// add_modifier: Modifier::empty(),
/// sub_modifier: Modifier::empty(),
/// },
/// buffer.get(0, 0).style(),
/// buffer[(0, 0)].style(),
/// );
/// ```
#[derive(Debug, Clone, Copy, PartialEq)]