Added verbosity flag (-v) for cp, mv and rm commands - #1129
Conversation
Sorry, something went wrong.
|
I had to force-push the same commit, because I've signed it, so that GitLab could unblock the PR. No code changed |
Sorry, something went wrong.
| 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().
Sorry, something went wrong.
| 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.
Good catch! 😄
Sorry, something went wrong.
| } | ||
|
|
||
| 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?
Sorry, something went wrong.
| 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 '
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 62.50000% with 9 lines in your changes missing coverage. Please review.
@@ 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 Harness.
|
Sorry, something went wrong.
|
It looks like there's a CI bug on the main branch (not a problem with your PR). I opened issue #1130 to work on that. |
Sorry, something went wrong.
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.