From 5417c89387b67c3192ae9043473b556cd669ee15 Mon Sep 17 00:00:00 2001 From: Maxim Zhiburt Date: Wed, 17 Jul 2024 00:21:42 +0300 Subject: [PATCH] Fix issue with truncation when head on border is used (#13389) close #13365 @fdncred thanks for reproducible hopefully there won't be issues with it after this one :sweat_smile: --- crates/nu-command/tests/commands/table.rs | 6 ++ crates/nu-table/src/table.rs | 122 ++++++++++++---------- 2 files changed, 70 insertions(+), 58 deletions(-) diff --git a/crates/nu-command/tests/commands/table.rs b/crates/nu-command/tests/commands/table.rs index a4869f6bfa..272369a1c9 100644 --- a/crates/nu-command/tests/commands/table.rs +++ b/crates/nu-command/tests/commands/table.rs @@ -2901,3 +2901,9 @@ 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 | |+---+----------+---+"); } + +#[test] +fn table_general_header_on_separator_issue1() { + let actual = nu!("$env.config.table.header_on_separator = true; [['Llll oo Bbbbbbbb' 'Bbbbbbbb Aaaa' Nnnnnn Ggggg 'Xxxxx Llllllll #' Bbb 'Pppp Ccccc' 'Rrrrrrrr Dddd' Rrrrrr 'Rrrrrr Ccccc II' 'Rrrrrr Ccccc Ppppppp II' 'Pppppp Dddddddd Tttt' 'Pppppp Dddddddd Dddd' 'Rrrrrrrrr Trrrrrr' 'Pppppp Ppppp Dddd' 'Ppppp Dddd' Hhhh]; [RRRRRRR FFFFFFFF UUUU VV 202407160001 BBB 1 '7/16/2024' '' AAA-1111 AAA-1111-11 '7 YEARS' 2555 'RRRRRRRR DDDD' '7/16/2031' '7/16/2031' NN]] | table --width=87 --theme basic"); + assert_eq!(actual.out, "+-#-+-Llll oo Bbbbbbbb-+-Bbbbbbbb Aaaa-+-Nnnnnn-+-Ggggg-+-Xxxxx Llllllll #-+-...-+| 0 | RRRRRRR | FFFFFFFF | UUUU | VV | 202407160001 | ... |+---+------------------+---------------+--------+-------+------------------+-----+"); +} diff --git a/crates/nu-table/src/table.rs b/crates/nu-table/src/table.rs index 0dbf1bda7e..bc23e48248 100644 --- a/crates/nu-table/src/table.rs +++ b/crates/nu-table/src/table.rs @@ -418,62 +418,7 @@ impl TableOption, ColoredConfig> for // we already must have been estimated that it's safe to do. // and all dims will be suffitient if self.trim_as_head { - if recs.is_empty() { - 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(); - 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(use_width); - if *try_to_keep_words { - wrap = wrap.keep_words(); - } - - Modify::new(Columns::single(i)) - .with(wrap) - .change(recs, cfg, dims); - } - TrimStrategy::Truncate { suffix } => { - let mut truncate = Width::truncate(use_width); - if let Some(suffix) = suffix { - truncate = truncate.suffix(suffix).suffix_try_color(true); - } - - Modify::new(Columns::single(i)) - .with(truncate) - .change(recs, cfg, dims); - } - } - } - + trim_as_header(recs, cfg, dims, self); return; } @@ -498,6 +443,67 @@ impl TableOption, ColoredConfig> for } } +fn trim_as_header( + recs: &mut VecRecords>, + cfg: &mut ColoredConfig, + dims: &mut CompleteDimensionVecRecords, + trim: TableTrim, +) { + if recs.is_empty() { + return; + } + + let headers = recs[0].to_owned(); + let headers_widths = headers + .iter() + .map(CellInfo::width) + .map(|v| v + trim.pad) + .collect::>(); + let min_width_use = get_total_width2(&headers_widths, cfg); + let mut free_width = trim.width_max.saturating_sub(min_width_use); + + // 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 + + for (i, head_width) in headers_widths.into_iter().enumerate() { + let head_width = head_width - trim.pad; + let column_width = trim.width[i] - trim.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 &trim.strategy { + TrimStrategy::Wrap { try_to_keep_words } => { + let mut wrap = Width::wrap(use_width); + if *try_to_keep_words { + wrap = wrap.keep_words(); + } + + Modify::new(Columns::single(i)) + .with(wrap) + .change(recs, cfg, dims); + } + TrimStrategy::Truncate { suffix } => { + let mut truncate = Width::truncate(use_width); + if let Some(suffix) = suffix { + truncate = truncate.suffix(suffix).suffix_try_color(true); + } + + Modify::new(Columns::single(i)) + .with(truncate) + .change(recs, cfg, dims); + } + } + } +} + fn align_table( table: &mut Table, alignments: Alignments, @@ -793,14 +799,14 @@ fn truncate_columns_by_head( let mut truncate_pos = 0; for (i, column_header) in head.iter().enumerate() { let column_header_width = Cell::width(column_header); - width += column_header_width; + width += column_header_width + pad; if i > 0 { width += has_vertical as usize; } if width >= termwidth { - width -= column_header_width + (i > 0 && has_vertical) as usize; + width -= column_header_width + (i > 0 && has_vertical) as usize + pad; break; }