Preserve metadata in more places (#12848)

# Description
This PR makes some commands and areas of code preserve pipeline
metadata. This is in an attempt to make the issue described in #12599
and #9456 less likely to occur. That is, reading and writing to the same
file in a pipeline will result in an empty file. Since we preserve
metadata in more places now, there will be a higher chance that we
successfully detect this error case and abort the pipeline.
This commit is contained in:
Ian Manske 2024-05-17 17:59:32 +00:00 committed by GitHub
parent c10aa2cf09
commit 580c60bb82
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 59 additions and 49 deletions

View File

@ -322,7 +322,9 @@ fn saving_to_source_file_error(dest: &Spanned<PathBuf>) -> ShellError {
dest.item.display() dest.item.display()
), ),
span: Some(dest.span), span: Some(dest.span),
help: Some("You should use `collect` to run your save command (see `help collect`). Or, you can put the file data in a variable and then pass the variable to `save`.".into()), help: Some(
"insert a `collect` command in the pipeline before `save` (see `help collect`).".into(),
),
inner: vec![], inner: vec![],
} }
} }

View File

@ -291,36 +291,38 @@ impl PipelineData {
F: FnMut(Value) -> Value + 'static + Send, F: FnMut(Value) -> Value + 'static + Send,
{ {
match self { match self {
PipelineData::Value(value, ..) => { PipelineData::Value(value, metadata) => {
let span = value.span(); let span = value.span();
match value { let pipeline = match value {
Value::List { vals, .. } => { Value::List { vals, .. } => {
Ok(vals.into_iter().map(f).into_pipeline_data(span, ctrlc)) vals.into_iter().map(f).into_pipeline_data(span, ctrlc)
} }
Value::Range { val, .. } => Ok(val Value::Range { val, .. } => val
.into_range_iter(span, ctrlc.clone()) .into_range_iter(span, ctrlc.clone())
.map(f) .map(f)
.into_pipeline_data(span, ctrlc)), .into_pipeline_data(span, ctrlc),
value => match f(value) { value => match f(value) {
Value::Error { error, .. } => Err(*error), Value::Error { error, .. } => return Err(*error),
v => Ok(v.into_pipeline_data()), v => v.into_pipeline_data(),
}, },
} };
Ok(pipeline.set_metadata(metadata))
} }
PipelineData::Empty => Ok(PipelineData::Empty), PipelineData::Empty => Ok(PipelineData::Empty),
PipelineData::ListStream(stream, ..) => { PipelineData::ListStream(stream, metadata) => {
Ok(PipelineData::ListStream(stream.map(f), None)) Ok(PipelineData::ListStream(stream.map(f), metadata))
} }
PipelineData::ByteStream(stream, ..) => { PipelineData::ByteStream(stream, metadata) => {
// TODO: is this behavior desired / correct ? // TODO: is this behavior desired / correct ?
let span = stream.span(); let span = stream.span();
match String::from_utf8(stream.into_bytes()?) { let value = match String::from_utf8(stream.into_bytes()?) {
Ok(mut str) => { Ok(mut str) => {
str.truncate(str.trim_end_matches(LINE_ENDING_PATTERN).len()); str.truncate(str.trim_end_matches(LINE_ENDING_PATTERN).len());
Ok(f(Value::string(str, span)).into_pipeline_data()) f(Value::string(str, span))
} }
Err(err) => Ok(f(Value::binary(err.into_bytes(), span)).into_pipeline_data()), Err(err) => f(Value::binary(err.into_bytes(), span)),
} };
Ok(value.into_pipeline_data_with_metadata(metadata))
} }
} }
} }
@ -339,36 +341,37 @@ impl PipelineData {
{ {
match self { match self {
PipelineData::Empty => Ok(PipelineData::Empty), PipelineData::Empty => Ok(PipelineData::Empty),
PipelineData::Value(value, ..) => { PipelineData::Value(value, metadata) => {
let span = value.span(); let span = value.span();
match value { let pipeline = match value {
Value::List { vals, .. } => { Value::List { vals, .. } => {
Ok(vals.into_iter().flat_map(f).into_pipeline_data(span, ctrlc)) vals.into_iter().flat_map(f).into_pipeline_data(span, ctrlc)
} }
Value::Range { val, .. } => Ok(val Value::Range { val, .. } => val
.into_range_iter(span, ctrlc.clone()) .into_range_iter(span, ctrlc.clone())
.flat_map(f) .flat_map(f)
.into_pipeline_data(span, ctrlc)), .into_pipeline_data(span, ctrlc),
value => Ok(f(value).into_iter().into_pipeline_data(span, ctrlc)), value => f(value).into_iter().into_pipeline_data(span, ctrlc),
} };
Ok(pipeline.set_metadata(metadata))
} }
PipelineData::ListStream(stream, ..) => { PipelineData::ListStream(stream, metadata) => Ok(PipelineData::ListStream(
Ok(stream.modify(|iter| iter.flat_map(f)).into()) stream.modify(|iter| iter.flat_map(f)),
} metadata,
PipelineData::ByteStream(stream, ..) => { )),
PipelineData::ByteStream(stream, metadata) => {
// TODO: is this behavior desired / correct ? // TODO: is this behavior desired / correct ?
let span = stream.span(); let span = stream.span();
match String::from_utf8(stream.into_bytes()?) { let iter = match String::from_utf8(stream.into_bytes()?) {
Ok(mut str) => { Ok(mut str) => {
str.truncate(str.trim_end_matches(LINE_ENDING_PATTERN).len()); str.truncate(str.trim_end_matches(LINE_ENDING_PATTERN).len());
Ok(f(Value::string(str, span)) f(Value::string(str, span))
.into_iter()
.into_pipeline_data(span, ctrlc))
} }
Err(err) => Ok(f(Value::binary(err.into_bytes(), span)) Err(err) => f(Value::binary(err.into_bytes(), span)),
.into_iter() };
.into_pipeline_data(span, ctrlc)), Ok(iter
} .into_iter()
.into_pipeline_data_with_metadata(span, ctrlc, metadata))
} }
} }
} }
@ -384,27 +387,31 @@ impl PipelineData {
{ {
match self { match self {
PipelineData::Empty => Ok(PipelineData::Empty), PipelineData::Empty => Ok(PipelineData::Empty),
PipelineData::Value(value, ..) => { PipelineData::Value(value, metadata) => {
let span = value.span(); let span = value.span();
match value { let pipeline = match value {
Value::List { vals, .. } => { Value::List { vals, .. } => {
Ok(vals.into_iter().filter(f).into_pipeline_data(span, ctrlc)) vals.into_iter().filter(f).into_pipeline_data(span, ctrlc)
} }
Value::Range { val, .. } => Ok(val Value::Range { val, .. } => val
.into_range_iter(span, ctrlc.clone()) .into_range_iter(span, ctrlc.clone())
.filter(f) .filter(f)
.into_pipeline_data(span, ctrlc)), .into_pipeline_data(span, ctrlc),
value => { value => {
if f(&value) { if f(&value) {
Ok(value.into_pipeline_data()) value.into_pipeline_data()
} else { } else {
Ok(Value::nothing(span).into_pipeline_data()) Value::nothing(span).into_pipeline_data()
} }
} }
} };
Ok(pipeline.set_metadata(metadata))
} }
PipelineData::ListStream(stream, ..) => Ok(stream.modify(|iter| iter.filter(f)).into()), PipelineData::ListStream(stream, metadata) => Ok(PipelineData::ListStream(
PipelineData::ByteStream(stream, ..) => { stream.modify(|iter| iter.filter(f)),
metadata,
)),
PipelineData::ByteStream(stream, metadata) => {
// TODO: is this behavior desired / correct ? // TODO: is this behavior desired / correct ?
let span = stream.span(); let span = stream.span();
let value = match String::from_utf8(stream.into_bytes()?) { let value = match String::from_utf8(stream.into_bytes()?) {
@ -414,11 +421,12 @@ impl PipelineData {
} }
Err(err) => Value::binary(err.into_bytes(), span), Err(err) => Value::binary(err.into_bytes(), span),
}; };
if f(&value) { let value = if f(&value) {
Ok(value.into_pipeline_data()) value
} else { } else {
Ok(Value::nothing(span).into_pipeline_data()) Value::nothing(span)
} };
Ok(value.into_pipeline_data_with_metadata(metadata))
} }
} }
} }