More refactorings

crates/nu-command/src/viewers/table.rs

@@ -11,8 +11,8 @@ use nu_protocol::{
     ByteStream, Config, DataSource, ListStream, PipelineMetadata, Signals, TableMode, ValueIterator,
 };
 use nu_table::{
-    common::create_nu_table_config, CollapsedTable, ExpandedTable, JustTable, NuTable, NuTableCell,
+    common::create_nu_table_config, CollapsedTable, ExpandedTable, JustTable, NuRecordsValue,
-    StringResult, TableOpts, TableOutput,
+    NuTable, StringResult, TableOpts, TableOutput,
 };
 use nu_utils::get_ls_colors;
 use std::{
@@ -522,14 +522,13 @@ fn handle_record(
         }
     }
 
-    let indent = (config.table.padding.left, config.table.padding.right);
     let opts = TableOpts::new(
         &config,
         styles,
         input.engine_state.signals(),
         span,
         cfg.term_width,
-        indent,
+        config.table.padding,
         cfg.theme,
         cfg.index.unwrap_or(0),
         cfg.index.is_none(),
@@ -826,7 +825,7 @@ impl PagingTableCreator {
             self.engine_state.signals(),
             self.head,
             self.cfg.term_width,
-            (cfg.table.padding.left, cfg.table.padding.right),
+            cfg.table.padding,
             self.cfg.theme,
             self.cfg.index.unwrap_or(0) + self.row_offset,
             self.cfg.index.is_none(),
@@ -1084,11 +1083,11 @@ fn create_empty_placeholder(
         return String::new();
     }
 
-    let cell = NuTableCell::new(format!("empty {}", value_type_name));
+    let cell = NuRecordsValue::new(format!("empty {}", value_type_name));
     let data = vec![vec![cell]];
     let mut table = NuTable::from(data);
     table.set_data_style(TextStyle::default().dimmed());
-    let out = TableOutput::new(table, false, false, 1);
+    let out = TableOutput::from_table(table, false, false);
 
     let style_computer = &StyleComputer::from_config(engine_state, stack);
     let config = create_nu_table_config(&config, style_computer, &out, false, TableMode::default());

crates/nu-explore/src/nu_common/table.rs

@@ -36,7 +36,7 @@ fn try_build_map(
         signals,
         Span::unknown(),
         usize::MAX,
-        (config.table.padding.left, config.table.padding.right),
+        config.table.padding,
         config.table.mode,
         0,
         false,
@@ -63,7 +63,7 @@ fn try_build_list(
         signals,
         Span::unknown(),
         usize::MAX,
-        (config.table.padding.left, config.table.padding.right),
+        config.table.padding,
         config.table.mode,
         0,
         false,

crates/nu-protocol/src/config/mod.rs

@@ -21,7 +21,7 @@ pub use plugin_gc::{PluginGcConfig, PluginGcConfigs};
 pub use reedline::{CursorShapeConfig, EditBindings, NuCursorShape, ParsedKeybinding, ParsedMenu};
 pub use rm::RmConfig;
 pub use shell_integration::ShellIntegrationConfig;
-pub use table::{FooterMode, TableConfig, TableIndexMode, TableMode, TrimStrategy};
+pub use table::{FooterMode, TableConfig, TableIndent, TableIndexMode, TableMode, TrimStrategy};
 
 mod completions;
 mod datetime_format;

crates/nu-protocol/src/config/table.rs

@@ -277,6 +277,12 @@ pub struct TableIndent {
     pub right: usize,
 }
 
+impl TableIndent {
+    pub fn new(left: usize, right: usize) -> Self {
+        Self { left, right }
+    }
+}
+
 impl IntoValue for TableIndent {
     fn into_value(self, span: Span) -> Value {
         record! {

crates/nu-table/src/common.rs

@@ -54,7 +54,7 @@ pub fn nu_value_to_string_colored(val: &Value, cfg: &Config, style: &StyleComput
 pub fn nu_value_to_string(val: &Value, cfg: &Config, style: &StyleComputer) -> NuText {
     let float_precision = cfg.float_precision as usize;
     let text = val.to_abbreviated_string(cfg);
-    make_styled_string(style, text, Some(val), float_precision)
+    make_styled_value(text, val, float_precision, style)
 }
 
 pub fn nu_value_to_string_clean(val: &Value, cfg: &Config, style_comp: &StyleComputer) -> NuText {
@@ -66,7 +66,11 @@ pub fn nu_value_to_string_clean(val: &Value, cfg: &Config, style_comp: &StyleCom
 }
 
 pub fn error_sign(style_computer: &StyleComputer) -> (String, TextStyle) {
-    make_styled_string(style_computer, String::from("❎"), None, 0)
+    // Though holes are not the same as null, the closure for "empty" is passed a null anyway.
+
+    let text = String::from("❎");
+    let style = style_computer.compute("empty", &Value::nothing(Span::unknown()));
+    (text, TextStyle::with_style(Alignment::Center, style))
 }
 
 pub fn wrap_text(text: &str, width: usize, config: &Config) -> String {
@@ -122,36 +126,23 @@ pub fn get_empty_style(style_computer: &StyleComputer) -> NuText {
     )
 }
 
-fn make_styled_string(
+fn make_styled_value(
-    style_computer: &StyleComputer,
     text: String,
-    value: Option<&Value>, // None represents table holes.
+    value: &Value,
     float_precision: usize,
+    style_computer: &StyleComputer,
 ) -> NuText {
     match value {
-        Some(value) => {
+        Value::Float { .. } => {
-            match value {
+            // set dynamic precision from config
-                Value::Float { .. } => {
+            let precise_number = match convert_with_precision(&text, float_precision) {
-                    // set dynamic precision from config
+                Ok(num) => num,
-                    let precise_number = match convert_with_precision(&text, float_precision) {
+                Err(e) => e.to_string(),
-                        Ok(num) => num,
+            };
-                        Err(e) => e.to_string(),
+
-                    };
+            (precise_number, style_computer.style_primitive(value))
-                    (precise_number, style_computer.style_primitive(value))
-                }
-                _ => (text, style_computer.style_primitive(value)),
-            }
-        }
-        None => {
-            // Though holes are not the same as null, the closure for "empty" is passed a null anyway.
-            (
-                text,
-                TextStyle::with_style(
-                    Alignment::Center,
-                    style_computer.compute("empty", &Value::nothing(Span::unknown())),
-                ),
-            )
         }
+        _ => (text, style_computer.style_primitive(value)),
     }
 }
 
@@ -220,3 +211,10 @@ fn need_footer(config: &Config, count_records: u64) -> bool {
         }
     }
 }
+
+pub fn check_value(value: &Value) -> Result<(), ShellError> {
+    match value {
+        Value::Error { error, .. } => Err(*error.clone()),
+        _ => Ok(()),
+    }
+}

crates/nu-table/src/lib.rs

@@ -9,7 +9,7 @@ pub mod common;
 
 pub use common::{StringResult, TableResult};
 pub use nu_color_config::TextStyle;
-pub use table::{NuTable, NuTableCell, NuTableConfig};
+pub use table::{NuRecordsValue, NuTable, NuTableConfig};
 pub use table_theme::TableTheme;
 pub use types::{CollapsedTable, ExpandedTable, JustTable, TableOpts, TableOutput};
 pub use unstructured_table::UnstructuredTable;

crates/nu-table/src/table.rs

@@ -29,7 +29,10 @@ use tabled::{
     Table,
 };
 
-use crate::{convert_style, table_theme::TableTheme};
+use crate::{convert_style, is_color_empty, table_theme::TableTheme};
+
+pub type NuRecords = VecRecords<NuRecordsValue>;
+pub type NuRecordsValue = Text<String>;
 
 /// NuTable is a table rendering implementation.
 #[derive(Debug, Clone)]
@@ -40,27 +43,18 @@ pub struct NuTable {
     indent: (usize, usize),
 }
 
-pub type NuRecords = VecRecords<NuTableCell>;
-pub type NuTableCell = Text<String>;
-
 #[derive(Debug, Default, Clone)]
-struct Styles {
+struct TableConfig<Value> {
-    data: Color,
+    data: Value,
-    index: Color,
+    index: Value,
-    header: Color,
+    header: Value,
-    columns: HashMap<usize, Color>,
+    columns: HashMap<usize, Value>,
-    cells: HashMap<Position, Color>,
+    cells: HashMap<Position, Value>,
 }
 
-// todo: generic
+type Alignments = TableConfig<AlignmentHorizontal>;
-#[derive(Debug, Clone)]
+
-struct Alignments {
+type Styles = TableConfig<Color>;
-    data: AlignmentHorizontal,
-    index: AlignmentHorizontal,
-    header: AlignmentHorizontal,
-    columns: HashMap<usize, AlignmentHorizontal>,
-    cells: HashMap<Position, AlignmentHorizontal>,
-}
 
 impl NuTable {
     /// Creates an empty [`NuTable`] instance.
@@ -93,6 +87,14 @@ impl NuTable {
         self.data[pos.0][pos.1] = Text::new(text);
     }
 
+    pub fn insert_row(&mut self, index: usize, row: Vec<String>) {
+        let data = &mut self.data[index];
+
+        for (col, text) in row.into_iter().enumerate() {
+            data[col] = Text::new(text);
+        }
+    }
+
     pub fn set_column_style(&mut self, column: usize, style: TextStyle) {
         if let Some(style) = style.color_style {
             let style = convert_style(style);
@@ -411,8 +413,8 @@ impl TableOption<NuRecords, ColoredConfig, CompleteDimensionVecRecords<'_>> for
             return;
         }
 
-        let need_expantion = self.cfg.expand && self.width_max > total_width;
+        let need_expansion = self.cfg.expand && self.width_max > total_width;
-        if need_expantion {
+        if need_expansion {
             let opt = (SetDimensions(self.width), Width::increase(self.width_max));
             TableOption::<VecRecords<_>, _, _>::change(opt, rec, cfg, dim);
             return;
@@ -1242,7 +1244,3 @@ fn strip_color_from_row(row: usize) -> ModifyList<Row, FormatContent<fn(&str) ->
 
     Modify::new(Rows::single(row)).with(Format::content(foo))
 }
-
-fn is_color_empty(c: &Color) -> bool {
-    c.get_prefix().is_empty() && c.get_suffix().is_empty()
-}

crates/nu-table/src/table_theme.rs

@@ -10,7 +10,7 @@ pub struct TableTheme {
 }
 
 impl TableTheme {
-    pub fn new(base: impl Into<Theme>, full: impl Into<Theme>) -> Self {
+    fn new(base: impl Into<Theme>, full: impl Into<Theme>) -> Self {
         Self {
             base: base.into(),
             full: full.into(),
@@ -160,26 +160,6 @@ impl TableTheme {
         Self::new(Style::blank(), Style::blank())
     }
 
-    // pub fn has_top(&self) -> bool {
-    //     self.theme.borders_has_top()
-    // }
-
-    // pub fn has_left(&self) -> bool {
-    //     self.theme.borders_has_left()
-    // }
-
-    // pub fn has_right(&self) -> bool {
-    //     self.theme.borders_has_right()
-    // }
-
-    // pub fn has_inner(&self) -> bool {
-    //     self.has_inner
-    // }
-
-    // pub fn has_horizontals(&self) -> bool {
-    //     self.full_theme.get_borders().has_horizontal()
-    // }
-
     pub fn as_full(&self) -> &Theme {
         &self.full
     }

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

@@ -3,46 +3,36 @@ use crate::{
     StringResult, TableOpts, UnstructuredTable,
 };
 use nu_color_config::StyleComputer;
-use nu_protocol::{Config, Record, TableMode, Value};
+use nu_protocol::{Config, Record, Value};
 use nu_utils::SharedCow;
 
 pub struct CollapsedTable;
 
 impl CollapsedTable {
     pub fn build(value: Value, opts: TableOpts<'_>) -> StringResult {
-        collapsed_table(
+        collapsed_table(value, opts)
-            value,
-            opts.config,
-            opts.width,
-            opts.style_computer,
-            opts.mode,
-        )
     }
 }
 
-fn collapsed_table(
+fn collapsed_table(mut value: Value, opts: TableOpts<'_>) -> StringResult {
-    mut value: Value,
+    colorize_value(&mut value, opts.config, opts.style_computer);
-    config: &Config,
-    term_width: usize,
-    style_computer: &StyleComputer,
-    mode: TableMode,
-) -> StringResult {
-    colorize_value(&mut value, config, style_computer);
 
-    let theme = load_theme(mode);
+    let mut table = UnstructuredTable::new(value, opts.config);
-    let mut table = UnstructuredTable::new(value, config);
+
-    let is_empty = table.truncate(&theme, term_width);
+    let theme = load_theme(opts.mode);
+    let is_empty = table.truncate(&theme, opts.width);
     if is_empty {
         return Ok(None);
     }
 
-    let indent = (config.table.padding.left, config.table.padding.right);
+    let table = table.draw(&theme, opts.config.table.padding, opts.style_computer);
-    let table = table.draw(style_computer, &theme, indent);
 
     Ok(Some(table))
 }
 
 fn colorize_value(value: &mut Value, config: &Config, style_computer: &StyleComputer) {
+    // todo: Remove recursion?
+
     match value {
         Value::Record { ref mut val, .. } => {
             let style = get_index_style(style_computer);

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

@@ -1,12 +1,12 @@
 use crate::{
     common::{
-        create_nu_table_config, error_sign, get_header_style, get_index_style, load_theme,
+        check_value, create_nu_table_config, error_sign, get_header_style, get_index_style,
-        nu_value_to_string, nu_value_to_string_clean, nu_value_to_string_colored, wrap_text,
+        load_theme, nu_value_to_string, nu_value_to_string_clean, nu_value_to_string_colored,
-        NuText, StringResult, TableResult, INDEX_COLUMN_NAME,
+        wrap_text, NuText, StringResult, TableResult, INDEX_COLUMN_NAME,
     },
     string_width,
     types::has_index,
-    NuTable, NuTableCell, TableOpts, TableOutput,
+    NuRecordsValue, NuTable, TableOpts, TableOutput,
 };
 use nu_color_config::{Alignment, StyleComputer, TextStyle};
 use nu_engine::column::get_columns;
@@ -133,15 +133,12 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
 
     if with_index {
         if with_header {
-            data[0].push(NuTableCell::exact(String::from("#"), 1, vec![]));
+            data[0].push(NuRecordsValue::exact(String::from("#"), 1, vec![]));
         }
 
         for (row, item) in input.iter().enumerate() {
             cfg.opts.signals.check(cfg.opts.span)?;
-
+            check_value(item)?;
-            if let Value::Error { error, .. } = item {
-                return Err(*error.clone());
-            }
 
             let index = row + row_offset;
             let text = item
@@ -152,7 +149,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
                 .unwrap_or_else(|| index.to_string());
 
             let row = row + with_header as usize;
-            let value = NuTableCell::new(text);
+            let value = NuRecordsValue::new(text);
             data[row].push(value);
         }
 
@@ -177,10 +174,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
 
         for (row, item) in input.iter().enumerate() {
             cfg.opts.signals.check(cfg.opts.span)?;
-
+            check_value(item)?;
-            if let Value::Error { error, .. } = item {
-                return Err(*error.clone());
-            }
 
             let inner_cfg = update_config(cfg.clone(), available_width);
             let mut cell = expanded_table_entry2(item, inner_cfg);
@@ -195,7 +189,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
                 cell.text = wrap_text(&cell.text, available_width, cfg.opts.config);
             }
 
-            let value = NuTableCell::new(cell.text);
+            let value = NuRecordsValue::new(cell.text);
             data[row].push(value);
             data_styles.insert((row, with_index as usize), cell.style);
 
@@ -203,7 +197,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
         }
 
         let mut table = NuTable::from(data);
-        table.set_indent(cfg.opts.indent.0, cfg.opts.indent.1);
+        table.set_indent(cfg.opts.indent.left, cfg.opts.indent.right);
         table.set_index_style(get_index_style(cfg.opts.style_computer));
         set_data_styles(&mut table, data_styles);
 
@@ -266,10 +260,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
 
         for (row, item) in input.iter().enumerate() {
             cfg.opts.signals.check(cfg.opts.span)?;
-
+            check_value(item)?;
-            if let Value::Error { error, .. } = item {
-                return Err(*error.clone());
-            }
 
             let inner_cfg = update_config(cfg.clone(), available);
             let mut cell = expanded_table_entry(item, header.as_str(), inner_cfg);
@@ -285,14 +276,14 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
 
             column_width = max(column_width, value_width);
 
-            let value = NuTableCell::new(cell.text);
+            let value = NuRecordsValue::new(cell.text);
             data[row + 1].push(value);
             data_styles.insert((row + 1, col + with_index as usize), cell.style);
 
             column_rows = column_rows.saturating_add(cell.size);
         }
 
-        let head_cell = NuTableCell::new(header);
+        let head_cell = NuRecordsValue::new(header);
         data[0].push(head_cell);
 
         if column_width > available {
@@ -354,7 +345,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
 
         let is_last_column = widths.len() == count_columns;
         if !is_last_column {
-            let shift = NuTableCell::exact(String::from("..."), 3, vec![]);
+            let shift = NuRecordsValue::exact(String::from("..."), 3, vec![]);
             for row in &mut data {
                 row.push(shift.clone());
             }
@@ -366,7 +357,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
     let mut table = NuTable::from(data);
     table.set_index_style(get_index_style(cfg.opts.style_computer));
     table.set_header_style(get_header_style(cfg.opts.style_computer));
-    table.set_indent(cfg.opts.indent.0, cfg.opts.indent.1);
+    table.set_indent(cfg.opts.indent.left, cfg.opts.indent.right);
     set_data_styles(&mut table, data_styles);
 
     Ok(Some(TableOutput::new(table, true, with_index, rows_count)))
@@ -410,8 +401,8 @@ fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
             key.insert(0, '\n');
         }
 
-        let key = NuTableCell::new(key);
+        let key = NuRecordsValue::new(key);
-        let val = NuTableCell::new(cell.text);
+        let val = NuRecordsValue::new(cell.text);
         let row = vec![key, val];
 
         data.push(row);
@@ -421,7 +412,7 @@ fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
 
     let mut table = NuTable::from(data);
     table.set_index_style(get_key_style(&cfg));
-    table.set_indent(cfg.opts.indent.0, cfg.opts.indent.1);
+    table.set_indent(cfg.opts.indent.left, cfg.opts.indent.right);
 
     let out = TableOutput::new(table, false, true, count_rows);
 

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

@@ -2,10 +2,10 @@ use super::has_index;
 use crate::{
     clean_charset, colorize_space,
     common::{
-        create_nu_table_config, get_empty_style, get_header_style, get_index_style,
+        check_value, create_nu_table_config, get_empty_style, get_header_style, get_index_style,
         get_value_style, nu_value_to_string_colored, NuText, INDEX_COLUMN_NAME,
     },
-    NuTable, NuTableCell, StringResult, TableOpts, TableOutput, TableResult,
+    NuRecordsValue, NuTable, StringResult, TableOpts, TableOutput, TableResult,
 };
 use nu_color_config::TextStyle;
 use nu_engine::column::get_columns;
@@ -15,7 +15,7 @@ pub struct JustTable;
 
 impl JustTable {
     pub fn table(input: &[Value], opts: TableOpts<'_>) -> StringResult {
-        create_table(input, opts)
+        list_table(input, opts)
     }
 
     pub fn kv_table(record: &Record, opts: TableOpts<'_>) -> StringResult {
@@ -23,32 +23,35 @@ impl JustTable {
     }
 }
 
-fn create_table(input: &[Value], opts: TableOpts<'_>) -> Result<Option<String>, ShellError> {
+fn list_table(input: &[Value], opts: TableOpts<'_>) -> Result<Option<String>, ShellError> {
-    match table(input, &opts)? {
+    let mut out = match create_table(input, &opts)? {
-        Some(mut out) => {
+        Some(out) => out,
-            let left = opts.config.table.padding.left;
+        None => return Ok(None),
-            let right = opts.config.table.padding.right;
+    };
-            out.table.set_indent(left, right);
 
-            colorize_space(out.table.get_records_mut(), opts.style_computer);
+    out.table.set_indent(
+        opts.config.table.padding.left,
+        opts.config.table.padding.right,
+    );
 
-            let table_config =
+    colorize_space(out.table.get_records_mut(), opts.style_computer);
-                create_nu_table_config(opts.config, opts.style_computer, &out, false, opts.mode);
+
-            Ok(out.table.draw(table_config, opts.width))
+    let config = create_nu_table_config(opts.config, opts.style_computer, &out, false, opts.mode);
-        }
+    let table = out.table.draw(config, opts.width);
-        None => Ok(None),
+
-    }
+    Ok(table)
 }
 
 fn kv_table(record: &Record, opts: TableOpts<'_>) -> StringResult {
     let mut data = vec![Vec::with_capacity(2); record.len()];
+
     for ((column, value), row) in record.iter().zip(data.iter_mut()) {
         opts.signals.check(opts.span)?;
 
-        let value = nu_value_to_string_colored(value, opts.config, opts.style_computer);
+        let key = NuRecordsValue::new(column.to_string());
 
-        let key = NuTableCell::new(column.to_string());
+        let value = nu_value_to_string_colored(value, opts.config, opts.style_computer);
-        let value = NuTableCell::new(value);
+        let value = NuRecordsValue::new(value);
 
         row.push(key);
         row.push(value);
@@ -56,14 +59,12 @@ fn kv_table(record: &Record, opts: TableOpts<'_>) -> StringResult {
 
     let mut table = NuTable::from(data);
     table.set_index_style(TextStyle::default_field());
-    let count_rows = table.count_rows();
+    table.set_indent(
-
+        opts.config.table.padding.left,
-    let mut out = TableOutput::new(table, false, true, count_rows);
+        opts.config.table.padding.right,
-
+    );
-    let left = opts.config.table.padding.left;
-    let right = opts.config.table.padding.right;
-    out.table.set_indent(left, right);
 
+    let out = TableOutput::from_table(table, false, true);
     let table_config =
         create_nu_table_config(opts.config, opts.style_computer, &out, false, opts.mode);
     let table = out.table.draw(table_config, opts.width);
@@ -71,29 +72,33 @@ fn kv_table(record: &Record, opts: TableOpts<'_>) -> StringResult {
     Ok(table)
 }
 
-fn table(input: &[Value], opts: &TableOpts<'_>) -> TableResult {
+fn create_table(input: &[Value], opts: &TableOpts<'_>) -> TableResult {
     if input.is_empty() {
         return Ok(None);
     }
 
-    let mut headers = get_columns(input);
+    let headers = get_columns(input);
     let with_index = has_index(opts, &headers);
+    let with_header = !headers.is_empty();
     let row_offset = opts.index_offset;
 
-    let with_header = !headers.is_empty();
+    let table = match (with_header, with_index) {
-    if !with_header {
+        (true, true) => create_table_with_header_and_index(input, headers, row_offset, opts)?,
-        let table = to_table_with_no_header(input, with_index, row_offset, opts)?;
+        (true, false) => create_table_with_header(input, headers, opts)?,
-        let table = table.map(|table| {
+        (false, true) => create_table_with_no_header_and_index(input, row_offset, opts)?,
-            let count_rows = table.count_rows();
+        (false, false) => create_table_with_no_header(input, opts)?,
-            TableOutput::new(table, false, with_index, count_rows)
+    };
-        });
-        return Ok(table);
-    }
 
-    if with_header && with_index {
+    let table = table.map(|table| TableOutput::from_table(table, with_header, with_index));
-        headers.insert(0, "#".into());
-    }
 
+    Ok(table)
+}
+
+fn create_table_with_header(
+    input: &[Value],
+    headers: Vec<String>,
+    opts: &TableOpts<'_>,
+) -> Result<Option<NuTable>, ShellError> {
     // The header with the INDEX is removed from the table headers since
     // it is added to the natural table index
     let headers: Vec<_> = headers
@@ -101,80 +106,113 @@ fn table(input: &[Value], opts: &TableOpts<'_>) -> TableResult {
         .filter(|header| header != INDEX_COLUMN_NAME)
         .collect();
 
-    let table = to_table_with_header(input, &headers, with_index, row_offset, opts)?;
-    let table = table.map(|table| {
-        let count_rows = table.count_rows();
-        TableOutput::new(table, true, with_index, count_rows)
-    });
-
-    Ok(table)
-}
-
-fn to_table_with_header(
-    input: &[Value],
-    headers: &[String],
-    with_index: bool,
-    row_offset: usize,
-    opts: &TableOpts<'_>,
-) -> Result<Option<NuTable>, ShellError> {
     let count_rows = input.len() + 1;
     let count_columns = headers.len();
     let mut table = NuTable::new(count_rows, count_columns);
+
     table.set_header_style(get_header_style(opts.style_computer));
     table.set_index_style(get_index_style(opts.style_computer));
 
-    for (i, text) in headers.iter().enumerate() {
+    table.insert_row(0, headers.clone());
-        table.insert((0, i), text.to_owned());
-    }
 
     for (row, item) in input.iter().enumerate() {
         opts.signals.check(opts.span)?;
+        check_value(item)?;
 
-        if let Value::Error { error, .. } = item {
+        for (col, header) in headers.iter().enumerate() {
-            return Err(*error.clone());
-        }
-
-        if with_index {
-            let text = get_table_row_index(item, opts.config, row, row_offset);
-            table.insert((row + 1, 0), text);
-        }
-
-        let skip_index = usize::from(with_index);
-        for (col, header) in headers.iter().enumerate().skip(skip_index) {
             let (text, style) = get_string_value_with_header(item, header, opts);
 
-            table.insert((row + 1, col), text);
+            let pos = (row + 1, col);
-            table.insert_style((row + 1, col), style);
+            table.insert(pos, text);
+            table.insert_style(pos, style);
         }
     }
 
     Ok(Some(table))
 }
 
-fn to_table_with_no_header(
+fn create_table_with_header_and_index(
     input: &[Value],
-    with_index: bool,
+    headers: Vec<String>,
     row_offset: usize,
     opts: &TableOpts<'_>,
 ) -> Result<Option<NuTable>, ShellError> {
-    let mut table = NuTable::new(input.len(), with_index as usize + 1);
+    // The header with the INDEX is removed from the table headers since
+    // it is added to the natural table index
+    let mut headers: Vec<_> = headers
+        .into_iter()
+        .filter(|header| header != INDEX_COLUMN_NAME)
+        .collect();
+
+    headers.insert(0, "#".into());
+
+    let count_rows = input.len() + 1;
+    let count_columns = headers.len();
+    let mut table = NuTable::new(count_rows, count_columns);
+
+    table.set_header_style(get_header_style(opts.style_computer));
+    table.set_index_style(get_index_style(opts.style_computer));
+
+    table.insert_row(0, headers.clone());
+
+    for (row, item) in input.iter().enumerate() {
+        opts.signals.check(opts.span)?;
+        check_value(item)?;
+
+        let text = get_table_row_index(item, opts.config, row, row_offset);
+        table.insert((row + 1, 0), text);
+
+        for (col, header) in headers.iter().enumerate().skip(1) {
+            let (text, style) = get_string_value_with_header(item, header, opts);
+
+            let pos = (row + 1, col);
+            table.insert(pos, text);
+            table.insert_style(pos, style);
+        }
+    }
+
+    Ok(Some(table))
+}
+
+fn create_table_with_no_header(
+    input: &[Value],
+    opts: &TableOpts<'_>,
+) -> Result<Option<NuTable>, ShellError> {
+    let mut table = NuTable::new(input.len(), 1);
     table.set_index_style(get_index_style(opts.style_computer));
 
     for (row, item) in input.iter().enumerate() {
         opts.signals.check(opts.span)?;
-
+        check_value(item)?;
-        if let Value::Error { error, .. } = item {
-            return Err(*error.clone());
-        }
-
-        if with_index {
-            let text = get_table_row_index(item, opts.config, row, row_offset);
-            table.insert((row, 0), text);
-        }
 
         let (text, style) = get_string_value(item, opts);
 
-        let pos = (row, with_index as usize);
+        let pos = (row, 0);
+        table.insert(pos, text);
+        table.insert_style(pos, style);
+    }
+
+    Ok(Some(table))
+}
+
+fn create_table_with_no_header_and_index(
+    input: &[Value],
+    row_offset: usize,
+    opts: &TableOpts<'_>,
+) -> Result<Option<NuTable>, ShellError> {
+    let mut table = NuTable::new(input.len(), 1 + 1);
+    table.set_index_style(get_index_style(opts.style_computer));
+
+    for (row, item) in input.iter().enumerate() {
+        opts.signals.check(opts.span)?;
+        check_value(item)?;
+
+        let text = get_table_row_index(item, opts.config, row, row_offset);
+        table.insert((row, 0), text);
+
+        let (text, style) = get_string_value(item, opts);
+
+        let pos = (row, 1);
         table.insert(pos, text);
         table.insert_style(pos, style);
     }
@@ -194,8 +232,9 @@ fn get_string_value_with_header(item: &Value, header: &str, opts: &TableOpts) ->
 
 fn get_string_value(item: &Value, opts: &TableOpts) -> NuText {
     let (mut text, style) = get_value_style(item, opts.config, opts.style_computer);
-    let is_string_value = matches!(item, Value::String { .. });
+
-    if is_string_value {
+    let is_string = matches!(item, Value::String { .. });
+    if is_string {
         text = clean_charset(&text);
     }
 

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

@@ -1,5 +1,5 @@
 use nu_color_config::StyleComputer;
-use nu_protocol::{Config, Signals, Span, TableIndexMode, TableMode};
+use nu_protocol::{Config, Signals, Span, TableIndent, TableIndexMode, TableMode};
 
 use crate::{common::INDEX_COLUMN_NAME, NuTable};
 
@@ -15,6 +15,8 @@ pub struct TableOutput {
     pub table: NuTable,
     pub with_header: bool,
     pub with_index: bool,
+    /// The value may be bigger then table.count_rows(),
+    /// Specifically in case of expanded table we collect the whole structure size here.
     pub count_rows: usize,
 }
 
@@ -27,6 +29,10 @@ impl TableOutput {
             count_rows,
         }
     }
+    pub fn from_table(table: NuTable, with_header: bool, with_index: bool) -> Self {
+        let count_rows = table.count_rows();
+        Self::new(table, with_header, with_index, count_rows)
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -36,7 +42,7 @@ pub struct TableOpts<'a> {
     style_computer: &'a StyleComputer<'a>,
     span: Span,
     width: usize,
-    indent: (usize, usize),
+    indent: TableIndent,
     mode: TableMode,
     index_offset: usize,
     index_remove: bool,
@@ -50,7 +56,7 @@ impl<'a> TableOpts<'a> {
         signals: &'a Signals,
         span: Span,
         width: usize,
-        indent: (usize, usize),
+        indent: TableIndent,
         mode: TableMode,
         index_offset: usize,
         index_remove: bool,

crates/nu-table/src/unstructured_table.rs

@@ -1,6 +1,6 @@
-use crate::{string_width, string_wrap, TableTheme};
 use nu_color_config::StyleComputer;
-use nu_protocol::{Config, Record, Span, Value};
+use nu_protocol::{Config, Record, Span, TableIndent, Value};
+
 use tabled::{
     grid::{
         ansi::{ANSIBuf, ANSIStr},
@@ -11,9 +11,11 @@ use tabled::{
     tables::{PoolTable, TableValue},
 };
 
+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.
+/// It doesn't support alignment and a proper width control (allthough it's possible to achieve).
 pub struct UnstructuredTable {
     value: TableValue,
 }
@@ -34,28 +36,23 @@ impl UnstructuredTable {
         truncate_table_value(&mut self.value, has_vertical, available).is_none()
     }
 
-    pub fn draw(
+    pub fn draw(self, theme: &TableTheme, indent: TableIndent, style: &StyleComputer) -> String {
-        self,
+        build_table(self.value, style, theme, indent)
-        style_computer: &StyleComputer,
-        theme: &TableTheme,
-        indent: (usize, usize),
-    ) -> String {
-        build_table(self.value, style_computer, theme, indent)
     }
 }
 
 fn build_table(
     val: TableValue,
-    style_computer: &StyleComputer,
+    style: &StyleComputer,
     theme: &TableTheme,
-    indent: (usize, usize),
+    indent: TableIndent,
 ) -> String {
     let mut table = PoolTable::from(val);
 
     let mut theme = theme.as_full().clone();
     theme.set_horizontal_lines(Default::default());
 
-    table.with(Padding::new(indent.0, indent.1, 0, 0));
+    table.with(Padding::new(indent.left, indent.right, 0, 0));
     table.with(SetRawStyle(theme));
     table.with(SetAlignment(AlignmentHorizontal::Left));
     table.with(PoolTableDimension::new(
@@ -64,25 +61,12 @@ fn build_table(
     ));
 
     // color_config closures for "separator" are just given a null.
-    let color = style_computer.compute("separator", &Value::nothing(Span::unknown()));
+    let color = style.compute("separator", &Value::nothing(Span::unknown()));
     let color = color.paint(" ").to_string();
     if let Ok(color) = Color::try_from(color) {
-        // # SAFETY
+        if !is_color_empty(&color) {
-        //
+            table.with(SetBorderColor(color_into_ansistr(color)));
-        // It's perfectly save to do cause table does not store the reference internally.
+        }
-        // We just need this unsafe section to cope with some limitations of [`PoolTable`].
-        // Mitigation of this is definitely on a todo list.
-
-        let color: ANSIBuf = color.into();
-        let prefix = color.get_prefix();
-        let suffix = color.get_suffix();
-        let prefix: &'static str = unsafe { std::mem::transmute(prefix) };
-        let suffix: &'static str = unsafe { std::mem::transmute(suffix) };
-
-        table.with(SetBorderColor(ANSIStr::new(prefix, suffix)));
-        let table = table.to_string();
-
-        return table;
     }
 
     table.to_string()
@@ -348,3 +332,19 @@ fn truncate_table_value(
         }
     }
 }
+
+fn color_into_ansistr(color: Color) -> ANSIStr<'static> {
+    // # SAFETY
+    //
+    // It's perfectly save to do cause table does not store the reference internally.
+    // We just need this unsafe section to cope with some limitations of [`PoolTable`].
+    // Mitigation of this is definitely on a todo list.
+
+    let color: ANSIBuf = color.into();
+    let prefix = color.get_prefix();
+    let suffix = color.get_suffix();
+    let prefix: &'static str = unsafe { std::mem::transmute(prefix) };
+    let suffix: &'static str = unsafe { std::mem::transmute(suffix) };
+
+    ANSIStr::new(prefix, suffix)
+}

crates/nu-table/src/util.rs

@@ -1,12 +1,15 @@
 use nu_color_config::StyleComputer;
+
 use tabled::{
-    builder::Builder,
     grid::{
         ansi::{ANSIBuf, ANSIStr},
         records::vec_records::Text,
         util::string::get_text_width,
     },
-    settings::{width::Truncate, Color, Modify, Padding, Style, Width},
+    settings::{
+        width::{Truncate, Wrap},
+        Color,
+    },
 };
 
 use crate::common::get_leading_trailing_space_style;
@@ -16,34 +19,16 @@ pub fn string_width(text: &str) -> usize {
 }
 
 pub fn string_wrap(text: &str, width: usize, keep_words: bool) -> String {
-    // todo: change me...
-    //
-    // well... it's not efficient to build a table to wrap a string,
-    // but ... it's better than a copy paste (is it?)
-
     if text.is_empty() {
         return String::new();
     }
 
-    let wrap = if keep_words {
+    Wrap::wrap(text, width, keep_words)
-        Width::wrap(width).keep_words(true)
-    } else {
-        Width::wrap(width)
-    };
-
-    Builder::from_iter([[text]])
-        .build()
-        .with(Style::empty())
-        .with(Padding::zero())
-        .with(Modify::new((0, 0)).with(wrap))
-        .to_string()
 }
 
 pub fn string_truncate(text: &str, width: usize) -> String {
-    // todo: change me...
-
     let line = match text.lines().next() {
-        Some(first_line) => first_line,
+        Some(line) => line,
         None => return String::new(),
     };
 
@@ -51,35 +36,75 @@ pub fn string_truncate(text: &str, width: usize) -> String {
 }
 
 pub fn clean_charset(text: &str) -> String {
-    // todo: optimize, I bet it can be done in 1 path
+    // TODO: We could make an optimization to take a String and modify it
-    text.replace('\t', "    ").replace('\r', "")
+    //       We could check if there was any changes and if not make no allocations at all and don't change the origin.
+    //       Why it's not done...
+    //       Cause I am not sure how the `if` in a loop will affect performance.
+    //       So it's better be profiled, but likely the optimization be worth it.
+    //       At least because it's a base case where we won't change anything....
+
+    // 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.
+    // We could shrink it but...it will be another realloc which make no scense.
+    let mut buf = String::with_capacity(text.len());
+
+    for c in text.chars() {
+        if c == '\n' {
+            buf.push(c);
+            continue;
+        }
+
+        if c == '\t' {
+            buf.push(' ');
+            buf.push(' ');
+            buf.push(' ');
+            buf.push(' ');
+            continue;
+        }
+
+        if c < ' ' {
+            continue;
+        }
+
+        buf.push(c);
+    }
+
+    buf
 }
 
 pub fn colorize_space(data: &mut [Vec<Text<String>>], style_computer: &StyleComputer<'_>) {
-    if let Some(style) = get_leading_trailing_space_style(style_computer).color_style {
+    let style = match get_leading_trailing_space_style(style_computer).color_style {
-        let style = ANSIBuf::from(convert_style(style));
+        Some(color) => color,
-        let style = style.as_ref();
+        None => return,
-        colorize_lead_trail_space(data, Some(style), Some(style));
+    };
-    }
-}
 
-pub fn colorize_space_str(text: &mut String, style_computer: &StyleComputer<'_>) {
+    let style = ANSIBuf::from(convert_style(style));
-    if let Some(style) = get_leading_trailing_space_style(style_computer).color_style {
+    let style = style.as_ref();
-        let style = ANSIBuf::from(convert_style(style));
+    if style.is_empty() {
-        let style = style.as_ref();
-        *text = colorize_space_one(text, Some(style), Some(style));
-    }
-}
-
-fn colorize_lead_trail_space(
-    data: &mut [Vec<Text<String>>],
-    lead: Option<ANSIStr<'_>>,
-    trail: Option<ANSIStr<'_>>,
-) {
-    if lead.is_none() && trail.is_none() {
         return;
     }
 
+    colorize_list(data, style, style);
+}
+
+pub fn colorize_space_str(text: &mut String, style_computer: &StyleComputer<'_>) {
+    let style = match get_leading_trailing_space_style(style_computer).color_style {
+        Some(color) => color,
+        None => return,
+    };
+
+    let style = ANSIBuf::from(convert_style(style));
+    let style = style.as_ref();
+    if style.is_empty() {
+        return;
+    }
+
+    *text = colorize_space_one(text, style, style);
+}
+
+fn colorize_list(data: &mut [Vec<Text<String>>], lead: ANSIStr<'_>, trail: ANSIStr<'_>) {
     for row in data.iter_mut() {
         for cell in row {
             let buf = colorize_space_one(cell.as_ref(), lead, trail);
@@ -88,7 +113,7 @@ fn colorize_lead_trail_space(
     }
 }
 
-fn colorize_space_one(text: &str, lead: Option<ANSIStr<'_>>, trail: Option<ANSIStr<'_>>) -> String {
+fn colorize_space_one(text: &str, lead: ANSIStr<'_>, trail: ANSIStr<'_>) -> String {
     use fancy_regex::Captures;
     use fancy_regex::Regex;
     use std::sync::LazyLock;
@@ -102,20 +127,20 @@ fn colorize_space_one(text: &str, lead: Option<ANSIStr<'_>>, trail: Option<ANSIS
 
     let mut buf = text.to_owned();
 
-    if let Some(color) = &lead {
+    if !lead.is_empty() {
         buf = RE_LEADING
             .replace_all(&buf, |cap: &Captures| {
                 let spaces = cap.get(1).expect("valid").as_str();
-                format!("{}{}{}", color.get_prefix(), spaces, color.get_suffix())
+                format!("{}{}{}", lead.get_prefix(), spaces, lead.get_suffix())
             })
             .into_owned();
     }
 
-    if let Some(color) = &trail {
+    if !trail.is_empty() {
         buf = RE_TRAILING
             .replace_all(&buf, |cap: &Captures| {
                 let spaces = cap.get(1).expect("valid").as_str();
-                format!("{}{}{}", color.get_prefix(), spaces, color.get_suffix())
+                format!("{}{}{}", trail.get_prefix(), spaces, trail.get_suffix())
             })
             .into_owned();
     }
@@ -126,3 +151,7 @@ fn colorize_space_one(text: &str, lead: Option<ANSIStr<'_>>, trail: Option<ANSIS
 pub fn convert_style(style: nu_ansi_term::Style) -> Color {
     Color::new(style.prefix().to_string(), style.suffix().to_string())
 }
+
+pub fn is_color_empty(c: &Color) -> bool {
+    c.get_prefix().is_empty() && c.get_suffix().is_empty()
+}