Added verbosity flag (-v) for cp, mv and rm commands #1129
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); |
| @@ -84,6 +85,10 @@ function copyFileSync(srcFile, destFile, options) { | |||
| fs.closeSync(fdr); | |||
| fs.closeSync(fdw); | |||
| } | |||
|
|
|||
| 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?
| @@ -59,6 +63,9 @@ function rmdirSyncRecursive(dir, force, fromSymlink) { | |||
| 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.
Add a space between directory and '
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1129 +/- ##
==========================================
- Coverage 97.19% 96.36% -0.83%
==========================================
Files 36 36
Lines 1354 1375 +21
==========================================
+ Hits 1316 1325 +9
- Misses 38 50 +12
☔ View full report in Codecov by Sentry. |
|
It looks like there's a CI bug on the |
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.