on master help sign in
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>

commitf5b0821860940782567a0baa17280aef04f845e3 authorMathis Brossier <mathis.brossier@gmail.com> dateSun 16 Jan 2022, 02:55 (5 years ago) parent9da0aba
9 files changed+95−47
helix-term/src/ui/editor.rs+2−4 helix-term/src/ui/menu.rs+1−1 helix-term/src/ui/picker.rs+6−6 helix-term/src/ui/prompt.rs+1−1 helix-tui/src/backend/test.rs+1−2 helix-tui/src/buffer.rs+71−20 helix-tui/src/widgets/block.rs+8−8 helix-tui/src/widgets/paragraph.rs+1−1 helix-view/src/graphics.rs+4−4
helix-term/src/ui/editor.rs+2−4
@@ -97,8 +97,7 @@ impl EditorView {
9797 let x = area.right();
9898 let border_style = theme.get("ui.window");
9999 for y in area.top()..area.bottom() {
100− surface
101− .get_mut(x, y)
100+ surface[(x, y)]
102101 .set_symbol(tui::symbols::line::VERTICAL)
103102 //.set_symbol(" ")
104103 .set_style(border_style);
@@ -393,8 +392,7 @@ impl EditorView {
393392 .add_modifier(Modifier::DIM)
394393 });
395394
396− surface
397− .get_mut(viewport.x + pos.col as u16, viewport.y + pos.row as u16)
395+ surface[(viewport.x + pos.col as u16, viewport.y + pos.row as u16)]
398396 .set_style(style);
399397 }
400398 }
helix-term/src/ui/menu.rs+1−1
@@ -301,7 +301,7 @@ impl<T: Item + 'static> Component for Menu<T> {
301301 let is_marked = i >= scroll_line && i < scroll_line + scroll_height;
302302
303303 if is_marked {
304− let cell = surface.get_mut(area.x + area.width - 2, area.y + i as u16);
304+ let cell = &mut surface[(area.x + area.width - 2, area.y + i as u16)];
305305 cell.set_symbol("▐ ");
306306 // cell.set_style(selected);
307307 // cell.set_style(if is_marked { selected } else { style });
helix-term/src/ui/picker.rs+6−6
@@ -159,6 +159,7 @@ impl<T: 'static> Component for FilePicker<T> {
159159 // |picker | | |
160160 // | | | |
161161 // +---------+ +---------+
162+
162163 let render_preview = area.width > MIN_SCREEN_WIDTH_FOR_PREVIEW;
163164 let area = inner_rect(area);
164165 // -- Render the frame:
@@ -492,10 +493,9 @@ impl<T: 'static> Component for Picker<T> {
492493 let sep_style = Style::default().fg(Color::Rgb(90, 89, 119));
493494 let borders = BorderType::line_symbols(BorderType::Plain);
494495 for x in inner.left()..inner.right() {
495− surface
496− .get_mut(x, inner.y + 1)
497− .set_symbol(borders.horizontal)
498− .set_style(sep_style);
496+ if let Some(cell) = surface.get_mut(x, inner.y + 1) {
497+ cell.set_symbol(borders.horizontal).set_style(sep_style);
498+ }
499499 }
500500
501501 // -- Render the contents:
@@ -505,7 +505,7 @@ impl<T: 'static> Component for Picker<T> {
505505 let selected = cx.editor.theme.get("ui.text.focus");
506506
507507 let rows = inner.height;
508− let offset = self.cursor / (rows as usize) * (rows as usize);
508+ let offset = self.cursor / std::cmp::max(1, (rows as usize) * (rows as usize));
509509
510510 let files = self.matches.iter().skip(offset).map(|(index, _score)| {
511511 (index, self.options.get(*index).unwrap()) // get_unchecked
@@ -513,7 +513,7 @@ impl<T: 'static> Component for Picker<T> {
513513
514514 for (i, (_index, option)) in files.take(rows as usize).enumerate() {
515515 if i == (self.cursor - offset) {
516− surface.set_string(inner.x - 2, inner.y + i as u16, ">", selected);
516+ surface.set_string(inner.x.saturating_sub(2), inner.y + i as u16, ">", selected);
517517 }
518518
519519 surface.set_string_truncated(
helix-term/src/ui/prompt.rs+1−1
@@ -330,7 +330,7 @@ impl Prompt {
330330 .max(BASE_WIDTH);
331331
332332 let cols = std::cmp::max(1, area.width / max_len);
333− let col_width = (area.width - (cols)) / cols;
333+ let col_width = (area.width.saturating_sub(cols)) / cols;
334334
335335 let height = ((self.completion.len() as u16 + cols - 1) / cols)
336336 .min(10) // at most 10 rows (or less)
helix-tui/src/backend/test.rs+1−2
@@ -111,8 +111,7 @@ impl Backend for TestBackend {
111111 I: Iterator<Item = (u16, u16, &'a Cell)>,
112112 {
113113 for (x, y, c) in content {
114− let cell = self.buffer.get_mut(x, y);
115− *cell = c.clone();
114+ self.buffer[(x, y)] = c.clone();
116115 }
117116 Ok(())
118117 }
helix-tui/src/buffer.rs+71−20
@@ -90,17 +90,17 @@ impl Default for Cell {
9090/// use helix_view::graphics::{Rect, Color, Style, Modifier};
9191///
9292/// let mut buf = Buffer::empty(Rect{x: 0, y: 0, width: 10, height: 5});
93−/// buf.get_mut(0, 2).set_symbol("x");
94−/// assert_eq!(buf.get(0, 2).symbol, "x");
93+/// buf[(0, 2)].set_symbol("x");
94+/// assert_eq!(buf[(0, 2)].symbol, "x");
9595/// buf.set_string(3, 0, "string", Style::default().fg(Color::Red).bg(Color::White));
96−/// assert_eq!(buf.get(5, 0), &Cell{
96+/// assert_eq!(buf[(5, 0)], Cell{
9797/// symbol: String::from("r"),
9898/// fg: Color::Red,
9999/// bg: Color::White,
100100/// modifier: Modifier::empty()
101101/// });
102−/// buf.get_mut(5, 0).set_char('x');
103−/// assert_eq!(buf.get(5, 0).symbol, "x");
102+/// buf[(5, 0)].set_char('x');
103+/// assert_eq!(buf[(5, 0)].symbol, "x");
104104/// ```
105105#[derive(Debug, Default, Clone, PartialEq)]
106106pub struct Buffer {
@@ -162,15 +162,38 @@ impl Buffer {
162162 }
163163
164164 /// Returns a reference to Cell at the given coordinates
165− pub fn get(&self, x: u16, y: u16) -> &Cell {
166− let i = self.index_of(x, y);
167− &self.content[i]
165+ pub fn get(&self, x: u16, y: u16) -> Option<&Cell> {
166+ self.index_of_opt(x, y).map(|i| &self.content[i])
168167 }
169168
170169 /// Returns a mutable reference to Cell at the given coordinates
171− pub fn get_mut(&mut self, x: u16, y: u16) -> &mut Cell {
172− let i = self.index_of(x, y);
173− &mut self.content[i]
170+ pub fn get_mut(&mut self, x: u16, y: u16) -> Option<&mut Cell> {
171+ self.index_of_opt(x, y).map(|i| &mut self.content[i])
172+ }
173+
174+ /// Tells whether the global (x, y) coordinates are inside the Buffer's area.
175+ ///
176+ /// Global coordinates are offset by the Buffer's area offset (`x`/`y`).
177+ ///
178+ /// # Examples
179+ ///
180+ /// ```
181+ /// # use helix_tui::buffer::Buffer;
182+ /// # use helix_view::graphics::Rect;
183+ /// let rect = Rect::new(200, 100, 10, 10);
184+ /// let buffer = Buffer::empty(rect);
185+ /// // Global coordinates inside the Buffer's area
186+ /// assert!(buffer.in_bounds(209, 100));
187+ /// // Global coordinates outside the Buffer's area
188+ /// assert!(!buffer.in_bounds(210, 100));
189+ /// ```
190+ ///
191+ /// Global coordinates are offset by the Buffer's area offset (`x`/`y`).
192+ pub fn in_bounds(&self, x: u16, y: u16) -> bool {
193+ x >= self.area.left()
194+ && x < self.area.right()
195+ && y >= self.area.top()
196+ && y < self.area.bottom()
174197 }
175198
176199 /// Returns the index in the Vec<Cell> for the given global (x, y) coordinates.
@@ -184,7 +207,7 @@ impl Buffer {
184207 /// # use helix_view::graphics::Rect;
185208 /// let rect = Rect::new(200, 100, 10, 10);
186209 /// let buffer = Buffer::empty(rect);
187− /// // Global coordinates to the top corner of this buffer's area
210+ /// // Global coordinates to the top corner of this Buffer's area
188211 /// assert_eq!(buffer.index_of(200, 100), 0);
189212 /// ```
190213 ///
@@ -193,10 +216,7 @@ impl Buffer {
193216 /// Panics when given an coordinate that is outside of this Buffer's area.
194217 pub fn index_of(&self, x: u16, y: u16) -> usize {
195218 debug_assert!(
196− x >= self.area.left()
197− && x < self.area.right()
198− && y >= self.area.top()
199− && y < self.area.bottom(),
219+ self.in_bounds(x, y),
200220 "Trying to access position outside the buffer: x={}, y={}, area={:?}",
201221 x,
202222 y,
@@ -205,6 +225,16 @@ impl Buffer {
205225 ((y - self.area.y) * self.area.width + (x - self.area.x)) as usize
206226 }
207227
228+ /// Returns the index in the Vec<Cell> for the given global (x, y) coordinates,
229+ /// or `None` if the coordinates are outside the buffer's area.
230+ fn index_of_opt(&self, x: u16, y: u16) -> Option<usize> {
231+ if self.in_bounds(x, y) {
232+ Some(self.index_of(x, y))
233+ } else {
234+ None
235+ }
236+ }
237+
208238 /// Returns the (global) coordinates of a cell given its index
209239 ///
210240 /// Global coordinates are offset by the Buffer's area offset (`x`/`y`).
@@ -278,6 +308,11 @@ impl Buffer {
278308 where
279309 S: AsRef<str>,
280310 {
311+ // prevent panic if out of range
312+ if !self.in_bounds(x, y) || width == 0 {
313+ return (x, y);
314+ }
315+
281316 let mut index = self.index_of(x, y);
282317 let mut x_offset = x as usize;
283318 let width = if ellipsis { width - 1 } else { width };
@@ -372,7 +407,7 @@ impl Buffer {
372407 pub fn set_background(&mut self, area: Rect, color: Color) {
373408 for y in area.top()..area.bottom() {
374409 for x in area.left()..area.right() {
375− self.get_mut(x, y).set_bg(color);
410+ self[(x, y)].set_bg(color);
376411 }
377412 }
378413 }
@@ -380,7 +415,7 @@ impl Buffer {
380415 pub fn set_style(&mut self, area: Rect, style: Style) {
381416 for y in area.top()..area.bottom() {
382417 for x in area.left()..area.right() {
383− self.get_mut(x, y).set_style(style);
418+ self[(x, y)].set_style(style);
384419 }
385420 }
386421 }
@@ -408,7 +443,7 @@ impl Buffer {
408443 pub fn clear(&mut self, area: Rect) {
409444 for x in area.left()..area.right() {
410445 for y in area.top()..area.bottom() {
411− self.get_mut(x, y).reset();
446+ self[(x, y)].reset();
412447 }
413448 }
414449 }
@@ -417,7 +452,7 @@ impl Buffer {
417452 pub fn clear_with(&mut self, area: Rect, style: Style) {
418453 for x in area.left()..area.right() {
419454 for y in area.top()..area.bottom() {
420− let cell = self.get_mut(x, y);
455+ let cell = &mut self[(x, y)];
421456 cell.reset();
422457 cell.set_style(style);
423458 }
@@ -509,6 +544,22 @@ impl Buffer {
509544 }
510545}
511546
547+impl std::ops::Index<(u16, u16)> for Buffer {
548+ type Output = Cell;
549+
550+ fn index(&self, (x, y): (u16, u16)) -> &Self::Output {
551+ let i = self.index_of(x, y);
552+ &self.content[i]
553+ }
554+}
555+
556+impl std::ops::IndexMut<(u16, u16)> for Buffer {
557+ fn index_mut(&mut self, (x, y): (u16, u16)) -> &mut Self::Output {
558+ let i = self.index_of(x, y);
559+ &mut self.content[i]
560+ }
561+}
562+
512563#[cfg(test)]
513564mod tests {
514565 use super::*;
helix-tui/src/widgets/block.rs+8−8
@@ -140,14 +140,14 @@ impl<'a> Widget for Block<'a> {
140140 // Sides
141141 if self.borders.intersects(Borders::LEFT) {
142142 for y in area.top()..area.bottom() {
143− buf.get_mut(area.left(), y)
143+ buf[(area.left(), y)]
144144 .set_symbol(symbols.vertical)
145145 .set_style(self.border_style);
146146 }
147147 }
148148 if self.borders.intersects(Borders::TOP) {
149149 for x in area.left()..area.right() {
150− buf.get_mut(x, area.top())
150+ buf[(x, area.top())]
151151 .set_symbol(symbols.horizontal)
152152 .set_style(self.border_style);
153153 }
@@ -155,7 +155,7 @@ impl<'a> Widget for Block<'a> {
155155 if self.borders.intersects(Borders::RIGHT) {
156156 let x = area.right() - 1;
157157 for y in area.top()..area.bottom() {
158− buf.get_mut(x, y)
158+ buf[(x, y)]
159159 .set_symbol(symbols.vertical)
160160 .set_style(self.border_style);
161161 }
@@ -163,7 +163,7 @@ impl<'a> Widget for Block<'a> {
163163 if self.borders.intersects(Borders::BOTTOM) {
164164 let y = area.bottom() - 1;
165165 for x in area.left()..area.right() {
166− buf.get_mut(x, y)
166+ buf[(x, y)]
167167 .set_symbol(symbols.horizontal)
168168 .set_style(self.border_style);
169169 }
@@ -171,22 +171,22 @@ impl<'a> Widget for Block<'a> {
171171
172172 // Corners
173173 if self.borders.contains(Borders::RIGHT | Borders::BOTTOM) {
174− buf.get_mut(area.right() - 1, area.bottom() - 1)
174+ buf[(area.right() - 1, area.bottom() - 1)]
175175 .set_symbol(symbols.bottom_right)
176176 .set_style(self.border_style);
177177 }
178178 if self.borders.contains(Borders::RIGHT | Borders::TOP) {
179− buf.get_mut(area.right() - 1, area.top())
179+ buf[(area.right() - 1, area.top())]
180180 .set_symbol(symbols.top_right)
181181 .set_style(self.border_style);
182182 }
183183 if self.borders.contains(Borders::LEFT | Borders::BOTTOM) {
184− buf.get_mut(area.left(), area.bottom() - 1)
184+ buf[(area.left(), area.bottom() - 1)]
185185 .set_symbol(symbols.bottom_left)
186186 .set_style(self.border_style);
187187 }
188188 if self.borders.contains(Borders::LEFT | Borders::TOP) {
189− buf.get_mut(area.left(), area.top())
189+ buf[(area.left(), area.top())]
190190 .set_symbol(symbols.top_left)
191191 .set_style(self.border_style);
192192 }
helix-tui/src/widgets/paragraph.rs+1−1
@@ -176,7 +176,7 @@ impl<'a> Widget for Paragraph<'a> {
176176 if y >= self.scroll.0 {
177177 let mut x = get_line_offset(current_line_width, text_area.width, self.alignment);
178178 for StyledGrapheme { symbol, style } in current_line {
179− buf.get_mut(text_area.left() + x, text_area.top() + y - self.scroll.0)
179+ buf[(text_area.left() + x, text_area.top() + y - self.scroll.0)]
180180 .set_symbol(if symbol.is_empty() {
181181 // If the symbol is empty, the last char which rendered last time will
182182 // leave on the line. It's a quick fix.
helix-view/src/graphics.rs+4−4
@@ -323,7 +323,7 @@ impl FromStr for Modifier {
323323/// ];
324324/// let mut buffer = Buffer::empty(Rect::new(0, 0, 1, 1));
325325/// for style in &styles {
326−/// buffer.get_mut(0, 0).set_style(*style);
326+/// buffer[(0, 0)].set_style(*style);
327327/// }
328328/// assert_eq!(
329329/// Style {
@@ -332,7 +332,7 @@ impl FromStr for Modifier {
332332/// add_modifier: Modifier::BOLD,
333333/// sub_modifier: Modifier::empty(),
334334/// },
335−/// buffer.get(0, 0).style(),
335+/// buffer[(0, 0)].style(),
336336/// );
337337/// ```
338338///
@@ -348,7 +348,7 @@ impl FromStr for Modifier {
348348/// ];
349349/// let mut buffer = Buffer::empty(Rect::new(0, 0, 1, 1));
350350/// for style in &styles {
351−/// buffer.get_mut(0, 0).set_style(*style);
351+/// buffer[(0, 0)].set_style(*style);
352352/// }
353353/// assert_eq!(
354354/// Style {
@@ -357,7 +357,7 @@ impl FromStr for Modifier {
357357/// add_modifier: Modifier::empty(),
358358/// sub_modifier: Modifier::empty(),
359359/// },
360−/// buffer.get(0, 0).style(),
360+/// buffer[(0, 0)].style(),
361361/// );
362362/// ```
363363#[derive(Debug, Clone, Copy, PartialEq)]