Refactoring nu_table (#6049)

* nu-table: Remove unused dependencies

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* nu-table: Small refactoring

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* nu-table: Refactoring

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* nu-table: Refactoring alignments

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* nu-table: Add width check

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* nu-table/ Use commit instead of branch of tabled

To be safe

* Update Cargo.lock

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

* nu-table: Bump tabled

Signed-off-by: Maxim Zhiburt <zhiburt@gmail.com>

Cargo.lock

@@ -94,22 +94,6 @@ dependencies = [
  "ansi-parser",
 ]
 
-[[package]]
-name = "ansi-str"
-version = "0.2.0"
-source = "git+https://github.com/zhiburt/ansi-str?rev=655cd8125a032286082794690c2cc6dc835345b4#655cd8125a032286082794690c2cc6dc835345b4"
-dependencies = [
- "ansi_rs",
-]
-
-[[package]]
-name = "ansi_rs"
-version = "0.1.0"
-source = "git+https://gitlab.com/zhiburt/ansi_rs#16173ad4a2812a2b0dc9faf1552d5985db4ee310"
-dependencies = [
- "nom 7.1.1",
-]
-
 [[package]]
 name = "ansi_term"
 version = "0.12.1"
@@ -2856,14 +2840,11 @@ dependencies = [
 name = "nu-table"
 version = "0.65.1"
 dependencies = [
- "ansi-str 0.2.0 (git+https://github.com/zhiburt/ansi-str?rev=655cd8125a032286082794690c2cc6dc835345b4)",
  "atty",
  "nu-ansi-term",
  "nu-protocol",
- "regex",
  "strip-ansi-escapes",
  "tabled",
- "unicode-width",
 ]
 
 [[package]]
@@ -3187,7 +3168,7 @@ checksum = "decf7381921fea4dcb2549c5667eda59b3ec297ab7e2b5fc33eac69d2e7da87b"
 [[package]]
 name = "papergrid"
 version = "0.4.0"
-source = "git+https://github.com/zhiburt/tabled?branch=master#a17e76783422de7db213ef07e81ed1ef78ca599e"
+source = "git+https://github.com/zhiburt/tabled?rev=cca285b1fc0eac48b8a386c8884092d894d0e7ae#cca285b1fc0eac48b8a386c8884092d894d0e7ae"
 dependencies = [
  "ansi-str 0.1.1",
  "bytecount",
@@ -4805,9 +4786,9 @@ dependencies = [
 [[package]]
 name = "tabled"
 version = "0.7.0"
-source = "git+https://github.com/zhiburt/tabled?branch=master#a17e76783422de7db213ef07e81ed1ef78ca599e"
+source = "git+https://github.com/zhiburt/tabled?rev=cca285b1fc0eac48b8a386c8884092d894d0e7ae#cca285b1fc0eac48b8a386c8884092d894d0e7ae"
 dependencies = [
- "ansi-str 0.2.0 (git+https://github.com/zhiburt/ansi-str?branch=master)",
+ "ansi-str 0.2.0",
  "papergrid",
  "tabled_derive",
  "unicode-width",
@@ -4816,7 +4797,7 @@ dependencies = [
 [[package]]
 name = "tabled_derive"
 version = "0.3.0"
-source = "git+https://github.com/zhiburt/tabled?branch=master#a17e76783422de7db213ef07e81ed1ef78ca599e"
+source = "git+https://github.com/zhiburt/tabled?rev=cca285b1fc0eac48b8a386c8884092d894d0e7ae#cca285b1fc0eac48b8a386c8884092d894d0e7ae"
 dependencies = [
  "heck 0.4.0",
  "proc-macro-error",

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

@@ -7,7 +7,7 @@ use nu_protocol::{
     format_error, Category, Config, DataSource, Example, IntoPipelineData, ListStream,
     PipelineData, PipelineMetadata, RawStream, ShellError, Signature, Span, SyntaxShape, Value,
 };
-use nu_table::{StyledString, TableTheme, TextStyle};
+use nu_table::{Alignments, StyledString, TableTheme, TextStyle};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 use std::time::Instant;
@@ -167,9 +167,11 @@ impl Command for Table {
                     ])
                 }
 
-                let table = nu_table::Table::new(None, output, load_theme_from_config(config));
+                let table =
+                    nu_table::Table::new(Vec::new(), output, load_theme_from_config(config));
 
-                let result = nu_table::draw_table(&table, term_width, &color_hm, config)
+                let result = table
+                    .draw_table(config, &color_hm, Alignments::default(), term_width)
                     .unwrap_or_else(|| format!("Couldn't fit table into {} columns!", term_width));
 
                 Ok(Value::String {
@@ -426,7 +428,6 @@ fn convert_to_table(
         }
 
         Ok(Some(nu_table::Table::new(
-            Some(
             headers
                 .into_iter()
                 .map(|x| StyledString {
@@ -437,7 +438,6 @@ fn convert_to_table(
                     },
                 })
                 .collect(),
-            ),
             data.into_iter()
                 .map(|x| {
                     x.into_iter()
@@ -554,7 +554,8 @@ impl Iterator for PagingTableCreator {
 
         match table {
             Ok(Some(table)) => {
-                let result = nu_table::draw_table(&table, term_width, &color_hm, &self.config)
+                let result = table
+                    .draw_table(&self.config, &color_hm, Alignments::default(), term_width)
                     .unwrap_or_else(|| format!("Couldn't fit table into {} columns!", term_width));
 
                 Some(Ok(result.as_bytes().to_vec()))

crates/nu-table/Cargo.toml

@@ -14,9 +14,6 @@ path = "src/main.rs"
 [dependencies]
 nu-ansi-term = "0.46.0"
 nu-protocol = { path = "../nu-protocol", version = "0.65.1" }
-regex = "1.4"
-unicode-width = "0.1.8"
 strip-ansi-escapes = "0.1.1"
-ansi-str = { git = "https://github.com/zhiburt/ansi-str", rev = "655cd8125a032286082794690c2cc6dc835345b4" }
 atty = "0.2.14"
-tabled = { git = "https://github.com/zhiburt/tabled", branch = "master", features = ["color"] }
+tabled = { git = "https://github.com/zhiburt/tabled", rev = "9c831d5bc5bcd5a7b7a349ce63f746a64bf1c278", features = ["color"] }

crates/nu-table/src/lib.rs

@@ -3,6 +3,6 @@ mod table_theme;
 mod textstyle;
 mod width_control;
 
-pub use table::{draw_table, Table};
+pub use table::{Alignments, Table};
 pub use table_theme::TableTheme;
 pub use textstyle::{Alignment, StyledString, TextStyle};

crates/nu-table/src/main.rs

@@ -1,5 +1,5 @@
 use nu_protocol::Config;
-use nu_table::{draw_table, StyledString, Table, TableTheme, TextStyle};
+use nu_table::{Alignments, StyledString, Table, TableTheme, TextStyle};
 use std::collections::HashMap;
 
 fn main() {
@@ -23,13 +23,14 @@ fn main() {
     // The table rows
     let rows = vec_of_str_to_vec_of_styledstr(&row_data, false);
     // The table itself
-    let table = Table::new(Some(headers), vec![rows; 3], TableTheme::rounded());
+    let table = Table::new(headers, vec![rows; 3], TableTheme::rounded());
     // FIXME: Config isn't available from here so just put these here to compile
     let color_hm: HashMap<String, nu_ansi_term::Style> = HashMap::new();
     // get the default config
     let config = Config::default();
     // Capture the table as a string
-    let output_table = draw_table(&table, width, &color_hm, &config)
+    let output_table = table
+        .draw_table(&config, &color_hm, Alignments::default(), width)
         .unwrap_or_else(|| format!("Couldn't fit table into {} columns!", width));
     // Draw the table
     println!("{}", output_table)

crates/nu-table/src/table.rs

@@ -5,14 +5,15 @@ use nu_protocol::{Config, FooterMode, TrimStrategy};
 use tabled::{
     builder::Builder,
     formatting_settings::AlignmentStrategy,
-    object::{Cell, Columns, Rows},
+    object::{Cell, Columns, Rows, Segment},
     papergrid,
     style::BorderColor,
-    Alignment, Modify, TableOption, Width,
+    Alignment, AlignmentHorizontal, Modify, ModifyObject, TableOption, Width,
 };
 
 use crate::{table_theme::TableTheme, width_control::maybe_truncate_columns, StyledString};
 
+/// Table represent a table view.
 #[derive(Debug)]
 pub struct Table {
     headers: Option<Vec<StyledString>>,
@@ -20,15 +21,36 @@ pub struct Table {
     theme: TableTheme,
 }
 
+#[derive(Debug)]
+pub struct Alignments {
+    data: AlignmentHorizontal,
+    index: AlignmentHorizontal,
+    header: AlignmentHorizontal,
+}
+
+impl Default for Alignments {
+    fn default() -> Self {
+        Self {
+            data: AlignmentHorizontal::Center,
+            index: AlignmentHorizontal::Right,
+            header: AlignmentHorizontal::Center,
+        }
+    }
+}
+
 impl Table {
+    /// Creates a [Table] instance.
+    ///
+    /// If `headers.is_empty` then no headers will be rendered.
     pub fn new(
-        headers: Option<Vec<StyledString>>,
+        headers: Vec<StyledString>,
         data: Vec<Vec<StyledString>>,
         theme: TableTheme,
     ) -> Table {
-        let headers = match headers {
+        let headers = if headers.is_empty() {
-            Some(headers) if headers.is_empty() => None,
+            None
-            headers => headers,
+        } else {
+            Some(headers)
         };
 
         Table {
@@ -37,34 +59,59 @@ impl Table {
             theme,
         }
     }
+
+    /// Draws a trable on a String.
+    ///
+    /// It returns None in case where table cannot be fit to a terminal width.
+    pub fn draw_table(
+        &self,
+        config: &Config,
+        color_hm: &HashMap<String, Style>,
+        alignments: Alignments,
+        termwidth: usize,
+    ) -> Option<String> {
+        draw_table(self, config, color_hm, alignments, termwidth)
+    }
 }
 
-pub fn draw_table(
+fn draw_table(
     table: &Table,
-    termwidth: usize,
-    color_hm: &HashMap<String, Style>,
     config: &Config,
+    color_hm: &HashMap<String, Style>,
+    alignments: Alignments,
+    termwidth: usize,
 ) -> Option<String> {
-    let (mut headers, mut data, count_columns) =
+    let mut headers = colorize_headers(table.headers.as_deref());
-        table_fix_lengths(table.headers.as_ref(), &table.data);
+    let mut data = colorize_data(&table.data, table.headers.as_ref().map_or(0, |h| h.len()));
+
+    let count_columns = table_fix_lengths(headers.as_mut(), &mut data);
 
     maybe_truncate_columns(&mut headers, &mut data, count_columns, termwidth);
 
-    let alignments = build_alignment_map(&table.data);
+    let table_data = &table.data;
-
-    let headers = table_header_to_strings(headers);
-    let data = table_data_to_strings(data, count_columns);
-
     let theme = &table.theme;
     let with_header = headers.is_some();
     let with_footer = with_header && need_footer(config, data.len() as u64);
+    let with_index = !config.disable_table_indexes;
 
-    let table = build_table(data, headers, Some(alignments), config, with_footer);
+    let table = build_table(data, headers, with_footer);
     let table = load_theme(table, color_hm, theme, with_footer, with_header);
-
+    let table = align_table(
+        table,
+        alignments,
+        with_index,
+        with_header,
+        with_footer,
+        table_data,
+    );
     let table = table_trim_columns(table, termwidth, &config.trim_strategy);
 
-    Some(print_table(table, config))
+    let table = print_table(table, config);
+    if table_width(&table) > termwidth {
+        None
+    } else {
+        Some(table)
+    }
 }
 
 fn print_table(table: tabled::Table, config: &Config) -> String {
@@ -83,19 +130,20 @@ fn print_table(table: tabled::Table, config: &Config) -> String {
     }
 }
 
-fn table_data_to_strings(
+fn table_width(table: &str) -> usize {
-    table_data: Vec<Vec<StyledString>>,
+    table.lines().next().map_or(0, papergrid::string_width)
-    count_headers: usize,
+}
-) -> Vec<Vec<String>> {
+
-    let mut data = vec![Vec::with_capacity(count_headers); table_data.len()];
+fn colorize_data(table_data: &[Vec<StyledString>], count_columns: usize) -> Vec<Vec<String>> {
-    for (row, row_data) in table_data.into_iter().enumerate() {
+    let mut data = vec![Vec::with_capacity(count_columns); table_data.len()];
+    for (row, row_data) in table_data.iter().enumerate() {
         for cell in row_data {
             let colored_text = cell
                 .style
                 .color_style
                 .as_ref()
                 .map(|color| color.paint(&cell.contents).to_string())
-                .unwrap_or(cell.contents);
+                .unwrap_or_else(|| cell.contents.clone());
 
             data[row].push(colored_text)
         }
@@ -104,8 +152,8 @@ fn table_data_to_strings(
     data
 }
 
-fn table_header_to_strings(table_headers: Option<Vec<StyledString>>) -> Option<Vec<String>> {
+fn colorize_headers(headers: Option<&[StyledString]>) -> Option<Vec<String>> {
-    table_headers.map(|table_headers| {
+    headers.map(|table_headers| {
         let mut headers = Vec::with_capacity(table_headers.len());
         for cell in table_headers {
             let colored_text = cell
@@ -113,7 +161,7 @@ fn table_header_to_strings(table_headers: Option<Vec<StyledString>>) -> Option<V
                 .color_style
                 .as_ref()
                 .map(|color| color.paint(&cell.contents).to_string())
-                .unwrap_or(cell.contents);
+                .unwrap_or_else(|| cell.contents.clone());
 
             headers.push(colored_text)
         }
@@ -122,28 +170,11 @@ fn table_header_to_strings(table_headers: Option<Vec<StyledString>>) -> Option<V
     })
 }
 
-fn build_alignment_map(data: &[Vec<StyledString>]) -> Vec<Vec<Alignment>> {
-    let mut v = vec![Vec::new(); data.len()];
-    for (i, row) in data.iter().enumerate() {
-        let mut row_alignments = Vec::with_capacity(row.len());
-        for col in row {
-            row_alignments.push(Alignment::Horizontal(col.style.alignment));
-        }
-
-        v[i] = row_alignments;
-    }
-
-    v
-}
-
 fn build_table(
     data: Vec<Vec<String>>,
     headers: Option<Vec<String>>,
-    alignment_map: Option<Vec<Vec<Alignment>>>,
-    config: &Config,
     need_footer: bool,
 ) -> tabled::Table {
-    let header_present = headers.is_some();
     let mut builder = Builder::from(data);
 
     if let Some(headers) = headers {
@@ -154,38 +185,65 @@ fn build_table(
         }
     }
 
-    let mut table = builder.build();
+    builder.build()
+}
 
+fn align_table(
+    mut table: tabled::Table,
+    alignments: Alignments,
+    with_index: bool,
+    with_header: bool,
+    with_footer: bool,
+    data: &[Vec<StyledString>],
+) -> tabled::Table {
     table = table.with(
-        Modify::new(Rows::new(1..))
+        Modify::new(Segment::all())
-            .with(Alignment::left())
+            .with(Alignment::Horizontal(alignments.data))
             .with(AlignmentStrategy::PerLine),
     );
 
-    if !config.disable_table_indexes {
+    if with_index {
-        table = table.with(Modify::new(Columns::first()).with(Alignment::right()));
+        table =
+            table.with(Modify::new(Columns::first()).with(Alignment::Horizontal(alignments.index)));
     }
 
-    if header_present {
+    if with_header {
-        table = table.with(Modify::new(Rows::first()).with(Alignment::center()));
+        let alignment = Alignment::Horizontal(alignments.header);
+        table = table.with(Modify::new(Rows::first()).with(alignment.clone()));
+
+        if with_footer {
+            table = table.with(Modify::new(Rows::last()).with(alignment));
+        }
     }
 
-    if let Some(alignments) = alignment_map {
+    table = override_alignments(table, data, with_header, with_index, alignments);
-        table = apply_alignments(table, alignments, header_present);
-    }
 
     table
 }
 
-fn apply_alignments(
+fn override_alignments(
     mut table: tabled::Table,
-    alignment: Vec<Vec<Alignment>>,
+    data: &[Vec<StyledString>],
     header_present: bool,
+    index_present: bool,
+    alignments: Alignments,
 ) -> tabled::Table {
     let offset = if header_present { 1 } else { 0 };
-    for (row, alignments) in alignment.into_iter().enumerate() {
+    for (row, rows) in data.iter().enumerate() {
-        for (col, alignment) in alignments.into_iter().enumerate() {
+        for (col, s) in rows.iter().enumerate() {
-            table = table.with(Modify::new(Cell(row + offset, col)).with(alignment));
+            if index_present && col == 0 && s.style.alignment == alignments.index {
+                continue;
+            }
+
+            if s.style.alignment == alignments.data {
+                continue;
+            }
+
+            table = table.with(
+                Cell(row + offset, col)
+                    .modify()
+                    .with(Alignment::Horizontal(s.style.alignment)),
+            );
         }
     }
 
@@ -307,31 +365,21 @@ impl tabled::TableOption for &TrimStrategyModifier<'_> {
     }
 }
 
-fn table_fix_lengths(
+fn table_fix_lengths(headers: Option<&mut Vec<String>>, data: &mut [Vec<String>]) -> usize {
-    headers: Option<&Vec<StyledString>>,
+    let length = table_find_max_length(headers.as_deref(), data);
-    data: &[Vec<StyledString>],
-) -> (Option<Vec<StyledString>>, Vec<Vec<StyledString>>, usize) {
-    let length = table_find_max_length(headers, data);
 
-    let headers_fixed = headers.map(|h| {
+    if let Some(headers) = headers {
-        let mut headers_fixed = Vec::with_capacity(length);
+        headers.extend(std::iter::repeat(String::default()).take(length - headers.len()));
-        headers_fixed.extend(h.iter().cloned());
+    }
-        headers_fixed.extend(std::iter::repeat(StyledString::default()).take(length - h.len()));
-        headers_fixed
-    });
 
-    let mut data_fixed = Vec::with_capacity(data.len());
     for row in data {
-        let mut row_fixed = Vec::with_capacity(length);
+        row.extend(std::iter::repeat(String::default()).take(length - row.len()));
-        row_fixed.extend(row.iter().cloned());
-        row_fixed.extend(std::iter::repeat(StyledString::default()).take(length - row.len()));
-        data_fixed.push(row_fixed);
     }
 
-    (headers_fixed, data_fixed, length)
+    length
 }
 
-fn table_find_max_length(headers: Option<&Vec<StyledString>>, data: &[Vec<StyledString>]) -> usize {
+fn table_find_max_length<T>(headers: Option<&Vec<T>>, data: &[Vec<T>]) -> usize {
     let mut length = headers.map_or(0, |h| h.len());
     for row in data {
         length = std::cmp::max(length, row.len());

crates/nu-table/src/width_control.rs

@@ -1,9 +1,6 @@
-use crate::textstyle::TextStyle;
-use crate::StyledString;
-
 pub(crate) fn maybe_truncate_columns(
-    headers: &mut Option<Vec<StyledString>>,
+    headers: &mut Option<Vec<String>>,
-    data: &mut [Vec<StyledString>],
+    data: &mut [Vec<String>],
     length: usize,
     termwidth: usize,
 ) {
@@ -14,20 +11,14 @@ pub(crate) fn maybe_truncate_columns(
     if let Some(headers) = headers {
         if max_num_of_columns < length {
             headers.truncate(max_num_of_columns);
-            headers.push(StyledString::new(
+            headers.push(String::from("..."));
-                String::from("..."),
-                TextStyle::basic_center(),
-            ));
         }
     }
 
     if max_num_of_columns < length {
         for entry in data.iter_mut() {
             entry.truncate(max_num_of_columns);
-            entry.push(StyledString::new(
+            entry.push(String::from("..."));
-                String::from("..."),
-                TextStyle::basic_center(),
-            ));
         }
     }
 }