Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix insert/upsert creation for nested lists (#15131) #15133

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MMukundi
Copy link

Description

This PR fixes #15131 by allowing the insert and upsert commands to create lists where they may be expected based on the cell path provided. For example, the below would have previously thrown an error, but now creates lists and list elements where necessary
Screenshot 2025-02-17 at 2 46 12 AM
Screenshot 2025-02-17 at 2 46 16 AM
Screenshot 2025-02-17 at 2 45 43 AM

User-Facing Changes

This change removes errors that were previously raised by insert_data_at_cell_path and upsert_data_at_cell_path. If one of these commands encountered an unknown cell path in cases such as these, it would either raise a "Not a list value" as the list index is used on a record:

Screenshot 2025-02-17 at 2 46 43 AM

Or a "Row number too large" when required to create a new list element along the way:
Screenshot 2025-02-17 at 2 46 51 AM

But both now succeed, which seems to be the intention as it is in parity with record behavior. Any consumers depending on this specific behavior will see these errors subside.

This change also includes the public static method Value::with_data_at_cell_path that creates a value with a given nested value at a given cell path, creating records or lists based on the path member type. Plugin developers may begin using this method as its part of the public API now, quite similar to related siblings Value::insert_data_at_cell_path andValue::upsert_data_at_cell_path.

Tests + Formatting

In addition to unit tests for the altered behavior, both affected user-facing commands (insert and upsert) gained a new command example to both explain and test this change at the user level.
Screenshot 2025-02-17 at 2 29 26 AM

Note: A single test did fail locally, due to my config directory differing from expected, but works where this variable is unset (with-env { XDG_CONFIG_HOME: null } {cargo test}):

---- repl::test_config_path::test_default_config_path stdout ----
thread 'repl::test_config_path::test_default_config_path' panicked at tests/repl/test_config_path.rs:101:5:
assertion failed: `(left == right)`

Diff < left / right > :
<[home_dir]/Library/Application Support/nushell
>[home_dir]/.config/nushell

After Submitting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insert/Upsert does not work as expected on nested lists
1 participant