Skip to content

mv: decline of overwrite does not return an error#4731

Closed
Senshyi wants to merge 1 commit intouutils:mainfrom
Senshyi:mv-i-should-not-show-error-if-overwrite-is-declined
Closed

mv: decline of overwrite does not return an error#4731
Senshyi wants to merge 1 commit intouutils:mainfrom
Senshyi:mv-i-should-not-show-error-if-overwrite-is-declined

Conversation

@Senshyi
Copy link

@Senshyi Senshyi commented Apr 11, 2023

solves issue #4724

@cakebaker cakebaker linked an issue Apr 12, 2023 that may be closed by this pull request
@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/mv/i-1. tests/mv/i-1 is passing on 'main'. Maybe you have to rebase?

@cakebaker
Copy link
Contributor

cakebaker commented Apr 12, 2023

Thanks for your PR.

Unfortunately, it doesn't solve the issue entirely. While it removes the error message, it also changes the exit code from 1 to 0 (this is why I used echo $? in the ticket, to highlight that the exit code should be 1).

Here's a test case to reproduce the issue. Currently, it will fail while all existing tests pass:

#[test]
fn test_mv_interactive_with_dir_as_target() {
    let (at, mut ucmd) = at_and_ucmd!();

    let file = "test_mv_interactive_file";
    let target_dir = "target";

    at.mkdir(target_dir);
    at.touch(file);
    at.touch(format!("{target_dir}/{file}"));

    ucmd.arg(file)
        .arg(target_dir)
        .arg("-i")
        .pipe_in("n")
        .fails()
        .stderr_does_not_contain("cannot move")
        .no_stdout();
}

@cakebaker
Copy link
Contributor

@KEEZII are you still working on this issue?

@cakebaker
Copy link
Contributor

Closing this PR as it has been superseded by #4773

@cakebaker cakebaker closed this Apr 23, 2023
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.

mv: -i shouldn't show error message if overwriting is declined

2 participants