Split scripts list in scripts or run-scripts #7464
Split scripts list in scripts or run-scripts #7464
Conversation
@@ -84,31 +84,52 @@ function runScript (args, cb) { | |||
|
|||
function list(cb) { | |||
var json = path.join(npm.localPrefix, "package.json") | |||
var cmdList = [ "publish", "install", "uninstall" |
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.
For bonus points, something that abstracted this list of lifecycle scripts out into a separate function would be useful, instead of having this scattered around in a few different places.
5b6e0ac
to
3efb83d
Compare
scripts.forEach(function(script) { | ||
console.log(prefix + script + s + d.scripts[script]) | ||
}) | ||
return cb(null, scripts) | ||
if (!scripts && runScripts) { |
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.
It's weird to have one console.log in a block and the other as a single-statement line. Make both branches into bracketed blocks?
One possible improvement, and two style nits. As usual, tests would be great. I see three test cases:
All tests should verify the output. Thanks for putting this together so quickly! |
Thanks for your review! After fixes that, Let me ping to you please. |
I wanted to get this into this week's release, so I powered through. I rebased and squashed your patch and landed it as a0a8777, and added the tests and some documentation in da269db and 687117a. This is a really nice little usability improvement! Thanks for thinking of it and doing the work to put this patch together! |
Wow, that’s amazing! I'm really happy that npm become user friendly. Thanks for following up so hard <3 |
I fixed #7463: Improvements in displaying npm run-script list
Run example: