Skip to content

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Update inquirer library #8411

wants to merge 10 commits into from

Conversation

inlined
Copy link
Member

@inlined inlined commented Apr 8, 2025

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.

@inlined inlined changed the title Inlined.inquirer Update inquirer library Apr 8, 2025
@inlined inlined force-pushed the inlined.inquirer branch from 7fcf8c0 to 63cdbc9 Compare April 8, 2025 05:49
@joehan joehan changed the base branch from next to master April 14, 2025 17:46
Copy link
Contributor

@joehan joehan left a 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 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;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/**
* Exported for testing only
*/

opts = { message: opts };
} else {
if (opts.force) {
// TODO: Should we print what we've forced?
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants