Fix bug where "env rm" fails silently if an additional arg is present

Fixes #1380

Argument parsing of "env rm" command was improved by migrating it to oclif

Change-type: patch
Signed-off-by: Lucian <lucian.buzzo@gmail.com>

automation/capitanodoc/capitanodoc.ts

@@ -51,6 +51,7 @@ const capitanoDoc = {
 			files: [
 				'build/actions/environment-variables.js',
 				'build/actions-oclif/env/add.js',
+				'build/actions-oclif/env/rm.js',
 			],
 		},
 		{

doc/cli.markdown

@@ -601,12 +601,12 @@ device uuid
 
 show config variables
 
-## env rm <id>
+## env rm ID
 
-Use this command to remove an environment variable from an application
-or device.
+Remove an environment variable from an application or device, as selected
+by command-line options.
 
-Notice this command asks for confirmation interactively.
+Note that this command asks for confirmation interactively.
 You can avoid this by passing the `--yes` boolean option.
 
 The --device option selects a device instead of an application.
@@ -620,15 +620,21 @@ Examples:
 	$ balena env rm 215 --yes
 	$ balena env rm 215 --device
 
+### Arguments
+
+#### ID
+
+environment variable id
+
 ### Options
 
-#### --yes, -y
+#### -d, --device
 
-confirm non interactively
+Selects a device environment variable instead of an application environment variable
 
-#### --device, -d
+#### -y, --yes
 
-device
+Run in non-interactive mode
 
 ## env add NAME [VALUE]
 

lib/actions-oclif/env/rm.ts

@@ -0,0 +1,111 @@
+/**
+ * @license
+ * Copyright 2019 Balena Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Command, flags } from '@oclif/command';
+import { stripIndent } from 'common-tags';
+
+import { CommandHelp } from '../../utils/oclif-utils';
+
+interface FlagsDef {
+	device: boolean;
+	yes: boolean;
+}
+
+interface ArgsDef {
+	id: number;
+}
+
+export default class EnvRmCmd extends Command {
+	public static description = stripIndent`
+		Remove an environment variable from an application or device.
+
+		Remove an environment variable from an application or device, as selected
+		by command-line options.
+
+		Note that this command asks for confirmation interactively.
+		You can avoid this by passing the \`--yes\` boolean option.
+
+		The --device option selects a device instead of an application.
+
+		Service-specific variables are not currently supported. The following
+		examples remove variables that apply to all services in an app or device.
+`;
+	public static examples = [
+		'$ balena env rm 215',
+		'$ balena env rm 215 --yes',
+		'$ balena env rm 215 --device',
+	];
+
+	public static args = [
+		{
+			name: 'id',
+			required: true,
+			description: 'environment variable id',
+		},
+	];
+
+	// hardcoded 'env add' to avoid oclif's 'env:add' topic syntax
+	public static usage =
+		'env rm ' + new CommandHelp({ args: EnvRmCmd.args }).defaultUsage();
+
+	public static flags: flags.Input<FlagsDef> = {
+		device: flags.boolean({
+			char: 'd',
+			description:
+				'Selects a device environment variable instead of an application environment variable',
+			default: false,
+		}),
+		yes: flags.boolean({
+			char: 'y',
+			description: 'Run in non-interactive mode',
+			default: false,
+		}),
+	};
+
+	public async run() {
+		const { args: params, flags: options } = this.parse<FlagsDef, ArgsDef>(
+			EnvRmCmd,
+		);
+		const balena = (await import('balena-sdk')).fromSharedOptions();
+		const patterns = await import('../../utils/patterns');
+
+		if (isNaN(params.id) || !Number.isInteger(Number(params.id))) {
+			patterns.exitWithExpectedError(
+				'The environment variable id must be an integer',
+			);
+		}
+
+		return patterns
+			.confirm(
+				options.yes || false,
+				'Are you sure you want to delete the environment variable?',
+			)
+			.then(function() {
+				if (options.device) {
+					return balena.pine.delete({
+						resource: 'device_environment_variable',
+						id: params.id,
+					});
+				} else {
+					return balena.pine.delete({
+						resource: 'application_environment_variable',
+						id: params.id,
+					});
+				}
+			});
+	}
+}

lib/actions/environment-variables.ts

@@ -101,67 +101,6 @@ export const list: CommandDefinition<
 	},
 };
 
-export const remove: CommandDefinition<
-	{
-		id: number;
-	},
-	{
-		yes: boolean;
-		device: boolean;
-	}
-> = {
-	signature: 'env rm <id>',
-	description: 'remove an environment variable',
-	help: stripIndent`
-		Use this command to remove an environment variable from an application
-		or device.
-
-		Notice this command asks for confirmation interactively.
-		You can avoid this by passing the \`--yes\` boolean option.
-
-		The --device option selects a device instead of an application.
-
-		Service-specific variables are not currently supported. The following
-		examples remove variables that apply to all services in an app or device.
-
-		Examples:
-
-			$ balena env rm 215
-			$ balena env rm 215 --yes
-			$ balena env rm 215 --device
-	`,
-	options: [commandOptions.yes, commandOptions.booleanDevice],
-	permission: 'user',
-	async action(params, options, done) {
-		const balena = (await import('balena-sdk')).fromSharedOptions();
-		const patterns = await import('../utils/patterns');
-
-		if (typeof params.id !== 'number') {
-			patterns.exitWithExpectedError('The environment variable id must be an integer');
-		}
-
-		return patterns
-			.confirm(
-				options.yes || false,
-				'Are you sure you want to delete the environment variable?',
-			)
-			.then(function() {
-				if (options.device) {
-					return balena.pine.delete({
-						resource: 'device_environment_variable',
-						id: params.id,
-					});
-				} else {
-					return balena.pine.delete({
-						resource: 'application_environment_variable',
-						id: params.id,
-					});
-				}
-			})
-			.nodeify(done);
-	},
-};
-
 export const rename: CommandDefinition<
 	{
 		id: number;

lib/actions/help_ts.ts

@@ -24,8 +24,9 @@ export function getOclifHelpLinePairs(): Array<[string, string]> {
 	// improvement would probably be to automatically scan the actions-oclif
 	// folder.
 	const EnvAddCmd = require('../actions-oclif/env/add').default;
+	const EnvRmCmd = require('../actions-oclif/env/rm').default;
 	const VersionCmd = require('../actions-oclif/version').default;
-	return [EnvAddCmd, VersionCmd].map(getCmdUsageDescriptionLinePair);
+	return [EnvAddCmd, EnvRmCmd, VersionCmd].map(getCmdUsageDescriptionLinePair);
 }
 
 function getCmdUsageDescriptionLinePair(cmd: typeof Command): [string, string] {

lib/app-capitano.coffee

@@ -86,7 +86,6 @@ capitano.command(actions.keys.remove)
 # ---------- Env Module ----------
 capitano.command(actions.env.list)
 capitano.command(actions.env.rename)
-capitano.command(actions.env.remove)
 
 # ---------- Tags Module ----------
 capitano.command(actions.tags.list)

lib/app.ts

@@ -140,6 +140,11 @@ function isOclifCommand(argvSlice: string[]): [boolean, boolean] {
 			if (argvSlice[0] === 'env' && argvSlice[1] === 'add') {
 				return [true, true];
 			}
+
+			// balena env rm
+			if (argvSlice[0] === 'env' && argvSlice[1] === 'rm') {
+				return [true, true];
+			}
 		}
 	}
 	return [false, false];

lib/utils/oclif-utils.ts

@@ -19,6 +19,7 @@ import * as Config from '@oclif/config';
 
 export const convertedCommands = {
 	'env:add': 'env add',
+	'env:rm': 'env rm',
 };
 
 /**