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

Option name and value are reversed #1

Open
sbj42 opened this issue Aug 29, 2017 · 3 comments
Open

Option name and value are reversed #1

sbj42 opened this issue Aug 29, 2017 · 3 comments
Labels

Comments

@sbj42
Copy link

sbj42 commented Aug 29, 2017

The sample code in README.md shows how the displayed option string and the corresponding answer string can be different. But running the example seems to have the two strings reversed.

For instance:

    {
      key: 'a',
      name: 'Overwrite this one and all next',
      value: 'overwrite_all'
    },

I would expect this to display as Overwrite this one and all next. But here is what I get when I run that sample:

? What action should be taken on file.js?  (yadXh)
  overwrite
  overwrite_all
  diff
 ────────
  abort
  Help
  Answer:
@jonschlinkert
Copy link
Member

Your issues are awesome! thank you. Well written and very clear, which makes it easy to act on and fix the problems described. again, thank you! I'm on it

@sbj42
Copy link
Author

sbj42 commented Aug 29, 2017

No problem. This one I thought I might have miscategorized. I went digging and I think part the issue is actually all the way down in prompt-choices, but I don't understand enough of the layers to trace it properly.

In general it looks like value is initialized from name, but then their usage gets muddled, e.g. in Choice.prototype.render:

Choice.prototype.render = function(idx, options) {
 // ...
    return this.value;

And Choice's line property:

Object.defineProperty(Choice.prototype, 'line', {
  // ...
  set: function(line) {
    if (typeof line === 'string') this.value = line;

And up in prompt-checkbox at Checkbox.prototype.renderAnswer:

Checkbox.prototype.renderAnswer = function() {
  var keys = this.choices.checked.map(function(choice) {
    return typeof choice === 'string' ? choice : choice.value;

I'm sure you've got a better grasp than I do of the implications here, but it does look like this particular one is pretty pervasive.

@jonschlinkert
Copy link
Member

jonschlinkert commented Aug 29, 2017

I already have a fix for this I think. Somewhere along the way I guess I removed support for the .format option, which was passed to each choice to format the line. no, I didn't remove it lol. I just need to fix the logic here. Working on it now.

fwiw, I probably did this because we've been refactoring incrementally to get closer to using a redux-like stateless object for rendering. Soon, when I have enough time, I'll just refactor everything to do that at once. The APIs for Enquirer and prompts shouldn't change much if at all, but it will make it much easier to understand how the prompts work internally, how to customize them, and how to debug for stuff like this.

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

No branches or pull requests

2 participants