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.

Sign up for GitHub

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

Jump to bottom

add uutils/coreutils mv command into nushell #10446

Closed
fdncred opened this issue Sep 20, 2023 · 5 comments · Fixed by #10822
Closed

add uutils/coreutils mv command into nushell #10446

fdncred opened this issue Sep 20, 2023 · 5 comments · Fixed by #10822
Assignees
@dmatos2012
Labels
coreutils-uutils Changes relating to coreutils/uutils enhancement New feature or request
Milestone
v0.90.0

Comments

@fdncred
Copy link
Collaborator

fdncred commented Sep 20, 2023 •
edited

Related problem

Following on the momentum of moving the uu_cp crate/command into nushell, we'd like to add the uu_mv crate/command too, as umv for now. We can use this issue to discuss parameters and other topics.

Looking for volunteers. Reply below and we can assign the issue to you.

Describe the solution you'd like

Use the ucp code as a guide and port umv into nushell.

Describe alternatives you've considered

No response

Additional context and details

No response
@fdncred fdncred added enhancement New feature or request coreutils-uutils Changes relating to coreutils/uutils labels Sep 20, 2023
@fdncred fdncred mentioned this issue Sep 20, 2023
Open
@fdncred fdncred changed the title add uutils/coreutils mv into nushell add uutils/coreutils mv command into nushell Sep 20, 2023
@tertsdiepraam tertsdiepraam mentioned this issue Sep 20, 2023
Closed
@fdncred
Copy link
Collaborator Author

fdncred commented Sep 30, 2023

Any takers? My dream of 1 coreutils command per week is sadly failing.

We need uutils/coreutils#5335 to land I think before we can land an integration with mv.

@dmatos2012
Copy link
Contributor

It looks like its already merged, so I will give mv a shot. Basically same procedure as ucp:

  • Implement first iteration as umv,
  • Write extra tests from uutils mv to umv tests.
  • Run for the 3 weeks or something that @fdncred would like probably

If we are happy:

  • Rename umv to mv and then mv to mv-old
  • After period of time, remove mv-old altogether.

If thats ok, I will start with rm @fdncred.

@fdncred
Copy link
Collaborator Author

fdncred commented Oct 16, 2023

Yup, I was thinking a very similar procedure as ucp. I've assigned it to you. Thanks so much for chipping in again!

@dmatos2012
Copy link
Contributor

I think I am almost done for first iteration of umv, @fdncred, but first I have to get some changes to uutils approved first, see PRs dir into itsef bug and making UpdateMode public.

other than this, I think there are only about 2 nushell mv tests that have different behavior but the new umv should almost be done. But I have got less time in these upcoming 2 days to work on this, but should have initial PR by the weekend or beginning of next week.

Just so that you know that I am working on the background on it and I havent disappeared :).

@fdncred
Copy link
Collaborator Author

fdncred commented Oct 19, 2023

Thanks @dmatos2012 for the update. Looks like one of your PRs has landed. I'm very excited to have umv in nushell. Thanks for working on it! Our next release is Nov-14, so we have some time to work out the kinks.

@dmatos2012 dmatos2012 mentioned this issue Oct 23, 2023
Merged
4 tasks
@fdncred fdncred mentioned this issue Jan 16, 2024
Open
16 tasks
fdncred pushed a commit that referenced this issue Jan 18, 2024
@dmatos2012
Initial implementation of umv from uutils (#10822)
ee6547d 
<!--
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
Hi,
This closes #10446 , wherein we start implementing `mv` from `uutils`.
There are some stuff to iron out, particularly
* Decide on behavior from ignored tests 
* Wait for release/PRs to be approved on `uutils` side, but still can be
tested for now. See [PR
approved](uutils/coreutils#5428), and
[pending](uutils/coreutils#5429).
* `--progress` does not seem to work on `uutils mv` either and have not
checked whether certain `X` size has to be achieved in order for it to
appear, thus something to investigate as well, but thought it wasnt
important enough to not make the PR.

See [issue
comment](#10446 (comment)),
on the possible strategy to follow, mainly copy what we did with `ucp`.

I still left some comments on purpose particularly on tests, which of
course would be removed before something is decided here. :) @fdncred
<!--
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:

- [X] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [X] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used`
to check that you're using the standard code style
- [X] `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))
- [X] `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
> ```
-->

# 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.
-->
@hustcer hustcer added this to the v0.90.0 milestone Jan 18, 2024
dmatos2012 added a commit to dmatos2012/nushell that referenced this issue Feb 20, 2024
@dmatos2012
Initial implementation of umv from uutils (nushell#10822)
865914c 
<!--
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
Hi,
This closes nushell#10446 , wherein we start implementing `mv` from `uutils`.
There are some stuff to iron out, particularly
* Decide on behavior from ignored tests 
* Wait for release/PRs to be approved on `uutils` side, but still can be
tested for now. See [PR
approved](uutils/coreutils#5428), and
[pending](uutils/coreutils#5429).
* `--progress` does not seem to work on `uutils mv` either and have not
checked whether certain `X` size has to be achieved in order for it to
appear, thus something to investigate as well, but thought it wasnt
important enough to not make the PR.

See [issue
comment](nushell#10446 (comment)),
on the possible strategy to follow, mainly copy what we did with `ucp`.

I still left some comments on purpose particularly on tests, which of
course would be removed before something is decided here. :) @fdncred
<!--
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:

- [X] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [X] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used`
to check that you're using the standard code style
- [X] `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))
- [X] `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
> ```
-->

# 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.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmatos2012 dmatos2012

Labels
coreutils-uutils Changes relating to coreutils/uutils enhancement New feature or request
Projects
None yet
Milestone
v0.90.0
Development

Successfully merging a pull request may close this issue.

Initial implementation of umv from uutils dmatos2012/nushell
3 participants
@fdncred @hustcer @dmatos2012