fixing minor issues with character picking and parsing description

mac- · Dec 16, 2014 · 24b2bc2 · 24b2bc2
1 parent 5743f51
commit 24b2bc2
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 19 deletions.
diff --git a/lib/opter.js b/lib/opter.js
@@ -14,15 +14,20 @@ var commander = require('commander'),
 			charater: 'o',
 			argument: 'value',
 			defaultValue: '',
-			description: 'an option',
 			required: true,
-			type: Boolean
+			schema: {
+				type: 'boolean',
+				description: 'an option'
+			}
 		},
 		options2: {
 			charater: 'O',
 			argument: 'value',
 			defaultValue: 10,
-			description: 'another option'
+			schema: {
+				type: 'integer',
+				description: 'another option'
+			}
 		}
 	}
 */
@@ -44,15 +49,21 @@ module.exports = function (options, appVersion, opterFileLocation) {
 		character,
 		allCharacters = 'abcdefgijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUWXYZ',
 		usedCharacters = { h: true, V: true },
+		isValidCharacter = function(char) {
+			return (allCharacters.indexOf(char) !== -1);
+		},
 		getCharacterForOption = function (optName) {
-			var i, moreChars, ch;
+			var i, moreChars, ch,
+				firstCharLC = optName[0].toLowerCase(),
+				firstCharUC = optName[0].toUpperCase();
+
 			// try the first character in the option name
-			if (!usedCharacters[optName[0]]) {
-				ch = optName[0];
+			if (!usedCharacters[firstCharLC] && isValidCharacter(firstCharLC)) {
+				ch = firstCharLC;
 			}
 			// try the first character in the option name (upper case)
-			else if (!usedCharacters[optName[0].toUpperCase()]) {
-				ch = optName[0].toUpperCase();
+			else if (!usedCharacters[firstCharUC] && isValidCharacter(firstCharUC)) {
+				ch = firstCharUC;
 			}
 			else {
 				// try the any of the upper case letters in the option name
@@ -70,14 +81,17 @@ module.exports = function (options, appVersion, opterFileLocation) {
 					}
 				}
 				if (!ch) {
+					var testCharLC, testCharUC;
 					// try the any character in the option name
 					for (i = 1; i < optName.length; i++) {
-						if (!usedCharacters[optName[i].toLowerCase()]) {
-							ch = optName[i].toLowerCase();
+						testCharLC = optName[i].toLowerCase();
+						testCharUC = optName[i].toUpperCase();
+						if (!usedCharacters[testCharLC] && isValidCharacter(testCharLC)) {
+							ch = testCharLC;
 							break;
 						}
-						else if (!usedCharacters[optName[i].toUpperCase()]) {
-							ch = optName[i].toUpperCase();
+						else if (!usedCharacters[testCharUC] && isValidCharacter(testCharUC)) {
+							ch = testCharUC;
 							break;
 						}
 					}
@@ -115,11 +129,11 @@ module.exports = function (options, appVersion, opterFileLocation) {
 		if (options.hasOwnProperty(optName)) {
 			option = options[optName];
 
-			if (option.hasOwnProperty('schema') && option.schema.type !== 'boolean') {
-				option.argument = option.schema.type || 'string';
+			if (option.hasOwnProperty('schema') && option.schema.type && option.schema.type !== 'boolean') {
+				option.argument = option.schema.type;
 			}
 			requiredText = (option.required) ? '(Required) ' : '(Optional) ';
-			description = option.description || 'No Description.';
+			description = (option.hasOwnProperty('schema') && option.schema.description) ? option.schema.description : 'No Description.';
 			description = requiredText + description;
 			if (option.hasOwnProperty('defaultValue') && option.defaultValue !== null) {
 				description += ' Defaults to: ';

diff --git a/package.json b/package.json
@@ -4,7 +4,7 @@
 	"contributors": [
 		"Mac Angell <mac.ang311@gmail.com>"
 	],
-	"version": "1.0.0",
+	"version": "1.0.1",
 	"dependencies": {
 		"commander": "2.x.x",
 		"underscore": "1.x.x",

diff --git a/test/opter.test.js b/test/opter.test.js
@@ -284,8 +284,10 @@ describe('Opter Unit Tests', function() {
 			myOptionFromDefault: {
 				character: 'd',
 				argument: 'string',
-				description: 'some description.',
-				defaultValue: 'default'
+				defaultValue: 'default',
+				schema: {
+					description: 'some description.'
+				}
 			}
 		}, '0.1.0');
 		var mockedCommander = opter.__get__('commander');
@@ -405,7 +407,9 @@ describe('Opter Unit Tests', function() {
 		var cfg = opter({
 			myOptionFromDefault: {
 				character: 'd',
-				description: 'some description.'
+				schema: {
+					description: 'some description.'
+				}
 			}
 		}, '0.1.0');
 
@@ -487,6 +491,36 @@ describe('Opter Unit Tests', function() {
 		done();
 	});
 
+	it('should filter out invalid characters when attempting to pick one', function(done) {
+
+		setCommandLineArgsAndEnvVars();
+
+		var cfg = opter({
+			a_b: { },
+			a_c: { },
+			a_d: { },
+			'!%@#$thing': { },
+			',./<>': { }
+		}, '0.1.0');
+
+		var mockedCommander = opter.__get__('commander');
+		var expectedFlagsString = '-d, --a_d';
+		var flagsMatch = _.find(mockedCommander.options, function(item) { return (item.flags === expectedFlagsString); });
+		assert(flagsMatch, 'option string should show short option as "d"');
+
+		expectedFlagsString = '-t, --!%@#$thing';
+		flagsMatch = _.find(mockedCommander.options, function(item) { return (item.flags === expectedFlagsString); });
+		assert(flagsMatch, 'option string should show short option as "t"');
+
+		expectedFlagsString = '-b, --,./<>';
+		flagsMatch = _.find(mockedCommander.options, function(item) { return (item.flags === expectedFlagsString); });
+		assert(flagsMatch, 'option string should show short option as "b"');
+
+		resetCommander();
+
+		done();
+	});
+
 	it('should read config from an opter.json file', function(done) {
 
 		// change the location of the "running file"