-
Notifications
You must be signed in to change notification settings - Fork 988
Update inquirer library #8411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Update inquirer library #8411
Conversation
7fcf8c0
to
63cdbc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a relatively detailed pass through this + played around with it locally, and I think it looks good. Some nits, but LGTM.
Thanks for taking the time to drive this significant refactor!
/** | ||
* Options for the checkbox function. | ||
* | ||
* Epxorted because Inquirer does not export its own input configs anymore. Some unused |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Epxorted because Inquirer does not export its own input configs anymore. Some unused | |
* Ecported because Inquirer does not export its own input configs anymore. Some unused |
force?: boolean; | ||
nonInteractive?: boolean; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/** | |
* Exported for testing only | |
*/ |
opts = { message: opts }; | ||
} else { | ||
if (opts.force) { | ||
// TODO: Should we print what we've forced? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question - I like the idea (especially since it gives a 'paper trail' when running noninteractive commands). We probably need to audit the places we use this tho to make sure it doesn't lead to too many redundant logs
/** | ||
* Options for the checkbox function. | ||
* | ||
* Epxorted because Inquirer does not export its own input configs anymore. Some unused |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Epxorted because Inquirer does not export its own input configs anymore. Some unused | |
* Ecported because Inquirer does not export its own input configs anymore. Some unused |
* Confirm if the user wants to continue | ||
* Options for the number function. | ||
* | ||
* Epxorted because Inquirer does not export its own input configs anymore. Some unused |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Epxorted because Inquirer does not export its own input configs anymore. Some unused | |
* Exported because Inquirer does not export its own input configs anymore. Some unused |
* Prompts for a directory name, and reprompts if that path does not exist | ||
* Options for the checkbox function. | ||
* | ||
* Epxorted because Inquirer does not export its own input configs anymore. Some unused |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Epxorted because Inquirer does not export its own input configs anymore. Some unused | |
* Exported because Inquirer does not export its own input configs anymore. Some unused |
The new inquirer library is said to have many fixes and performance improvements. Upgrade our use. While I intended to only upgrade a bit at a time, I kept pulling at the string trying to fix the graphical glitches and was never successful. Will continue trying to fix glitches in a dependent PR
Accidentally closed the last PR while trying to fix the conflicts. Recreated.