Adding Support for defining one script to run in watch mode #38
Conversation
cli.js
Outdated
| @@ -2,14 +2,17 @@ | |||
| 'use strict'; | |||
| var path = require('path') | |||
|
|
|||
| var windows = process.platform === 'win32' | |||
| var windows = process.platform === 'win32' | |||
There was a problem hiding this comment.
I think my / our company Formater reformarted a lot of stuff i will change it back.
cli.js
Outdated
| process.env[pathVarName] += path.delimiter + path.join(__dirname, 'node_modules', '.bin') | ||
| process.env[pathVarName] += | ||
| path.delimiter + | ||
| path.join(__dirname, 'node_modules', '.bin') |
There was a problem hiding this comment.
Could you please revert this formatting? It does not seem to add anything in this case
cli.js
Outdated
|
|
||
| var watchPackage = require('./watch-package') | ||
| var watchPackage = require('./watch-package') |
cli.js
Outdated
|
|
||
| var watcher = watchPackage(process.argv[2] || process.cwd(), process.exit) | ||
| var watcher = watchPackage( | ||
| process.argv[3] || process.cwd(), process.exit, process.argv[2]) |
watch-package.js
Outdated
| var pkg = require(path.join(pkgDir, 'package.json')) | ||
| var processes = {} | ||
|
|
||
| if (typeof taskName !== 'undefined' && taskName === '') { | ||
| console.info('No Task specified. Will go trough all possible tasks'); |
There was a problem hiding this comment.
task should be lowercase.
trough -> through
There was a problem hiding this comment.
Thanks for the correction.
| var pkgDir = ''; | ||
| var stdin = null; | ||
|
|
||
| module.exports = function watchPackage(_pkgDir, exit, taskName) { |
There was a problem hiding this comment.
can taskName ever come in with only white-space?
if so, it should be trimmed as soon as possible
There was a problem hiding this comment.
You are right it would be possible if anyone would call it "npm-watch ' '".
Not thought about it and will add the trim.
| @@ -8,16 +8,24 @@ var through = require('through2') | |||
| var npm = process.platform === 'win32' ? 'npm.cmd' : 'npm'; | |||
| var nodemon = process.platform === 'win32' ? 'nodemon.cmd' : 'nodemon'; | |||
|
|
|||
| module.exports = function watchPackage(pkgDir, exit) { | |||
| var pkgDir = ''; | |||
| var stdin = null; | |||
There was a problem hiding this comment.
What is the reasoning for creating these variables?
There was a problem hiding this comment.
The Problem is that if i not create these variables in the Global scope i would need to pass them as parameter to the function. So in this case they are in the module scope and not can be refered from outside.
|
Happens to be, this was on my to-do list of things to add. 😀 |
|
Is nice that I could help you. Kind regards. |
watch-package.js
Outdated
| @@ -36,11 +44,47 @@ module.exports = function watchPackage(pkgDir, exit) { | |||
| stdin.stderr = through() | |||
| stdin.stdout = through() | |||
|
|
|||
| if (typeof taskName !== 'undefined' && taskName.trim() !== '') { | |||
| if (!pkg.scripts[taskName]) { | |||
There was a problem hiding this comment.
Might be worth while to do taskName = taskName.trim() before the if
There was a problem hiding this comment.
hmm but what if taskName is undefined cause there was no param on vargs[2] than it would crash cause we can not trim on undefined. Otherwise we can do a default action after the function definition. How you like best.
There was a problem hiding this comment.
I was thinking about in between lines 47&48 so after we already checked for undefined.
But I will have to pull down the code before deciding on this, so will get back to you after the weekend
There was a problem hiding this comment.
No problem.
Take your time.
Just updated it to be like an default param.
There was a problem hiding this comment.
I like the default param implementation, so will leave it at that
|
🎉 |
|
Thanks for accepting my Pullrequest. Kind regards Marcel Ploch |
Hey,
I added and Update so the people can define one script inside the task to run in watch mode.
This is a nice feature if you define more than just one task inside the "watch" Object and not want to run all at once.
Kind regards
Marcel Ploch
This change is