Skip to content

ensure get doesn't delve too deep in nested lists#7497

Merged
rgwood merged 2 commits intonushell:mainfrom
merelymyself:module_constant
Dec 16, 2022
Merged

ensure get doesn't delve too deep in nested lists#7497
rgwood merged 2 commits intonushell:mainfrom
merelymyself:module_constant

Conversation

@merelymyself
Copy link
Copy Markdown
Contributor

@merelymyself merelymyself commented Dec 16, 2022

Description

Fixes #7494.

/home/gabriel/CodingProjects/nushell〉[[{foo: bar}]] | get foo          12/16/2022 12:31:17 PM
Error: nu::parser::not_found (link)

  × Not found.
   ╭─[entry #1:1:1]
 1 │ [[{foo: bar}]] | get foo
   · ───────┬──────
   ·        ╰── did not find anything under this name
   ╰────

User-Facing Changes

cell paths no longer drill into nested tables.

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 -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace to check that all tests pass

After Submitting

If your PR had any user-facing changes, update the documentation after the PR is merged, if necessary. This will help us keep the docs up to date.

@rgwood
Copy link
Copy Markdown
Contributor

rgwood commented Dec 16, 2022

Nice, thank you for the quick fix! That looks good to me once the tests pass; I think we have some tests that (strangely enough) depend on the current behaviour.

@stormasm
Copy link
Copy Markdown
Contributor

stormasm commented Dec 16, 2022

[ [{ foo: "bar" }] ] | get 0 | to nuon

[[foo]; [bar]]

[ [{ foo: "bar" }] ] | get 0.foo | to nuon

[bar]

This should continue to work after this fix though...

@rgwood
Copy link
Copy Markdown
Contributor

rgwood commented Dec 16, 2022

@stormasm Those commands return the same thing as before this PR.

@rgwood rgwood merged commit 2d07c6e into nushell:main Dec 16, 2022
@rgwood rgwood added the notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes label Dec 16, 2022
@stormasm
Copy link
Copy Markdown
Contributor

@rgwood cool ! thanks for checking...

rgwood added a commit that referenced this pull request Dec 17, 2022
A follow-up to #7497. That change made it so that `get foo` would
eliminate non-record rows; I think that was an unintentional and
undesirable side-effect.

Before #7497:
```bash
〉[$nothing, { item: "foo" }] | get item
╭───┬─────╮
│ 0 │     │
│ 1 │ foo │
╰───┴─────╯
```
After #7497:
```bash
〉[$nothing, {item: "foo"}] | get item
╭───┬─────╮
│ 0 │ foo │
╰───┴─────╯
```

After this PR:
```bash
〉[$nothing, { item: "foo" }] | get item
╭───┬─────╮
│ 0 │     │
│ 1 │ foo │
╰───┴─────╯
```

cc: @merelymyself
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes:breaking-changes This PR implies a change affecting users and has to be noted in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cell paths drill into nested tables

3 participants