Repository: zhiburt/tabled
Pull request #352 (2023-07-25 23:34:54)

tabled/ Fix column names alignment (remove offset)


Patch:
diff --git a/tabled/src/grid/dimension/complete_dimension_vec_records.rs b/tabled/src/grid/dimension/complete_dimension_vec_records.rs
index ddc806a..54d48ed 100644
--- a/tabled/src/grid/dimension/complete_dimension_vec_records.rs
+++ b/tabled/src/grid/dimension/complete_dimension_vec_records.rs
@@ -43,6 +43,17 @@ impl CompleteDimensionVecRecords<'_> {
         true
     }
 
+    /// Get column widths.
+    ///
+    /// In general the method is only considered to be useful to a [`TableOption`].
+    ///
+    /// BE CAREFUL WITH THIS METHOD as it supposed that the content is not bigger than the provided widths.
+    ///
+    /// [`TableOption`]: crate::settings::TableOption
+    pub fn get_widths(&self) -> Option<&'_ [usize]> {
+        self.width.as_deref()
+    }
+
     /// Set rows heights.
     ///
     /// In general the method is only considered to be useful to a [`TableOption`].
@@ -56,6 +67,17 @@ impl CompleteDimensionVecRecords<'_> {
         true
     }
 
+    /// Get row heights.
+    ///
+    /// In general the method is only considered to be useful to a [`TableOption`].
+    ///
+    /// BE CAREFUL WITH THIS METHOD as it supposed that the content is not bigger than the provided widths.
+    ///
+    /// [`TableOption`]: crate::settings::TableOption
+    pub fn get_heights(&self) -> Option<&'_ [usize]> {
+        self.height.as_deref()
+    }
+
     /// Force width estimation.
     pub fn clear_width(&mut self) {
         self.width = None;
diff --git a/tabled/src/settings/style/border_text.rs b/tabled/src/settings/style/border_text.rs
index 163a198..236c120 100644
--- a/tabled/src/settings/style/border_text.rs
+++ b/tabled/src/settings/style/border_text.rs
@@ -35,6 +35,7 @@ use super::Offset;
 /// ```
 #[derive(Debug)]
 pub struct BorderText<L> {
+    // todo: change to T and specify to be As<str>
     text: String,
     offset: Offset,
     color: Option<AnsiColor<'static>>,
diff --git a/tabled/src/settings/themes/column_names.rs b/tabled/src/settings/themes/column_names.rs
index 0284816..7ceb752 100644
--- a/tabled/src/settings/themes/column_names.rs
+++ b/tabled/src/settings/themes/column_names.rs
@@ -1,6 +1,8 @@
+use std::cmp;
+
 use crate::{
     grid::{
-        config::{AlignmentHorizontal, ColoredConfig},
+        config::{AlignmentHorizontal, ColoredConfig, Position},
         dimension::{CompleteDimensionVecRecords, Dimension, Estimate},
         records::{
             vec_records::{CellInfo, VecRecords},
@@ -22,7 +24,7 @@ use crate::{
 ///
 /// ```
 /// use std::iter::FromIterator;
-/// use tabled::{Table, settings::themes::ColumnNames};
+/// use tabled::{Table, settings::themes::ColumnNames, grid::config::AlignmentHorizontal};
 ///
 /// let data = vec![
 ///     vec!["Hello", "World"],
@@ -30,15 +32,15 @@ use crate::{
 /// ];
 ///
 /// let mut table = Table::from_iter(data);
-/// table.with(ColumnNames::new(["head1", "head2"]).set_offset(3).set_line(2));
+/// table.with(ColumnNames::new(["head1", "head2"]).set_line(2).set_alignment(AlignmentHorizontal::Right));
 ///
 /// assert_eq!(
 ///     table.to_string(),
-///     "+--------+--------+\n\
-///      | Hello  | World  |\n\
-///      +--------+--------+\n\
-///      | Hello  | World  |\n\
-///      +---head1+---head2+"
+///     "+-------+-------+\n\
+///      | Hello | World |\n\
+///      +-------+-------+\n\
+///      | Hello | World |\n\
+///      +--head1+--head2+"
 /// );
 /// ```
 ///
@@ -67,7 +69,6 @@ use crate::{
 pub struct ColumnNames {
     names: Option<Vec<String>>,
     colors: Vec<Option<Color>>,
-    offset: usize,
     line: usize,
     alignment: AlignmentHorizontal,
 }
@@ -77,7 +78,6 @@ impl Default for ColumnNames {
         Self {
             names: Default::default(),
             colors: Default::default(),
-            offset: Default::default(),
             line: Default::default(),
             alignment: AlignmentHorizontal::Left,
         }
@@ -114,7 +114,6 @@ impl ColumnNames {
         Self {
             names: Some(names),
             colors: Vec::new(),
-            offset: 0,
             line: 0,
             alignment: AlignmentHorizontal::Left,
         }
@@ -149,43 +148,12 @@ impl ColumnNames {
         let colors = colors.into_iter().map(Into::into).collect::<Vec<_>>();
         Self {
             names: self.names,
-            offset: self.offset,
             line: self.line,
             alignment: self.alignment,
             colors,
         }
     }
 
-    /// Set a left offset after which the names will be used.
-    ///
-    /// By default there's no offset.
-    ///
-    /// # Example
-    ///
-    /// ```
-    /// use std::iter::FromIterator;
-    /// use tabled::{Table, settings::themes::ColumnNames};
-    ///
-    /// let mut table = Table::from_iter(vec![vec!["Hello", "World"]]);
-    /// table.with(ColumnNames::new(["head1", "head2"]).set_offset(1));
-    ///
-    /// assert_eq!(
-    ///     table.to_string(),
-    ///     "+-head1-+-head2-+\n\
-    ///      | Hello | World |\n\
-    ///      +-------+-------+"
-    /// );
-    /// ```
-    pub fn set_offset(self, i: usize) -> Self {
-        Self {
-            names: self.names,
-            colors: self.colors,
-            line: self.line,
-            alignment: self.alignment,
-            offset: i,
-        }
-    }
-
     /// Set a horizontal line the names will be applied to.
     ///
     /// The default value is 0 (the top horizontal line).
@@ -210,7 +178,6 @@ impl ColumnNames {
         Self {
             names: self.names,
             colors: self.colors,
-            offset: self.offset,
             alignment: self.alignment,
             line: i,
         }
@@ -244,7 +211,6 @@ impl ColumnNames {
         Self {
             names: self.names,
             colors: self.colors,
-            offset: self.offset,
             line: self.line,
             alignment,
         }
@@ -260,78 +226,77 @@ impl TableOption<VecRecords<CellInfo<String>>, CompleteDimensionVecRecords<'stat
         cfg: &mut ColoredConfig,
         dims: &mut CompleteDimensionVecRecords<'static>,
     ) {
-        let names = match self.names {
-            Some(names) => names,
-            None => {
-                if records.count_rows() == 0 || records.count_columns() == 0 {
-                    return;
-                }
-
-                let names = (0..records.count_columns())
-                    .map(|column| records.get_text((0, column)))
-                    .map(ToString::to_string)
-                    .collect::<Vec<_>>();
+        let count_rows = records.count_rows();
+        let count_columns = records.count_columns();
 
-                records.remove_row(0);
+        if count_columns == 0 || count_rows == 0 || self.line > count_rows {
+            return;
+        }
 
-                names
-            }
+        let names = match self.names {
+            Some(names) => names,
+            None => collect_head(records),
         };
 
-        let names = names.iter().map(|name| name.lines().next().unwrap_or(""));
+        let names = names
+            .iter()
+            .take(count_columns)
+            .map(|name| name.lines().next().unwrap_or(""))
+            .collect::<Vec<_>>();
 
         dims.estimate(&*records, cfg);
 
-        let mut widths = (0..records.count_columns())
-            .map(|column| dims.get_width(column))
-            .collect::<Vec<_>>();
-
-        let names = names.take(widths.len());
+        let widths = (0..count_columns)
+            .map(|column| {
+                let text = names.get(column).unwrap_or(&"");
+                let width = dims.get_width(column);
 
-        let offset = self.offset;
-        widths
-            .iter_mut()
-            .zip(names.clone())
-            .for_each(|(width, text)| {
-                let name_width = string_width(text) + offset;
-                *width = std::cmp::max(name_width, *width);
-            });
+                (width, text)
+            })
+            .map(|(width, text)| cmp::max(string_width(text), width))
+            .collect::<Vec<_>>();
 
         let _ = dims.set_widths(widths.clone());
 
         let mut total_width = 0;
-        for (i, (width, name)) in widths.iter().zip(names).enumerate() {
+        for (i, (width, name)) in widths.into_iter().zip(names).enumerate() {
+            let left_vertical = get_vertical_width(cfg, (self.line, i), count_columns);
             let color = get_color(&self.colors, i);
-            let offset = total_width + 1;
-            let btext = get_border_text(
-                name,
-                offset,
-                *width,
-                self.alignment,
-                self.offset,
-                self.line,
-                color,
-            );
+            let grid_offset = total_width + left_vertical;
+            let btext = get_border_text(name, grid_offset, width, self.alignment, self.line, color);
             btext.change(records, cfg, dims);
 
-            total_width += width + 1;
+            total_width += width + left_vertical;
         }
     }
 }
 
+fn collect_head(records: &mut VecRecords<CellInfo<String>>) -> Vec<String> {
+    if records.count_rows() == 0 || records.count_columns() == 0 {
+        return Vec::new();
+    }
+
+    let names = (0..records.count_columns())
+        .map(|column| records.get_text((0, column)))
+        .map(ToString::to_string)
+        .collect();
+
+    records.remove_row(0);
+
+    names
+}
+
 fn get_border_text(
     text: &str,
     offset: usize,
     available: usize,
-    alignment: AlignmentHorizontal,
-    alignment_offset: usize,
+    align: AlignmentHorizontal,
     line: usize,
     color: Option<&Color>,
 ) -> BorderText<usize> {
-    let name = text.to_string();
-    let left_indent = get_indent(text, alignment, alignment_offset, available);
-    let left_offset = Offset::Begin(offset + left_indent);
-    let mut btext = BorderText::new(name).horizontal(line).offset(left_offset);
+    let left_indent = get_indent(text, align, available);
+    let offset = Offset::Begin(offset + left_indent);
+    let mut btext = BorderText::new(text).horizontal(line).offset(offset);
     if let Some(color) = color {
         btext = btext.color(color.clone());
     }
@@ -346,10 +311,16 @@ fn get_color(colors: &[Option<Color>], i: usize) -> Option<&Color> {
     })
 }
 
-fn get_indent(text: &str, align: AlignmentHorizontal, offset: usize, available: usize) -> usize {
+fn get_indent(text: &str, align: AlignmentHorizontal, available: usize) -> usize {
     match align {
-        AlignmentHorizontal::Left => offset,
-        AlignmentHorizontal::Right => available - string_width(text) - offset,
+        AlignmentHorizontal::Left => 0,
+        AlignmentHorizontal::Right => available - string_width(text),
         AlignmentHorizontal::Center => (available - string_width(text)) / 2,
     }
 }
+
+fn get_vertical_width(cfg: &mut ColoredConfig, pos: Position, count_columns: usize) -> usize {
+    cfg.get_vertical(pos, count_columns)
+        .and_then(unicode_width::UnicodeWidthChar::width)
+        .unwrap_or(0)
+}
diff --git a/tabled/src/tables/table.rs b/tabled/src/tables/table.rs
index d6ff376..b63902b 100644
--- a/tabled/src/tables/table.rs
+++ b/tabled/src/tables/table.rs
@@ -183,6 +183,9 @@ impl Table {
             ColoredConfig,
         >,
     {
+        // todo: because we do it we can't reuse cached values....
+        // better we set a clear flag and then after option is done check if we need to clear or it was reset by the option.
+
         self.dimension.clear_width();
         self.dimension.clear_height();
 


Test patch:
diff --git a/tabled/tests/settings/column_names_test.rs b/tabled/tests/settings/column_names_test.rs
index c2b23f1..37b1a26 100644
--- a/tabled/tests/settings/column_names_test.rs
+++ b/tabled/tests/settings/column_names_test.rs
@@ -113,78 +113,6 @@ test_table!(
     "+-------+----------+"
 );
 
-test_table!(
-    alignment_left_offset,
-    Table::new([("Hello", "World"), ("and", "looooong\nword")])
-        .with(ColumnNames::default().set_alignment(AlignmentHorizontal::Left).set_offset(1)),
-    "+-&str--+-&str-----+"
-    "| Hello | World    |"
-    "+-------+----------+"
-    "| and   | looooong |"
-    "|       | word     |"
-    "+-------+----------+"
-);
-
-test_table!(
-    alignment_left_offset_big,
-    Table::new([("Hello", "World"), ("and", "looooong\nword")])
-        .with(ColumnNames::default().set_alignment(AlignmentHorizontal::Left).set_offset(30)),
-    "+------------------------------&str+------------------------------&str+"
-    "| Hello                            | World                            |"
-    "+----------------------------------+----------------------------------+"
-    "| and                              | looooong                         |"
-    "|                                  | word                             |"
-    "+----------------------------------+----------------------------------+"
-);
-
-test_table!(
-    alignment_left_offset_0,
-    Table::new([("Hello", "World"), ("and", "looooong\nword")])
-        .with(ColumnNames::default().set_alignment(AlignmentHorizontal::Left).set_offset(0)),
-    "+&str---+&str------+"
-    "| Hello | World    |"
-    "+-------+----------+"
-    "| and   | looooong |"
-    "|       | word     |"
-    "+-------+----------+"
-);
-
-test_table!(
-    alignment_right_offset,
-    Table::new([("Hello", "World"), ("and", "looooong\nword")])
-        .with(ColumnNames::default().set_alignment(AlignmentHorizontal::Right).set_offset(1)),
-    "+--&str-+-----&str-+"
-    "| Hello | World    |"
-    "+-------+----------+"
-    "| and   | looooong |"
-    "|       | word     |"
-    "+-------+----------+"
-);
-
-test_table!(
-    alignment_right_offset_big,
-    Table::new([("Hello", "World"), ("and", "looooong\nword")])
-        .with(ColumnNames::default().set_alignment(AlignmentHorizontal::Right).set_offset(30)),
-    "+&str------------------------------+&str------------------------------+"
-    "| Hello                            | World                            |"
-    "+----------------------------------+----------------------------------+"
-    "| and                              | looooong                         |"
-    "|                                  | word                             |"
-    "+----------------------------------+----------------------------------+"
-);
-
-test_table!(
-    alignment_right_offset_0,
-    Table::new([("Hello", "World"), ("and", "looooong\nword")])
-        .with(ColumnNames::default().set_alignment(AlignmentHorizontal::Right).set_offset(0)),
-    "+---&str+------&str+"
-    "| Hello | World    |"
-    "+-------+----------+"
-    "| and   | looooong |"
-    "|       | word     |"
-    "+-------+----------+"
-);
-
 test_table!(
     set_line,
     Matrix::new(3, 3).with(ColumnNames::default().set_line(1)),
@@ -200,13 +128,15 @@ test_table!(
 test_table!(
     set_line_max_out,
     Matrix::new(3, 3).with(ColumnNames::default().set_line(100)),
-    "+---+--------+--------+--------+"
-    "| 0 |  0-0   |  0-1   |  0-2   |"
-    "+---+--------+--------+--------+"
-    "| 1 |  1-0   |  1-1   |  1-2   |"
-    "+---+--------+--------+--------+"
-    "| 2 |  2-0   |  2-1   |  2-2   |"
-    "+---+--------+--------+--------+"
+    "+---+----------+----------+----------+"
+    "| N | column 0 | column 1 | column 2 |"
+    "+---+----------+----------+----------+"
+    "| 0 |   0-0    |   0-1    |   0-2    |"
+    "+---+----------+----------+----------+"
+    "| 1 |   1-0    |   1-1    |   1-2    |"
+    "+---+----------+----------+----------+"
+    "| 2 |   2-0    |   2-1    |   2-2    |"
+    "+---+----------+----------+----------+"
 );
 
 test_table!(