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

Handle questions as a plain object {[name]: question} #885

Merged
merged 2 commits into from May 21, 2021

Conversation

caub
Copy link
Contributor

@caub caub commented Jan 22, 2020

I think it's worth to support a questions object, like this:

const { foo, bar } = await inquirer.prompt({
  foo: {
    message: '...',
    default: '...',
  },
  bar: {
    default: '...',
  }
}):

it looks better imo, { foo, bar } visually matches the corresponding keys in the question object

and object literals keys are ordered (as long as they're not numeric)

This PR detects such an object, safely normally, since a question should contain a name prop, there can't be a conflict with a single question object

@caub caub force-pushed the patch-1 branch 2 times, most recently from bf48047 to bf365e1 Compare January 22, 2020 20:07
@caub
Copy link
Contributor Author

caub commented Mar 2, 2020

@SBoudrias Do you think we could do that?

@codinghusi
Copy link

Hey, will this be merged one day?

@caub
Copy link
Contributor Author

caub commented Mar 12, 2021

@codinghusi I gave up, using http://npm.im/prompt instead

@codinghusi
Copy link

Oh nice, there's another package for that :)
(But doesn't have that nice highlighting and stuff 🙃 )
Btw. I'm currently working on my own wrapper for inquirer.js where this feature here will be also included.

May I ask, what kind of problem you had with merging? As I could see, it had only problems with code formatting guidelines.

@caub caub force-pushed the patch-1 branch 2 times, most recently from 0bb05b2 to 5e79408 Compare March 13, 2021 10:13
This is a breaking change, but I believe it's worth, it's easier to wrap a single question in []

and

```js
const { foo, bar } = await inquirer.prompt({
  foo: {
    message: '...',
    default: '...',
  },
  bar: {
    default: '...',
  }
}):
```
looks better imo

and object literals keys are ordered (as long as they're not numeric)
@codecov
Copy link

codecov bot commented Mar 13, 2021

Codecov Report

Merging #885 (fdfd9af) into master (6bc00c0) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #885      +/-   ##
==========================================
+ Coverage   93.30%   93.32%   +0.01%     
==========================================
  Files          27       27              
  Lines        1121     1123       +2     
  Branches       23       23              
==========================================
+ Hits         1046     1048       +2     
  Misses         75       75              
Impacted Files Coverage Δ
packages/inquirer/lib/ui/prompt.js 100.00% <100.00%> (ø)

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 6bc00c0...fdfd9af. Read the comment docs.

@caub
Copy link
Contributor Author

caub commented Mar 13, 2021

We use prompt package because it's light, support json-schema and we don't need fancy highlighting

Ok I fixed the eslint issues, I don't have more time to spend on this sorry, it'd probably need tests

@caub caub force-pushed the patch-1 branch 2 times, most recently from 8874fc1 to 059bc6d Compare March 13, 2021 10:40
@SBoudrias
Copy link
Owner

Thanks for the PR. I think that's a good idea to reduce boilerplate a little.

@SBoudrias SBoudrias merged commit 06203f8 into SBoudrias:master May 21, 2021
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.

None yet

3 participants