fix(select,multiselect): properly handle defaults and prompts#642
Merged
fix(select,multiselect): properly handle defaults and prompts#642
Conversation
caarlos0
added
bug
caarlos0
requested review from
bashbunni and
Copilot
and removed request for
a team
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the handling of defaults and prompts for select and multiselect inputs. Key changes include introducing an atoi helper to process input strings, updating prompt error messages for clarity, and refining the default value handling logic in select/multiselect fields.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/accessibility/accessibility.go | Adds a helper function (atoi) to handle empty string inputs better. |
| huh_test.go | Updates test assertions to use Fatalf in order to fail tests fast. |
| field_select.go | Adjusts default value assignment in select fields by ensuring s.selected is set. |
| field_multiselect.go | Modifies the input condition to catch negative values by using <= instead of ==. |
Comments suppressed due to low confidence (2)
huh_test.go:1458
- [nitpick] Using tb.Fatalf halts the test execution on the first failure, possibly masking subsequent errors. Consider using tb.Errorf if you wish to aggregate multiple assertion failures in a single test run.
tb.Fatalf("expected %v to be equal to %v", a, b)
field_select.go:724
- Ensure that s.selectOption() reliably sets s.selected before it is used to compute defaultValue. Adding an inline comment or assertion could help clarify the intended behavior and prevent off-by-one errors.
s.selectOption() // make sure s.selected is set
meowgorithm
reviewed
Apr 29, 2025
meowgorithm
reviewed
Apr 29, 2025
meowgorithm
reviewed
Apr 29, 2025
Co-authored-by: Christian Rocha <[email protected]>
| idx := s.selected + 1 | ||
| defaultValue = &idx | ||
| } | ||
| prompt := fmt.Sprintf("Enter a number between %d and %d: ", 1, len(s.options.val)) |
Contributor
There was a problem hiding this comment.
Just a note, this change from Input to Enter wasn't also reflected in the multiselect, so they are different now.
Demonstrating Single Select
Favorite cuisine? (default: Italian)
1. Italian
2. Greek
3. Indian
4. Japanese
5. American
Enter a number between 1 and 5: 1
Favorite cuisine: Italian
Demonstrating Multi Select
Favorite cuisines?
Select up to 5 options.
1. Italian
2. Greek
3. Indian
4. Japanese
5. American
0. Confirm selection
Input a number between 0 and 5:
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #639
closes #638