From 65ea3c550bb32ec1f6aab1628dbde426d109c09a Mon Sep 17 00:00:00 2001
From: Chris Kastorff <encryptio@gmail.com>
Date: Fri, 29 Jul 2016 16:18:00 -0500
Subject: [PATCH] CLI parsing rework (#690)

* cli: refactor main.js to send arguments to command modules, not parse them itself

* cli: transition existing commands to new command module form
---
 cli/src/create-cert.js | 14 ++++----
 cli/src/init.js        | 12 +++----
 cli/src/main.js        | 73 ++++++++++++++++++++++--------------------
 cli/src/make-token.js  | 15 ++++++---
 cli/src/schema.js      | 35 +++++++++++---------
 cli/src/serve.js       | 15 ++++++---
 cli/src/version.js     |  6 ----
 7 files changed, 92 insertions(+), 78 deletions(-)

diff --git a/cli/src/create-cert.js b/cli/src/create-cert.js
index 4772dbaf9..258d2fbdd 100644
--- a/cli/src/create-cert.js
+++ b/cli/src/create-cert.js
@@ -1,10 +1,16 @@
 'use strict';
 const hasbin = require('hasbin');
 const spawn = require('child_process').spawn;
+const process = require('process');
 
 const helpText = 'Generate a certificate';
 
-const runCommand = () => {
+const runCommand = (args) => {
+  if (args.length) {
+    console.error("create-cert takes no arguments");
+    process.exit(1);
+  }
+
   // TODO: user configuration?
   const settings = {
     binaryName: 'openssl',
@@ -56,13 +62,7 @@ const runCommand = () => {
   });
 };
 
-const processConfig = () => ({});
-
-const addArguments = () => {};
-
 module.exports = {
-  addArguments,
-  processConfig,
   runCommand,
   helpText,
 };
diff --git a/cli/src/init.js b/cli/src/init.js
index 85625bcc0..d986a9400 100644
--- a/cli/src/init.js
+++ b/cli/src/init.js
@@ -5,6 +5,7 @@
 const fs = require('fs');
 const crypto = require('crypto');
 const process = require('process');
+const argparse = require('argparse');
 const checkProjectName = require('./utils/check-project-name');
 const rethrow = require('./utils/rethrow');
 
@@ -156,13 +157,15 @@ rethinkdb_data
 .hz/secrets.toml
 `
 
-const addArguments = (parser) => {
+function parseArguments(args) {
+  const parser = new argparse.ArgumentParser({prog: 'hz init'});
   parser.addArgument([ 'projectName' ],
     { action: 'store',
       help: 'Name of directory to create. Defaults to current directory',
       nargs: '?',
     }
   );
+  return parser.parseArgs(args);
 };
 
 const fileExists = (pathName) => {
@@ -174,8 +177,6 @@ const fileExists = (pathName) => {
   }
 };
 
-const processConfig = (parsed) => parsed;
-
 function maybeMakeDir(createDir, dirName) {
   if (createDir) {
     try {
@@ -283,7 +284,8 @@ function populateDir(projectName, dirWasPopulated, chdirTo, dirName) {
   }
 }
 
-const runCommand = (parsed) => {
+function runCommand(args) {
+  const parsed = parseArguments(args);
   const check = checkProjectName(
     parsed.projectName,
     process.cwd(),
@@ -303,8 +305,6 @@ const runCommand = (parsed) => {
 
 
 module.exports = {
-  addArguments,
   runCommand,
-  processConfig,
   helpText,
 };
diff --git a/cli/src/main.js b/cli/src/main.js
index 258a7e655..2fde67c22 100755
--- a/cli/src/main.js
+++ b/cli/src/main.js
@@ -4,8 +4,9 @@
 // To support `pidof horizon`, by default it shows in `pidof node`
 process.title = 'horizon';
 
-const argparse = require('argparse');
 const chalk = require('chalk');
+const path = require('path');
+
 const initCommand = require('./init');
 const serveCommand = require('./serve');
 const versionCommand = require('./version');
@@ -16,9 +17,7 @@ const makeTokenCommand = require('./make-token');
 // Mapping from command line strings to modules. To add a new command,
 // add an entry in this object, and create a module with the following
 // exported:
-// - processConfig: merge parsed command line options with config
 // - runCommand: main function for the command
-// - addArguments: receives a parser and adds any options it needs
 // - helpText: a string to display in the hz help text
 const commands = {
   init: initCommand,
@@ -29,42 +28,48 @@ const commands = {
   schema: schemaCommand,
 };
 
-function parseArgs() {
-  const parser = new argparse.ArgumentParser();
-  const subparsers = parser.addSubparsers({
-    title: 'commands',
-    dest: 'command_name',
-  });
+const programName = path.basename(process.argv[1]);
 
-  Object.keys(commands).forEach((commandName) => {
-    const command = commands[commandName];
-    const subparser = subparsers.addParser(commandName, {
-      addHelp: true,
-      help: command.helpText,
-    });
-    command.addArguments(subparser);
+function help() {
+  console.log(`Usage: ${programName} subcommand [args...]`);
+  console.log(`Available subcommands:`);
+  Object.keys(commands).forEach(function (cmdName) {
+    console.log(`  ${cmdName} - ${commands[cmdName].helpText}`);
   });
+}
 
-  return parser.parseArgs();
+const allArgs = process.argv.slice(2);
+if (allArgs.length == 0) {
+  help();
+  process.exit(1);
 }
 
-function runCommand(command, parsedOptions) {
-  const options = command.processConfig(parsedOptions);
-  const done = (err) => {
-    if (err) {
-      console.log(chalk.red.bold(
-        `${parsedOptions.command_name} failed ` +
-          `with ${options.debug ? err.stack : err}`));
-      process.exit(1);
-    }
-  };
-  try {
-    command.runCommand(options, done);
-  } catch (e) {
-    done(e);
-  }
+const cmdName = allArgs[0];
+const cmdArgs = allArgs.slice(1);
+
+if (cmdName == "-h" || cmdName == "--help" || cmdName == "help") {
+  help();
+  process.exit(0);
 }
 
-const parsed = parseArgs();
+var command = commands[cmdName];
+if (!command) {
+  console.log(chalk.red.bold(
+    `No such subcommand ${cmdName}, run with -h for help`));
+  process.exit(1);
+}
 
-runCommand(commands[parsed.command_name], parsed);
+function done(err) {
+  if (err) {
+    console.log(chalk.red.bold(
+      `${cmdName} failed ` +
+      `with ${err.stack}`));
+    process.exit(1);
+  }
+};
+
+try {
+  command.runCommand(cmdArgs, done);
+} catch (e) {
+  done(e);
+}
diff --git a/cli/src/make-token.js b/cli/src/make-token.js
index 4026027d5..74e22f356 100644
--- a/cli/src/make-token.js
+++ b/cli/src/make-token.js
@@ -10,10 +10,13 @@ const jwt = require('jsonwebtoken');
 
 const r = horizon_server.r;
 const logger = horizon_server.logger;
+const argparse = require('argparse');
 
 const helpText = 'Generate a token to log in as a user';
 
-const addArguments = (parser) => {
+function parseArguments(args) {
+  const parser = new argparse.ArgumentParser({prog: "hz make-token"});
+
   parser.addArgument([ 'project_path' ],
     { type: 'string', nargs: '?',
       help: 'Change to this directory before serving' });
@@ -45,7 +48,9 @@ const addArguments = (parser) => {
   parser.addArgument([ 'user' ],
     { type: 'string', metavar: 'USER_ID',
       help: 'The ID of the user to issue a token for.' });
-};
+
+  return parser.parseArgs(args);
+}
 
 const processConfig = (parsed) => {
   let config;
@@ -65,7 +70,9 @@ const processConfig = (parsed) => {
   return Object.assign(config, { user: parsed.user });
 };
 
-const runCommand = (options, done) => {
+const runCommand = (args, done) => {
+  const options = processConfig(parseArguments(args));
+
   const db = options.project_name;
   let conn;
 
@@ -116,8 +123,6 @@ const runCommand = (options, done) => {
 };
 
 module.exports = {
-  addArguments,
-  processConfig,
   runCommand,
   helpText,
 };
diff --git a/cli/src/schema.js b/cli/src/schema.js
index da67d396e..71f58e473 100644
--- a/cli/src/schema.js
+++ b/cli/src/schema.js
@@ -11,6 +11,7 @@ const Joi = require('joi');
 const parse_yes_no_option = require('./utils/parse_yes_no_option');
 const path = require('path');
 
+const argparse = require('argparse');
 const serve = require('./serve');
 const start_rdb_server = require('./utils/start_rdb_server');
 const toml = require('toml');
@@ -23,7 +24,9 @@ const name_to_info = horizon_index.name_to_info;
 
 const helpText = 'Apply and save the schema from a horizon database';
 
-const addArguments = (parser) => {
+function parseArguments(args) {
+  const parser = new argparse.ArgumentParser({prog: 'hz schema'});
+
   const subparsers = parser.addSubparsers({
     title: 'subcommands',
     dest: 'subcommand_name',
@@ -136,7 +139,9 @@ const addArguments = (parser) => {
     defaultValue: '.hz/schema.toml',
     help: 'File to write the horizon schema to, defaults to .hz/schema.toml.',
   });
-};
+
+  return parser.parseArgs(args);
+}
 
 const schema_schema = Joi.object().unknown(false).keys({
   collections: Joi.object().unknown(true).pattern(/.*/,
@@ -527,22 +532,22 @@ const runSaveCommand = (options, done, shutdown) => {
   });
 };
 
+function processConfig(options) {
+  // Determine if we are saving or applying and use appropriate config processing
+  switch (options.subcommand_name) {
+  case 'apply':
+    return processApplyConfig(options);
+  case 'save':
+    return processSaveConfig(options);
+  default:
+    throw new Error(`Unrecognized schema subcommand: "${options.subcommand_name}"`);
+  }
+}
 
 // Avoiding cyclical depdendencies
 module.exports = {
-  addArguments,
-  processConfig: (options) => {
-    // Determine if we are saving or applying and use appropriate config processing
-    switch (options.subcommand_name) {
-    case 'apply':
-      return processApplyConfig(options);
-    case 'save':
-      return processSaveConfig(options);
-    default:
-      throw new Error(`Unrecognized schema subcommand: "${options.subcommand_name}"`);
-    }
-  },
-  runCommand: (options, done) => {
+  runCommand: (args, done) => {
+    const options = processConfig(parseArguments(args));
     // Determine if we are saving or applying and use appropriate runCommand
     //  Also shutdown = true in this case since we are calling from the CLI.
     switch (options.subcommand_name) {
diff --git a/cli/src/serve.js b/cli/src/serve.js
index 3141f55a6..bd72c32d0 100644
--- a/cli/src/serve.js
+++ b/cli/src/serve.js
@@ -8,6 +8,7 @@ const http = require('http');
 const https = require('https');
 const open = require('open');
 const path = require('path');
+const argparse = require('argparse');
 const toml = require('toml');
 const url = require('url');
 
@@ -31,7 +32,9 @@ const default_rdb_timeout = 20;
 
 const helpText = 'Serve a Horizon app';
 
-const addArguments = (parser) => {
+function parseArguments(args) {
+  const parser = new argparse.ArgumentParser({prog: "hz serve"});
+
   parser.addArgument([ 'project_path' ],
     { type: 'string', nargs: '?',
       help: 'Change to this directory before serving' });
@@ -152,7 +155,9 @@ const addArguments = (parser) => {
     { action: 'storeTrue',
       help: 'Open index.html in the static files folder once Horizon is ready to' +
       ' receive connections' });
-};
+
+  return parser.parseArgs(args);
+}
 
 const make_default_config = () => ({
   config: null,
@@ -674,7 +679,9 @@ const change_to_project_dir = (project_path) => {
 };
 
 // Actually serve based on the already validated options
-const runCommand = (opts, done) => {
+const runCommand = (args, done) => {
+  const opts = processConfig(parseArguments(args));
+
   if (opts.debug) {
     logger.level = 'debug';
   } else {
@@ -765,8 +772,6 @@ const runCommand = (opts, done) => {
   }).catch(done);
 };
 
-serve.addArguments = addArguments;
-serve.processConfig = processConfig;
 serve.runCommand = runCommand;
 serve.helpText = helpText;
 serve.merge_configs = merge_configs;
diff --git a/cli/src/version.js b/cli/src/version.js
index adef38cd1..f6f15d225 100644
--- a/cli/src/version.js
+++ b/cli/src/version.js
@@ -8,13 +8,7 @@ const runCommand = () => {
   console.info(package_json.version);
 };
 
-const processConfig = () => ({});
-
-const addArguments = () => {};
-
 module.exports = {
-  addArguments,
-  processConfig,
   runCommand,
   helpText,
 };