Skip to content

Don't allow skip on external stream#12559

Merged
IanManske merged 3 commits intonushell:mainfrom
WindSoilder:skip_external_stream
Apr 19, 2024
Merged

Don't allow skip on external stream#12559
IanManske merged 3 commits intonushell:mainfrom
WindSoilder:skip_external_stream

Conversation

@WindSoilder
Copy link
Copy Markdown
Contributor

@WindSoilder WindSoilder commented Apr 17, 2024

Description

Close: #12514

User-Facing Changes

^ls | skip 1 will raise an error

❯ ^ls | skip 1
Error: nu::shell::only_supports_this_input_type

  × Input type not supported.
   ╭─[entry #1:1:2]
 1 │ ^ls | skip 1
   ·  ─┬   ──┬─
   ·   │     ╰── only list, binary or range input data is supported
   ·   ╰── input type: raw data
   ╰────

Tests + Formatting

Sorry I can't add it because of the issue: #12558

After Submitting

Nan

@IanManske IanManske added notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes deprecated:pr-commands (deprecated: too vague) This PR changes our commands in some way labels Apr 18, 2024
IanManske
IanManske approved these changes Apr 18, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@IanManske IanManske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense that skip should only work on structured data. In the future we can add/extend the bytes commands for this use case on byte streams. Thanks for the PR!

@IanManske IanManske merged commit 187b87c into nushell:main Apr 19, 2024
@hustcer hustcer added this to the v0.93.0 milestone Apr 20, 2024
@WindSoilder WindSoilder deleted the skip_external_stream branch May 23, 2024 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IanManske IanManske IanManske approved these changes

Assignees

No one assigned

Labels

deprecated:pr-commands (deprecated: too vague) This PR changes our commands in some way notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes

Projects

None yet

Milestone

v0.93.0

Development

Successfully merging this pull request may close these issues.

skip shouldn't work with external streams

3 participants

@WindSoilder @IanManske @hustcer