-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Adjust -i behavior for ln, cp & mv #4630
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
Conversation
Matches the change upstream 7a69df88999bedd8e9fccf9f3dfa9ac6907fab66
Just like mv Note that cp -i -u won't show the overwrite question Matches the change upstream 7a69df88999bedd8e9fccf9f3dfa9ac6907fab66
Just like mv & cp Matches the change upstream 7a69df88999bedd8e9fccf9f3dfa9ac6907fab66
GNU ln uses "replace" instead of "overwrite"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While reviewing I noticed that we use "overwrite" in the prompt of ln
instead of "replace" and fixed it.
@@ -420,7 +420,7 @@ fn rename( | |||
OverwriteMode::NoClobber => return Ok(()), | |||
OverwriteMode::Interactive => { | |||
if !prompt_yes!("overwrite {}?", to.quote()) { | |||
return Ok(()); | |||
return Err(io::Error::new(io::ErrorKind::Other, "")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do something else than constructing an io::Error
? This is going to give a weird error message isn't it? For a PR like this, I guess it's OK, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, there isn't any error message :)
$ touch a b && echo "n"|./target/debug/coreutils mv -i a b; echo $?
mv: overwrite 'b'?
1
GNU testsuite comparison:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
Mentioned in issue #4627
touch a b && echo "n"|./target/debug/coreutils ln -i a b; echo $?
was 0, it is now 1
See:
coreutils/coreutils@7a69df8
(no problem reading the code here as there isn't much)