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

Prompt multi select #3929

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

joepjoosten
Copy link
Contributor

Type

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Added prompt to select multiple choices

Copy link

changeset-bot bot commented Nov 12, 2024

🦋 Changeset detected

Latest commit: bdf2254

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@effect/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@gcanti gcanti added the cli label Nov 12, 2024
@joepjoosten
Copy link
Contributor Author

Scratchpad to see it in action:

import { Command, Prompt } from "@effect/cli"
import { NodeContext, NodeRuntime } from "@effect/platform-node"
import { Array, Console, Effect, pipe } from "effect"

const run = Command.make("example", {}, () =>
  Effect.gen(function*() {
    const choices = pipe(Array.range(1, 20), Array.map((n) => ({ title: `Option ${n}`, value: n })))
    const selectMulti = yield* Prompt.selectMulti({
      message: "select a option",
      choices
    })
    yield* Console.log(`You selected: ${selectMulti.join(", ")}`)
  })).pipe(
    Command.run({
      name: "Test",
      version: "1.0.0"
    })
  )

const main = Effect.suspend(() => run(globalThis.process.argv))

main.pipe(
  Effect.provide(NodeContext.layer),
  Effect.tapErrorCause(Effect.logError),
  NodeRuntime.runMain
)

@IMax153
Copy link
Member

IMax153 commented Nov 13, 2024

@joepjoosten - thank you so much for all the work you put into this! It looks great so far! I did notice one small thing that I wanted to get your opinion on.

If I select a bunch of options, then invert the selection, and then select all the options, the returned results are out-of-order:

Multi-select reproduction of out of order results

It might be a good idea if we order the results by option number on submission before returning the final list. Wdyt?

@IMax153
Copy link
Member

IMax153 commented Nov 13, 2024

@joepjoosten - this looks good to me! The only additional request I have is that we make this a patch release since we're only adding a feature, which is backward compatible.

@joepjoosten
Copy link
Contributor Author

Thanks!

Should i add min/max choices support? Then it can be used in the wizard when there is a repeated choice option?

@IMax153
Copy link
Member

IMax153 commented Nov 13, 2024

Sure! That would be a cool addition :)

@joepjoosten
Copy link
Contributor Author

I'm also not really fond of using default text for the select all/none and inverse selection. This is not very international because there in English. I don't know if there are universal symbols for select all/none and inverse?

@joepjoosten
Copy link
Contributor Author

And nothing more difficult than naming things: "selectMulti", i'm still not happy with this name...

@IMax153
Copy link
Member

IMax153 commented Nov 13, 2024

And nothing more difficult than naming things: "selectMulti", i'm still not happy with this name...

Maybe MultiSelect instead?

@joepjoosten
Copy link
Contributor Author

I've added the min max functionality, and renamed it to multiSelect

@IMax153
Copy link
Member

IMax153 commented Nov 19, 2024

The only additional request I have is that we make this a patch release since we're only adding a feature, which is backward compatible.

@joepjoosten - only one thing left to take care of before we can merge this in 👍

@joepjoosten
Copy link
Contributor Author

Sorry, i missed that comment. Is this what you mean? (see latest commit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Discussion Ongoing
Development

Successfully merging this pull request may close these issues.

3 participants