From a51c5698c704a39efdbd87b9c77ee66e0208b3f1 Mon Sep 17 00:00:00 2001
From: Peter Jaszkowiak <p.jaszkow@gmail.com>
Date: Wed, 6 Jan 2021 18:36:50 -0700
Subject: [PATCH] fix: `--help` usage info

yargs (via nconf) would exit when detecting a help flag

also improves the speed of `./nodebb help build`
---
 install/package.json |  1 +
 src/cli/index.js     | 48 +++++++++++++++++++++++++-------------------
 src/cli/manage.js    | 23 ---------------------
 src/meta/aliases.js  | 44 ++++++++++++++++++++++++++++++++++++++++
 src/meta/build.js    | 26 ++++--------------------
 5 files changed, 76 insertions(+), 66 deletions(-)
 create mode 100644 src/meta/aliases.js

diff --git a/install/package.json b/install/package.json
index eaf379ea38..0a448564e9 100644
--- a/install/package.json
+++ b/install/package.json
@@ -146,6 +146,7 @@
         "winston": "3.3.3",
         "xml": "^1.0.1",
         "xregexp": "^4.3.0",
+        "yargs": "16.2.0",
         "zxcvbn": "^4.4.2"
     },
     "devDependencies": {
diff --git a/src/cli/index.js b/src/cli/index.js
index 8d53f069f7..88683d24ba 100644
--- a/src/cli/index.js
+++ b/src/cli/index.js
@@ -1,3 +1,5 @@
+/* eslint-disable import/order */
+
 'use strict';
 
 const fs = require('fs');
@@ -35,13 +37,13 @@ try {
 try {
 	fs.accessSync(path.join(paths.nodeModules, 'semver/package.json'), fs.constants.R_OK);
 
-	var semver = require('semver');
-	var defaultPackage = require('../../install/package.json');
+	const semver = require('semver');
+	const defaultPackage = require('../../install/package.json');
 
-	var checkVersion = function (packageName) {
-		var version = JSON.parse(fs.readFileSync(path.join(paths.nodeModules, packageName, 'package.json'), 'utf8')).version;
+	const checkVersion = function (packageName) {
+		const version = JSON.parse(fs.readFileSync(path.join(paths.nodeModules, packageName, 'package.json'), 'utf8')).version;
 		if (!semver.satisfies(version, defaultPackage.dependencies[packageName])) {
-			var e = new TypeError('Incorrect dependency version: ' + packageName);
+			const e = new TypeError('Incorrect dependency version: ' + packageName);
 			e.code = 'DEP_WRONG_VERSION';
 			throw e;
 		}
@@ -67,14 +69,13 @@ try {
 }
 
 require('colors');
-// eslint-disable-next-line
-var nconf = require('nconf');
-// eslint-disable-next-line
-var program = require('commander');
+const nconf = require('nconf');
+const { program } = require('commander');
+const yargs = require('yargs');
 
-var pkg = require('../../package.json');
-var file = require('../file');
-var prestart = require('../prestart');
+const pkg = require('../../package.json');
+const file = require('../file');
+const prestart = require('../prestart');
 
 program
 	.name('./nodebb')
@@ -86,19 +87,23 @@ program
 	.option('-d, --dev', 'Development mode, including verbose logging', false)
 	.option('-l, --log', 'Log subprocess output to console', false);
 
-nconf.argv().env({
+// provide a yargs object ourselves
+// otherwise yargs will consume `--help` or `help`
+// and `nconf` will exit with useless usage info
+const opts = yargs(process.argv.slice(2)).help(false).exitProcess(false);
+nconf.argv(opts).env({
 	separator: '__',
 });
 
-var env = program.dev ? 'development' : (process.env.NODE_ENV || 'production');
+const env = program.dev ? 'development' : (process.env.NODE_ENV || 'production');
 process.env.NODE_ENV = env;
 global.env = env;
 
 prestart.setupWinston();
 
 // Alternate configuration file support
-var	configFile = path.resolve(paths.baseDir, nconf.get('config') || 'config.json');
-var configExists = file.existsSync(configFile) || (nconf.get('url') && nconf.get('secret') && nconf.get('database'));
+const	configFile = path.resolve(paths.baseDir, nconf.get('config') || 'config.json');
+const configExists = file.existsSync(configFile) || (nconf.get('url') && nconf.get('secret') && nconf.get('database'));
 
 prestart.loadConfig(configFile);
 prestart.versionCheck();
@@ -195,7 +200,7 @@ program
 		require('./manage').build(targets.length ? targets : true, options);
 	})
 	.on('--help', function () {
-		require('./manage').buildTargets();
+		require('../meta/aliases').buildTargets();
 	});
 program
 	.command('activate [plugin]')
@@ -223,7 +228,7 @@ program
 	});
 
 // reset
-var resetCommand = program.command('reset');
+const resetCommand = program.command('reset');
 
 resetCommand
 	.description('Reset plugins, themes, settings, etc')
@@ -233,7 +238,7 @@ resetCommand
 	.option('-s, --settings', 'Reset settings to their default values')
 	.option('-a, --all', 'All of the above')
 	.action(function (options) {
-		var valid = ['theme', 'plugin', 'widgets', 'settings', 'all'].some(function (x) {
+		const valid = ['theme', 'plugin', 'widgets', 'settings', 'all'].some(function (x) {
 			return options[x];
 		});
 		if (!valid) {
@@ -295,10 +300,11 @@ program
 			return program.help();
 		}
 
-		var command = program.commands.find(function (command) { return command._name === name; });
+		const command = program.commands.find(function (command) { return command._name === name; });
 		if (command) {
 			command.help();
 		} else {
+			console.log(`error: unknown command '${command}'.`);
 			program.help();
 		}
 	});
@@ -311,4 +317,4 @@ if (process.argv.length === 2) {
 
 program.executables = false;
 
-program.parse(process.argv);
+program.parse();
diff --git a/src/cli/manage.js b/src/cli/manage.js
index c986a2725f..d586484bf6 100644
--- a/src/cli/manage.js
+++ b/src/cli/manage.js
@@ -2,7 +2,6 @@
 
 const winston = require('winston');
 const childProcess = require('child_process');
-const _ = require('lodash');
 const CliGraph = require('cli-graph');
 
 const build = require('../meta/build');
@@ -13,27 +12,6 @@ const analytics = require('../analytics');
 const reset = require('./reset');
 const { pluginNamePattern, themeNamePattern } = require('../constants');
 
-function buildTargets() {
-	var aliases = build.aliases;
-	var length = 0;
-	var output = Object.keys(aliases).map(function (name) {
-		var arr = aliases[name];
-		if (name.length > length) {
-			length = name.length;
-		}
-
-		return [name, arr.join(', ')];
-	}).map(function (tuple) {
-		return '     ' + _.padEnd('"' + tuple[0] + '"', length + 2).magenta + '  |  ' + tuple[1];
-	}).join('\n');
-	console.log(
-		'\n\n  Build targets:\n' +
-		('\n     ' + _.padEnd('Target', length + 2) + '  |  Aliases').green +
-		'\n     ------------------------------------------------------\n'.blue +
-		output + '\n'
-	);
-}
-
 async function activate(plugin) {
 	if (themeNamePattern.test(plugin)) {
 		await reset.reset({
@@ -176,7 +154,6 @@ async function buildWrapper(targets, options) {
 }
 
 exports.build = buildWrapper;
-exports.buildTargets = buildTargets;
 exports.activate = activate;
 exports.listPlugins = listPlugins;
 exports.listEvents = listEvents;
diff --git a/src/meta/aliases.js b/src/meta/aliases.js
new file mode 100644
index 0000000000..d35df972f1
--- /dev/null
+++ b/src/meta/aliases.js
@@ -0,0 +1,44 @@
+'use strict';
+
+const _ = require('lodash');
+
+const aliases = {
+	'plugin static dirs': ['staticdirs'],
+	'requirejs modules': ['rjs', 'modules'],
+	'client js bundle': ['clientjs', 'clientscript', 'clientscripts'],
+	'admin js bundle': ['adminjs', 'adminscript', 'adminscripts'],
+	javascript: ['js'],
+	'client side styles': [
+		'clientcss', 'clientless', 'clientstyles', 'clientstyle',
+	],
+	'admin control panel styles': [
+		'admincss', 'adminless', 'adminstyles', 'adminstyle', 'acpcss', 'acpless', 'acpstyles', 'acpstyle',
+	],
+	styles: ['css', 'less', 'style'],
+	templates: ['tpl'],
+	languages: ['lang', 'i18n'],
+};
+
+exports.aliases = aliases;
+
+function buildTargets() {
+	var length = 0;
+	var output = Object.keys(aliases).map(function (name) {
+		var arr = aliases[name];
+		if (name.length > length) {
+			length = name.length;
+		}
+
+		return [name, arr.join(', ')];
+	}).map(function (tuple) {
+		return '     ' + _.padEnd('"' + tuple[0] + '"', length + 2).magenta + '  |  ' + tuple[1];
+	}).join('\n');
+	console.log(
+		'\n\n  Build targets:\n' +
+		('\n     ' + _.padEnd('Target', length + 2) + '  |  Aliases').green +
+		'\n     ------------------------------------------------------\n'.blue +
+		output + '\n'
+	);
+}
+
+exports.buildTargets = buildTargets;
diff --git a/src/meta/build.js b/src/meta/build.js
index 9c63902d3a..dbf69296e1 100644
--- a/src/meta/build.js
+++ b/src/meta/build.js
@@ -8,6 +8,7 @@ const path = require('path');
 const mkdirp = require('mkdirp');
 
 const cacheBuster = require('./cacheBuster');
+const { aliases } = require('./aliases');
 let meta;
 
 const targetHandlers = {
@@ -47,26 +48,7 @@ const targetHandlers = {
 	},
 };
 
-let aliases = {
-	'plugin static dirs': ['staticdirs'],
-	'requirejs modules': ['rjs', 'modules'],
-	'client js bundle': ['clientjs', 'clientscript', 'clientscripts'],
-	'admin js bundle': ['adminjs', 'adminscript', 'adminscripts'],
-	javascript: ['js'],
-	'client side styles': [
-		'clientcss', 'clientless', 'clientstyles', 'clientstyle',
-	],
-	'admin control panel styles': [
-		'admincss', 'adminless', 'adminstyles', 'adminstyle', 'acpcss', 'acpless', 'acpstyles', 'acpstyle',
-	],
-	styles: ['css', 'less', 'style'],
-	templates: ['tpl'],
-	languages: ['lang', 'i18n'],
-};
-
-exports.aliases = aliases;
-
-aliases = Object.keys(aliases).reduce(function (prev, key) {
+const aliasMap = Object.keys(aliases).reduce(function (prev, key) {
 	var arr = aliases[key];
 	arr.forEach(function (alias) {
 		prev[alias] = key;
@@ -151,7 +133,7 @@ exports.build = async function (targets, options) {
 		// get full target name
 		.map(function (target) {
 			target = target.toLowerCase().replace(/-/g, '');
-			if (!aliases[target]) {
+			if (!aliasMap[target]) {
 				winston.warn('[build] Unknown target: ' + target);
 				if (target.includes(',')) {
 					winston.warn('[build] Are you specifying multiple targets? Separate them with spaces:');
@@ -161,7 +143,7 @@ exports.build = async function (targets, options) {
 				return false;
 			}
 
-			return aliases[target];
+			return aliasMap[target];
 		})
 		// filter nonexistent targets
 		.filter(Boolean);