From 8deecc0137d41ca84d1896c366b5921b7c3a9f2c Mon Sep 17 00:00:00 2001 From: Maxim Zhiburt Date: Wed, 22 Feb 2023 21:35:45 +0300 Subject: [PATCH] table --collapse dont do truncation return message instead (#8172) Reverts #8042 I've just noticed that #8042 was merged, but I didn't addressed your @fdncred last comment. This PR reverts #8042 and returns a message in cases where we need truncation/wrapping. https://github.com/nushell/nushell/blob/157b7e0b603c43d8e4f3bd0b028218d1e1d29114/crates/nu-command/tests/commands/table.rs#L234-L240 Signed-off-by: Maxim Zhiburt --- crates/nu-command/src/viewers/table.rs | 25 +---- crates/nu-command/tests/commands/table.rs | 8 ++ crates/nu-table/src/nu_protocol_table.rs | 122 +++------------------- 3 files changed, 25 insertions(+), 130 deletions(-) diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 67c1cd712..1119617bc 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -354,17 +354,9 @@ fn build_collapsed_table( colorize_value(&mut value, config, style_computer); let theme = load_theme_from_config(config); - let table = nu_table::NuTable::new( - value, - true, - term_width, - config, - style_computer, - &theme, - false, - ); + let table = nu_table::NuTable::new(value, true, config, style_computer, &theme, false); - let table = table.draw(); + let table = table.draw(term_width); Ok(table) } @@ -1674,17 +1666,10 @@ impl PagingTableCreator { colorize_value(&mut value, config, &style_computer); - let table = nu_table::NuTable::new( - value, - true, - term_width, - config, - &style_computer, - &theme, - need_footer, - ); + let table = + nu_table::NuTable::new(value, true, config, &style_computer, &theme, need_footer); - Ok(table.draw()) + Ok(table.draw(term_width)) } fn build_general(&mut self, batch: &[Value]) -> Result, ShellError> { diff --git a/crates/nu-command/tests/commands/table.rs b/crates/nu-command/tests/commands/table.rs index 8f3b3e530..8736a58fd 100644 --- a/crates/nu-command/tests/commands/table.rs +++ b/crates/nu-command/tests/commands/table.rs @@ -231,6 +231,14 @@ fn table_collapse_hearts() { ); } +#[test] +fn table_collapse_doesnot_support_width_control() { + let actual = nu!( + r#"[[a]; [11111111111111111111111111111111111111111111111111111111111111111111111111111111]] | table --collapse"# + ); + assert_eq!(actual.out, "Couldn't fit table into 80 columns!"); +} + #[test] fn table_expand_0() { let actual = nu!(r#"[[a b, c]; [1 2 3] [4 5 [1 2 3]]] | table --expand"#); diff --git a/crates/nu-table/src/nu_protocol_table.rs b/crates/nu-table/src/nu_protocol_table.rs index e5174a195..21e05fb6f 100644 --- a/crates/nu-table/src/nu_protocol_table.rs +++ b/crates/nu-table/src/nu_protocol_table.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use crate::{string_truncate, string_width, Alignments, TableTheme}; +use crate::{string_width, Alignments, TableTheme}; use nu_color_config::StyleComputer; use nu_protocol::{Config, Span, Value}; use tabled::{ @@ -17,14 +17,13 @@ use serde_json::Value as Json; /// /// It doesn't support alignment and a proper width control. pub struct NuTable { - inner: Option, + inner: String, } impl NuTable { pub fn new( value: Value, collapse: bool, - termwidth: usize, config: &Config, style_computer: &StyleComputer, theme: &TableTheme, @@ -34,44 +33,19 @@ impl NuTable { load_theme(&mut table, style_computer, theme); let cfg = table.get_config().clone(); - let mut val = nu_protocol_value_to_json(value, config, with_footer); + let val = nu_protocol_value_to_json(value, config, with_footer); + let table = build_table(val, cfg, collapse); - let table = build_table(val.clone(), cfg.clone(), collapse); - let table_width = string_width(&table); - - if table_width > termwidth { - // Doing a soffisticated width control would require some deep rooted changes. - // (Which is might neessery to be done) - // - // Instead we peek biggest cells 1 by 1 and truncating them. - - loop { - match get_biggest_value(&mut val) { - Some((value, width)) => { - if width == 0 { - return Self { inner: None }; - } - - let need_to_cut = width - 1; - __truncate_value(value, need_to_cut); - - let table = build_table(val.clone(), cfg.clone(), collapse); - let table_width = string_width(&table); - - if table_width <= termwidth { - return Self { inner: Some(table) }; - } - } - None => return Self { inner: None }, - } - } - } - - Self { inner: Some(table) } + Self { inner: table } } - pub fn draw(&self) -> Option { - self.inner.clone() + pub fn draw(&self, termwidth: usize) -> Option { + let table_width = string_width(&self.inner); + if table_width > termwidth { + None + } else { + Some(self.inner.clone()) + } } } @@ -251,75 +225,3 @@ where .with(AlignmentStrategy::PerLine), ); } - -fn __truncate_value(value: &mut Json, width: usize) { - match value { - Json::Null => *value = Json::String(string_truncate("null", width)), - Json::Bool(b) => { - let val = if *b { "true" } else { "false" }; - - *value = Json::String(string_truncate(val, width)); - } - Json::Number(n) => { - let n = n.to_string(); - *value = Json::String(string_truncate(&n, width)); - } - Json::String(s) => { - *value = Json::String(string_truncate(s, width)); - } - Json::Array(_) | Json::Object(_) => { - unreachable!("must never happen") - } - } -} - -fn get_biggest_value(value: &mut Json) -> Option<(&mut Json, usize)> { - match value { - Json::Null => Some((value, 4)), - Json::Bool(_) => Some((value, 4)), - Json::Number(n) => { - let width = n.to_string().len(); - Some((value, width)) - } - Json::String(s) => { - let width = string_width(s); - Some((value, width)) - } - Json::Array(arr) => { - if arr.is_empty() { - return None; - } - - let mut width = 0; - let mut index = 0; - for (i, value) in arr.iter_mut().enumerate() { - if let Some((_, w)) = get_biggest_value(value) { - if w >= width { - index = i; - width = w; - } - } - } - - get_biggest_value(&mut arr[index]) - } - Json::Object(map) => { - if map.is_empty() { - return None; - } - - let mut width = 0; - let mut index = String::new(); - for (key, mut value) in map.clone() { - if let Some((_, w)) = get_biggest_value(&mut value) { - if w >= width { - index = key; - width = w; - } - } - } - - get_biggest_value(&mut map[&index]) - } - } -}