feat: allow entering non-default values in multi-select #15935
Conversation
| @@ -101,6 +101,39 @@ func TestMultiSelect(t *testing.T) { | |||
| }() | |||
| require.Equal(t, items, <-msgChan) | |||
| }) | |||
|
|
|||
| t.Run("MultiSelectWithCustomInput", func(t *testing.T) { | |||
There was a problem hiding this comment.
couldn't add tests to check the custom input flow, we had this check here and not sure if its safe to remove and add interactive tests, please do let me know if there's a better way to incorporate those tests
Line 310 in 63572d9
There was a problem hiding this comment.
I think historically we've manually tested this using prompt-example. I agree that it's not ideal :-(
|
Hi @johnstcn , could you please have a look at this PR? I do have a question regarding duplicates, if users enters the same custom value twice should we treat it as a single option ? right now it will create 2 duplicate options, what do you think will be the better way to handle that scenario? |
I think it would make more sense to either de-duplicate or validate that the custom option isn't already present. |
cli/cliui/select.go
Outdated
| message string | ||
| canceled bool | ||
| selected bool | ||
| isInputMode bool // New field to track if we're adding a custom option |
There was a problem hiding this comment.
suggestion: isCustomInputMode for consistency
| m.cursor = len(options) - 1 | ||
| m.cursor = maxIndex |
There was a problem hiding this comment.
I think this logic needs to handle when EnableCustomInput is false. Currently it lets you go over the last valid option.
cli/prompts.go
Outdated
| }, | ||
| Defaults: []string{"Code"}, | ||
| Defaults: []string{"Code"}, | ||
| EnableCustomInput: true, |
There was a problem hiding this comment.
Can we expose this as a flag so we can easily test both with and without custom input?
I've addressed the comments and handled duplicates as shown in the recording
Screen.Recording.2024-12-20.at.11.39.55.AM.mov |
cli/cliui/select.go
Outdated
|
|
||
| // Check for duplicates | ||
| for i, opt := range m.options { | ||
| if strings.EqualFold(opt.option, m.customInput) { |
There was a problem hiding this comment.
I'm not 100% sure about the case-insensitive matching. On the surface, it seems innocuous enough but I'm not sure if there's going to be a case where someone is going to want to differentiate A from a. Let's keep matching case-sensitive for now.
We can add an option for case-insensitive matching later if someone requests it.
Closes #15488
cliui.Multiselect currently does not allow users to input custom options if zero choices are provided.
Proposed Solution
cliui.MultiSelect should always allow users to specify another option.
For example, when running coder exp prompt-example multi-select:
PS: I have modified the text a bit from the original issue
Attaching a recording for the same
Screen.Recording.2024-12-19.at.4.25.12.PM.mov