Skip to content

move-Command Tests, Refactor, Fix#11904

Merged
fdncred merged 11 commits intonushell:mainfrom
dannou812:dannou812/move-command
Feb 20, 2024
Merged

move-Command Tests, Refactor, Fix#11904
fdncred merged 11 commits intonushell:mainfrom
dannou812:dannou812/move-command

Conversation

@dannou812
Copy link
Copy Markdown
Contributor

fixes #11783

Description

Firstly Tests for the move Command have been added. Afterwards some duplicate Code has been removed and finally an error-message has been added for when a column is tried to be moved based on itself. This should fix #11783 .

To reiterate, the example of the initial issue now plays out as follows:

> {a: 1} | move a --after a                             
Error: nu::shell::incompatible_parameters

  × Incompatible parameters.
   ╭─[entry #1:1:15]
 1 │ {a: 1} | move a --after a
   ·               ┬         ┬
   ·               │         ╰── relative to itself
   ·               ╰── Column cannot be moved
   ╰────

User-Facing Changes

The error message shown above.

Tests + Formatting

I added some Tests for the behavior of the command. If I should add more, please let me know but I added everything that came to mind when thinking about the command.

@dannou812
Copy link
Copy Markdown
Contributor Author

I am sorry for all the commits but somehow I am not getting all the clippy warnings on my local machine. If anyone knows what the problem could be please let me know (I am running cargo clippy --workspace -- -D warnings -D clippy::unwrap_used)

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Feb 19, 2024

Other than my comment above I think this looks good. Thanks!

@dannou812
Copy link
Copy Markdown
Contributor Author

Great, I will change and commit it immediately.

@fdncred
Copy link
Copy Markdown
Contributor

fdncred commented Feb 20, 2024

Thanks for helping make nushell better!

@fdncred fdncred merged commit 7c1eb6b into nushell:main Feb 20, 2024
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
fixes nushell#11783 

# Description
Firstly Tests for the `move` Command have been added. Afterwards some
duplicate Code has been removed and finally an error-message has been
added for when a column is tried to be moved based on itself. This
should fix nushell#11783 .

To reiterate, the example of the initial issue now plays out as follows:
```shell
> {a: 1} | move a --after a                             
Error: nu::shell::incompatible_parameters

  × Incompatible parameters.
   ╭─[entry nushell#1:1:15]
 1 │ {a: 1} | move a --after a
   ·               ┬         ┬
   ·               │         ╰── relative to itself
   ·               ╰── Column cannot be moved
   ╰────
``` 

# User-Facing Changes
The error message shown above.

# Tests + Formatting
I added some Tests for the behavior of the command. If I should add
more, please let me know but I added everything that came to mind when
thinking about the command.

---------

Co-authored-by: dannou812 <dannou281@gmail.com>
@hustcer hustcer added this to the v0.91.0 milestone Feb 21, 2024
kik4444 pushed a commit to kik4444/nushell-fork that referenced this pull request Feb 28, 2024
fixes nushell#11783 

# Description
Firstly Tests for the `move` Command have been added. Afterwards some
duplicate Code has been removed and finally an error-message has been
added for when a column is tried to be moved based on itself. This
should fix nushell#11783 .

To reiterate, the example of the initial issue now plays out as follows:
```shell
> {a: 1} | move a --after a                             
Error: nu::shell::incompatible_parameters

  × Incompatible parameters.
   ╭─[entry nushell#1:1:15]
 1 │ {a: 1} | move a --after a
   ·               ┬         ┬
   ·               │         ╰── relative to itself
   ·               ╰── Column cannot be moved
   ╰────
``` 

# User-Facing Changes
The error message shown above.

# Tests + Formatting
I added some Tests for the behavior of the command. If I should add
more, please let me know but I added everything that came to mind when
thinking about the command.

---------

Co-authored-by: dannou812 <dannou281@gmail.com>
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.

move to the same column creates duplicate keys on records

3 participants