Refactor Prompts #43
Merged
Refactor Prompts #43
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
terkelg
reviewed
Mar 4, 2018
|
|
||
| if (!prompts.hasOwnProperty(question.type)) { | ||
| throw new Error(`prompt type ${question.type} not defined`); | ||
| if (prompts[type] === void 0) { |
There was a problem hiding this comment.
Yep and effectively does same thing.
terkelg
reviewed
Mar 4, 2018
| let MAP = prompt._map || {}; | ||
|
|
||
| for (question of questions) { | ||
| name = question.name; | ||
| ({ name, type } = question); |
There was a problem hiding this comment.
Haha, little out of ordinary but hey
|
This is so good and much needed! Nothing is better than removing code |
terkelg
reviewed
Mar 4, 2018
| const onAbort = opts.onAbort || noop; | ||
| const onSubmit = opts.onSubmit || noop; | ||
| p.on('state', args.onState || noop); | ||
| p.on('submit', x => res(onSubmit(x))); |
There was a problem hiding this comment.
Now all prompts also have individual onAbort and onSubmit. That's cool
There was a problem hiding this comment.
Individual prompt functions, yeah. But not the user-defined prompt objects.
tunnckoCore
reviewed
Mar 4, 2018
lib/prompts.js
Outdated
| * @param {Function} [args.suggest] Function to filter results based on user input. Defaults to sort by `title` | ||
| * @param {number} [args.limit=10] Max number of results to show | ||
| * @param {string} [args.style="default"] Render style ('default', 'password', 'invisible') | ||
| * @param {function} [args.onState] On state change callback | ||
| * @returns {Promise} Promise with user input | ||
| */ | ||
| function autocomplete({ message, choices, suggest, limit, style, onState = noop }) { |
There was a problem hiding this comment.
it is still destructed object, but not args as expected below.
There was a problem hiding this comment.
Old commit. Fixed in 6457a15
This was referenced Mar 6, 2018
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were destructuring all prompt-objects, only to reassemble them in nearly the same way. While this made it a little more legible in some ways, it also added a lot of noise.
Looking at the underling
elements, they were also picking out the keys that they cared about, which meant that we could safely pass everything down to them. In most cases, hardly anything from theargsis accessory anyway -- it's pretty much just theonStatekey that gets discarded.Anyway, after clearing up a bunch of the pattern matching, it was pretty clear that our
prompts.jswas doing the same thing for every type, with a tiny exception here n' there.The entire file is now 44 lines of code, plus 102 lines of whitespace and docs 馃槅
Also, I moved the
messagechecking into the main loop. The errors were being swallowed by theonCancelhandler, immediately skipping thepromptinstead. I also converted thethrowfor ensuringchoiceswas an Array into the action of just ensuring it's an Array. It'll be pretty obvious that something's wrong when there are 0 choices 馃槈TLDR:
promptstypes to build from single functionmessageinstead of silent skipchoicesis always an Array instead of silent skip