Skip to content

Commit

Permalink
fix(ui): avoid rendering area more than u16::MAX (#7867)
Browse files Browse the repository at this point in the history
### Description

Fix for #7843

### Testing Instructions

Verify that there is no longer a panic when persisting tasks with large
amount of lines e.g. `yes | head -n 1000`

(Waiting on user test to confirm fix works)


Closes TURBO-2735
  • Loading branch information
chris-olszewski committed Apr 15, 2024
1 parent ab248ec commit 7c5b0c4
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 10 deletions.
22 changes: 17 additions & 5 deletions crates/turborepo-ui/src/tui/pane.rs
Expand Up @@ -12,7 +12,7 @@ use ratatui::{
},
Terminal,
};
use tracing::debug;
use tracing::{debug, debug_span};
use tui_term::widget::PseudoTerminal;
use turborepo_vt100 as vt100;

Expand Down Expand Up @@ -188,13 +188,24 @@ impl<W> TerminalOutput<W> {
self.cols = cols;
}

#[tracing::instrument(skip(self, terminal))]
fn persist_screen<B: Backend>(
&mut self,
task_name: &str,
terminal: &mut Terminal<B>,
) -> Result<(), Error> {
let screen = self.parser.entire_screen();
let mut screen = self.parser.entire_screen();
let width = terminal.size()?.width;
// number of lines we can render before hitting the hidden area limit of ratatui
let max_lines = (u16::MAX / width).saturating_sub(2);
let (rows, _) = screen.size();
let lines_to_render = if rows <= (max_lines as usize) {
(rows + 2) as u16
} else {
screen.with_max_lines(Some(max_lines as usize));
max_lines + 2
};
debug!("rendering {} lines", lines_to_render);
let mut cursor = tui_term::widget::Cursor::default();
cursor.hide();
let title = format!(" {task_name} >");
Expand All @@ -203,9 +214,10 @@ impl<W> TerminalOutput<W> {
.title(title.as_str())
.title(Title::from(title.as_str()).position(Position::Bottom));
let term = PseudoTerminal::new(&screen).cursor(cursor).block(block);
terminal.insert_before((rows as u16).saturating_add(2), |buf| {
term.render(buf.area, buf)
})?;
let span = debug_span!("insert before");
let _guard = span.enter();
terminal.insert_before(lines_to_render, |buf| term.render(buf.area, buf))?;
drop(_guard);
self.has_been_persisted = true;

Ok(())
Expand Down
39 changes: 35 additions & 4 deletions crates/turborepo-vt100/src/entire_screen.rs
@@ -1,21 +1,52 @@
pub struct EntireScreen<'a>(pub(crate) &'a crate::Screen);
pub struct EntireScreen<'a> {
screen: &'a crate::Screen,
// If present, screen will be truncated to only display lines that fit in those cells
max_lines: Option<usize>,
size: (usize, u16),
}

impl<'a> EntireScreen<'a> {
#[must_use]
pub fn new(screen: &'a crate::Screen) -> Self {
Self {
size: screen.grid().size_with_contents(),
screen,
max_lines: None,
}
}

pub fn with_max_lines(&mut self, max_lines: Option<usize>) {
self.max_lines = max_lines;
}

#[must_use]
pub fn cell(&self, row: u16, col: u16) -> Option<&crate::Cell> {
self.0.grid().all_row(row).and_then(|r| r.get(col))
match self.max_lines {
// We need to do some trimming
Some(max_lines) if self.size().0 > max_lines => {
// in this case we fuck ourselves :) HARD
let (height, _) = self.size();
// Skip over these
let lines_to_cut = (height - max_lines) as u16;

Check warning on line 30 in crates/turborepo-vt100/src/entire_screen.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust clippy

casting `usize` to `u16` may truncate the value

Check warning on line 30 in crates/turborepo-vt100/src/entire_screen.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust clippy

using a potentially dangerous silent `as` conversion
self.screen
.grid()
.all_row(lines_to_cut + row)
.and_then(|r| r.get(col))
}
_ => self.screen.grid().all_row(row).and_then(|r| r.get(col)),
}
}

#[must_use]
pub fn contents(&self) -> String {
let mut s = String::new();
self.0.grid().write_full_contents(&mut s);
self.screen.grid().write_full_contents(&mut s);
s
}

/// Size required to render all contents
#[must_use]
pub fn size(&self) -> (usize, u16) {
self.0.grid().size_with_contents()
self.size
}
}
2 changes: 1 addition & 1 deletion crates/turborepo-vt100/src/parser.rs
Expand Up @@ -62,7 +62,7 @@ impl Parser {
/// terminal state where all contents including scrollback and displayed.
#[must_use]
pub fn entire_screen(&self) -> crate::EntireScreen {
crate::EntireScreen(self.screen())
crate::EntireScreen::new(self.screen())
}
}

Expand Down

0 comments on commit 7c5b0c4

Please sign in to comment.