Merge remote-tracking branch 'origin/main' into porting-format

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
+/// on values", but lead to a few unfortunate issues.
-/// Value's stream case
+///
+/// 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.

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.")]

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<T>
 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,

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,

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<dyn Iterator<Item = Value> + Send + 'static>,
     pub ctrlc: Option<Arc<AtomicBool>>,
@@ -60,62 +66,3 @@ impl Iterator for ValueStream {
         }
     }
 }
-
-// impl Serialize for ValueStream {
-//     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-//     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<D>(deserializer: D) -> Result<Self, D::Error>
-//     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<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
-//     where
-//         A: serde::de::SeqAccess<'a>,
-//     {
-//         let mut output: Vec<Value> = 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<T> IntoValueStream for T
-// where
-//     T: Iterator<Item = Value> + 'static,
-// {
-//     fn into_value_stream(self) -> ValueStream {
-//         ValueStream::from_stream(self)
-//     }
-// }