Skip to content

Commit

Permalink
fix(command)!: commands will throw when duplicate short hands are found
Browse files Browse the repository at this point in the history
Previously it was possible to define multiple flags with the same
shorthand which causes indeterminate behavior when using them. Now a
command class will throw an exception when this situation is encountered

fixes: #113
  • Loading branch information
esatterwhite committed Sep 5, 2022
1 parent f19151a commit cdaddbb
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 12 deletions.
2 changes: 1 addition & 1 deletion examples/commands/hello.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module.exports = new cli.Command({

, excited: {
'type': Boolean
, 'shorthand': 'e'
, 'shorthand': 'n'
, 'description': 'Say hello in a very excited manner'
, 'default': false
}
Expand Down
2 changes: 1 addition & 1 deletion examples/commands/password-confirm.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
'use strict'
'puse strict'

const cli = require('seeli')

Expand Down
31 changes: 27 additions & 4 deletions lib/command/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ class Command extends Registry {

this.setOptions(defaults, ...options)

this._shcache = this.shorthands
this.ui = ora({
color: conf.get('color')
, spinner: this.options.ui
Expand Down Expand Up @@ -273,7 +274,17 @@ class Command extends Registry {
this._shcache = Object.create(null)

for (const [key, value] of Object.entries(this.options.flags)) {
if (value.shorthand) this._shcache[value.shorthand] = [`--${key}`]
if (value.shorthand) {
if (hasOwn(this._shcache, value.shorthand)) {
const [previous] = this._shcache[value.shorthand]
throw new exceptions.DuplicateShorthandException(
value.shorthand
, key
, previous.replace('--', '')
)
}
this._shcache[value.shorthand] = [`--${key}`]
}
}

return this._shcache
Expand All @@ -287,7 +298,7 @@ class Command extends Registry {
const error = new exceptions.InvalidFieldException(
`Invalided Field Value for ${chalk.yellow(key)} - ${msg}`
)
this.emit('error', error)
throw error
}

/**
Expand Down Expand Up @@ -472,7 +483,7 @@ class Command extends Registry {
if (stop_flags.has(flag)) continue
const validator = cfg.validate
const value = this.parsed[flag]
const choices = new Set(toArray(cfg.choices))
const choices = getChoices(cfg, this.parsed, value)

this.debug('validating %s', flag)
if (this._required.has(flag) && value === UNDEF) {
Expand Down Expand Up @@ -639,4 +650,16 @@ function toQuestion(flag, opts, answers) {
return arg
}


function getChoices(cfg, answers) {
return new Set(
toArray(cfg.choices)
.filter((choice) => {
if (choice.type === 'separator') return false
return true
})
.map((choice) => {
if (typeOf(choice) === 'object') return choice.value
return choice
})
)
}
39 changes: 38 additions & 1 deletion lib/exceptions.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* jshint laxcomma: true, smarttabs: true, node: true, esnext: true*/
'use strict'
/**
* Default exception types for seeli
Expand Down Expand Up @@ -156,6 +155,43 @@ class UnknownFlagException extends Error {
}
}

/**
* @class module:seeli/lib/exceptions.DuplicateShorthandException
* @param {String} field Name of the erroneous flag
* @extends Error
*/
class DuplicateShorthandException extends Error {
constructor(shorthand, flag, prev) {
super()

/**
* @readonly
* @instance
* @memberof module:seeli/lib/exceptions.DuplicateShorthandException
* @name name
* @property {String} name=DuplicateShorthandException Exception name
**/
this.name = 'DuplicateShorthandException'
/**
* @readonly
* @instance
* @memberof module:seeli/lib/exceptions.DuplicateShorthandException
* @name message
* @property {String} message Message to include in error output
**/
this.message = `shorthand ${shorthand} for flag ${flag} `
+ `- duplicates flag ${prev}`
/**
* @readonly
* @instance
* @memberof module:seeli/lib/exceptions.UnkownFlagException
* @name code
* @property {String} message A Unique error code identifier
**/
this.code = 'ESHORTHAND'
}
}

class CommandException extends Error {
constructor(msg) {
super()
Expand Down Expand Up @@ -218,6 +254,7 @@ class PluginException extends Error {

module.exports = {
RequiredFieldException: RequiredFieldException
, DuplicateShorthandException: DuplicateShorthandException
, InvalidChoiceException: InvalidChoiceException
, InvalidFieldException: InvalidFieldException
, UnknownFlagException: UnknownFlagException
Expand Down
29 changes: 24 additions & 5 deletions test/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const test = require('tap').test
test('command', async (t) => {

// description parsing
t.test('~description', async (tt) => {
test('should accept a single string', async (t) => {
t.test('~description', async (t) => {
t.test('should accept a single string', async (t) => {
const DescriptionCommand = new Command({
description: 'a test command'
})
Expand All @@ -28,6 +28,25 @@ test('command', async (t) => {
})
})

t.test('duplicate shorthands', async (t) => {
t.throws(() => {
return new Command({
flags: {
foo: {
type: String
, description: 'this is foo'
, shorthand: 'f'
}
, bar: {
type: String
, description: 'this is bar'
, shorthand: 'f'
}
}
})
}, {code: 'ESHORTHAND', name: 'DuplicateShorthandException'})
})

t.test('nested flags', async (t) => {
t.plan(3)
const NestedCommand = new Command({
Expand Down Expand Up @@ -738,12 +757,12 @@ test('command', async (t) => {
t.deepEqual(cmd.tree, {
'-': base_flags
, '--': base_flags
, two: {
three: base_flags
, 'two': {
'three': base_flags
, '-': base_flags
, '--': base_flags
}
, one: [...base_flags, '--option', '--single']
, 'one': [...base_flags, '--option', '--single']
}, 'serialized command tree')
})

Expand Down

0 comments on commit cdaddbb

Please sign in to comment.