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

Inquirer.js now accepts input/output options #2

Closed
ruyadorno opened this issue Sep 14, 2015 · 5 comments
Closed

Inquirer.js now accepts input/output options #2

ruyadorno opened this issue Sep 14, 2015 · 5 comments

Comments

@ruyadorno
Copy link
Contributor

Hi @knownasilya, while I was playing with Inquirer's new input/output streams options (SBoudrias/Inquirer.js#280) I stumbled upon some problems with cli-width making direct access to the process.stdout stream - As you can see in the file changes from the linked PR, the output stream now can be user defined and not only process.stdout.

Now, I don't want to force any design/api but it would be really nice if the module could accept an object as the function parameter, that let us set the output stream (it can always default to process.stdout) and even defining the defaultWidth value too - I find it quite non-standard the way we define the default value as a property before invoking the function.

Let me know if a PR for these changes is welcome and most importantly, many thanks for all the good work 🏆

@ruyadorno
Copy link
Contributor Author

relates to SBoudrias/Inquirer.js#285

@knownasilya
Copy link
Owner

Sounds like a nice change. Would totally accept a PR. Like you said, it should default to what it is now, even if options aren't specified.

I'm not a fan of the way that the default width is currently set, so I do like your idea of setting it in the options hash (removing the way that the default width is currently set), but that would probably mean that we have to push a major version, and update inquirer to use that version as well.

cliWidth({
  output: customOut,
  defaultWidth: 200,
  tty: require('customtty') // custom tty, not sure if that would be useful.
});

@knownasilya
Copy link
Owner

@ruyadorno are you still planning to submit a PR for this change?

@ruyadorno
Copy link
Contributor Author

yes 😊 I'm quite busy with personal matters for now so feel free to implement it if you have the time - if you don't, I'll be happy to submit the PR once everything settles for me 👍

ruyadorno added a commit to ruyadorno/cli-width that referenced this issue Feb 7, 2016
ruyadorno added a commit to ruyadorno/cli-width that referenced this issue Feb 7, 2016
ruyadorno added a commit to ruyadorno/cli-width that referenced this issue Feb 7, 2016
ruyadorno added a commit to ruyadorno/cli-width that referenced this issue Feb 7, 2016
ruyadorno added a commit to ruyadorno/cli-width that referenced this issue Feb 7, 2016
knownasilya pushed a commit that referenced this issue Feb 7, 2016
@knownasilya
Copy link
Owner

Released as v2.0.0

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

No branches or pull requests

2 participants