Updated how the execute functions operate and made string commands better

Updated execute commands to use spawn-command making sure we run the command in the actual shell. This means that we do not need special handling for &&, & and cd anymore.
String commands will now automatically be get access to node_modules/.bin when for the extension in question making it easier to proxy CLI commands from npm.
Added support to pass extra arguments to proxied CLI tools by using --. These arguments will also be available to command functions through the new extraArguments property on the commandObject.
Improved how string commands are managed for better error messages.

Author: dlmr

docs/API.md

@@ -122,18 +122,40 @@ Will deeply merge two objects together and return a new object.
 ## `Execute`
 __These functions should be seen as experimental and might change without a mayor version change to Roc.__
 
-Roc has a simple implementation of execute that makes it easy to invoke string commands as they would have been invoked through a npm script. Does not currently automatically bind `node_modules` to the _PATH_ meaning that correct paths needs to be used. Not a complete mapping to the shell, supports `&`, `&&` for running commands in sequence and `cd`.
+Roc has a simple implementation of execute that makes it easy to invoke string commands as they would have been invoked through a npm script. Supports binding of  `node_modules` to the _$PATH_ using the options.
+
+__`options`__  
+All of the variants of execute takes in the same option object as the second argument.
+
+```js
+{
+  args: [], // Additional arguments as an array that should be used with the command.
+  context: '/some/path', // A path that where a lookup will be done for node_modules/.bin and if found it will be added to the $PATH
+  cwd: 'some/path/', // The directory the command should be invoked inside
+  silent: true // A boolean that will enable and disable output from the command
+}
+```
+
+__`ExecuteError`__  
+If an error happens in one of the execute functions an `ExecuteError` will be thrown, or in the case of `execute` the promise will be rejected with the error.
+
+It extends the normal `Error` with the following methods.
+
+```
+error.getCommand() - The command that was used that caused the problem
+error.getExitCode() - The exit code from the process
+error.getStderr() - The result from stderr
+error.getStdout() - The result from stdout
+```
 
 ### `execute`
 ```javascript
 import { execute } from 'roc';
 
-execute('git log')
-  .then(() => {
+execute('git log', options).then(() => {
     // Completed
   })
-  .catch((exitStatus) => {
-	  // A problem happened, status from the process available
+  .catch((executeError) => {
   });
 ```
 Runs a string in the shell asynchronous and returns a promise that resolves when the command is completed or when a error happened.
@@ -142,36 +164,20 @@ Runs a string in the shell asynchronous and returns a promise that resolves when
 ```javascript
 import { executeSync } from 'roc';
 
-// Will no print anything to console, result available in return value
-const output = executeSync('git log', true);
-
-// Will print to console
-executeSync('git log');
+const output = executeSync('git log', options);
 ```
-Runs a string in the shell synchronous. If running in silent mode, the second argument set to true, the function will return the output.
+Runs a string in the shell synchronously.
 
-The function will throw if an error happens. The error object will be extended with the following extra properties.
-
-```
-error.command - The command that was used that caused the problem
-error.arguments - The arguments that where used that caused the problem
-error.exitStatus - The status from the process
-error.stderr - The result from stderr
-error.stdout - The result from stdout
-```
+The function will throw if an `ExecuteError` if an error happens.
 
 ### `executeSyncExit`
 ```javascript
 import { executeSyncExit } from 'roc';
 
-// Will no print anything to console, result available in return value
-const output = executeSyncExit('git log', true);
-
-// Will print to console
-executeSyncExit('git log');
+const output = executeSyncExit('git log', options);
 ```
 
-A wrapper around `executeSync` with the difference that it will terminate the process if an error happens with the same status.
+A wrapper around `executeSync` with the difference that it will terminate the process if an error happens using the same exit code.
 
 ## Configuration
 

docs/Commands.md

@@ -20,10 +20,14 @@ __Important!__
 The property name `command` is reserved and should not be used as the name for a command group or command.
 
 ### String command
-A string command is a string that will managed as if it was typed into the terminal directly. Great for creating aliases to other commands. Does not work exactly as one would run things in a terminal but `&` and `&&` can be used to chain commands together. This feature is experimental in it’s current state and will be improved going forward.
+A string command is a string that will managed as if it was typed into the terminal directly. Great for creating aliases to other commands. Uses [`spawn-command`](https://github.com/mmalecki/spawn-command) internally meaning that the command will be launched in either `cmd.exe` or `/bin/sh`. By default will the `node_modules/.bin` for a extension be added making it easy to proxy CLI tools to Roc projects.
+
+This feature is experimental in it’s current state and will be improved going forward.
 
 String commands uses [execute](/docs/API.md#execute) internally.
 
+Tip: You can provide additional arguments to a command in Roc that is using a string command by using `--`.
+
 ### Function command
 The function will be invoked with an object called `commandObject`.
 
@@ -52,7 +56,8 @@ __Object structure__
   options: {
     managed: {}
     unmanaged: {}
-  }
+  },
+  extraArguments: []
 }
 ```
 
@@ -82,6 +87,9 @@ An object with options matching the defined options from the commands meta infor
 #### `unmanaged`
 An object with options that was not managed by Roc.
 
+### `extraArguments`
+An array with the arguments that are defined after `--` on the command line.
+
 ## Define new commands & groups
 To define new commands that extension will need to either use the [`command`](/docs/RocObject.md#commands)  property on the [Roc object](/docs/RocObject.md) or return a command property form the [`init`](/docs/RocObject.md#init) function.
 

docs/Context.md

@@ -65,7 +65,13 @@ actions             Action objects.
 ### `commands`
 An object with the merged commands.
 
-The structure of the object is the same as the one used in the [Roc object](#commands) with the exception for `__extensions` that Roc will add when building the context to all the groups and commands. This property is an array with strings that list all of the extensions that have modified it in some way. This is used in the core together with `override` to make sure that extensions knowingly override groups and commands defined by other extensions.
+The structure of the object is the same as the one used in the [Roc object](#commands) with the exception for `__extensions` and `__context` that Roc will add when building the context to the groups and the commands.
+
+__`__extensions`__  
+This property is an array with strings that list all of the extensions that have modified it in some way. This is used in the core together with `override` to make sure that extensions knowingly override groups and commands defined by other extensions.
+
+__`__context`__  
+This is the path to the extension that registered the command. This is used internally for giving access to `node_modules/.bin` when invoking a string command.
 
 ### `config`
 An object containing the final configuration. This means that the project configuration will have been merged with the configuration from the packages as well as the settings that was defined in the cli at runtime and the `__raw` values added to their properties in `settings`.

package.json

@@ -81,6 +81,7 @@
     "mocha": "~3.0.2",
     "npm-check-updates": "~2.5.6",
     "nyc": "7.1.0",
+    "proxyquire": "1.7.10",
     "rimraf": "~2.5.0"
   },
   "dependencies": {
@@ -96,13 +97,16 @@
     "leven": "~2.0.0",
     "lodash": "~4.13.1",
     "loud-rejection": "~1.3.0",
+    "manage-path": "2.0.0",
     "merge-options": "0.0.64",
     "minimist": "~1.2.0",
     "redent": "~1.0.0",
     "replace": "~0.3.0",
     "resolve": "~1.1.6",
     "semver": "5.1.0",
+    "shell-escape": "0.2.0",
     "source-map-support": "~0.4.0",
+    "spawn-command": "0.0.2-1",
     "strip-ansi": "~3.0.0",
     "tar": "~2.2.1",
     "temp": "~0.8.3",

src/cli/runCli.js

@@ -10,7 +10,7 @@ import execute from '../execute';
 import getAbsolutePath from '../helpers/getAbsolutePath';
 import getSuggestions from '../helpers/getSuggestions';
 import initContext from '../context/initContext';
-import log from '../log/default/large';
+import log from '../log/default';
 import merge from '../helpers/merge';
 import runHook from '../hooks/runHook';
 import validateSettingsWrapper from '../validation/validateSettingsWrapper';
@@ -36,8 +36,8 @@ export default function runCli({
 }) {
     const {
         _, h, help, V, verbose, v, version, c, config, d, directory, b, 'better-feedback': betterFeedback,
-        ...restOptions,
-    } = minimist(argv.slice(2));
+        '--': extraArguments, ...restOptions,
+    } = minimist(argv.slice(2), { '--': true });
 
     // The first should be our command if there is one
     const [groupOrCommand, ...args] = _;
@@ -110,7 +110,7 @@ export default function runCli({
     }
 
     if (!commands[command]) {
-        log.error(
+        log.large.error(
             getSuggestions([command], suggestions),
             'Invalid command'
         );
@@ -164,15 +164,22 @@ export default function runCli({
     if (invoke) {
         // If string run as shell command
         if (isString(commands[command].command)) {
-            return execute(commands[command].command)
-                .catch(process.exit);
+            return execute(commands[command].command, {
+                context: commands[command].__context,
+                args: extraArguments,
+                cwd: directory,
+            }).catch((error) => {
+                process.exitCode = error.getCode ? error.getCode() : 1;
+                log.small.error('An error happened when running the Roc command', error);
+            });
         }
 
         // Run the command
         return commands[command].command({
             info,
             arguments: parsedArguments,
             options: parsedOptions,
+            extraArguments,
 
             // Roc Context
             context,

src/context/extensions/helpers/processCommands.js

@@ -8,17 +8,17 @@ import merge from '../../../helpers/merge';
 import buildList from './buildList';
 
 // Updates the command object and validates it
-export default function processCommands(name, extensionCommands, stateCommands) {
+export default function processCommands(name, path, extensionCommands, stateCommands) {
     return validateCommands(
         name,
-        normalizeCommands(name, extensionCommands, stateCommands),
+        normalizeCommands(name, path, extensionCommands, stateCommands),
         stateCommands,
         true
     );
 }
 
 // Updated the command object
-export function normalizeCommands(name, extensionCommands, stateCommands = {}) {
+export function normalizeCommands(name, path, extensionCommands, stateCommands = {}) {
     const normalizeCommandsHelper = (newCommands, existingCommands = {}, oldPath = '') => {
         const localCommands = { ...newCommands };
         Object.keys(localCommands).forEach((command) => {
@@ -30,14 +30,19 @@ export function normalizeCommands(name, extensionCommands, stateCommands = {}) {
                     localCommands[command] = {
                         command: localCommands[command],
                         __extensions: [name],
+                        __context: path,
                     };
+                } else if (localCommands[command].command) {
+                    // If the command has been changed we would like to update the context for it
+                    localCommands[command].__context = path;
                 }
 
                 localCommands[command].__extensions = union(existingExtensions, [name]);
 
                 // If it was a command group and now is a command
                 if (isCommandGroup(existingCommands)(command) && isCommand(localCommands)(command)) {
                     localCommands[command].__extensions = [name];
+                    localCommands[command].__context = path;
                 }
             } else if (isCommand(localCommands)(command)) {
                 if (!isPlainObject(localCommands[command])) {
@@ -47,6 +52,7 @@ export function normalizeCommands(name, extensionCommands, stateCommands = {}) {
                 }
 
                 localCommands[command].__extensions = [name];
+                localCommands[command].__context = path;
             }
 
             if (

src/context/extensions/helpers/processRocObject.js

@@ -96,7 +96,7 @@ export default function processRocObject(
         if (roc.commands) {
             state.context.commands = merge(
                 state.context.commands,
-                processCommands(roc.name, roc.commands, state.context.commands)
+                processCommands(roc.name, roc.path, roc.commands, state.context.commands)
             );
         }
 

src/context/helpers/getDefaults.js

@@ -16,7 +16,7 @@ export default function getDefaults(context, name = 'roc', directory) {
         commands: getDefaultCommands(directory) || {},
     });
 
-    newContext.commands = normalizeCommands(name, newContext.commands);
+    newContext.commands = normalizeCommands(name, null, newContext.commands);
 
     newContext.hooks = registerHooks(getDefaultHooks(), 'roc', newContext.hooks);
 

src/execute/executeSync.js

@@ -1,63 +1,24 @@
-import { join } from 'path';
-
-import { sync } from 'cross-spawn';
-
-import getParts from './helpers/getParts';
-
-/**
- * Executes a command string in a synchronous manner.
- *
- * Quite simple in its current state and should be expected to change in the future.
- * Can manage multiple commands if they are divided by either & or &&. Important that there is spacing on both sides.
- *
- * @param {string} command - A command string that should run.
- * @param {boolean} [silent=false] - If the command should run in silent mode, no output.
- *
- * @returns {string[]} - The output to stdout if silent was used.
- */
-export default function executeSync(command, silent = false) {
-    // Will run them in parallel anyway, nothing we can do about it currently
-    const parallelCommands = command.split(/ & /);
-    return parallelCommands.map((syncCommand) => {
-        const syncCommands = syncCommand.split(/ && /);
-        return runCommandSync(syncCommands, silent);
+import spawnCommandSync from './helpers/spawnCommandSync';
+import getEnv from './helpers/getEnv';
+import getCommand from './helpers/getCommand';
+import ExecuteError from './helpers/ExecuteError';
+
+export default function executeSync(command, { context, cwd, args, silent } = {}) {
+    const cmd = getCommand(command, args);
+    const { status, stdout, stderr } = spawnCommandSync(cmd, {
+        stdio: silent ? undefined : 'inherit',
+        env: getEnv(context),
+        cwd,
     });
-}
-
-function runCommandSync(syncCommands, silent, path = process.cwd(), results = []) {
-    const command = syncCommands.shift();
-
-    if (command) {
-        const parts = getParts(command);
-        const cmd = parts[0];
-        const args = parts.slice(1);
-
-        // If the command is to change directory we will carry the path over to the next command.
-        if (cmd === 'cd') {
-            // If the path is absolute, starts with a /, we will not join in with the previous
-            const newPath = args[0].charAt(0) === '/' ?
-                args[0] : join(path, args[0]);
-            return runCommandSync(syncCommands, silent, newPath, results);
-        }
-
-        const { status, stdout, stderr } = sync(cmd, args, { cwd: path, stdio: silent ? undefined : 'inherit' });
-
-        const newResults = [...results, stdout && stdout.toString()];
-
-        if (status) {
-            const error = new Error(`The following command returned exit status [${status}]: ${cmd} ${args.join(' ')}`);
-
-            error.command = cmd;
-            error.arguments = args;
-            error.exitStatus = status;
-            error.stderr = stderr && stderr.toString();
-            error.stdout = newResults;
-
-            throw error;
-        }
 
-        return runCommandSync(syncCommands, silent, path, newResults);
+    if (status) {
+        throw new ExecuteError(`The command "${cmd}" failed with error code ${status}`,
+            cmd,
+            status,
+            stderr && stderr.toString(),
+            stdout && stdout.toString()
+        );
     }
 
-    return results;
+    return stdout && stdout.toString();
 }

src/execute/executeSyncExit.js

@@ -5,21 +5,16 @@ import executeSync from './executeSync';
  * without throwing an exception.
  *
  * This can be useful for when the command that is running is handling the error output itself.
- *
- * @param {string} command - A command string that should run.
- * @param {boolean} [silent=false] - If the command should run in silent mode, no output.
- *
- * @returns {string[]} - The output to stdout if silent was used.
  */
-export default function executeSyncExit(command, silent = false) {
+export default function executeSyncExit(command, options) {
     try {
-        return executeSync(command, silent);
-    } catch (err) {
-        // Only process if we got the error from below that sets the exitStatus.
-        if (!err.exitStatus) {
-            throw err;
+        return executeSync(command, options);
+    } catch (error) {
+        // Only process if we got an error that have getCode
+        if (!error.getExitCode) {
+            throw error;
         }
 
-        return process.exit(err.exitStatus); // eslint-disable-line
+        return process.exit(error.getExitCode()); // eslint-disable-line
     }
 }