holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use Record's public API in a bunch of places (#10927)

Browse Source 
# Description
Since #10841 the goal is to remove the implementation details of
`Record` outside of core operations.

To this end use Record iterators and map-like accessors in a bunch of
places. In this PR I try to collect the boring cases where I don't
expect any dramatic performance impacts or don't have doubts about the
correctness afterwards

- Use checked record construction in `nu_plugin_example`
- Use `Record::into_iter` in `columns`
- Use `Record` iterators in `headers` cmd
- Use explicit record iterators in `split-by`
- Use `Record::into_iter` in variable completions
- Use `Record::values` iterator in `into sqlite`
- Use `Record::iter_mut` for-loop in `default`
- Change `nu_engine::nonexistent_column` to use iterator
- Use `Record::columns` iter in `nu-cmd-base`
- Use `Record::get_index` in `nu-command/network/http`
- Use `Record.insert()` in `merge`
- Refactor `move` to use encapsulated record API
- Use `Record.insert()` in `explore`
- Use proper `Record` API in `explore`
- Remove defensiveness around record in `explore`
- Use encapsulated record API in more `nu-command`s

# User-Facing Changes
None intentional

# Tests + Formatting
(-)
This commit is contained in:
Stefan Holderbach 2023-11-08 14:24:00 +01:00 committed by GitHub
parent b03ef56bcb
commit edbf3aaccb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 70 additions and 108 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
crates/nu-cli/src/completions/variable_completions.rs
View File

@ -237,9 +237,9 @@ fn nested_suggestions(
match value {
Value::Record { val, .. } => {
// Add all the columns as completion
for item in val.cols {
for (col, _) in val.into_iter() {
output.push(Suggestion {
value: item,
value: col,
description: None,
extra: None,
span: current_span,

2
crates/nu-cmd-base/src/formats/to/delimited.rs
View File

@ -6,7 +6,7 @@ pub fn merge_descriptors(values: &[Value]) -> Vec<String> {
let mut seen: IndexSet<String> = indexset! {};
for value in values {
let data_descriptors = match value {
Value::Record { val, .. } => val.cols.clone(),
Value::Record { val, .. } => val.columns().cloned().collect(),
_ => vec!["".to_string()],
};
for desc in data_descriptors {

3
crates/nu-command/src/database/commands/into_sqlite.rs
View File

@ -128,8 +128,7 @@ fn action(
"({})",
match list_value {
Value::Record { val, .. } => {
val.vals
.iter()
val.values()
.map(|rec_val| {
format!("'{}'", nu_value_to_string(rec_val.clone(), ""))
})

3
crates/nu-command/src/filters/columns.rs
View File

@ -124,9 +124,8 @@ fn getcol(
})
}
Value::Record { val, .. } => Ok(val
.cols
.into_iter()
.map(move |x| Value::string(x, head))
.map(move |(x, _)| Value::string(x, head))
.into_pipeline_data(ctrlc)
.set_metadata(metadata)),
// Propagate errors

10
crates/nu-command/src/filters/default.rs
View File

@ -91,17 +91,15 @@ fn default(
Value::Record {
val: mut record, ..
} => {
let mut idx = 0;
let mut found = false;
while idx < record.len() {
if record.cols[idx] == column.item {
for (col, val) in record.iter_mut() {
if *col == column.item {
found = true;
if matches!(record.vals[idx], Value::Nothing { .. }) {
record.vals[idx] = value.clone();
if matches!(val, Value::Nothing { .. }) {
*val = value.clone();
}
}
idx += 1;
}
if !found {

4
crates/nu-command/src/filters/flatten.rs
View File

@ -314,7 +314,7 @@ fn flat_value(columns: &[CellPath], item: &Value, name_tag: Span, all: bool) ->
// this can avoid output column order changed.
if index == parent_column_index {
for (col, val) in inner_cols.iter().zip(inner_vals.iter()) {
if record.cols.contains(col) {
if record.contains(col) {
record.push(format!("{parent_column_name}_{col}"), val.clone());
} else {
record.push(col, val.clone());
@ -329,7 +329,7 @@ fn flat_value(columns: &[CellPath], item: &Value, name_tag: Span, all: bool) ->
// the flattened column may be the last column in the original table.
if index == parent_column_index {
for (col, val) in inner_cols.iter().zip(inner_vals.iter()) {
if record.cols.contains(col) {
if record.contains(col) {
record.push(format!("{parent_column_name}_{col}"), val.clone());
} else {
record.push(col, val.clone());

7
crates/nu-command/src/filters/headers.rs
View File

@ -129,7 +129,7 @@ fn extract_headers(
let span = value.span();
match value {
Value::Record { val: record, .. } => {
for v in &record.vals {
for v in record.values() {
if !is_valid_header(v) {
return Err(ShellError::TypeMismatch {
err_message: "needs compatible type: Null, String, Bool, Float, Int"
@ -139,10 +139,9 @@ fn extract_headers(
}
}
let old_headers = record.cols.clone();
let old_headers = record.columns().cloned().collect();
let new_headers = record
.vals
.iter()
.values()
.enumerate()
.map(|(idx, value)| {
let col = value.into_string("", config);

12
crates/nu-command/src/filters/merge.rs
View File

@ -153,20 +153,12 @@ repeating this process with row 1, and so on."#
}
}
// TODO: rewrite to mutate the input record
fn do_merge(input_record: &Record, to_merge_record: &Record) -> Record {
let mut result = input_record.clone();
for (col, val) in to_merge_record {
let pos = result.cols.iter().position(|c| c == col);
// if find, replace existing data, else, push new data.
match pos {
Some(index) => {
result.vals[index] = val.clone();
}
None => {
result.push(col, val.clone());
}
}
result.insert(col, val.clone());
}
result
}

14
crates/nu-command/src/filters/move_.rs
View File

@ -197,7 +197,7 @@ fn move_record_columns(
// Check if before/after column exist
match &before_or_after.item {
BeforeOrAfter::After(after) => {
if !record.cols.contains(after) {
if !record.contains(after) {
return Err(ShellError::GenericError(
"Cannot move columns".to_string(),
"column does not exist".to_string(),
@ -208,7 +208,7 @@ fn move_record_columns(
}
}
BeforeOrAfter::Before(before) => {
if !record.cols.contains(before) {
if !record.contains(before) {
return Err(ShellError::GenericError(
"Cannot move columns".to_string(),
"column does not exist".to_string(),
@ -224,11 +224,7 @@ fn move_record_columns(
for column in columns.iter() {
let column_str = column.as_string()?;
if let Some(idx) = record
.cols
.iter()
.position(|inp_col| &column_str == inp_col)
{
if let Some(idx) = record.index_of(&column_str) {
column_idx.push(idx);
} else {
return Err(ShellError::GenericError(
@ -249,7 +245,7 @@ fn move_record_columns(
out.push(inp_col.clone(), inp_val.clone());
for idx in column_idx.iter() {
if let (Some(col), Some(val)) = (record.cols.get(*idx), record.vals.get(*idx)) {
if let Some((col, val)) = record.get_index(*idx) {
out.push(col.clone(), val.clone());
} else {
return Err(ShellError::NushellFailedSpanned {
@ -262,7 +258,7 @@ fn move_record_columns(
}
BeforeOrAfter::Before(before) if before == inp_col => {
for idx in column_idx.iter() {
if let (Some(col), Some(val)) = (record.cols.get(*idx), record.vals.get(*idx)) {
if let Some((col, val)) = record.get_index(*idx) {
out.push(col.clone(), val.clone());
} else {
return Err(ShellError::NushellFailedSpanned {

2
crates/nu-command/src/filters/sort.rs
View File

@ -254,7 +254,7 @@ pub fn sort(
) -> Result<(), ShellError> {
match vec.first() {
Some(Value::Record { val, .. }) => {
let columns = val.cols.clone();
let columns: Vec<String> = val.columns().cloned().collect();
vec.sort_by(|a, b| process(a, b, &columns, span, insensitive, natural));
}
_ => {

10
crates/nu-command/src/filters/split_by.rs
View File

@ -185,15 +185,15 @@ pub fn data_split(
let span = v.span();
match v {
Value::Record { val: grouped, .. } => {
for (idx, list) in grouped.vals.iter().enumerate() {
match data_group(list, splitter, span) {
for (outer_key, list) in grouped.into_iter() {
match data_group(&list, splitter, span) {
Ok(grouped_vals) => {
if let Value::Record { val: sub, .. } = grouped_vals {
for (inner_idx, subset) in sub.vals.iter().enumerate() {
for (inner_key, subset) in sub.into_iter() {
let s: &mut IndexMap<String, Value> =
splits.entry(sub.cols[inner_idx].clone()).or_default();
splits.entry(inner_key).or_default();
s.insert(grouped.cols[idx].clone(), subset.clone());
s.insert(outer_key.clone(), subset.clone());
}
}
}

28
crates/nu-command/src/filters/uniq.rs
View File

@ -3,8 +3,8 @@ use itertools::Itertools;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
record, Category, Example, IntoPipelineData, PipelineData, PipelineMetadata, Record,
ShellError, Signature, Span, Type, Value,
record, Category, Example, IntoPipelineData, PipelineData, PipelineMetadata, ShellError,
Signature, Span, Type, Value,
};
use std::collections::hash_map::IntoIter;
use std::collections::HashMap;
@ -190,10 +190,10 @@ fn clone_to_lowercase(value: &Value) -> Value {
Value::list(vec.iter().map(clone_to_lowercase).collect(), span)
}
Value::Record { val: record, .. } => Value::record(
Record {
cols: record.cols.clone(),
vals: record.vals.iter().map(clone_to_lowercase).collect(),
},
record
.iter()
.map(|(k, v)| (k.to_owned(), clone_to_lowercase(v)))
.collect(),
span,
),
other => other.clone(),
@ -204,24 +204,18 @@ fn sort_attributes(val: Value) -> Value {
let span = val.span();
match val {
Value::Record { val, .. } => {
// TODO: sort inplace
let sorted = val
.into_iter()
.sorted_by(|a, b| a.0.cmp(&b.0))
.collect_vec();
let sorted_cols = sorted.clone().into_iter().map(|a| a.0).collect_vec();
let sorted_vals = sorted
let record = sorted
.into_iter()
.map(|a| sort_attributes(a.1))
.collect_vec();
.map(|(k, v)| (k, sort_attributes(v)))
.collect();
Value::record(
Record {
cols: sorted_cols,
vals: sorted_vals,
},
span,
)
Value::record(record, span)
}
Value::List { vals, .. } => {
Value::list(vals.into_iter().map(sort_attributes).collect_vec(), span)

2
crates/nu-command/src/filters/uniq_by.rs
View File

@ -126,7 +126,7 @@ fn validate(vec: &[Value], columns: &[String], span: Span) -> Result<(), ShellEr
));
}
if let Some(nonexistent) = nonexistent_column(columns, &record.cols) {
if let Some(nonexistent) = nonexistent_column(columns, record.columns()) {
return Err(ShellError::CantFindColumn {
col_name: nonexistent,
span,

2
crates/nu-command/src/formats/to/md.rs
View File

@ -210,7 +210,7 @@ pub fn group_by(values: PipelineData, head: Span, config: &Config) -> (PipelineD
} = val
{
lists
.entry(record.cols.concat())
.entry(record.columns().map(|c| c.as_str()).collect::<String>())
.and_modify(|v: &mut Vec<Value>| v.push(val.clone()))
.or_insert_with(|| vec![val.clone()]);
} else {

2
crates/nu-command/src/formats/to/nuon.rs
View File

@ -209,7 +209,7 @@ pub fn value_to_string(
let mut row = vec![];
if let Value::Record { val, .. } = val {
for val in &val.vals {
for val in val.values() {
row.push(value_to_string_without_quotes(
val,
span,

9
crates/nu-command/src/network/http/client.rs
View File

@ -602,9 +602,12 @@ fn headers_to_nu(headers: &Headers, span: Span) -> Result<PipelineData, ShellErr
for (name, values) in headers {
let is_duplicate = vals.iter().any(|val| {
if let Value::Record { val, .. } = val {
if let Some(Value::String {
val: header_name, ..
}) = val.vals.get(0)
if let Some((
_col,
Value::String {
val: header_name, ..
},
)) = val.get_index(0)
{
return name == header_name;
}

2
crates/nu-command/src/sort_utils.rs
View File

@ -77,7 +77,7 @@ pub fn sort(
));
}
if let Some(nonexistent) = nonexistent_column(&sort_columns, &record.cols) {
if let Some(nonexistent) = nonexistent_column(&sort_columns, record.columns()) {
return Err(ShellError::CantFindColumn {
col_name: nonexistent,
span,

17
crates/nu-command/src/strings/str_/trim/trim_.rs
View File

@ -3,8 +3,8 @@ use nu_engine::CallExt;
use nu_protocol::{
ast::{Call, CellPath},
engine::{Command, EngineState, Stack},
Category, Example, PipelineData, Record, ShellError, Signature, Span, Spanned, SyntaxShape,
Type, Value,
Category, Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, Type,
Value,
};
#[derive(Clone)]
@ -181,15 +181,12 @@ fn action(input: &Value, arg: &Arguments, head: Span) -> Value {
match mode {
ActionMode::Global => match other {
Value::Record { val: record, .. } => {
let new_vals = record.vals.iter().map(|v| action(v, arg, head)).collect();
let new_record = record
.iter()
.map(|(k, v)| (k.clone(), action(v, arg, head)))
.collect();
Value::record(
Record {
cols: record.cols.to_vec(),
vals: new_vals,
},
span,
)
Value::record(new_record, span)
}
Value::List { vals, .. } => {
let new_vals = vals.iter().map(|v| action(v, arg, head)).collect();

7
crates/nu-engine/src/column.rs
View File

@ -19,8 +19,11 @@ pub fn get_columns(input: &[Value]) -> Vec<String> {
}
// If a column doesn't exist in the input, return it.
pub fn nonexistent_column(inputs: &[String], columns: &[String]) -> Option<String> {
let set: HashSet<String> = HashSet::from_iter(columns.iter().cloned());
pub fn nonexistent_column<'a, I>(inputs: &[String], columns: I) -> Option<String>
where
I: IntoIterator<Item = &'a String>,
{
let set: HashSet<&String> = HashSet::from_iter(columns);
for input in inputs {
if set.contains(input) {

2
crates/nu-explore/src/commands/help.rs
View File

@ -165,7 +165,7 @@ fn help_frame_data(
let (cols, mut vals) = help_manual_data(manual, aliases);
let vals = vals.remove(0);
Value::record(Record { cols, vals }, NuSpan::unknown())
Value::record(Record::from_raw_cols_vals(cols, vals), NuSpan::unknown())
})
.collect();
let commands = Value::list(commands, NuSpan::unknown());

5
crates/nu-explore/src/nu_common/value.rs
View File

@ -90,7 +90,10 @@ fn collect_external_stream(
pub fn collect_input(value: Value) -> (Vec<String>, Vec<Vec<Value>>) {
let span = value.span();
match value {
Value::Record { val: record, .. } => (record.cols, vec![record.vals]),
Value::Record { val: record, .. } => {
let (key, val) = record.into_iter().unzip();
(key, vec![val])
}
Value::List { vals, .. } => {
let mut columns = get_columns(&vals);
let data = convert_records_to_dataset(&columns, vals);

16
crates/nu-explore/src/pager/mod.rs
View File

@ -1041,21 +1041,9 @@ fn set_config(hm: &mut HashMap<String, Value>, path: &[&str], value: Value) -> b
match val {
Value::Record { val: record, .. } => {
if path.len() == 2 {
if record.cols.len() != record.vals.len() {
return false;
}
let key = path[1];
let key = &path[1];
let pos = record.cols.iter().position(|v| v == key);
match pos {
Some(i) => {
record.vals[i] = value;
}
None => {
record.push(*key, value);
}
}
record.insert(key, value);
} else {
let mut hm2: HashMap<String, Value> = HashMap::new();
for (k, v) in record.iter() {

7
crates/nu-explore/src/views/record/mod.rs
View File

@ -708,10 +708,7 @@ fn build_table_as_list(v: &RecordView) -> Value {
.cloned()
.map(|vals| {
Value::record(
Record {
cols: headers.clone(),
vals,
},
Record::from_raw_cols_vals(headers.clone(), vals),
NuSpan::unknown(),
)
})
@ -726,7 +723,7 @@ fn build_table_as_record(v: &RecordView) -> Value {
let cols = layer.columns.to_vec();
let vals = layer.records.get(0).map_or(Vec::new(), |row| row.clone());
Value::record(Record { cols, vals }, NuSpan::unknown())
Value::record(Record::from_raw_cols_vals(cols, vals), NuSpan::unknown())
}
fn report_cursor_position(mode: UIMode, cursor: XYCursor) -> String {

8
crates/nu_plugin_example/src/example.rs
View File

@ -67,13 +67,7 @@ impl Example {
.map(|v| Value::int(v * i, call.head))
.collect::<Vec<Value>>();
Value::record(
Record {
cols: cols.clone(),
vals,
},
call.head,
)
Value::record(Record::from_raw_cols_vals(cols.clone(), vals), call.head)
})
.collect::<Vec<Value>>();