Fix Bug in copy.lua, mkdir.lua and rename.lua (updated) #411
Conversation
|
Are you using Codecuriosity? |
No, it wouldn't. If you appear on this page at all, you will appear in the credits. Either way, next time you could do a rebase to fix the faulty commits. |
| @@ -10,7 +10,9 @@ local sDest = shell.resolve( tArgs[2] ) | |||
| local tFiles = fs.find( sSource ) | |||
| if #tFiles > 0 then | |||
| for n,sFile in ipairs( tFiles ) do | |||
| if fs.isDir( sDest ) then | |||
| if fs.exists( sDest ) == true then | |||
There was a problem hiding this comment.
This check is incorrect. "copy existingfile existingdir" is perfectly valid, hence the isDir() check on the very next line, please move it inside the "#tFiles == 1" branch and remove the "== true"
| @@ -5,5 +5,11 @@ if #tArgs < 1 then | |||
| end | |||
|
|
|||
| local sNewDir = shell.resolve( tArgs[1] ) | |||
|
|
|||
| if fs.exists( sNewDir ) == true then | |||
There was a problem hiding this comment.
This check is unnecessary. Making a dir that already exists should just silently suceed.
There was a problem hiding this comment.
Yes, when it is a Directory. But if it is a normal file, it will give a lua error.
| @@ -6,4 +6,9 @@ end | |||
|
|
|||
| local sSource = shell.resolve( tArgs[1] ) | |||
| local sDest = shell.resolve( tArgs[2] ) | |||
|
|
|||
| if fs.exists( sDest ) == true then | |||
There was a problem hiding this comment.
This is the only check in this PR that's actually meaningful...
Please remove the "== true" though.
|
@dan200 Changes are now done. btw: Here's a screen, that shows, that mkdir will crash, if the file exists. |
| @@ -5,5 +5,11 @@ if #tArgs < 1 then | |||
| end | |||
|
|
|||
| local sNewDir = shell.resolve( tArgs[1] ) | |||
|
|
|||
| if fs.exists( sNewDir ) then | |||
There was a problem hiding this comment.
Change to "fs.exists(sNewDir) and not fs.isDir(sNewDir)"
It's perfectly okay to "make" a directory that already exists (it should silently succeed
|
@dan200 The Check is now added |
| @@ -13,7 +13,11 @@ if #tFiles > 0 then | |||
| if fs.isDir( sDest ) then | |||
| fs.copy( sFile, fs.combine( sDest, fs.getName(sFile) ) ) | |||
| elseif #tFiles == 1 then | |||
| fs.copy( sFile, sDest ) | |||
| if fs.exists( sDest ) then | |||
| printError( "Destination exists" ) | |||
There was a problem hiding this comment.
SHOULD request of overwrite file instead of drop error and quit.
fs.copy(sFile, sDest) like use --force argument in POSIX/GNU cp.
|
CraftOS has never aimed to be POSIX compliant, but I agree it'd be nice to have. |
Fix Bug in copy.lua, mkdir.lua and rename.lua (updated)
This is a Updated Version of #404
The Reason for the Update is, that a Commit is not bound to my Account and maybe this will make Problems with Credits Generation.