Skip to content
This repository has been archived by the owner on Apr 26, 2022. It is now read-only.

feat: Prompt Bypass supports Object choice values #209

Merged
merged 2 commits into from
Oct 18, 2021
Merged

feat: Prompt Bypass supports Object choice values #209

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06b1c79
feat: Prompt Bypass supports Object choice values
knikolov-nuvolo Oct 6, 2021
f41e798
simplify code and tests as per PR comments
knikolov-nuvolo Oct 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/prompt-bypass.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,23 @@ const getChoiceValue = (choice) => {
return choice;
};

// check if the choice value matches the bypass value
function checkChoiceValue(choiceValue, value) {
return typeof choiceValue === 'string'
&& choiceValue.toLowerCase() === value.toLowerCase();
}

// check if a bypass value matches some aspect of
// a particular choice option (index, value, key, etc)
const choiceMatchesValue = (choice, choiceIdx, value) => {
const choiceValue = getChoiceValue(choice);

const valueMatchesChoice = choiceValue && choiceValue.toLowerCase() === value.toLowerCase();
const valueMatchesChoiceKey = typeof choice.key === 'string' && choice.key.toLowerCase() === value.toLowerCase();
const valueMatchesChoiceName = typeof choice.name === 'string' && choice.name.toLowerCase() === value.toLowerCase();
const valueMatchesChoiceIndex = choiceIdx.toString() === value;

return (
valueMatchesChoice
|| valueMatchesChoiceKey
|| valueMatchesChoiceName
|| valueMatchesChoiceIndex
);
};
function choiceMatchesValue (choice, choiceIdx, value) {
return [
choice,
choice.value,
choice.key,
choice.name,
choiceIdx.toString()
].some(choiceValue => checkChoiceValue(choiceValue, value));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally find this logic a bit hard to parse in terms of early-return (despite understanding some usage).

I think I'd rather move this back to using logical ORs. What do you think of this?

Suggested change
function choiceMatchesValue (choice, choiceIdx, value) {
return [
choice,
choice.value,
choice.key,
choice.name,
choiceIdx.toString()
].some(choiceValue => checkChoiceValue(choiceValue, value));
}
function choiceMatchesValue(choice, choiceIdx, value) {
return checkChoiceValue(choice, value) ||
checkChoiceValue(choice.value, value) ||
checkChoiceValue(choice.key, value) ||
checkChoiceValue(choice.name, value) ||
checkChoiceValue(choiceIdx.toString(), value)
}


// check if a value matches a particular set of flagged input options
const isFlag = (list, v) => list.includes(v.toLowerCase());
Expand Down
76 changes: 40 additions & 36 deletions tests/prompt-bypass-list.ava.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,52 +6,56 @@ const plop = nodePlop();

const prompts = [{
type:'list',
name:'list', message:'listMsg',
name:'list',
message:'listMsg',
choices: [
'eh',
{key: 'b', value:'bee'},
{name: 'c', value: 'see'},
{value: 'd'},
{name: 'e'}
{name: 'e'},
{key: 'f', name: 'ff', value: { prop: 'value'}}
]
}];

test('verify good bypass input', function (t) {
const [, byValue] = promptBypass(prompts, ['eh'], plop);
t.is(byValue.list, 'eh');

const [, byKey] = promptBypass(prompts, ['b'], plop);
t.is(byKey.list, 'bee');

const [, byName] = promptBypass(prompts, ['c'], plop);
t.is(byName.list, 'see');

const [, byValueProp] = promptBypass(prompts, ['d'], plop);
t.is(byValueProp.list, 'd');

const [, byNameNoValue] = promptBypass(prompts, ['e'], plop);
t.is(byNameNoValue.list, 'e');

const [, byIndexValue] = promptBypass(prompts, ['0'], plop);
t.is(byIndexValue.list, 'eh');

const [, byIndexKey] = promptBypass(prompts, ['1'], plop);
t.is(byIndexKey.list, 'bee');

const [, byIndexName] = promptBypass(prompts, ['2'], plop);
t.is(byIndexName.list, 'see');

const [, byIndexValueProp] = promptBypass(prompts, ['3'], plop);
t.is(byIndexValueProp.list, 'd');

const [, byIndexNameNoValue] = promptBypass(prompts, ['4'], plop);
t.is(byIndexNameNoValue.list, 'e');

const [, byIndexNumber] = promptBypass(prompts, [4], plop);
t.is(byIndexNumber.list, 'e');
// answer is string
[
{ bypassValue: 'eh', expectedAnswer: 'eh' }, // by value
{ bypassValue: 'b', expectedAnswer: 'bee' }, // by key
{ bypassValue: 'c', expectedAnswer: 'see' }, // by name
{ bypassValue: 'd', expectedAnswer: 'd' }, // by value prop
{ bypassValue: 'e', expectedAnswer: 'e' }, // by name, no value
{ bypassValue: '0', expectedAnswer: 'eh' }, // by index - value
{ bypassValue: '1', expectedAnswer: 'bee' }, // by index - key
{ bypassValue: '2', expectedAnswer: 'see' }, // by index - name
{ bypassValue: '3', expectedAnswer: 'd' }, // by index - value prop
{ bypassValue: '4', expectedAnswer: 'e' }, // by index - name, no value
{ bypassValue: 4, expectedAnswer: 'e' }, // by index number
].forEach(testCase => {
const [, value] = promptBypass(prompts, [testCase.bypassValue], plop);
t.is(value.list, testCase.expectedAnswer);
});

// answer is object
const objValue = { prop: 'value' };
[
{ bypassValue: 'f', expectedAnswer: objValue }, // by key
{ bypassValue: 'ff', expectedAnswer: objValue }, // by name
{ bypassValue: '5', expectedAnswer: objValue }, // by index
{ bypassValue: 5, expectedAnswer: objValue }, // by index number
].forEach(testCase => {
const [, value] = promptBypass(prompts, [testCase.bypassValue], plop);
t.deepEqual(value.list, testCase.expectedAnswer);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deeply appreciate you having added unit tests for this functionality!

However, can we revert the migration of tests to use forEach (both here and below). While DRY code is useful in application logic, I try my best to discourage adding any un-neccisary level of complexity to unit/integration tests, even if it means copy+pasting large swaths of code.

IMO it makes tests easier to immediately grok and less potential to need to debug the tests later on

});

test('verify bad bypass input', function (t) {
t.throws(() => promptBypass(prompts, ['asdf'], plop));
t.throws(() => promptBypass(prompts, ['5'], plop));
[
'asdf',
'6',
6,
].forEach(bypassValue => {
t.throws(() => promptBypass(prompts, [bypassValue], plop));
});
});