From a68e38e59e6735c0a99139303b1609669d2c38da Mon Sep 17 00:00:00 2001 From: Florian Dehau Date: Sun, 1 Aug 2021 16:27:05 +0200 Subject: [PATCH] fix(table): use `Layout` in table column widths computation --- src/widgets/table.rs | 84 +++++++++++------------------------------- tests/widgets_table.rs | 44 +++++++++++++++++++--- 2 files changed, 60 insertions(+), 68 deletions(-) diff --git a/src/widgets/table.rs b/src/widgets/table.rs index b45eeb04..bb06f34f 100644 --- a/src/widgets/table.rs +++ b/src/widgets/table.rs @@ -1,16 +1,10 @@ use crate::{ buffer::Buffer, - layout::{Constraint, Rect}, + layout::{Constraint, Direction, Layout, Rect}, style::Style, text::Text, widgets::{Block, StatefulWidget, Widget}, }; -use cassowary::{ - strength::{MEDIUM, REQUIRED, WEAK}, - WeightedRelation::*, - {Expression, Solver}, -}; -use std::collections::HashMap; use unicode_width::UnicodeWidthStr; /// A [`Cell`] contains the [`Text`] to be displayed in a [`Row`] of a [`Table`]. @@ -273,69 +267,33 @@ impl<'a> Table<'a> { } fn get_columns_widths(&self, max_width: u16, has_selection: bool) -> Vec { - let mut solver = Solver::new(); - let mut var_indices = HashMap::new(); - let mut ccs = Vec::new(); - let mut variables = Vec::new(); - for i in 0..self.widths.len() { - let var = cassowary::Variable::new(); - variables.push(var); - var_indices.insert(var, i); - } - let spacing_width = (variables.len() as u16).saturating_sub(1) * self.column_spacing; - let mut available_width = max_width.saturating_sub(spacing_width); + let mut constraints = Vec::with_capacity(self.widths.len() * 2 + 1); if has_selection { let highlight_symbol_width = self.highlight_symbol.map(|s| s.width() as u16).unwrap_or(0); - available_width = available_width.saturating_sub(highlight_symbol_width); + constraints.push(Constraint::Length(highlight_symbol_width)); } - for (i, constraint) in self.widths.iter().enumerate() { - ccs.push(variables[i] | GE(WEAK) | 0.); - ccs.push(match *constraint { - Constraint::Length(v) => variables[i] | EQ(MEDIUM) | f64::from(v), - Constraint::Percentage(v) => { - variables[i] | EQ(WEAK) | (f64::from(v * available_width) / 100.0) - } - Constraint::Ratio(n, d) => { - variables[i] - | EQ(WEAK) - | (f64::from(available_width) * f64::from(n) / f64::from(d)) - } - Constraint::Min(v) => variables[i] | GE(WEAK) | f64::from(v), - Constraint::Max(v) => variables[i] | LE(WEAK) | f64::from(v), - }) + for constraint in self.widths { + constraints.push(*constraint); + constraints.push(Constraint::Length(self.column_spacing)); } - solver - .add_constraint( - variables - .iter() - .fold(Expression::from_constant(0.), |acc, v| acc + *v) - | LE(REQUIRED) - | f64::from(available_width), - ) - .unwrap(); - solver.add_constraints(&ccs).unwrap(); - let mut widths = vec![0; variables.len()]; - for &(var, value) in solver.fetch_changes() { - let index = var_indices[&var]; - let value = if value.is_sign_negative() { - 0 - } else { - value.round() as u16 - }; - widths[index] = value; + if !self.widths.is_empty() { + constraints.pop(); } - // Cassowary could still return columns widths greater than the max width when there are - // fixed length constraints that cannot be satisfied. Therefore, we clamp the widths from - // left to right. - let mut available_width = max_width; - for w in &mut widths { - *w = available_width.min(*w); - available_width = available_width - .saturating_sub(*w) - .saturating_sub(self.column_spacing); + let mut chunks = Layout::default() + .direction(Direction::Horizontal) + .constraints(constraints) + .expand_to_fill(false) + .split(Rect { + x: 0, + y: 0, + width: max_width, + height: 1, + }); + if has_selection { + chunks.remove(0); } - widths + chunks.iter().step_by(2).map(|c| c.width).collect() } fn get_row_bounds( diff --git a/tests/widgets_table.rs b/tests/widgets_table.rs index f71169c1..a790a5a5 100644 --- a/tests/widgets_table.rs +++ b/tests/widgets_table.rs @@ -354,12 +354,12 @@ fn widgets_table_columns_widths_can_use_mixed_constraints() { ], Buffer::with_lines(vec![ "┌────────────────────────────┐", - "│Hea Head2 Hea│", + "│Hea Head2 He │", "│ │", - "│Row Row12 Row│", - "│Row Row22 Row│", - "│Row Row32 Row│", - "│Row Row42 Row│", + "│Row Row12 Ro │", + "│Row Row22 Ro │", + "│Row Row32 Ro │", + "│Row Row42 Ro │", "│ │", "│ │", "└────────────────────────────┘", @@ -715,3 +715,37 @@ fn widgets_table_should_render_even_if_empty() { terminal.backend().assert_buffer(&expected); } + +#[test] +fn widgets_table_columns_dont_panic() { + let test_case = |state: &mut TableState, table: Table, width: u16| { + let backend = TestBackend::new(width, 8); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|f| { + let size = f.size(); + f.render_stateful_widget(table, size, state); + }) + .unwrap(); + }; + + // based on https://github.com/fdehau/tui-rs/issues/470#issuecomment-852562848 + let table1_width = 98; + let table1 = Table::new(vec![Row::new(vec!["r1", "r2", "r3", "r4"])]) + .header(Row::new(vec!["h1", "h2", "h3", "h4"])) + .block(Block::default().borders(Borders::ALL)) + .highlight_symbol(">> ") + .column_spacing(1) + .widths(&[ + Constraint::Percentage(15), + Constraint::Percentage(15), + Constraint::Percentage(25), + Constraint::Percentage(45), + ]); + + let mut state = TableState::default(); + + // select first, which would cause a panic before fix + state.select(Some(0)); + test_case(&mut state, table1.clone(), table1_width); +}