diff --git a/crates/nu-protocol/src/pipeline_data.rs b/crates/nu-protocol/src/pipeline_data.rs index 47b10b40db..182c253092 100644 --- a/crates/nu-protocol/src/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline_data.rs @@ -14,9 +14,20 @@ use crate::{ast::PathMember, ShellError, Span, Value, ValueStream}; /// Namely, how do you know the difference between a single string and a list of one string. How do you know /// when to flatten the data given to you from a data source into the stream or to keep it as an unflattened /// list? +/// /// * We tried putting the stream into Value. This had some interesting properties as now commands "just worked -/// on values", but the inability to pass Value to threads as-is meant a lot of workarounds for dealing with -/// Value's stream case +/// on values", but lead to a few unfortunate issues. +/// +/// The first is that you can't easily clone Values in a way that felt largely immutable. For example, if +/// you cloned a Value which contained a stream, and in one variable drained some part of it, then the second +/// variable would see different values based on what you did to the first. +/// +/// To make this kind of mutation thread-safe, we would have had to produce a lock for the stream, which in +/// practice would have meant always locking the stream before reading from it. But more fundamentally, it +/// felt wrong in practice that observation of a value at runtime could affect other values which happen to +/// alias the same stream. By separating these, we don't have this effect. Instead, variables could get +/// concrete list values rather than streams, and be able to view them without non-local effects. +/// /// * A balance of the two approaches is what we've landed on: Values are thread-safe to pass, and we can stream /// them into any sources. Streams are still available to model the infinite streams approach of original /// Nushell. diff --git a/crates/nu-protocol/src/shell_error.rs b/crates/nu-protocol/src/shell_error.rs index 425fec37b6..2579856452 100644 --- a/crates/nu-protocol/src/shell_error.rs +++ b/crates/nu-protocol/src/shell_error.rs @@ -4,6 +4,9 @@ use thiserror::Error; use crate::{ast::Operator, Span, Type}; +/// The fundamental error type for the evaluation engine. These cases represent different kinds of errors +/// the evaluator might face, along with helpful spans to label. An error renderer will take this error value +/// and pass it into an error viewer to display to the user. #[derive(Debug, Clone, Error, Diagnostic, Serialize, Deserialize)] pub enum ShellError { #[error("Type mismatch during operation.")] diff --git a/crates/nu-protocol/src/span.rs b/crates/nu-protocol/src/span.rs index b4b32cec64..aa6192c603 100644 --- a/crates/nu-protocol/src/span.rs +++ b/crates/nu-protocol/src/span.rs @@ -1,6 +1,7 @@ use miette::SourceSpan; use serde::{Deserialize, Serialize}; +/// A spanned area of interest, generic over what kind of thing is of interest #[derive(Clone, Debug)] pub struct Spanned where @@ -10,6 +11,9 @@ where pub span: Span, } +/// Spans are a global offset across all seen files, which are cached in the engine's state. The start and +/// end offset together make the inclusive start/exclusive end pair for where to underline to highlight +/// a given point of interest. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub struct Span { pub start: usize, diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index 24e6e6b7b8..63949d98da 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -130,18 +130,6 @@ impl Range { } } -// impl IntoIterator for Range { -// type Item = Value; - -// type IntoIter = RangeIterator; - -// fn into_iter(self) -> Self::IntoIter { -// let span = self.from.span(); - -// RangeIterator::new(self, span) -// } -// } - pub struct RangeIterator { curr: Value, end: Value, diff --git a/crates/nu-protocol/src/value/stream.rs b/crates/nu-protocol/src/value/stream.rs index 3c69cc504e..94cca448dd 100644 --- a/crates/nu-protocol/src/value/stream.rs +++ b/crates/nu-protocol/src/value/stream.rs @@ -7,6 +7,12 @@ use std::{ }, }; +/// A potentially infinite stream of values, optinally with a mean to send a Ctrl-C signal to stop +/// the stream from continuing. +/// +/// In practice, a "stream" here means anything which can be iterated and produce Values as it iterates. +/// Like other iterators in Rust, observing values from this stream will drain the items as you view them +/// and the stream cannot be replayed. pub struct ValueStream { pub stream: Box + Send + 'static>, pub ctrlc: Option>, @@ -60,62 +66,3 @@ impl Iterator for ValueStream { } } } - -// impl Serialize for ValueStream { -// fn serialize(&self, serializer: S) -> Result -// where -// S: serde::Serializer, -// { -// let mut seq = serializer.serialize_seq(None)?; - -// for element in self.0.borrow_mut().into_iter() { -// seq.serialize_element(&element)?; -// } -// seq.end() -// } -// } - -// impl<'de> Deserialize<'de> for ValueStream { -// fn deserialize(deserializer: D) -> Result -// where -// D: serde::Deserializer<'de>, -// { -// deserializer.deserialize_seq(MySeqVisitor) -// } -// } - -// struct MySeqVisitor; - -// impl<'a> serde::de::Visitor<'a> for MySeqVisitor { -// type Value = ValueStream; - -// fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { -// formatter.write_str("a value stream") -// } - -// fn visit_seq(self, mut seq: A) -> Result -// where -// A: serde::de::SeqAccess<'a>, -// { -// let mut output: Vec = vec![]; - -// while let Some(value) = seq.next_element()? { -// output.push(value); -// } - -// Ok(ValueStream(Rc::new(RefCell::new(output.into_iter())))) -// } -// } - -// pub trait IntoValueStream { -// fn into_value_stream(self) -> ValueStream; -// } - -// impl IntoValueStream for T -// where -// T: Iterator + 'static, -// { -// fn into_value_stream(self) -> ValueStream { -// ValueStream::from_stream(self) -// } -// }