Refactorings

crates/nu-table/src/common.rs

@@ -1,9 +1,11 @@
+use nu_color_config::{Alignment, StyleComputer, TextStyle};
+use nu_protocol::{Config, FooterMode, ShellError, Span, TableMode, TrimStrategy, Value};
+
+use terminal_size::{terminal_size, Height, Width};
+
 use crate::{
     clean_charset, colorize_space_str, string_wrap, NuTableConfig, TableOutput, TableTheme,
 };
-use nu_color_config::{Alignment, StyleComputer, TextStyle};
-use nu_protocol::{Config, FooterMode, ShellError, Span, TableMode, TrimStrategy, Value};
-use terminal_size::{terminal_size, Height, Width};
 
 pub type NuText = (String, TextStyle);
 pub type TableResult = Result<Option<TableOutput>, ShellError>;
@@ -37,15 +39,20 @@ pub fn create_nu_table_config(
     }
 }
 
-pub fn nu_value_to_string_colored(val: &Value, cfg: &Config, style: &StyleComputer) -> String {
-    let (mut text, value_style) = nu_value_to_string(val, cfg, style);
-    if let Some(color) = value_style.color_style {
+pub fn nu_value_to_string_colored(val: &Value, cfg: &Config, comp: &StyleComputer) -> String {
+    let (mut text, style) = nu_value_to_string(val, cfg, comp);
+
+    let is_string = matches!(val, Value::String { .. });
+    if is_string {
+        text = clean_charset(&text);
+    }
+
+    if let Some(color) = style.color_style {
         text = color.paint(text).to_string();
     }
 
-    if matches!(val, Value::String { .. }) {
-        text = clean_charset(&text);
-        colorize_space_str(&mut text, style);
+    if is_string {
+        colorize_space_str(&mut text, comp);
     }
 
     text

crates/nu-table/src/lib.rs

@@ -1,4 +1,5 @@
 #![doc = include_str!("../README.md")]
+
 mod table;
 mod table_theme;
 mod types;

crates/nu-table/src/table.rs

@@ -264,7 +264,7 @@ fn draw_table(
 ) -> Option<String> {
     let structure = get_table_structure(&data, &cfg);
     let sep_color = cfg.split_color;
-    let border_header = cfg.header_on_border;
+    let border_header = structure.with_header && cfg.header_on_border;
 
     let data: Vec<Vec<_>> = data.into();
     let mut table = Builder::from(data).build();
@@ -277,13 +277,7 @@ fn draw_table(
     let pad = indent.0 + indent.1;
     let width_ctrl = WidthCtrl::new(widths, cfg, termwidth, pad);
 
-    let need_border_header = structure.with_header && border_header;
-    adjust_table(
-        &mut table,
-        width_ctrl,
-        need_border_header,
-        structure.with_footer,
-    );
+    adjust_table(&mut table, width_ctrl, border_header, structure.with_footer);
 
     table_to_string(table, termwidth)
 }

crates/nu-table/src/types/collapse.rs

@@ -1,10 +1,11 @@
+use nu_color_config::StyleComputer;
+use nu_protocol::{Config, Record, Value};
+use nu_utils::SharedCow;
+
 use crate::{
     common::{get_index_style, load_theme, nu_value_to_string_clean},
     StringResult, TableOpts, UnstructuredTable,
 };
-use nu_color_config::StyleComputer;
-use nu_protocol::{Config, Record, Value};
-use nu_utils::SharedCow;
 
 pub struct CollapsedTable;
 

crates/nu-table/src/types/expanded.rs

@@ -1,3 +1,11 @@
+use std::{cmp::max, collections::HashMap};
+
+use nu_color_config::{Alignment, StyleComputer, TextStyle};
+use nu_engine::column::get_columns;
+use nu_protocol::{Config, Record, ShellError, Span, Value};
+
+use tabled::grid::config::Position;
+
 use crate::{
     common::{
         check_value, create_nu_table_config, error_sign, get_header_style, get_index_style,
@@ -8,11 +16,6 @@ use crate::{
     types::has_index,
     NuRecordsValue, NuTable, TableOpts, TableOutput,
 };
-use nu_color_config::{Alignment, StyleComputer, TextStyle};
-use nu_engine::column::get_columns;
-use nu_protocol::{Config, Record, ShellError, Span, Value};
-use std::{cmp::max, collections::HashMap};
-use tabled::grid::config::Position;
 
 #[derive(Debug, Clone)]
 pub struct ExpandedTable {
@@ -31,12 +34,14 @@ impl ExpandedTable {
     }
 
     pub fn build_value(self, item: &Value, opts: TableOpts<'_>) -> NuText {
-        let cell = expanded_table_entry2(item, Cfg { opts, format: self });
+        let cfg = Cfg { opts, format: self };
+        let cell = expanded_table_entry2(item, cfg);
         (cell.text, cell.style)
     }
 
     pub fn build_map(self, record: &Record, opts: TableOpts<'_>) -> StringResult {
-        expanded_table_kv(record, Cfg { opts, format: self }).map(|cell| cell.map(|cell| cell.text))
+        let cfg = Cfg { opts, format: self };
+        expanded_table_kv(record, cfg).map(|cell| cell.map(|cell| cell.text))
     }
 
     pub fn build_list(self, vals: &[Value], opts: TableOpts<'_>) -> StringResult {
@@ -387,7 +392,7 @@ fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
     for (key, value) in record {
         cfg.opts.signals.check(cfg.opts.span)?;
 
-        let cell = match expand_table_value(value, value_width, &cfg)? {
+        let cell = match expand_value(value, value_width, &cfg)? {
             Some(val) => val,
             None => return Ok(None),
         };
@@ -421,14 +426,11 @@ fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
 }
 
 // the flag is used as an optimization to not do `value.lines().count()` search.
-fn expand_table_value(value: &Value, value_width: usize, cfg: &Cfg<'_>) -> CellResult {
+fn expand_value(value: &Value, value_width: usize, cfg: &Cfg<'_>) -> CellResult {
     let is_limited = matches!(cfg.format.expand_limit, Some(0));
     if is_limited {
-        return Ok(Some(CellOutput::clean(
-            value_to_string_clean(value, cfg),
-            1,
-            false,
-        )));
+        let value = value_to_string_clean(value, cfg);
+        return Ok(Some(CellOutput::clean(value, 1, false)));
     }
 
     let span = value.span();
@@ -448,40 +450,32 @@ fn expand_table_value(value: &Value, value_width: usize, cfg: &Cfg<'_>) -> CellR
                 }
                 None => {
                     // it means that the list is empty
-                    Ok(Some(CellOutput::text(value_to_wrapped_string(
-                        value,
-                        cfg,
-                        value_width,
-                    ))))
+                    let value = value_to_wrapped_string(value, cfg, value_width);
+                    Ok(Some(CellOutput::text(value)))
                 }
             }
         }
         Value::Record { val: record, .. } => {
             if record.is_empty() {
                 // Like list case return styled string instead of empty value
-                return Ok(Some(CellOutput::text(value_to_wrapped_string(
-                    value,
-                    cfg,
-                    value_width,
-                ))));
+                let value = value_to_wrapped_string(value, cfg, value_width);
+                return Ok(Some(CellOutput::text(value)));
             }
 
             let inner_cfg = update_config(dive_options(cfg, span), value_width);
             let result = expanded_table_kv(record, inner_cfg)?;
             match result {
                 Some(result) => Ok(Some(CellOutput::clean(result.text, result.size, true))),
-                None => Ok(Some(CellOutput::text(value_to_wrapped_string(
-                    value,
-                    cfg,
-                    value_width,
-                )))),
+                None => {
+                    let value = value_to_wrapped_string(value, cfg, value_width);
+                    Ok(Some(CellOutput::text(value)))
+                }
             }
         }
-        _ => Ok(Some(CellOutput::text(value_to_wrapped_string_clean(
-            value,
-            cfg,
-            value_width,
-        )))),
+        _ => {
+            let value = value_to_wrapped_string_clean(value, cfg, value_width);
+            Ok(Some(CellOutput::text(value)))
+        }
     }
 }
 

crates/nu-table/src/types/general.rs

@@ -1,3 +1,7 @@
+use nu_color_config::TextStyle;
+use nu_engine::column::get_columns;
+use nu_protocol::{Config, Record, ShellError, Value};
+
 use super::has_index;
 use crate::{
     clean_charset, colorize_space,
@@ -7,9 +11,6 @@ use crate::{
     },
     NuRecordsValue, NuTable, StringResult, TableOpts, TableOutput, TableResult,
 };
-use nu_color_config::TextStyle;
-use nu_engine::column::get_columns;
-use nu_protocol::{Config, Record, ShellError, Value};
 
 pub struct JustTable;
 

crates/nu-table/src/unstructured_table.rs

@@ -15,7 +15,7 @@ use crate::{is_color_empty, string_width, string_wrap, TableTheme};
 
 /// UnstructuredTable has a recursive table representation of nu_protocol::Value.
 ///
-/// It doesn't support alignment and a proper width control (allthough it's possible to achieve).
+/// It doesn't support alignment and a proper width control (although it's possible to achieve).
 pub struct UnstructuredTable {
     value: TableValue,
 }

crates/nu-table/src/util.rs

@@ -23,6 +23,11 @@ pub fn string_wrap(text: &str, width: usize, keep_words: bool) -> String {
         return String::new();
     }
 
+    let text_width = string_width(text);
+    if text_width <= width {
+        return text.to_owned();
+    }
+
     Wrap::wrap(text, width, keep_words)
 }
 
@@ -46,7 +51,7 @@ pub fn clean_charset(text: &str) -> String {
     // allocating at least the text size,
     // in most cases the buf will be a copy of text anyhow.
     //
-    // but yes sometimes we will alloc more then nessary.
+    // but yes sometimes we will alloc more then necessary.
     // We could shrink it but...it will be another realloc which make no scense.
     let mut buf = String::with_capacity(text.len());