TopCli / prompts

Node.js user prompt library for command-line interfaces.
ISC License
25 stars 4 forks source link

Feature: Autocomplete for select prompt #92

Closed noxify closed 8 months ago

noxify commented 8 months ago

Hey,

I'm currently extending our cli ( it's more a refactoring 🙈 ) and have currently the case that the user can provide a value via a cli option, but if the option isn't provided, I want to show a select field.

Showing a select field isn't the problem and works as expected, but since we have a lot of tables it could be hard to find the correct table.

Could we extend the select field to support autocomplete?

My first idea was to use the existing implementation from the multiselect prompt. If this could be a possible solution this should not break any existing integration, because without setting autocomplete:true in the select configuration, everything is working as before.

What do you think?

PierreDemailly commented 8 months ago

Hi, yes :)

Implementing an autocomplete option for the select prompt seems logical to me. We haven't implemented it yet as we don't have the need, but it's certainly a reasonable feature to consider.

Would you like to contribute ?

noxify commented 8 months ago

Would you like to contribute ?

Forked the repo already and currently doing some try&error.

Question: Is it easier to use the multiselect prompt as base and remove the possibility to choose multiple values?

PierreDemailly commented 8 months ago

Well, nice 👍

Semantically it doesn't make sense to have a multi-select that does not allow to choose multiple values (even if it only via an option) In terms of DX, it's the same, for the developer it's not intuitive to disable "multiple values" from the multi select to have a select with autocomplete, it makes much more sense to have a select with autocomplete option.

What could make sense is to remove the multi-select, and add a multi option to the select, but at the time I implemented the multi-select, having a dedicated prompt was easier because it needed too many abstraction/branch conditions if we wanted a single one select prompt.

So, yeah I think its easier but it's not what we want IMO, Also, it should not be too complicated if you get inspired by https://github.com/TopCli/prompts/pull/62

Don't hesitate if you have any questions / need any guidance ;)

noxify commented 8 months ago

@PierreDemailly - Thanks! Created the Draft PR with the first running example.

I used the multiselect code and removed everything which isn't needed. For me, this was easier then merging the snippets into the existing code.

Commented my current findings in the PR.

noxify commented 8 months ago

@PierreDemailly - one question:

I'm currently creating the additional tests for autocomplete. Here I'm using the multiselect tests and update them so they match the select output.

What should/could be the expected output for returning an empty list ( e.g. we have foo,bar,baz as choices and the filter is something )?

Currently there is a missing catch case / fallback handling and we get an exception.

PierreDemailly commented 8 months ago

I think we should return an empty string to match the question-prompt behavior.

Maybe fallback to an empty string:

const choice = this.filteredChoices[this.activeIndex] ?? "";

And display a cross symbol if empty string (again, to match the question-prompt behavior)

#showAnsweredQuestion(choice: Choice | string) {
  const symbolPrefix = choice === "" ? SYMBOLS.Cross : SYMBOLS.Tick;
  const prefix = `${symbolPrefix} ${kleur.bold(this.message)} ${SYMBOLS.Pointer}`;
  const formattedChoice = kleur.yellow(typeof choice === "string" ? choice : choice.label);

  this.write(`${prefix} ${formattedChoice}${EOL}`);
}

WDYT ?

noxify commented 8 months ago

Makes sense - updated the PR ( code + tests )

noxify commented 8 months ago

@PierreDemailly - thanks for the help & input today - was fun to leave my conform zone and trying something new :)

PierreDemailly commented 8 months ago

@PierreDemailly - thanks for the help & input today - was fun to leave my conform zone and trying something new :)

You're welcome, thanks you for the PR :)

If you wanna try another something new you can take a look at NodeSecure 😈 (if you're interested don't hesitates to join our discord)