Patch after fix after fix 7380 (#7501)

> I'm not sure how i feel about that. I mean if there are a lot of
columns, it should probably have a max width so 1 column doesn't take
the entire width of your screen. Ideally it would work closely like
table worked before we migrated to tabled, as far as how column widths
were allocated.

I believe it still not completely matched.
*To be honest I am not against the #7446 approach.

The PR makes a switch between logics on a premise of `termwidth`.
So if `termwidth > 120` we start prioritizing amount of columns we can
show (We try to show as many columns as we can).
Otherwise we do what I've described in #7446 (We show the least columns
but with least truncation involvement).

In case it's OK,
I guess we could make the value configurable.

cc @fdncred 
ref #7446

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

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

@@ -1701,6 +1701,10 @@ impl Iterator for PagingTableCreator {
             }
         }
 
+        if batch.is_empty() {
+            return None;
+        }
+
         let table = match &self.view {
             TableView::General => self.build_general(&batch),
             TableView::Collapsed => self.build_collapsed(batch),
@@ -1723,8 +1727,12 @@ impl Iterator for PagingTableCreator {
 
                 Some(Ok(bytes))
             }
+            Ok(None) => {
+                let term_width = get_width_param(self.width_param);
+                let msg = format!("Couldn't fit table into {} columns!", term_width);
+                Some(Ok(msg.as_bytes().to_vec()))
+            }
             Err(err) => Some(Err(err)),
-            _ => None,
         }
     }
 }

crates/nu-table/src/table.rs

@@ -14,7 +14,7 @@ use tabled::{
             cell_info::CellInfo, tcell::TCell, vec_records::VecRecords, Records, RecordsMut,
         },
         util::string_width_multiline,
-        width::CfgWidthFunction,
+        width::{CfgWidthFunction, WidthEstimator},
         Estimate,
     },
     peaker::Peaker,
@@ -175,9 +175,7 @@ impl Default for Alignments {
 }
 
 fn build_table(mut data: Data, cfg: TableConfig, termwidth: usize) -> Option<String> {
-    let priority = TruncationPriority::Content;
+    let is_empty = maybe_truncate_columns(&mut data, &cfg.theme, termwidth);
-    let is_empty = maybe_truncate_columns(&mut data, &cfg.theme, termwidth, priority);
-
     if is_empty {
         return None;
     }
@@ -388,30 +386,20 @@ where
     }
 }
 
-enum TruncationPriority {
+fn maybe_truncate_columns(data: &mut Data, theme: &TableTheme, termwidth: usize) -> bool {
-    // VERSION where we are showing AS LITTLE COLUMNS AS POSSIBLE but WITH AS MUCH CONTENT AS POSSIBLE.
+    const TERMWIDTH_TRESHHOLD: usize = 120;
-    Content,
-    // VERSION where we are showing AS MANY COLUMNS AS POSSIBLE but as a side affect they MIGHT CONTAIN AS LITTLE CONTENT AS POSSIBLE
-    //
-    // not used so far.
-    #[allow(dead_code)]
-    Columns,
-}
 
-fn maybe_truncate_columns(
-    data: &mut Data,
-    theme: &TableTheme,
-    termwidth: usize,
-    priority: TruncationPriority,
-) -> bool {
     if data.count_columns() == 0 {
         return true;
     }
 
-    match priority {
+    let truncate = if termwidth > TERMWIDTH_TRESHHOLD {
-        TruncationPriority::Content => truncate_columns_by_content(data, theme, termwidth),
+        truncate_columns_by_columns
-        TruncationPriority::Columns => truncate_columns_by_columns(data, theme, termwidth),
+    } else {
-    }
+        truncate_columns_by_content
+    };
+
+    truncate(data, theme, termwidth)
 }
 
 // VERSION where we are showing AS LITTLE COLUMNS AS POSSIBLE but WITH AS MUCH CONTENT AS POSSIBLE.
@@ -433,7 +421,7 @@ fn truncate_columns_by_content(data: &mut Data, theme: &TableTheme, termwidth: u
         return false;
     }
 
-    let mut width_ctrl = tabled::papergrid::width::WidthEstimator::default();
+    let mut width_ctrl = WidthEstimator::default();
     width_ctrl.estimate(&*data, &config);
     let widths = Vec::from(width_ctrl);
 
@@ -499,10 +487,10 @@ fn truncate_columns_by_content(data: &mut Data, theme: &TableTheme, termwidth: u
     false
 }
 
+// VERSION where we are showing AS MANY COLUMNS AS POSSIBLE but as a side affect they MIGHT CONTAIN AS LITTLE CONTENT AS POSSIBLE
 fn truncate_columns_by_columns(data: &mut Data, theme: &TableTheme, termwidth: usize) -> bool {
-    const MIN_ACCEPTABLE_WIDTH: usize = 3;
+    const ACCEPTABLE_WIDTH: usize = 10 + 2;
-    const TRAILING_COLUMN_WIDTH: usize = 3;
+    const TRAILING_COLUMN_WIDTH: usize = 3 + 2;
-    const TRAILING_COLUMN_PADDING: usize = 2;
     const TRAILING_COLUMN_STR: &str = "...";
 
     let config;
@@ -518,14 +506,14 @@ fn truncate_columns_by_columns(data: &mut Data, theme: &TableTheme, termwidth: u
         return false;
     }
 
-    let mut width_ctrl = tabled::papergrid::width::WidthEstimator::default();
+    let mut width_ctrl = WidthEstimator::default();
     width_ctrl.estimate(&*data, &config);
     let widths = Vec::from(width_ctrl);
     let widths_total = widths.iter().sum::<usize>();
 
     let min_widths = widths
         .iter()
-        .map(|w| min(*w, MIN_ACCEPTABLE_WIDTH))
+        .map(|w| min(*w, ACCEPTABLE_WIDTH))
         .sum::<usize>();
     let mut min_total = total - widths_total + min_widths;
 
@@ -533,15 +521,14 @@ fn truncate_columns_by_columns(data: &mut Data, theme: &TableTheme, termwidth: u
         return false;
     }
 
+    let mut i = 0;
     while data.count_columns() > 0 {
-        let column = data.count_columns() - 1;
+        i += 1;
 
-        data.truncate(column);
+        let column = data.count_columns() - 1 - i;
+        let width = min(widths[column], ACCEPTABLE_WIDTH);
+        min_total -= width;
 
-        let width = widths[column];
-        let min_width = min(width, MIN_ACCEPTABLE_WIDTH);
-
-        min_total -= min_width;
         if config.get_borders().has_vertical() {
             min_total -= 1;
         }
@@ -551,15 +538,16 @@ fn truncate_columns_by_columns(data: &mut Data, theme: &TableTheme, termwidth: u
         }
     }
 
-    if data.count_columns() == 0 {
+    if i + 1 == data.count_columns() {
         return true;
     }
 
-    // Append columns with a trailing column
+    data.truncate(data.count_columns() - i);
 
+    // Append columns with a trailing column
     let diff = termwidth - min_total;
-    if diff > TRAILING_COLUMN_WIDTH + TRAILING_COLUMN_PADDING {
+    if diff > TRAILING_COLUMN_WIDTH {
-        let cell = Table::create_cell(String::from(TRAILING_COLUMN_STR), TextStyle::default());
+        let cell = Table::create_cell(TRAILING_COLUMN_STR, TextStyle::default());
         data.push(cell);
     } else {
         if data.count_columns() == 1 {
@@ -568,7 +556,7 @@ fn truncate_columns_by_columns(data: &mut Data, theme: &TableTheme, termwidth: u
 
         data.truncate(data.count_columns() - 1);
 
-        let cell = Table::create_cell(String::from(TRAILING_COLUMN_STR), TextStyle::default());
+        let cell = Table::create_cell(TRAILING_COLUMN_STR, TextStyle::default());
         data.push(cell);
     }
 

crates/nu-table/tests/common.rs

@@ -63,6 +63,6 @@ pub fn create_row(count_columns: usize) -> Vec<TCell<CellInfo<'static>, TextStyl
 }
 
 #[allow(dead_code)]
-pub fn styled_str(s: &str) -> TCell<CellInfo<'static>, TextStyle> {
+pub fn _str(s: &str) -> TCell<CellInfo<'static>, TextStyle> {
     Table::create_cell(s.to_string(), TextStyle::default())
 }