nu-table/ Do footer_inheritance by accouting for rows rather then a f… (#14380)

So it's my take on the comments in #14060 

The change could be seen in this test.
Looks like it works :) but I haven't done a lot of testing.


0b1af77415/crates/nu-command/tests/commands/table.rs (L3032-L3062)

```nushell
$env.config.table.footer_inheritance = true;
$env.config.footer_mode = 7;
[[a b]; ['kv' {0: [[field]; [0] [1] [2] [3] [4] [5]]} ], ['data' 0], ['data' 0] ] | table --expand --width=80
```

```text
╭───┬──────┬───────────────────────╮
│ # │  a   │           b           │
├───┼──────┼───────────────────────┤
│ 0 │ kv   │ ╭───┬───────────────╮ │
│   │      │ │   │ ╭───┬───────╮ │ │
│   │      │ │ 0 │ │ # │ field │ │ │
│   │      │ │   │ ├───┼───────┤ │ │
│   │      │ │   │ │ 0 │     0 │ │ │
│   │      │ │   │ │ 1 │     1 │ │ │
│   │      │ │   │ │ 2 │     2 │ │ │
│   │      │ │   │ │ 3 │     3 │ │ │
│   │      │ │   │ │ 4 │     4 │ │ │
│   │      │ │   │ │ 5 │     5 │ │ │
│   │      │ │   │ ╰───┴───────╯ │ │
│   │      │ ╰───┴───────────────╯ │
│ 1 │ data │                     0 │
│ 2 │ data │                     0 │
├───┼──────┼───────────────────────┤
│ # │  a   │           b           │
╰───┴──────┴───────────────────────╯
```

Maybe it will also solve the issue you @fdncred encountered.

close #14060
cc: @NotTheDr01ds
This commit is contained in:
Maxim Zhiburt 2024-11-20 00:31:28 +03:00 committed by GitHub
parent 9cffbdb42a
commit b6ce907928
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 170 additions and 65 deletions

View file

@ -1088,7 +1088,7 @@ fn create_empty_placeholder(
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, false);
let out = TableOutput::new(table, false, false, 1);
let style_computer = &StyleComputer::from_config(engine_state, stack);
let config = create_nu_table_config(&config, style_computer, &out, false, TableMode::default());

View file

@ -2941,3 +2941,123 @@ fn table_footer_inheritance() {
assert_eq!(actual.out.match_indices("x2").count(), 1);
assert_eq!(actual.out.match_indices("x3").count(), 1);
}
#[test]
fn table_footer_inheritance_kv_rows() {
let actual = nu!(
concat!(
"$env.config.table.footer_inheritance = true;",
"$env.config.footer_mode = 7;",
"[[a b]; ['kv' {0: 0, 1: 1, 2: 2, 3: 3, 4: 4} ], ['data' 0], ['data' 0] ] | table --expand --width=80",
)
);
assert_eq!(
actual.out,
"╭───┬──────┬───────────╮\
# a b \
\
0 kv \
0 0 \
1 1 \
2 2 \
3 3 \
4 4 \
\
1 data 0 \
2 data 0 \
"
);
let actual = nu!(
concat!(
"$env.config.table.footer_inheritance = true;",
"$env.config.footer_mode = 7;",
"[[a b]; ['kv' {0: 0, 1: 1, 2: 2, 3: 3, 4: 4, 5: 5} ], ['data' 0], ['data' 0] ] | table --expand --width=80",
)
);
assert_eq!(
actual.out,
"╭───┬──────┬───────────╮\
# a b \
\
0 kv \
0 0 \
1 1 \
2 2 \
3 3 \
4 4 \
5 5 \
\
1 data 0 \
2 data 0 \
\
# a b \
"
);
}
#[test]
fn table_footer_inheritance_list_rows() {
let actual = nu!(
concat!(
"$env.config.table.footer_inheritance = true;",
"$env.config.footer_mode = 7;",
"[[a b]; ['kv' {0: [[field]; [0] [1] [2] [3] [4]]} ], ['data' 0], ['data' 0] ] | table --expand --width=80",
)
);
assert_eq!(
actual.out,
"╭───┬──────┬───────────────────────╮\
# a b \
\
0 kv \
\
0 # field \
\
0 0 \
1 1 \
2 2 \
3 3 \
4 4 \
\
\
1 data 0 \
2 data 0 \
"
);
let actual = nu!(
concat!(
"$env.config.table.footer_inheritance = true;",
"$env.config.footer_mode = 7;",
"[[a b]; ['kv' {0: [[field]; [0] [1] [2] [3] [4] [5]]} ], ['data' 0], ['data' 0] ] | table --expand --width=80",
)
);
assert_eq!(
actual.out,
"╭───┬──────┬───────────────────────╮\
# a b \
\
0 kv \
\
0 # field \
\
0 0 \
1 1 \
2 2 \
3 3 \
4 4 \
5 5 \
\
\
1 data 0 \
2 data 0 \
\
# a b \
"
);
}

View file

@ -18,8 +18,12 @@ pub fn create_nu_table_config(
expand: bool,
mode: TableMode,
) -> NuTableConfig {
let with_footer = (config.table.footer_inheritance && out.with_footer)
|| with_footer(config, out.with_header, out.table.count_rows());
let mut count_rows = out.table.count_rows();
if config.table.footer_inheritance {
count_rows = out.count_rows;
}
let with_footer = with_footer(config, out.with_header, count_rows);
NuTableConfig {
theme: load_theme(mode),

View file

@ -615,12 +615,15 @@ fn load_theme(
if let Some(style) = sep_color {
let color = convert_style(style);
let color = ANSIBuf::from(color);
// todo: use .modify(Segment::all(), color) --> it has this optimization
table.get_config_mut().set_border_color_default(color);
}
if !with_header {
// todo: remove and use theme.remove_horizontal_lines();
table.with(RemoveHorizontalLine);
} else if with_footer {
// todo: remove and set it on theme rather then here...
table.with(CopyFirstHorizontalLineAtLast);
}
}
@ -1257,6 +1260,7 @@ fn remove_row(recs: &mut NuRecords, row: usize) -> Vec<String> {
columns
}
// todo; use Format?
struct StripColorFromRow(usize);
impl TableOption<NuRecords, ColoredConfig, CompleteDimensionVecRecords<'_>> for StripColorFromRow {

View file

@ -5,7 +5,7 @@ use crate::{
NuText, StringResult, TableResult, INDEX_COLUMN_NAME,
},
string_width,
types::{has_footer, has_index},
types::has_index,
NuTable, NuTableCell, TableOpts, TableOutput,
};
use nu_color_config::{Alignment, StyleComputer, TextStyle};
@ -63,22 +63,22 @@ struct Cfg<'a> {
struct CellOutput {
text: String,
style: TextStyle,
is_big: bool,
size: usize,
is_expanded: bool,
}
impl CellOutput {
fn new(text: String, style: TextStyle, is_big: bool, is_expanded: bool) -> Self {
fn new(text: String, style: TextStyle, size: usize, is_expanded: bool) -> Self {
Self {
text,
style,
is_big,
size,
is_expanded,
}
}
fn clean(text: String, is_big: bool, is_expanded: bool) -> Self {
Self::new(text, Default::default(), is_big, is_expanded)
fn clean(text: String, size: usize, is_expanded: bool) -> Self {
Self::new(text, Default::default(), size, is_expanded)
}
fn text(text: String) -> Self {
@ -86,7 +86,7 @@ impl CellOutput {
}
fn styled(text: NuText) -> Self {
Self::new(text.0, text.1, false, false)
Self::new(text.0, text.1, 1, false)
}
}
@ -117,7 +117,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
let with_index = has_index(&cfg.opts, &headers);
let row_offset = cfg.opts.index_offset;
let mut is_footer_used = false;
let mut rows_count = 0usize;
// The header with the INDEX is removed from the table headers since
// it is added to the natural table index
@ -199,9 +199,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
data[row].push(value);
data_styles.insert((row, with_index as usize), cell.style);
if cell.is_big {
is_footer_used = cell.is_big;
}
rows_count = rows_count.saturating_add(cell.size);
}
let mut table = NuTable::from(data);
@ -209,12 +207,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
table.set_index_style(get_index_style(cfg.opts.style_computer));
set_data_styles(&mut table, data_styles);
return Ok(Some(TableOutput::new(
table,
false,
with_index,
is_footer_used,
)));
return Ok(Some(TableOutput::new(table, false, with_index, rows_count)));
}
if !headers.is_empty() {
@ -269,6 +262,8 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
}
}
let mut column_rows = 0usize;
for (row, item) in input.iter().enumerate() {
cfg.opts.signals.check(cfg.opts.span)?;
@ -294,9 +289,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
data[row + 1].push(value);
data_styles.insert((row + 1, col + with_index as usize), cell.style);
if cell.is_big {
is_footer_used = cell.is_big;
}
column_rows = column_rows.saturating_add(cell.size);
}
let head_cell = NuTableCell::new(header);
@ -316,6 +309,8 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
available_width -= pad_space + column_width;
rendered_column += 1;
rows_count = std::cmp::max(rows_count, column_rows);
}
if truncate && rendered_column == 0 {
@ -374,9 +369,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
table.set_indent(cfg.opts.indent.0, cfg.opts.indent.1);
set_data_styles(&mut table, data_styles);
let has_footer = is_footer_used || has_footer(&cfg.opts, table.count_rows() as u64);
Ok(Some(TableOutput::new(table, true, with_index, has_footer)))
Ok(Some(TableOutput::new(table, true, with_index, rows_count)))
}
fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
@ -395,7 +388,7 @@ fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
let value_width = cfg.opts.width - key_width - count_borders - padding - padding;
let mut with_footer = false;
let mut count_rows = 0usize;
let mut data = Vec::with_capacity(record.len());
for (key, value) in record {
@ -420,19 +413,17 @@ fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> CellResult {
data.push(row);
if cell.is_big {
with_footer = cell.is_big;
}
count_rows = count_rows.saturating_add(cell.size);
}
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);
let out = TableOutput::new(table, false, true, with_footer);
let out = TableOutput::new(table, false, true, count_rows);
maybe_expand_table(out, cfg.opts.width, &cfg.opts)
.map(|value| value.map(|value| CellOutput::clean(value, with_footer, false)))
.map(|value| value.map(|value| CellOutput::clean(value, count_rows, false)))
}
// the flag is used as an optimization to not do `value.lines().count()` search.
@ -441,7 +432,7 @@ fn expand_table_value(value: &Value, value_width: usize, cfg: &Cfg<'_>) -> CellR
if is_limited {
return Ok(Some(CellOutput::clean(
value_to_string_clean(value, cfg),
false,
1,
false,
)));
}
@ -457,7 +448,7 @@ fn expand_table_value(value: &Value, value_width: usize, cfg: &Cfg<'_>) -> CellR
let cfg = create_table_cfg(cfg, &out);
let value = out.table.draw(cfg, value_width);
match value {
Some(value) => Ok(Some(CellOutput::clean(value, out.with_footer, true))),
Some(value) => Ok(Some(CellOutput::clean(value, out.count_rows, true))),
None => Ok(None),
}
}
@ -484,7 +475,7 @@ fn expand_table_value(value: &Value, value_width: usize, cfg: &Cfg<'_>) -> CellR
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.is_big, true))),
Some(result) => Ok(Some(CellOutput::clean(result.text, result.size, true))),
None => Ok(Some(CellOutput::text(value_to_wrapped_string(
value,
cfg,
@ -575,7 +566,7 @@ fn expanded_table_entry2(item: &Value, cfg: Cfg<'_>) -> CellOutput {
let table_config = create_table_cfg(&cfg, &out);
let table = out.table.draw(table_config, usize::MAX);
match table {
Some(table) => CellOutput::clean(table, out.with_footer, false),
Some(table) => CellOutput::clean(table, out.count_rows, false),
None => CellOutput::styled(nu_value_to_string(
item,
cfg.opts.config,

View file

@ -56,8 +56,9 @@ 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();
let mut out = TableOutput::new(table, false, true, false);
let mut out = TableOutput::new(table, false, true, count_rows);
let left = opts.config.table.padding.left;
let right = opts.config.table.padding.right;
@ -82,7 +83,10 @@ fn table(input: &[Value], opts: &TableOpts<'_>) -> TableResult {
let with_header = !headers.is_empty();
if !with_header {
let table = to_table_with_no_header(input, with_index, row_offset, opts)?;
let table = table.map(|table| TableOutput::new(table, false, with_index, false));
let table = table.map(|table| {
let count_rows = table.count_rows();
TableOutput::new(table, false, with_index, count_rows)
});
return Ok(table);
}
@ -98,7 +102,10 @@ fn table(input: &[Value], opts: &TableOpts<'_>) -> TableResult {
.collect();
let table = to_table_with_header(input, &headers, with_index, row_offset, opts)?;
let table = table.map(|table| TableOutput::new(table, true, with_index, false));
let table = table.map(|table| {
let count_rows = table.count_rows();
TableOutput::new(table, true, with_index, count_rows)
});
Ok(table)
}

View file

@ -1,8 +1,7 @@
use terminal_size::{terminal_size, Height, Width};
use nu_color_config::StyleComputer;
use nu_protocol::{Config, Signals, Span, TableIndexMode, TableMode};
use crate::{common::INDEX_COLUMN_NAME, NuTable};
use nu_color_config::StyleComputer;
use nu_protocol::{Config, FooterMode, Signals, Span, TableIndexMode, TableMode};
mod collapse;
mod expanded;
@ -16,16 +15,16 @@ pub struct TableOutput {
pub table: NuTable,
pub with_header: bool,
pub with_index: bool,
pub with_footer: bool,
pub count_rows: usize,
}
impl TableOutput {
pub fn new(table: NuTable, with_header: bool, with_index: bool, with_footer: bool) -> Self {
pub fn new(table: NuTable, with_header: bool, with_index: bool, count_rows: usize) -> Self {
Self {
table,
with_header,
with_index,
with_footer,
count_rows,
}
}
}
@ -79,23 +78,3 @@ fn has_index(opts: &TableOpts<'_>, headers: &[String]) -> bool {
with_index && !opts.index_remove
}
fn has_footer(opts: &TableOpts<'_>, count_records: u64) -> bool {
match opts.config.footer_mode {
// Only show the footer if there are more than RowCount rows
FooterMode::RowCount(limit) => count_records > limit,
// Always show the footer
FooterMode::Always => true,
// Never show the footer
FooterMode::Never => false,
// Calculate the screen height and row count, if screen height is larger than row count, don't show footer
FooterMode::Auto => {
let (_width, height) = match terminal_size() {
Some((w, h)) => (Width(w.0).0 as u64, Height(h.0).0 as u64),
None => (Width(0).0 as u64, Height(0).0 as u64),
};
height <= count_records
}
}
}