feat: Prompt Bypass supports Object choice values #209
Conversation
tests/prompt-bypass-list.ava.js
Outdated
| // 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); | ||
| }); |
There was a problem hiding this comment.
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
src/prompt-bypass.js
Outdated
| function choiceMatchesValue (choice, choiceIdx, value) { | ||
| return [ | ||
| choice, | ||
| choice.value, | ||
| choice.key, | ||
| choice.name, | ||
| choiceIdx.toString() | ||
| ].some(choiceValue => checkChoiceValue(choiceValue, value)); | ||
| } |
There was a problem hiding this comment.
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?
| 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) | |
| } |
|
Hey hey @knikolov-nuvolo! Thanks a ton for submitting a PR! 😊 Left a few thoughts - once addressed we can merge! |
|
Thanks for the feedback. I made the requested changes :) |
There was a problem hiding this comment.
Hey hey! Super super sorry for not communicating more.
Past few days have been JAMMED packed. This is awesome and ready-to-go (I actually read thru code on my phone the other night).
However, I don't yet have time (and won't for a few days, still) to merge in the PR and release 😅 I'll try my best to tackle this no later than Friday.
Please feel free to ping me if I miss this deadline. That all said, I have this email marked as unread (which I notoriously never have unread emails so I can bug myself with stuff like this) and is at the top of my TODO list that I look at often. :)
|
No worries, and thanks for the feedback :) As I mentioned in the issue, we have a temp solution for now. I'll be on vacation for most of this week, so whenever you find time. |
|
Pushed as-of |
Closes plop/issues/284
Check for choice value type, before comparing it with the bypass value.
When the value is not a string, check for key, name, and index (as usual).
Add unit tests.