Skip to content
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

Add option to print file being deleted (-V or --verbose) #183

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ var rimraf = require('./')
var help = false
var dashdash = false
var noglob = false
var verbose = false

var args = process.argv.slice(2).filter(function(arg) {
if (dashdash)
return !!arg
Expand All @@ -14,6 +16,8 @@ var args = process.argv.slice(2).filter(function(arg) {
noglob = true
else if (arg === '--glob' || arg === '-g')
noglob = false
else if (arg === '--verbose' || arg === '-V')
verbose = true
else if (arg.match(/^(-+|\/)(h(elp)?|\?)$/))
help = true
else
Expand All @@ -32,16 +36,23 @@ if (help || args.length === 0) {
log(' -h, --help Display this usage info')
log(' -G, --no-glob Do not expand glob patterns in arguments')
log(' -g, --glob Expand glob patterns in arguments (default)')
log(' -V, --verbose Prints paths being deleted')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rm command supports -v (lowercase) instead of -V. We could support both.

process.exit(help ? 0 : 1)
} else
go(0)

function go (n) {
if (n >= args.length)
return

var options = {}

if (noglob)
options = { glob: false }
options.glob = false

if (verbose)
options.verbose = verbose

rimraf(args[n], options, function (er) {
if (er)
throw er
Expand Down
4 changes: 4 additions & 0 deletions rimraf.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ function defaults (options) {
}
options.disableGlob = options.disableGlob || false
options.glob = options.glob || defaultGlobOpts
options.verbose = options.verbose || false
}

function rimraf (p, options, cb) {
Expand Down Expand Up @@ -107,6 +108,9 @@ function rimraf (p, options, cb) {
if (er.code === "ENOENT") er = null
}

if (options.verbose)
console.log('deleting', p)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw, we should use console.info

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It's just writing a string to stdout, in this case they're 100% equivalent, I almost always use console.log() for this purpose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of terminal profiles can be (and are) setup to visually highlight different forms of logs. afaik there is also a screen reader for terminal output which can differentiate between log & info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may dismiss this suggestion if it isn't aligned with other code here. I did prefix this with fwiw - not a deal breaker for most.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are literally the same function: https://github.com/nodejs/node/blob/main/lib/internal/console/constructor.js#L682

The only way you could highlight different forms of logs is by patching the global console object.

They're displayed differently in the browser, but in Node, they're 100% identical, just same function with two different names.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, thanks for informing me. I wasn't aware of this.


timeout = 0
next(er)
})
Expand Down