From 4bd87d04962989f47636be9b8279b98ac85fcd5e Mon Sep 17 00:00:00 2001 From: Maxim Zhiburt Date: Thu, 11 Jul 2024 23:13:16 +0300 Subject: [PATCH] Fix unused space when truncation is used and header on border is configured (#13353) Somehow this logic was missed on my end. ( I mean I was not even thinking about it in original patch :smile: ) Please recheck Added a regression test too. close #13336 cc: @fdncred --- crates/nu-command/tests/commands/table.rs | 6 ++ crates/nu-table/src/table.rs | 89 ++++++++++++++++++----- 2 files changed, 76 insertions(+), 19 deletions(-) diff --git a/crates/nu-command/tests/commands/table.rs b/crates/nu-command/tests/commands/table.rs index 16d5eb54ab..a4869f6bfa 100644 --- a/crates/nu-command/tests/commands/table.rs +++ b/crates/nu-command/tests/commands/table.rs @@ -2895,3 +2895,9 @@ fn table_kv_header_on_separator_trim_algorithm() { let actual = nu!("$env.config.table.header_on_separator = true; {key1: '111111111111111111111111111111111111111111111111111111111111'} | table --width=60 --theme basic"); assert_eq!(actual.out, "+------+---------------------------------------------------+| key1 | 1111111111111111111111111111111111111111111111111 || | 11111111111 |+------+---------------------------------------------------+"); } + +#[test] +fn table_general_header_on_separator_trim_algorithm() { + let actual = nu!("$env.config.table.header_on_separator = true; [[a b]; ['11111111111111111111111111111111111111' 2] ] | table --width=20 --theme basic"); + assert_eq!(actual.out, "+-#-+----a-----+-b-+| 0 | 11111111 | 2 || | 11111111 | || | 11111111 | || | 11111111 | || | 111111 | |+---+----------+---+"); +} diff --git a/crates/nu-table/src/table.rs b/crates/nu-table/src/table.rs index d2cf8f4d63..0dbf1bda7e 100644 --- a/crates/nu-table/src/table.rs +++ b/crates/nu-table/src/table.rs @@ -255,7 +255,8 @@ fn draw_table( align_table(&mut table, alignments, with_index, with_header, with_footer); colorize_table(&mut table, styles, with_index, with_header, with_footer); - let width_ctrl = TableWidthCtrl::new(widths, cfg, termwidth); + let pad = indent.0 + indent.1; + let width_ctrl = TableWidthCtrl::new(widths, cfg, termwidth, pad); if with_header && border_header { set_border_head(&mut table, with_footer, width_ctrl); @@ -336,12 +337,18 @@ fn table_to_string(table: Table, termwidth: usize) -> Option { struct TableWidthCtrl { width: Vec, cfg: NuTableConfig, - max: usize, + width_max: usize, + pad: usize, } impl TableWidthCtrl { - fn new(width: Vec, cfg: NuTableConfig, max: usize) -> Self { - Self { width, cfg, max } + fn new(width: Vec, cfg: NuTableConfig, max: usize, pad: usize) -> Self { + Self { + width, + cfg, + width_max: max, + pad, + } } } @@ -354,19 +361,20 @@ impl TableOption, ColoredConfig> for ) { let total_width = get_total_width2(&self.width, cfg); - if total_width > self.max { + if total_width > self.width_max { let has_header = self.cfg.with_header && rec.count_rows() > 1; let trim_as_head = has_header && self.cfg.header_on_border; - TableTrim { - max: self.max, - strategy: self.cfg.trim, - width: self.width, + TableTrim::new( + self.width, + self.width_max, + self.cfg.trim, trim_as_head, - } + self.pad, + ) .change(rec, cfg, dim); - } else if self.cfg.expand && self.max > total_width { - Settings::new(SetDimensions(self.width), Width::increase(self.max)) + } else if self.cfg.expand && self.width_max > total_width { + Settings::new(SetDimensions(self.width), Width::increase(self.width_max)) .change(rec, cfg, dim) } else { SetDimensions(self.width).change(rec, cfg, dim); @@ -376,9 +384,28 @@ impl TableOption, ColoredConfig> for struct TableTrim { width: Vec, + width_max: usize, strategy: TrimStrategy, - max: usize, trim_as_head: bool, + pad: usize, +} + +impl TableTrim { + fn new( + width: Vec, + width_max: usize, + strategy: TrimStrategy, + trim_as_head: bool, + pad: usize, + ) -> Self { + Self { + width, + strategy, + pad, + width_max, + trim_as_head, + } + } } impl TableOption, ColoredConfig> for TableTrim { @@ -395,13 +422,37 @@ impl TableOption, ColoredConfig> for return; } + // even though it's safe to trim columns by header there might be left unused space + // so we do use it if possible prioritizing left columns + let headers = recs[0].to_owned(); - for (i, head) in headers.into_iter().enumerate() { - let head_width = CellInfo::width(&head); + let headers_widths = headers + .iter() + .map(CellInfo::width) + .map(|v| v + self.pad) + .collect::>(); + + let min_width_use = get_total_width2(&headers_widths, cfg); + + let mut free_width = self.width_max.saturating_sub(min_width_use); + + for (i, head_width) in headers_widths.into_iter().enumerate() { + let head_width = head_width - self.pad; + let column_width = self.width[i] - self.pad; // safe to assume width is bigger then paddding + + let mut use_width = head_width; + if free_width > 0 { + // it's safe to assume that column_width is always bigger or equal to head_width + debug_assert!(column_width >= head_width); + + let additional_width = min(free_width, column_width - head_width); + free_width -= additional_width; + use_width += additional_width; + } match &self.strategy { TrimStrategy::Wrap { try_to_keep_words } => { - let mut wrap = Width::wrap(head_width); + let mut wrap = Width::wrap(use_width); if *try_to_keep_words { wrap = wrap.keep_words(); } @@ -411,7 +462,7 @@ impl TableOption, ColoredConfig> for .change(recs, cfg, dims); } TrimStrategy::Truncate { suffix } => { - let mut truncate = Width::truncate(self.max); + let mut truncate = Width::truncate(use_width); if let Some(suffix) = suffix { truncate = truncate.suffix(suffix).suffix_try_color(true); } @@ -428,7 +479,7 @@ impl TableOption, ColoredConfig> for match self.strategy { TrimStrategy::Wrap { try_to_keep_words } => { - let mut wrap = Width::wrap(self.max).priority::(); + let mut wrap = Width::wrap(self.width_max).priority::(); if try_to_keep_words { wrap = wrap.keep_words(); } @@ -436,7 +487,7 @@ impl TableOption, ColoredConfig> for Settings::new(SetDimensions(self.width), wrap).change(recs, cfg, dims); } TrimStrategy::Truncate { suffix } => { - let mut truncate = Width::truncate(self.max).priority::(); + let mut truncate = Width::truncate(self.width_max).priority::(); if let Some(suffix) = suffix { truncate = truncate.suffix(suffix).suffix_try_color(true); }