mirror of
https://github.com/nushell/nushell.git
synced 2024-11-29 03:44:19 +01:00
In my view we should revert nushell/nushell#8395 for now
## Potentially inconsistent application of semantic change
#8395 (1d5e7b441b
) was loosening the type
coercion rules significantly, to let missing data / void returns that
were either expressed by `PipelineData::Empty` or the `Value::nothing`
be accept by specifically those commands/operations that made use of
`PipelineData::into_iter_strict()`. This could apply the new rules
inconsistently.
## Turning explicit failures into silent continuations
Furthermore the effect of this breaking change to the missing data
semantics could make previous errors into silent failures.
This could either just reduce the effectiveness of teaching error
messages in interactive use:
### Contrived example before
```bash
> cd . | where blah
Error: nu:🐚:only_supports_this_input_type
× Input type not supported.
╭─[entry #13:1:1]
1 │ cd . | where blah
· ──┬──┬
· │ ╰── input type: null
· ╰── only list, binary, raw data or range input data is supported
╰────
```
### ...after, with #8395
```bash
> cd . | where blah
╭────────────╮
│ empty list │
╰────────────╯
```
In rare cases people could already try to rely on catching an error of a
downstream command to actually deal with the missing data, so it would
be a breaking change for their existing code.
## Problem with `PipelineData::into_iter_strict()`
Maybe this makes `_strict` a bit of a misnomer for this particular
iterator construction.
Further we did not actively test the `PipelineData::empty` branch before
![grafik](https://github.com/nushell/nushell/assets/15833959/c377bf1d-d47c-4c25-a342-9a348539f242)
## Parsimonious solution exists
For the motivating issue https://github.com/nushell/nushell/issues/8393
there already exists a fix that makes `ls` more consistent with the type
system by returning an empty `Value::List`
https://github.com/nushell/nushell/pull/8439
This commit is contained in:
parent
2ec1364925
commit
1be4eaeae3
@ -189,10 +189,3 @@ fn where_gt_null() {
|
||||
let actual = nu!("[{foo: 123} {}] | where foo? > 10 | to nuon");
|
||||
assert_eq!(actual.out, "[[foo]; [123]]");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pass_through_empty_pipelines() {
|
||||
let actual = nu!(cwd: ".", pipeline(r#"null | where name == "foo" | to json"#));
|
||||
|
||||
assert_eq!(actual.out, "[]");
|
||||
}
|
||||
|
@ -302,7 +302,6 @@ impl PipelineData {
|
||||
},
|
||||
// Propagate errors by explicitly matching them before the final case.
|
||||
Value::Error { error } => Err(*error),
|
||||
Value::Nothing { .. } => Ok(PipelineIterator(PipelineData::empty())),
|
||||
other => Err(ShellError::OnlySupportsThisInputType {
|
||||
exp_input_type: "list, binary, raw data or range".into(),
|
||||
wrong_type: other.get_type().to_string(),
|
||||
@ -310,7 +309,12 @@ impl PipelineData {
|
||||
src_span: other.expect_span(),
|
||||
}),
|
||||
},
|
||||
PipelineData::Empty => Ok(PipelineIterator(PipelineData::empty())),
|
||||
PipelineData::Empty => Err(ShellError::OnlySupportsThisInputType {
|
||||
exp_input_type: "list, binary, raw data or range".into(),
|
||||
wrong_type: "null".into(),
|
||||
dst_span: span,
|
||||
src_span: span,
|
||||
}),
|
||||
other => Ok(PipelineIterator(other)),
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user