Added verbosity flag (-v) for cp, mv and rm commands#1129
Added verbosity flag (-v) for cp, mv and rm commands#1129nicky1038 wants to merge 1 commit intoshelljs:mainfrom
Conversation
nicky1038
force-pushed
the
cp-mv-rm-verbose-output
branch
2 times, most recently
from
5c298ee to
dd92b29
Compare
nicky1038
force-pushed
the
cp-mv-rm-verbose-output
branch
from
dd92b29 to
1e3df2a
Compare
|
I had to force-push the same commit, because I've signed it, so that GitLab could unblock the PR. No code changed |
| common.unlinkSync(file); | ||
| if (options.verbose) { | ||
| console.log("removed '" + file + "'"); | ||
| } |
There was a problem hiding this comment.
Can you change this so that file is a method parameter? I'd also suggest passing options as a second parameter and then moving this function definition to the global scope, similar to what you've done with handleFIFO().
| 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); |
| } | ||
|
|
||
| if (options.verbose) { | ||
| console.log("copied '" + srcFile + "' -> '" + destFile + "'"); |
There was a problem hiding this comment.
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:
$ cp -vr src/ dest/
'src/' -> 'dest/'
'src/file.txt' -> 'dest/file.txt'
'src/sub1' -> 'dest/sub1'
'src/sub1/sub2' -> 'dest/sub1/sub2'
'src/sub1/sub2/file2.txt' -> 'dest/sub1/sub2/file2.txt'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?
| 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.
Add a space between directory and '
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1129 +/- ##
=======================================
Coverage ? 96.36%
=======================================
Files ? 36
Lines ? 1375
Branches ? 0
=======================================
Hits ? 1325
Misses ? 50
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
It looks like there's a CI bug on the |
2 participants
Closes #1127
The suggested verbose output is not claimed to be fully identical to the corresponding output in the original commands from coreutils. Though it is very similar, I also tried to make as few edits as possible.