-
Notifications
You must be signed in to change notification settings - Fork 722
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
Added verbosity flag (-v) for cp, mv and rm commands #1129
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
cmdOptions: { | ||
'f': '!no_force', | ||
'n': 'no_force', | ||
'v': 'verbose', | ||
}, | ||
}); | ||
|
||
|
@@ -102,15 +103,18 @@ | |
|
||
try { | ||
fs.renameSync(src, thisDest); | ||
if (options.verbose) { | ||
console.log("renamed '" + src + "' -> '" + thisDest + "'"); | ||
} | ||
} catch (e) { | ||
/* istanbul ignore next */ | ||
if (e.code === 'EXDEV') { | ||
// If we're trying to `mv` to an external partition, we'll actually need | ||
// to perform a copy and then clean up the original file. If either the | ||
// copy or the rm fails with an exception, we should allow this | ||
// exception to pass up to the top level. | ||
cp({ recursive: true }, src, thisDest); | ||
rm({ recursive: true, force: true }, src); | ||
cp({ recursive: true, verbose: options.verbose }, src, thisDest); | ||
rm({ recursive: true, force: true, verbose: options.verbose }, src); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! 😄 |
||
} | ||
} | ||
}); // forEach(src) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
'f': 'force', | ||
'r': 'recursive', | ||
'R': 'recursive', | ||
'v': 'verbose', | ||
}, | ||
}); | ||
|
||
|
@@ -17,7 +18,7 @@ | |
// | ||
// Licensed under the MIT License | ||
// http://www.opensource.org/licenses/mit-license.php | ||
function rmdirSyncRecursive(dir, force, fromSymlink) { | ||
function rmdirSyncRecursive(dir, force, verbose, fromSymlink) { | ||
var files; | ||
|
||
files = fs.readdirSync(dir); | ||
|
@@ -28,11 +29,14 @@ | |
var currFile = common.statNoFollowLinks(file); | ||
|
||
if (currFile.isDirectory()) { // Recursive function back to the beginning | ||
rmdirSyncRecursive(file, force); | ||
rmdirSyncRecursive(file, force, verbose); | ||
} else if (force || isWriteable(file)) { | ||
// Assume it's a file - perhaps a try/catch belongs here? | ||
try { | ||
common.unlinkSync(file); | ||
if (verbose) { | ||
console.log("removed '" + file + "'"); | ||
} | ||
} catch (e) { | ||
/* istanbul ignore next */ | ||
common.error('could not remove file (code ' + e.code + '): ' + file, { | ||
|
@@ -59,6 +63,9 @@ | |
try { | ||
result = fs.rmdirSync(dir); | ||
if (fs.existsSync(dir)) throw { code: 'EAGAIN' }; | ||
if (verbose) { | ||
console.log("removed directory'" + dir + "'"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a space between |
||
} | ||
break; | ||
} catch (er) { | ||
/* istanbul ignore next */ | ||
|
@@ -98,6 +105,9 @@ | |
if (options.force || isWriteable(file)) { | ||
// -f was passed, or file is writable, so it can be removed | ||
common.unlinkSync(file); | ||
if (options.verbose) { | ||
console.log("removed '" + file + "'"); | ||
} | ||
} else { | ||
common.error('permission denied: ' + file, { continue: true }); | ||
} | ||
|
@@ -106,43 +116,53 @@ | |
function handleDirectory(file, options) { | ||
if (options.recursive) { | ||
// -r was passed, so directory can be removed | ||
rmdirSyncRecursive(file, options.force); | ||
rmdirSyncRecursive(file, options.force, options.verbose); | ||
} else { | ||
common.error('path is a directory', { continue: true }); | ||
} | ||
} | ||
|
||
function handleSymbolicLink(file, options) { | ||
function _unlink() { | ||
common.unlinkSync(file); | ||
if (options.verbose) { | ||
console.log("removed '" + file + "'"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you change this so that |
||
} | ||
|
||
var stats; | ||
try { | ||
stats = common.statFollowLinks(file); | ||
} catch (e) { | ||
// symlink is broken, so remove the symlink itself | ||
common.unlinkSync(file); | ||
_unlink(); | ||
return; | ||
} | ||
|
||
if (stats.isFile()) { | ||
common.unlinkSync(file); | ||
_unlink(); | ||
} else if (stats.isDirectory()) { | ||
if (file[file.length - 1] === '/') { | ||
// trailing separator, so remove the contents, not the link | ||
if (options.recursive) { | ||
// -r was passed, so directory can be removed | ||
var fromSymlink = true; | ||
rmdirSyncRecursive(file, options.force, fromSymlink); | ||
rmdirSyncRecursive(file, options.force, options.verbose, fromSymlink); | ||
} else { | ||
common.error('path is a directory', { continue: true }); | ||
} | ||
} else { | ||
// no trailing separator, so remove the link | ||
common.unlinkSync(file); | ||
_unlink(); | ||
} | ||
} | ||
} | ||
|
||
function handleFIFO(file) { | ||
function handleFIFO(file, options) { | ||
common.unlinkSync(file); | ||
if (options.verbose) { | ||
console.log("removed '" + file + "'"); | ||
} | ||
} | ||
|
||
//@ | ||
|
@@ -193,7 +213,7 @@ | |
} else if (lstats.isSymbolicLink()) { | ||
handleSymbolicLink(file, options); | ||
} else if (lstats.isFIFO()) { | ||
handleFIFO(file); | ||
handleFIFO(file, options); | ||
} | ||
}); // forEach(file) | ||
return ''; | ||
|
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.
I wonder if we should drop the "copied" prefix. When I tested this on my linux machine, I don't see this in the output:
On the other hand, keeping "copied" would be more consistent with the other commands, since those do say "removed" or "renamed" before the file name.
What do you think?