cp --force: add force option to files cp command #4079
Conversation
te0d
force-pushed
the
feat/commands/cp-force
branch
from
77f314d to
cd4e72e
Compare
|
Fixes: #2074 |
core/commands/files/files.go
Outdated
|
|
||
| child, err := pdir.Child(name) | ||
| if err == nil && child.Type() == mfs.TFile { | ||
| pdir.Unlink(name) |
There was a problem hiding this comment.
Should check the error returned from Unlink
| } | ||
|
|
||
| child, err := pdir.Child(name) | ||
| if err == nil && child.Type() == mfs.TFile { |
There was a problem hiding this comment.
how should this behave in the context of symlinks?
There was a problem hiding this comment.
Ok, wasn't aware of symlinks. I'll check it out.
core/commands/files/files.go
Outdated
|
|
||
| pdir, ok := parent.(*mfs.Directory) | ||
| if !ok { | ||
| err = fmt.Errorf("No such file or directory: %s", dir) |
There was a problem hiding this comment.
Not necessarily true, this could be a file, but not a directory. Probably should have the error message be "PATH was not a directory" or something
whyrusleeping
left a comment
whyrusleeping
left a comment
There was a problem hiding this comment.
some comments, would also be good to add a test for this to test/sharness/t0250-files-api.sh
The force option was added to the `files cp` command. Specifying force will overwrite an existing file with the source file. License: MIT Signed-off-by: Tom O'Donnell <[email protected]>
License: MIT Signed-off-by: Tom O'Donnell <[email protected]>
The --force flag would produce an error when the target file did not exist. It now ignores errors trying to find the node but observes errors when attempting to unlink. Added a test case to verify --force allows cp to overwrite a target. License: MIT Signed-off-by: Tom O'Donnell <[email protected]>
te0d
force-pushed
the
feat/commands/cp-force
branch
from
fd238f4 to
4f1889c
Compare
schomatis
added
kind/enhancement
|
@te0d So, if you're feeling adventurous and would like to learn some more about how IPFS work, particularly the MFS layer, I think we could leverage the If not, and you would just like to get this merged and get it over with (which is more than understandable since we made you wait more than an year as of now), please rebase this PR and we can give it the final touches to bring it home. |
|
@schomatis Yes, I can look into adapting Mv to perform both operations. Haven't looked at ipfs in a while and a bit busy the upcoming week, but I'll submit something the following wee. |
|
@te0d No problem, take your time. There's no need to submit a PR from the get-go, I would advice you to first analyze the implementation differences between |
| pdir, ok := parent.(*mfs.Directory) | ||
| if !ok { | ||
| err = fmt.Errorf("No such directory: %s", dir) | ||
| return err | ||
| } | ||
|
|
||
| // Attempt to unlink if child is a file, ignore error since | ||
| // we are only concerned with unlinking an existing file. | ||
| child, err := pdir.Child(name) | ||
| if err == nil && child.Type() == mfs.TFile { | ||
| err := pdir.Unlink(name) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil |
There was a problem hiding this comment.
| pdir, ok := parent.(*mfs.Directory) | |
| if !ok { | |
| err = fmt.Errorf("No such directory: %s", dir) | |
| return err | |
| } | |
| // Attempt to unlink if child is a file, ignore error since | |
| // we are only concerned with unlinking an existing file. | |
| child, err := pdir.Child(name) | |
| if err == nil && child.Type() == mfs.TFile { | |
| err := pdir.Unlink(name) | |
| if err != nil { | |
| return err | |
| } | |
| } | |
| return nil | |
| pdir, ok := parent.(*mfs.Directory) | |
| if !ok { | |
| return fmt.Errorf("not a directory: %s", dir) | |
| } | |
| // Attempt to unlink if child is a file, ignore error since | |
| // we are only concerned with unlinking an existing file. | |
| child, err := pdir.Child(name) | |
| if err != nil { | |
| return nil // no child file, nothing to unlink | |
| } | |
| if child.Type() != mfs.TFile { | |
| return fmt.Errorf("not a file: %s", path) | |
| } | |
| return pdir.Unlink(name) |
5 participants
cp --force: add force option to files cp command
The force option was added to the
files cpcommand. Specifying forcewill overwrite an existing file with the source file.
License: MIT
Signed-off-by: Tom O'Donnell [email protected]