Create Record type (#10103)

# Description
This PR creates a new `Record` type to reduce duplicate code and
possibly bugs as well. (This is an edited version of #9648.)
- `Record` implements `FromIterator` and `IntoIterator` and so can be
iterated over or collected into. For example, this helps with
conversions to and from (hash)maps. (Also, no more
`cols.iter().zip(vals)`!)
- `Record` has a `push(col, val)` function to help insure that the
number of columns is equal to the number of values. I caught a few
potential bugs thanks to this (e.g. in the `ls` command).
- Finally, this PR also adds a `record!` macro that helps simplify
record creation. It is used like so:
   ```rust
   record! {
       "key1" => some_value,
       "key2" => Value::string("text", span),
       "key3" => Value::int(optional_int.unwrap_or(0), span),
       "key4" => Value::bool(config.setting, span),
   }
   ```
Since macros hinder formatting, etc., the right hand side values should
be relatively short and sweet like the examples above.

Where possible, prefer `record!` or `.collect()` on an iterator instead
of multiple `Record::push`s, since the first two automatically set the
record capacity and do less work overall.

# User-Facing Changes
Besides the changes in `nu-protocol` the only other breaking changes are
to `nu-table::{ExpandedTable::build_map, JustTable::kv_table}`.
This commit is contained in:
Ian Manske
2023-08-24 19:50:29 +00:00
committed by GitHub
parent 030e749fe7
commit 8da27a1a09
195 changed files with 4211 additions and 6245 deletions

View File

@ -40,14 +40,14 @@ fn collapsed_table(
fn colorize_value(value: &mut Value, config: &Config, style_computer: &StyleComputer) {
match value {
Value::Record { cols, vals, .. } => {
for val in vals {
Value::Record { val: record, .. } => {
for val in &mut record.vals {
colorize_value(val, config, style_computer);
}
let style = get_index_style(style_computer);
if let Some(color) = style.color_style {
for header in cols {
for header in &mut record.cols {
*header = color.paint(header.to_owned()).to_string();
}
}

View File

@ -3,7 +3,7 @@ use std::collections::HashMap;
use nu_color_config::{Alignment, StyleComputer, TextStyle};
use nu_engine::column::get_columns;
use nu_protocol::{ast::PathMember, Config, ShellError, Span, TableIndexMode, Value};
use nu_protocol::{ast::PathMember, Config, Record, ShellError, Span, TableIndexMode, Value};
use tabled::grid::config::Position;
use crate::{
@ -35,8 +35,8 @@ impl ExpandedTable {
expanded_table_entry2(item, Cfg { opts, format: self })
}
pub fn build_map(self, cols: &[String], vals: &[Value], opts: TableOpts<'_>) -> StringResult {
expanded_table_kv(cols, vals, Cfg { opts, format: self })
pub fn build_map(self, record: &Record, opts: TableOpts<'_>) -> StringResult {
expanded_table_kv(record, Cfg { opts, format: self })
}
pub fn build_list(self, vals: &[Value], opts: TableOpts<'_>) -> StringResult {
@ -340,9 +340,14 @@ fn expanded_table_list(input: &[Value], cfg: Cfg<'_>) -> TableResult {
Ok(Some(TableOutput::new(table, true, with_index)))
}
fn expanded_table_kv(cols: &[String], vals: &[Value], cfg: Cfg<'_>) -> StringResult {
fn expanded_table_kv(record: &Record, cfg: Cfg<'_>) -> StringResult {
let theme = load_theme_from_config(cfg.opts.config);
let key_width = cols.iter().map(|col| string_width(col)).max().unwrap_or(0);
let key_width = record
.cols
.iter()
.map(|col| string_width(col))
.max()
.unwrap_or(0);
let count_borders =
theme.has_inner() as usize + theme.has_right() as usize + theme.has_left() as usize;
let padding = 2;
@ -352,8 +357,8 @@ fn expanded_table_kv(cols: &[String], vals: &[Value], cfg: Cfg<'_>) -> StringRes
let value_width = cfg.opts.width - key_width - count_borders - padding - padding;
let mut data = Vec::with_capacity(cols.len());
for (key, value) in cols.iter().zip(vals) {
let mut data = Vec::with_capacity(record.len());
for (key, value) in record {
if nu_utils::ctrl_c::was_pressed(&cfg.opts.ctrlc) {
return Ok(None);
}
@ -422,8 +427,8 @@ fn expand_table_value(
}
}
}
Value::Record { cols, vals, span } => {
if cols.is_empty() {
Value::Record { val: record, span } => {
if record.is_empty() {
// Like list case return styled string instead of empty value
return Ok(Some((
value_to_wrapped_string(value, cfg, value_width),
@ -433,7 +438,7 @@ fn expand_table_value(
let mut inner_cfg = dive_options(cfg, *span);
inner_cfg.opts.width = value_width;
let result = expanded_table_kv(cols, vals, inner_cfg)?;
let result = expanded_table_kv(record, inner_cfg)?;
match result {
Some(result) => Ok(Some((result, true))),
None => Ok(Some((
@ -480,14 +485,14 @@ fn expanded_table_entry2(item: &Value, cfg: Cfg<'_>) -> NuText {
}
match &item {
Value::Record { cols, vals, span } => {
if cols.is_empty() && vals.is_empty() {
Value::Record { val: record, span } => {
if record.is_empty() {
return nu_value_to_string(item, cfg.opts.config, cfg.opts.style_computer);
}
// we verify what is the structure of a Record cause it might represent
let inner_cfg = dive_options(&cfg, *span);
let table = expanded_table_kv(cols, vals, inner_cfg);
let table = expanded_table_kv(record, inner_cfg);
match table {
Ok(Some(table)) => (table, TextStyle::default()),

View File

@ -1,6 +1,6 @@
use nu_color_config::TextStyle;
use nu_engine::column::get_columns;
use nu_protocol::{ast::PathMember, Config, ShellError, Span, TableIndexMode, Value};
use nu_protocol::{ast::PathMember, Config, Record, ShellError, Span, TableIndexMode, Value};
use crate::{
clean_charset,
@ -18,8 +18,8 @@ impl JustTable {
create_table(input, opts)
}
pub fn kv_table(cols: &[String], vals: &[Value], opts: TableOpts<'_>) -> StringResult {
kv_table(cols, vals, opts)
pub fn kv_table(record: &Record, opts: TableOpts<'_>) -> StringResult {
kv_table(record, opts)
}
}
@ -38,9 +38,9 @@ fn create_table(input: &[Value], opts: TableOpts<'_>) -> Result<Option<String>,
}
}
fn kv_table(cols: &[String], vals: &[Value], opts: TableOpts<'_>) -> StringResult {
let mut data = vec![Vec::with_capacity(2); cols.len()];
for ((column, value), row) in cols.iter().zip(vals.iter()).zip(data.iter_mut()) {
fn kv_table(record: &Record, opts: TableOpts<'_>) -> StringResult {
let mut data = vec![Vec::with_capacity(2); record.len()];
for ((column, value), row) in record.iter().zip(data.iter_mut()) {
if nu_utils::ctrl_c::was_pressed(&opts.ctrlc) {
return Ok(None);
}

View File

@ -1,5 +1,5 @@
use nu_color_config::StyleComputer;
use nu_protocol::{Config, Span, Value};
use nu_protocol::{Config, Record, Span, Value};
use tabled::{
grid::{
color::{AnsiColor, StaticColor},
@ -90,7 +90,7 @@ fn build_table(
fn convert_nu_value_to_table_value(value: Value, config: &Config) -> TableValue {
match value {
Value::Record { cols, vals, .. } => build_vertical_map(cols, vals, config),
Value::Record { val, .. } => build_vertical_map(val, config),
Value::List { vals, .. } => {
let rebuild_array_as_map = is_valid_record(&vals) && count_columns_in_record(&vals) > 0;
if rebuild_array_as_map {
@ -110,9 +110,9 @@ fn convert_nu_value_to_table_value(value: Value, config: &Config) -> TableValue
}
}
fn build_vertical_map(cols: Vec<String>, vals: Vec<Value>, config: &Config) -> TableValue {
let mut rows = Vec::with_capacity(cols.len());
for (key, value) in cols.into_iter().zip(vals) {
fn build_vertical_map(record: Record, config: &Config) -> TableValue {
let mut rows = Vec::with_capacity(record.len());
for (key, value) in record {
let val = convert_nu_value_to_table_value(value, config);
let row = TableValue::Row(vec![TableValue::Cell(key), val]);
rows.push(row);
@ -160,14 +160,14 @@ fn is_valid_record(vals: &[Value]) -> bool {
let mut used_cols: Option<&[String]> = None;
for val in vals {
match val {
Value::Record { cols, .. } => {
Value::Record { val, .. } => {
let cols_are_not_equal =
used_cols.is_some() && !matches!(used_cols, Some(used) if cols == used);
used_cols.is_some() && !matches!(used_cols, Some(used) if val.cols == used);
if cols_are_not_equal {
return false;
}
used_cols = Some(cols);
used_cols = Some(&val.cols);
}
_ => return false,
}
@ -178,7 +178,7 @@ fn is_valid_record(vals: &[Value]) -> bool {
fn count_columns_in_record(vals: &[Value]) -> usize {
match vals.iter().next() {
Some(Value::Record { cols, .. }) => cols.len(),
Some(Value::Record { val, .. }) => val.len(),
_ => 0,
}
}
@ -194,8 +194,8 @@ fn build_map_from_record(vals: Vec<Value>, config: &Config) -> TableValue {
for val in vals {
match val {
Value::Record { vals, .. } => {
for (i, cell) in vals.into_iter().take(count_columns).enumerate() {
Value::Record { val, .. } => {
for (i, cell) in val.vals.into_iter().take(count_columns).enumerate() {
let cell = convert_nu_value_to_table_value(cell, config);
list[i].push(cell);
}
@ -211,7 +211,7 @@ fn build_map_from_record(vals: Vec<Value>, config: &Config) -> TableValue {
fn get_columns_in_record(vals: &[Value]) -> Vec<String> {
match vals.iter().next() {
Some(Value::Record { cols, .. }) => cols.clone(),
Some(Value::Record { val, .. }) => val.cols.clone(),
_ => vec![],
}
}