Refactor table cmd and nu-table with Record API (#10930)

# Description
- Simplify `table` record highlight with `.get_mut`
  - pretty straight forward
- Use record iterators in `table` abbreviation logic
- This required some rework if we go from guaranted contiguous arrays to
iterators
- Refactor `nu-table` internals to new record API
# User-Facing Changes
None intened

# Tests + Formatting
(-)
This commit is contained in:
Stefan Holderbach 2023-11-08 01:22:47 +01:00 committed by GitHub
parent f45aed257f
commit 7ebae0b5f7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 80 additions and 71 deletions

View file

@ -354,10 +354,14 @@ fn handle_record(
}; };
if let Some(limit) = cfg.abbreviation { if let Some(limit) = cfg.abbreviation {
if record.cols.len() > limit * 2 + 1 { let prev_len = record.len();
record.cols = abbreviate_list(&record.cols, limit, String::from("...")); if record.len() > limit * 2 + 1 {
record.vals = // TODO: see if the following table builders would be happy with a simple iterator
abbreviate_list(&record.vals, limit, Value::string("...", Span::unknown())); let mut record_iter = record.into_iter();
record = Record::with_capacity(limit * 2 + 1);
record.extend(record_iter.by_ref().take(limit));
record.push(String::from("..."), Value::string("...", Span::unknown()));
record.extend(record_iter.skip(prev_len - 2 * limit));
} }
} }
@ -456,26 +460,19 @@ fn handle_row_stream(
None => None, None => None,
}; };
let ls_colors = get_ls_colors(ls_colors_env_str); let ls_colors = get_ls_colors(ls_colors_env_str);
let span = input.call.head;
ListStream::from_stream( ListStream::from_stream(
stream.map(move |mut x| match &mut x { stream.map(move |mut x| match &mut x {
Value::Record { val: record, .. } => { Value::Record { val: record, .. } => {
let mut idx = 0; // Only the name column gets special colors, for now
if let Some(value) = record.get_mut("name") {
while idx < record.len() { let span = value.span();
// Only the name column gets special colors, for now if let Value::String { val, .. } = value {
if record.cols[idx] == "name" { if let Some(val) = render_path_name(val, &config, &ls_colors, span)
let span = record.vals.get(idx).map(|v| v.span()).unwrap_or(span); {
if let Some(Value::String { val, .. }) = record.vals.get(idx) { *value = val;
let val = render_path_name(val, &config, &ls_colors, span);
if let Some(val) = val {
record.vals[idx] = val;
}
} }
} }
idx += 1;
} }
x x
@ -490,36 +487,34 @@ fn handle_row_stream(
data_source: DataSource::HtmlThemes, data_source: DataSource::HtmlThemes,
}) => { }) => {
let ctrlc = ctrlc.clone(); let ctrlc = ctrlc.clone();
let span = input.call.head;
ListStream::from_stream( ListStream::from_stream(
stream.map(move |mut x| match &mut x { stream.map(move |mut x| match &mut x {
Value::Record { val: record, .. } => { Value::Record { val: record, .. } => {
let mut idx = 0; for (rec_col, rec_val) in record.iter_mut() {
// Every column in the HTML theme table except 'name' is colored // Every column in the HTML theme table except 'name' is colored
while idx < record.len() { if rec_col != "name" {
if record.cols[idx] != "name" { continue;
// Simple routine to grab the hex code, convert to a style, }
// then place it in a new Value::String. // Simple routine to grab the hex code, convert to a style,
// then place it in a new Value::String.
let span = record.vals.get(idx).map(|v| v.span()).unwrap_or(span);
if let Some(Value::String { val, .. }) = record.vals.get(idx) { let span = rec_val.span();
let s = match color_from_hex(val) { if let Value::String { val, .. } = rec_val {
Ok(c) => match c { let s = match color_from_hex(val) {
// .normal() just sets the text foreground color. Ok(c) => match c {
Some(c) => c.normal(), // .normal() just sets the text foreground color.
None => nu_ansi_term::Style::default(), Some(c) => c.normal(),
}, None => nu_ansi_term::Style::default(),
Err(_) => nu_ansi_term::Style::default(), },
}; Err(_) => nu_ansi_term::Style::default(),
record.vals[idx] = Value::string( };
// Apply the style (ANSI codes) to the string *rec_val = Value::string(
s.paint(val).to_string(), // Apply the style (ANSI codes) to the string
span, s.paint(&*val).to_string(),
); span,
} );
} }
idx += 1;
} }
x x
} }
@ -756,10 +751,19 @@ impl Iterator for PagingTableCreator {
if limit > 0 && is_record_list { if limit > 0 && is_record_list {
// in case it's a record list we set a default text to each column instead of a single value. // in case it's a record list we set a default text to each column instead of a single value.
let cols = batch[0].as_record().expect("ok").cols.clone(); let dummy: Record = batch[0]
let vals = .as_record()
vec![Value::string(String::from("..."), Span::unknown()); cols.len()]; .expect("ok")
batch[limit] = Value::record(Record { cols, vals }, Span::unknown()); .columns()
.map(|k| {
(
k.to_owned(),
Value::string(String::from("..."), Span::unknown()),
)
})
.collect();
batch[limit] = Value::record(dummy, Span::unknown());
} }
} }
} }

View file

@ -1,5 +1,5 @@
use nu_color_config::StyleComputer; use nu_color_config::StyleComputer;
use nu_protocol::{Config, Value}; use nu_protocol::{Config, Record, Value};
use crate::UnstructuredTable; use crate::UnstructuredTable;
@ -40,17 +40,22 @@ fn collapsed_table(
fn colorize_value(value: &mut Value, config: &Config, style_computer: &StyleComputer) { fn colorize_value(value: &mut Value, config: &Config, style_computer: &StyleComputer) {
match value { match value {
Value::Record { val: record, .. } => { Value::Record { ref mut val, .. } => {
for val in &mut record.vals {
colorize_value(val, config, style_computer);
}
let style = get_index_style(style_computer); let style = get_index_style(style_computer);
if let Some(color) = style.color_style { // Take ownership of the record and reassign to &mut
for header in &mut record.cols { // We do this to have owned keys through `.into_iter`
*header = color.paint(header.to_owned()).to_string(); let record = std::mem::take(val);
} *val = record
} .into_iter()
.map(|(mut header, mut val)| {
colorize_value(&mut val, config, style_computer);
if let Some(color) = style.color_style {
header = color.paint(header).to_string();
}
(header, val)
})
.collect::<Record>();
} }
Value::List { vals, .. } => { Value::List { vals, .. } => {
for val in vals { for val in vals {

View file

@ -344,8 +344,7 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> StringResult { fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> StringResult {
let theme = load_theme_from_config(cfg.opts.config); let theme = load_theme_from_config(cfg.opts.config);
let key_width = record let key_width = record
.cols .columns()
.iter()
.map(|col| string_width(col)) .map(|col| string_width(col))
.max() .max()
.unwrap_or(0); .unwrap_or(0);

View file

@ -157,17 +157,18 @@ fn build_vertical_array(vals: Vec<Value>, config: &Config) -> TableValue {
} }
fn is_valid_record(vals: &[Value]) -> bool { fn is_valid_record(vals: &[Value]) -> bool {
let mut used_cols: Option<&[String]> = None; let mut first_record: Option<&Record> = None;
for val in vals { for val in vals {
match val { match val {
Value::Record { val, .. } => { Value::Record { val, .. } => {
let cols_are_not_equal = if let Some(known) = first_record {
used_cols.is_some() && !matches!(used_cols, Some(used) if val.cols == used); let equal = known.columns().eq(val.columns());
if cols_are_not_equal { if !equal {
return false; return false;
} }
} else {
used_cols = Some(&val.cols); first_record = Some(val)
};
} }
_ => return false, _ => return false,
} }
@ -195,8 +196,8 @@ fn build_map_from_record(vals: Vec<Value>, config: &Config) -> TableValue {
for val in vals { for val in vals {
match val { match val {
Value::Record { val, .. } => { Value::Record { val, .. } => {
for (i, cell) in val.vals.into_iter().take(count_columns).enumerate() { for (i, (_key, val)) in val.into_iter().take(count_columns).enumerate() {
let cell = convert_nu_value_to_table_value(cell, config); let cell = convert_nu_value_to_table_value(val, config);
list[i].push(cell); list[i].push(cell);
} }
} }
@ -211,7 +212,7 @@ fn build_map_from_record(vals: Vec<Value>, config: &Config) -> TableValue {
fn get_columns_in_record(vals: &[Value]) -> Vec<String> { fn get_columns_in_record(vals: &[Value]) -> Vec<String> {
match vals.iter().next() { match vals.iter().next() {
Some(Value::Record { val, .. }) => val.cols.clone(), Some(Value::Record { val, .. }) => val.columns().cloned().collect(),
_ => vec![], _ => vec![],
} }
} }