Fix cp to match unix behavior #271
Conversation
|
It might be helpful to add unit tests for this behavior. When I ran a quick test, it seemed to work as expected. |
|
@freitagbr Could you rebase off master? |
In unix, if src is a directory and dest doesn't exist, 'cp -r src dest' will copy src/* into dest. Fix cp('-r', 'src', 'dest') to behave the same way.
|
@nfischer Done. |
|
Looks like some of the tests are still failing. If we could get these fixed (or have the tests adjusted, if appropriate), that'd be good |
| @@ -125,17 +125,23 @@ assert.equal(shell.cat('resources/cp/dir_a/z'), shell.cat('tmp/cp/dir_a/z')); // | |||
|
|
|||
| //recursive, creates dest dir since it's only one level deep (see Github issue #44) | |||
| shell.rm('-rf', 'tmp/*'); | |||
| shell.cp('-r', 'resources/issue44/*', 'tmp/dir2'); | |||
| shell.cp('-r', 'resources/issue44', 'tmp/dir2'); | |||
There was a problem hiding this comment.
Should these test cases be changed? What was wrong with the previous behavior?
There was a problem hiding this comment.
In *nix, cp -r resources/issue44/* tmp/dir2 is not valid. If there is only one file in resources/issue44, it will be copied into tmp and be renamed dir2. If there is more than one file in resources/issue44, the command will fail.
The intention here is to copy all of the files in resources/issue44 to tmp/dir2, where dir2 will be created. This is achieved with the new command (and in *nix as well).
There was a problem hiding this comment.
Awesome explanation! The logic sounds right to me. It sounds like this is indeed a bug in the test. Thanks for fixing!
|
LGTM! @ariporad |
Fix cp to match unix behavior
Fixes #256, #101.