Make tee work more nicely with non-collections (#13652)

# Description

This changes the behavior of `tee` to be more transparent when given a
value that isn't a list or range. Previously, anything that wasn't a
byte stream would converted to a list stream using the iterator
implementation, which led to some surprising results. Instead, now, if
the value is a string or binary, it will be treated the same way a byte
stream is, and the output of `tee` is a byte stream instead of the
original value. This is done so that we can synchronize with the other
thread on collect, and potentially capture any error produced by the
closure.

For values that can't be converted to streams, the closure is just run
with a clone of the value instead on another thread. Because we can't
wait for the other thread, there is no way to send an error back to the
original thread, so instead it's just written to stderr using
`report_error_new()`.

There are a couple of follow up edge cases I see where byte streams
aren't necessarily treated exactly the same way strings are, but this
should mostly be a good experience.

Fixes #13489.

# User-Facing Changes

Breaking change.

- `tee` now outputs and sends string/binary stream for string/binary
input.
- `tee` now outputs and sends the original value for any other input
other than lists/ranges.

# Tests + Formatting

Added for new behavior.

# After Submitting

- [ ] release notes: breaking change, command change
This commit is contained in:
Devyn Cairns
2024-09-01 10:03:46 -07:00
committed by GitHub
parent ee997ef3dd
commit 39bda8986e
3 changed files with 116 additions and 18 deletions

View File

@ -142,6 +142,40 @@ impl PipelineData {
}
}
/// Converts any `Value` variant that can be represented as a stream into its stream variant.
///
/// This means that lists and ranges are converted into list streams, and strings and binary are
/// converted into byte streams.
///
/// Returns an `Err` with the original stream if the variant couldn't be converted to a stream
/// variant. If the variant is already a stream variant, it is returned as-is.
pub fn try_into_stream(self, engine_state: &EngineState) -> Result<PipelineData, PipelineData> {
let span = self.span().unwrap_or(Span::unknown());
match self {
PipelineData::ListStream(..) | PipelineData::ByteStream(..) => Ok(self),
PipelineData::Value(Value::List { .. } | Value::Range { .. }, ref metadata) => {
let metadata = metadata.clone();
Ok(PipelineData::ListStream(
ListStream::new(self.into_iter(), span, engine_state.signals().clone()),
metadata,
))
}
PipelineData::Value(Value::String { val, .. }, metadata) => {
Ok(PipelineData::ByteStream(
ByteStream::read_string(val, span, engine_state.signals().clone()),
metadata,
))
}
PipelineData::Value(Value::Binary { val, .. }, metadata) => {
Ok(PipelineData::ByteStream(
ByteStream::read_binary(val, span, engine_state.signals().clone()),
metadata,
))
}
_ => Err(self),
}
}
/// Writes all values or redirects all output to the current [`OutDest`]s in `stack`.
///
/// For [`OutDest::Pipe`] and [`OutDest::Capture`], this will return the `PipelineData` as is