extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-07-01 07:00:37 +02:00
Code Issues Packages Projects Releases Wiki Activity

Remove lazy records (#12682)

Browse Source 
# Description
Removes lazy records from the language, following from the reasons
outlined in #12622. Namely, this should make semantics more clear and
will eliminate concerns regarding maintainability.

# User-Facing Changes
- Breaking change: `lazy make` is removed.
- Breaking change: `describe --collect-lazyrecords` flag is removed.
- `sys` and `debug info` now return regular records.

# After Submitting
- Update nushell book if necessary.
- Explore new `sys` and `debug info` APIs to prevent them from taking
too long (e.g., subcommands or taking an optional column/cell-path
argument).
This commit is contained in:
Ian Manske
2024-05-03 00:36:10 +00:00
committed by GitHub
parent ad6deadf24
commit 847646e44e
32 changed files with 133 additions and 867 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
crates/nu-protocol/src/errors/shell_error.rs
View File

@ -1178,16 +1178,6 @@ pub enum ShellError {
span: Option<Span>,
},
/// An attempt to access a record column failed.
#[error("Access failure: {message}")]
#[diagnostic(code(nu::shell::lazy_record_access_failed))]
LazyRecordAccessFailed {
message: String,
column_name: String,
#[label("Could not access '{column_name}' on this record")]
span: Span,
},
/// Operation interrupted by user
#[error("Operation interrupted by user")]
InterruptedByUser {

29
crates/nu-protocol/src/value/lazy_record.rs
View File

@ -1,29 +0,0 @@
use crate::{Record, ShellError, Span, Value};
use std::fmt;
// Trait definition for a lazy record (where columns are evaluated on-demand)
// typetag is needed to make this implement Serialize+Deserialize... even though we should never actually serialize a LazyRecord.
// To serialize a LazyRecord, collect it into a Value::Record with collect() first.
pub trait LazyRecord<'a>: fmt::Debug + Send + Sync {
// All column names
fn column_names(&'a self) -> Vec<&'a str>;
// Get 1 specific column value
fn get_column_value(&self, column: &str) -> Result<Value, ShellError>;
fn span(&self) -> Span;
// Convert the lazy record into a regular Value::Record by collecting all its columns
fn collect(&'a self) -> Result<Value, ShellError> {
self.column_names()
.into_iter()
.map(|col| {
let val = self.get_column_value(col)?;
Ok((col.to_owned(), val))
})
.collect::<Result<Record, _>>()
.map(|record| Value::record(record, self.span()))
}
fn clone_value(&self, span: Span) -> Value;
}

145
crates/nu-protocol/src/value/mod.rs
View File

@ -4,7 +4,6 @@ mod filesize;
mod from;
mod from_value;
mod glob;
mod lazy_record;
mod range;
pub mod record;
@ -13,7 +12,6 @@ pub use duration::*;
pub use filesize::*;
pub use from_value::FromValue;
pub use glob::*;
pub use lazy_record::LazyRecord;
pub use range::{FloatRange, IntRange, Range};
pub use record::Record;
@ -164,13 +162,6 @@ pub enum Value {
#[serde(rename = "span")]
internal_span: Span,
},
#[serde(skip)]
LazyRecord {
val: Box<dyn for<'a> LazyRecord<'a>>,
// note: spans are being refactored out of Value
// please use .span() instead of matching this span value
internal_span: Span,
},
}
impl Clone for Value {
@ -212,7 +203,6 @@ impl Clone for Value {
val: val.clone(),
internal_span: *internal_span,
},
Value::LazyRecord { val, internal_span } => val.clone_value(*internal_span),
Value::List {
vals,
internal_span,
@ -672,24 +662,6 @@ impl Value {
}
}
/// Returns a reference to the inner [`LazyRecord`] trait object or an error if this `Value` is not a lazy record
pub fn as_lazy_record(&self) -> Result<&dyn for<'a> LazyRecord<'a>, ShellError> {
if let Value::LazyRecord { val, .. } = self {
Ok(val.as_ref())
} else {
self.cant_convert_to("lazy record")
}
}
/// Unwraps the inner [`LazyRecord`] trait object or returns an error if this `Value` is not a lazy record
pub fn into_lazy_record(self) -> Result<Box<dyn for<'a> LazyRecord<'a>>, ShellError> {
if let Value::LazyRecord { val, .. } = self {
Ok(val)
} else {
self.cant_convert_to("lazy record")
}
}
/// Get the span for the current value
pub fn span(&self) -> Span {
match self {
@ -709,7 +681,6 @@ impl Value {
| Value::Binary { internal_span, .. }
| Value::CellPath { internal_span, .. }
| Value::Custom { internal_span, .. }
| Value::LazyRecord { internal_span, .. }
| Value::Error { internal_span, .. } => *internal_span,
}
}
@ -727,7 +698,6 @@ impl Value {
| Value::String { internal_span, .. }
| Value::Glob { internal_span, .. }
| Value::Record { internal_span, .. }
| Value::LazyRecord { internal_span, .. }
| Value::List { internal_span, .. }
| Value::Closure { internal_span, .. }
| Value::Nothing { internal_span, .. }
@ -784,10 +754,6 @@ impl Value {
None => Type::List(Box::new(Type::Any)),
}
}
Value::LazyRecord { val, .. } => match val.collect() {
Ok(val) => val.get_type(),
Err(..) => Type::Error,
},
Value::Nothing { .. } => Type::Nothing,
Value::Closure { .. } => Type::Closure,
Value::Error { .. } => Type::Error,
@ -893,10 +859,6 @@ impl Value {
.collect::<Vec<_>>()
.join(separator)
),
Value::LazyRecord { val, .. } => val
.collect()
.unwrap_or_else(|err| Value::error(err, span))
.to_expanded_string(separator, config),
Value::Closure { val, .. } => format!("<Closure {}>", val.block_id),
Value::Nothing { .. } => String::new(),
Value::Error { error, .. } => format!("{error:?}"),
@ -919,7 +881,6 @@ impl Value {
/// - "[list {n} items]"
/// - "[record {n} fields]"
pub fn to_abbreviated_string(&self, config: &Config) -> String {
let span = self.span();
match self {
Value::Date { val, .. } => match &config.datetime_table_format {
Some(format) => self.format_datetime(val, format),
@ -945,10 +906,6 @@ impl Value {
val.len(),
if val.len() == 1 { "" } else { "s" }
),
Value::LazyRecord { val, .. } => val
.collect()
.unwrap_or_else(|err| Value::error(err, span))
.to_abbreviated_string(config),
val => val.to_expanded_string(", ", config),
}
}
@ -1087,7 +1044,7 @@ impl Value {
}
// Records (and tables) are the only built-in which support column names,
// so only use this message for them.
Value::Record { .. } | Value::LazyRecord { .. } => {
Value::Record { .. } => {
return Err(ShellError::TypeMismatch {
err_message:"Can't access record values with a row index. Try specifying a column name instead".into(),
span: *origin_span,
@ -1137,32 +1094,6 @@ impl Value {
});
}
}
Value::LazyRecord { val, .. } => {
let columns = val.column_names();
if let Some(col) = columns.iter().rev().find(|&col| {
if insensitive {
col.eq_ignore_case(column_name)
} else {
col == column_name
}
}) {
current = val.get_column_value(col)?;
} else if *optional {
return Ok(Value::nothing(*origin_span)); // short-circuit
} else if let Some(suggestion) = did_you_mean(&columns, column_name) {
return Err(ShellError::DidYouMean {
suggestion,
span: *origin_span,
});
} else {
return Err(ShellError::CantFindColumn {
col_name: column_name.clone(),
span: *origin_span,
src_span: span,
});
}
}
// String access of Lists always means Table access.
// Create a List which contains each matching value for contained
// records in the source list.
@ -1327,11 +1258,6 @@ impl Value {
record.to_mut().push(col_name, new_col);
}
}
Value::LazyRecord { val, .. } => {
// convert to Record first.
*self = val.collect()?;
self.upsert_data_at_cell_path(cell_path, new_val)?;
}
Value::Error { error, .. } => return Err(*error.clone()),
v => {
return Err(ShellError::CantFindColumn {
@ -1443,11 +1369,6 @@ impl Value {
});
}
}
Value::LazyRecord { val, .. } => {
// convert to Record first.
*self = val.collect()?;
self.update_data_at_cell_path(cell_path, new_val)?;
}
Value::Error { error, .. } => return Err(*error.clone()),
v => {
return Err(ShellError::CantFindColumn {
@ -1532,11 +1453,6 @@ impl Value {
}
Ok(())
}
Value::LazyRecord { val, .. } => {
// convert to Record first.
*self = val.collect()?;
self.remove_data_at_cell_path(cell_path)
}
v => Err(ShellError::CantFindColumn {
col_name: col_name.clone(),
span: *span,
@ -1616,11 +1532,6 @@ impl Value {
}
Ok(())
}
Value::LazyRecord { val, .. } => {
// convert to Record first.
*self = val.collect()?;
self.remove_data_at_cell_path(cell_path)
}
v => Err(ShellError::CantFindColumn {
col_name: col_name.clone(),
span: *span,
@ -1739,11 +1650,6 @@ impl Value {
record.to_mut().push(col_name, new_col);
}
}
Value::LazyRecord { val, .. } => {
// convert to Record first.
*self = val.collect()?;
self.insert_data_at_cell_path(cell_path, new_val, v_span)?;
}
other => {
return Err(ShellError::UnsupportedInput {
msg: "table or record".into(),
@ -1797,8 +1703,6 @@ impl Value {
///
/// If the closure returns `Err`, the traversal will stop.
///
/// If collecting lazy records to check them as well is desirable, make sure to do it in your
/// closure. The traversal continues on whatever modifications you make during the closure.
/// Captures of closure values are currently visited, as they are values owned by the closure.
pub fn recurse_mut<E>(
&mut self,
@ -1837,7 +1741,7 @@ impl Value {
| Value::Binary { .. }
| Value::CellPath { .. } => Ok(()),
// These could potentially contain values, but we expect the closure to handle them
Value::LazyRecord { .. } | Value::Custom { .. } => Ok(()),
Value::Custom { .. } => Ok(()),
}
}
@ -1998,13 +1902,6 @@ impl Value {
}
}
pub fn lazy_record(val: Box<dyn for<'a> LazyRecord<'a>>, span: Span) -> Value {
Value::LazyRecord {
val,
internal_span: span,
}
}
/// Note: Only use this for test data, *not* live data, as it will point into unknown source
/// when used in errors.
pub fn test_bool(val: bool) -> Value {
@ -2101,17 +1998,11 @@ impl Value {
Value::custom(val, Span::test_data())
}
/// Note: Only use this for test data, *not* live data, as it will point into unknown source
/// when used in errors.
pub fn test_lazy_record(val: Box<dyn for<'a> LazyRecord<'a>>) -> Value {
Value::lazy_record(val, Span::test_data())
}
/// Note: Only use this for test data, *not* live data,
/// as it will point into unknown source when used in errors.
///
/// Returns a `Vec` containing one of each value case (`Value::Int`, `Value::String`, etc.)
/// except for `Value::LazyRecord` and `Value::CustomValue`.
/// except for `Value::CustomValue`.
pub fn test_values() -> Vec<Value> {
vec![
Value::test_bool(false),
@ -2127,7 +2018,6 @@ impl Value {
Value::test_float(0.0),
Value::test_string(String::new()),
Value::test_record(Record::new()),
// Value::test_lazy_record(Box::new(todo!())),
Value::test_list(Vec::new()),
Value::test_closure(Closure {
block_id: 0,
@ -2181,7 +2071,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2201,7 +2090,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2221,7 +2109,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2241,7 +2128,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2261,7 +2147,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2281,7 +2166,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2301,7 +2185,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Less),
Value::Glob { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2321,7 +2204,6 @@ impl PartialOrd for Value {
Value::String { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2341,7 +2223,6 @@ impl PartialOrd for Value {
Value::String { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Glob { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2387,13 +2268,6 @@ impl PartialOrd for Value {
// that the shorter sequence is less than the longer one
lhs.len().partial_cmp(&rhs.len())
}
Value::LazyRecord { val, .. } => {
if let Ok(rhs) = val.collect() {
self.partial_cmp(&rhs)
} else {
None
}
}
Value::List { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2413,7 +2287,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { vals: rhs, .. } => lhs.partial_cmp(rhs),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2433,7 +2306,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::Closure { val: rhs, .. } => lhs.block_id.partial_cmp(&rhs.block_id),
Value::Nothing { .. } => Some(Ordering::Less),
@ -2453,7 +2325,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::Closure { .. } => Some(Ordering::Greater),
Value::Nothing { .. } => Some(Ordering::Equal),
@ -2473,7 +2344,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::Closure { .. } => Some(Ordering::Greater),
Value::Nothing { .. } => Some(Ordering::Greater),
@ -2493,7 +2363,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::Closure { .. } => Some(Ordering::Greater),
Value::Nothing { .. } => Some(Ordering::Greater),
@ -2513,7 +2382,6 @@ impl PartialOrd for Value {
Value::String { .. } => Some(Ordering::Greater),
Value::Glob { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::Closure { .. } => Some(Ordering::Greater),
Value::Nothing { .. } => Some(Ordering::Greater),
@ -2523,13 +2391,6 @@ impl PartialOrd for Value {
Value::Custom { .. } => Some(Ordering::Less),
},
(Value::Custom { val: lhs, .. }, rhs) => lhs.partial_cmp(rhs),
(Value::LazyRecord { val, .. }, rhs) => {
if let Ok(val) = val.collect() {
val.partial_cmp(rhs)
} else {
None
}
}
}
}
}