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

fix of #1042 rawlist throw #1045

Merged
merged 1 commit into from
Sep 15, 2021
Merged

fix of #1042 rawlist throw #1045

merged 1 commit into from
Sep 15, 2021

Conversation

SanariSan
Copy link
Contributor

@SanariSan SanariSan commented Sep 14, 2021

Fix of #1042

At the moment of pressing Enter key while Error being shown, index (1) which comes as an argument equals to empty string.
At the same time this.selected equals to undefined (2) here.

  getCurrentValue(index) { // (1)
    if (index == null) {
      index = this.rawDefault;
    } else if (index === '') {
      index = this.selected; // (2)
    } else {
      index -= 1;
    }

    const choice = this.opt.choices.getChoice(index); // (4)
    return choice ? choice.value : null;
  }

This happens because (3) here it gets undefined value every time non-existing index inserted, so at the time Error message shown, this value persists, which leads to assertion throw (4) here -> (5) here.

  onKeypress() {
    const index = this.rl.line.length ? Number(this.rl.line) - 1 : 0;


    if (this.opt.choices.getChoice(index)) {
      this.selected = index;
    } else {
      this.selected = undefined; // (3)
    }
    this.render();
  }
  getChoice(selector) {
    assert(_.isNumber(selector)); // (5)
    return this.realChoices[selector];
  }

To prevent this from happening I decided to re-assign (!) this.selected value if needed by checking with nullish coalescing operator.
This way, if user presses Enter without entering non-existing index beforehand (i.e. sending empty string), he gets the first (default) option selected, as it was before the change.
However, if Error been shown, this.selected will get -1 value, which is number (so assert pass + no negative effect) and allows user to press Enter as much as he would like to (nothing happens, expected behavior; alternative to -1 could be assigning this.rawDefault, then second Enter press just chooses first option available).
At the same time this allows to get back to selection by just pressing arrows or entering number.

getCurrentValue(index) {
    if (index == null) {
      index = this.rawDefault;
    } else if (index === '') {
      this.selected = this.selected ?? -1; // (!)
      index = this.selected;
    } else {
      index -= 1;
    }

    const choice = this.opt.choices.getChoice(index);
    return choice ? choice.value : null;
  }

@SBoudrias SBoudrias merged commit 5d47d7e into SBoudrias:master Sep 15, 2021
@SBoudrias
Copy link
Owner

Thanks!

@hingsir
Copy link

hingsir commented Sep 15, 2021

Node v10 don't support Nullish coalescing operator this.selected = this.selected ?? -1

/usr/local/lib/node_modules/emrnvm/node_modules/inquirer/lib/prompts/rawlist.js:126
      this.selected = this.selected ?? -1;
                                     ^

SyntaxError: Unexpected token ?
    at Module._compile (internal/modules/cjs/loader.js:723:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Function.promptModule.restoreDefaultPrompts (/usr/local/lib/node_modules/emrnvm/node_modules/inquirer/lib/inquirer.js:65:36)
    at Object.inquirer.createPromptModule (/usr/local/lib/node_modules/emrnvm/node_modules/inquirer/lib/inquirer.js:72:16)
    at Object.<anonymous> (/usr/local/lib/node_modules/emrnvm/node_modules/inquirer/lib/inquirer.js:84:28)

Node Version 10.24.1

@yiyu-liao
Copy link

same error , my Node Version is v12.22.1

This was referenced Sep 15, 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.

4 participants