Span `ListStream` and use `PipelineMismatch`

[sholderbach]

Description

To properly use ShellError::PipelineMismatch in PipelineData::collect_string_strict(), ListStream needs to carry a Span as well. Introduces the span field on ListStream and adjusts the respective .into_pipeline_data(.*) methods.

Requires changes to many commands.

User-Facing Changes

This change directly improves the from ... error messages:

Before:

/home/stefan/nushell: {a:1 b:2} | from nuon
Error: nu::shell::type_mismatch (link)

  × Type mismatch
   ╭─[entry #1:1:1]
 1 │ {a:1 b:2} | from nuon
   · ────┬────
   ·     ╰── needs string
   ╰────

After:

/home/stefan/nushell: {a:1 b:2} | from nuon
Error: nu::shell::pipeline_mismatch (link)

  × Pipeline mismatch.
   ╭─[entry #1:1:1]
 1 │ {a:1 b:2} | from nuon
   · ────┬────   ────┬────
   ·     │           ╰── expected: string
   ·     ╰── value originates from here
   ╰────

Tests + Formatting

None added

[webbedspace]

Wait, shouldn't this line be updated?

[webbedspace]

Just so you know, these two comments also need to be updated. Since ListStreams' .span() now produces Some, these lines only need the PipelineData::Empty check (i.e. remove the mention of ListStream from the comments)

[sholderbach]

Just so you know, these two comments also need to be updated. Since ListStreams' .span() now produces Some, these lines only need the PipelineData::Empty check (i.e. remove the mention of ListStream from the comments)

Thanks for the thorough review! Will have to look into it, wasn't fully sure what the comment is referring to (but updated the list stream span extraction.) We should probably also come up with somewhat clearer guidelines what the spans should track accross commands. I was slapping on call.head in most cases as it is the preceding stage but that might not carry the particular change to the types/values that might be relevant to a particular error.

Along those span semantics is also the question what the PipelineData::into_value semantics should be going forward.

[codecov-commenter]

Codecov Report

Base: 55.24% // Head: 54.86% // Decreases project coverage by -0.38% :warning:

Coverage data is based on head (8239d73) compared to base (2894668). Patch coverage: 53.21% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7637      +/-   ##
==========================================
- Coverage   55.24%   54.86%   -0.38%     
==========================================
  Files         606      606              
  Lines       99060    99126      +66     
==========================================
- Hits        54721    54387     -334     
- Misses      44339    44739     +400     

Impacted Files	Coverage Δ
crates/nu-command/src/core_commands/do_.rs	54.68% <0.00%> (-0.44%)	:arrow_down:
crates/nu-command/src/core_commands/for_.rs	40.12% <0.00%> (ø)
...rates/nu-command/src/core_commands/help_aliases.rs	54.60% <0.00%> (ø)
...ates/nu-command/src/core_commands/help_commands.rs	47.58% <0.00%> (ø)
...rates/nu-command/src/core_commands/help_modules.rs	26.86% <0.00%> (ø)
...tes/nu-command/src/core_commands/help_operators.rs	6.33% <0.00%> (ø)
crates/nu-command/src/date/list_timezone.rs	58.00% <0.00%> (ø)
crates/nu-command/src/filesystem/cp.rs	17.45% <0.00%> (ø)
crates/nu-command/src/filesystem/glob.rs	59.09% <0.00%> (ø)
crates/nu-command/src/filesystem/mkdir.rs	42.16% <0.00%> (ø)
... and 70 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[sholderbach]

I have no clue how the virtualenv test stack overflows again on windows. cc @kubouch

[kubouch]

I have no idea either. I had the same problem caused by allowing profiling of pipeline elements https://github.com/nushell/nushell/pull/7854 and solved it by boxing the metadata. You can try if the CI works without the PR (rebase this PR on top of commit right before my PR). Otherwise, you can try boxing the ListStream. But I have no idea why it is overflowing the stack in this particular case...

[sholderbach]

Closing this as it probably makes sense to just redo it from first principles instead of resolving a bunch of conflicts and continuing the work.