Skip to content

prevent select (negative number) from hanging shell#12393

Merged
fdncred merged 1 commit intonushell:mainfrom
merelymyself:patch-2
Apr 6, 2024
Merged

prevent select (negative number) from hanging shell#12393
fdncred merged 1 commit intonushell:mainfrom
merelymyself:patch-2

Conversation

@merelymyself
Copy link
Copy Markdown
Contributor

Description

Resolves #11756.
Resolves #12346.

As per description, shell no longer hangs:

~/CodingProjects/nushell> [1 2 3] | select (-2) 
Error: nu::shell::cant_convert

  × Can't convert to cell path.
   ╭─[entry #1:1:18]
 1 │ [1 2 3] | select (-2)
   ·                  ──┬─
   ·                    ╰── can't convert negative number to cell path
   ╰────

User-Facing Changes

Tests + Formatting

Added relevant test 🚀

After Submitting

Possibly support get getting negative numbers, as per #12346 discussion. Alternatively, we can consider adding a cellpath for negative indexing?

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Apr 4, 2024

Possibly support get getting negative numbers, as per #12346 discussion. Alternatively, we can consider adding a cellpath for negative indexing?

In my perfect world, get/select would support all the range syntax. I'm just not sure if that's possible anymore?

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Apr 6, 2024

Thanks!

@fdncred fdncred merged commit 75fedcc into nushell:main Apr 6, 2024
@fdncred fdncred added notes:fixes Include the release notes summary in the "Bug fixes" section deprecated:pr-commands (deprecated: too vague) This PR changes our commands in some way labels Apr 6, 2024
@hustcer hustcer added this to the v0.93.0 milestone Apr 6, 2024
NotTheDr01ds pushed a commit to NotTheDr01ds/nushell that referenced this pull request Apr 12, 2024
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
Resolves nushell#11756.
Resolves nushell#12346. 

As per description, shell no longer hangs:
```
~/CodingProjects/nushell> [1 2 3] | select (-2) 
Error: nu::shell::cant_convert

  × Can't convert to cell path.
   ╭─[entry nushell#1:1:18]
 1 │ [1 2 3] | select (-2)
   ·                  ──┬─
   ·                    ╰── can't convert negative number to cell path
   ╰────
```


<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

Added relevant test 🚀 

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

Possibly support `get` `get`ting negative numbers, as per nushell#12346
discussion. Alternatively, we can consider adding a cellpath for
negative indexing?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecated:pr-commands (deprecated: too vague) This PR changes our commands in some way notes:fixes Include the release notes summary in the "Bug fixes" section

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[1 2 3] | select (-2) makes nushell unresponsive select (-1) hangs Nushell forever

3 participants