Skip to content
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

Produce error on prompt in non-tty environment. #891

Merged
merged 4 commits into from
Feb 29, 2020

Conversation

jhorbulyk
Copy link
Contributor

Fixes #495

@codecov
Copy link

codecov bot commented Feb 14, 2020

Codecov Report

Merging #891 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #891      +/-   ##
==========================================
+ Coverage   93.23%   93.26%   +0.02%     
==========================================
  Files          26       26              
  Lines        1065     1069       +4     
  Branches       23       23              
==========================================
+ Hits          993      997       +4     
  Misses         72       72              

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a883e6...3e5df0f. Read the comment docs.

packages/inquirer/lib/inquirer.js Outdated Show resolved Hide resolved
@SBoudrias
Copy link
Owner

The rest looks good to me, we should be able to merge and release quickly.

@jhorbulyk
Copy link
Contributor Author

@SBoudrias Updated

@SBoudrias SBoudrias merged commit 8270551 into SBoudrias:master Feb 29, 2020
@SBoudrias
Copy link
Owner

Perfect thanks! I'll make a new release soon.

@PetrSnobelt
Copy link

PetrSnobelt commented Mar 2, 2020

Hi, this PR causes the problem when used with when property, for example:

responses = await inquirer.prompt([{
          name: 'name',
          message: 'give a name to your app',
          default: '@kiwicom/fe-app',
          when: () => !flags.name,
        }])

We provide name in flags on CI, but it break our CI with "Error: Prompts can not be meaningfully rendered in non-TTY environments"

@jhorbulyk
Copy link
Contributor Author

@PetrSnobelt Fixed in #896

@ruyadorno
Copy link
Contributor

Hi @SBoudrias and @jhorbulyk sorry but unfortunately this change introduces a regression since stdin can be configured to something else other than just process.stdin:

var input = opt.input || process.stdin;

it completely broke ipt in your latest patch release since it actually leverages the configurable stdin in order to provide the functionality there 😭

ruyadorno added a commit to ruyadorno/Inquirer.js that referenced this pull request Mar 8, 2020
The implementation which introduces thrown errors when using non-tty
environments didn't took into account the fact that a different input
stream can be provided via config.

This changeset fixes it by introducing the appropriate checks and tests
to ensure no further regressions.

ref: SBoudrias#891
@dan-barrett
Copy link

I'm somewhat confused as to where this issue stands. Looks like it was partly reverted in a future commit?

Can I explicitly configure inquirer to fail in non-tty environments or is that no longer possible? Having hard time finding documentation about it. Thanks!

@jhorbulyk
Copy link
Contributor Author

I'm somewhat confused as to where this issue stands.

My understanding is that after the latest merged PRs, inquirer will fail in non-tty environments but only if:

  • inquirer hasn't been configured to consume something other than stdin
  • when() functions still cause the prompt to be rendered

jdoyle65 pushed a commit to jdoyle65/Inquirer.js that referenced this pull request Jan 19, 2021
* Produce error on prompt in non-tty environment.

* Fix build on node 8.

* Fix line order.

* Use Promise.reject()
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.

prompt() fails silently if stdin isn't a TTY
5 participants