Use npm commander for parsing argv

[katanacrimson]

As on tin; would also make creation of a --help command much easier and make it a bit easier to determine how to run or configure the application from commandline.

[katanacrimson]

looking in app.js further, this would be far more ideal than interacting directly with process.std*, it'd be much more readable.

[julianlam]

Nice -- I was not aware of the commander module. Will integrate for the next release.

[katanacrimson]

Also worth considering is nconf - it's a bit heavier and lacks the prompting, but provides a mixture of configuration methods.

https://github.com/flatiron/nconf

That means you'd also have defaults, hierarchies, ENV variables, etc.

[julianlam]

Merged into master at 3c62b86 from branch nconf (since deleted)

[katanacrimson]

145898c#L0R41

Candidate for path.join imo.

145898c#L0R82

Better written as templates.ready(webserver.init); don't need to pass a function to call a function when all we need is a callable. Won't need to waste the memory.

145898c#L0R115

You might as well use both nconf and commander here - specifically here, commander would be the ideal.

Relevant:
https://github.com/visionmedia/commander.js/#promptmsg-fn
https://github.com/visionmedia/commander.js/#passwordmsg-mask-fn

Also look into async.series using an object for storing functions - each function would then be your ask() call, and you'd get away from indent hell that way. Would look far cleaner and easier to read.

145898c#L4R89

Look into node-validator / express-validator for a better method of email validation.

145898c#L0R159

Rather than defining default configs here, why not do so at the top of the file beside this?

Relevant, https://github.com/flatiron/nconf#hierarchical-configuration - look at nconf.defaults()

---

Other than that, looking like a decent improvement.

[julianlam]

Thanks,

Re: indent hell - I've actually taken out all of the nested asks and placed them inside an async.eachSeries in /src/install.js: https://github.com/designcreateplay/NodeBB/blob/3feb977cc44a695dc5d35305c60cbdb1f6b8d60d/src/install.js#L39

As for nconf.defaults(), I wasn't 100% sure whether I could use nconf.save(), since the way I understand, it saves the entire config contents into the file, including other values that are nconf.set-ed. It isn't a problem now, but if I were to update the current admin configs (themes, motd, etc) to use nconf-redis, then I'm not exactly sure what would be saved where. Let me know if I am wrong...

The latest commit has the defaults defined in install.js as well: 3c62b86#L6R17

[julianlam]

commander should be good, especially with the prompt method.

[katanacrimson]

@julianlam regarding nconf-redis, that is something of which I am not familiar with.

I would suggest asking the developer straight up about the behavior.