-
Notifications
You must be signed in to change notification settings - Fork 20
Add cancelKey parameter to singleChoicePrompt #760
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
base: main
Are you sure you want to change the base?
Conversation
- Add optional cancelKey property to SingleChoicePrompt struct
- Implement runCancelable() methods that return Optional<T>
- Add keystroke handling for cancel key in runCancelable
- Generate dynamic instruction text showing cancel key to users
- Add protocol methods to Noorable for cancelable variants
- Add public API overloads in Noora class (both main and convenience)
- Maintain full backwards compatibility (existing code unchanged)
- Zero breaking changes - cancelKey is opt-in via new overloads
Example usage:
if let fruit = noora.singleChoicePrompt(
question: "Choose a fruit",
cancelKey: "q"
) as Fruit? {
print("Selected: \(fruit)")
} else {
print("Canceled")
}
fortmarek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution 💯
I'm very aligned with the idea. I'd try to reuse more of the existing implementation to ease the future maintenance of this component. Additionally, I'd suggest adding some tests in SingleChoicePromptTests. Let me know if you have any questions or need help with anything 😌
We really appreciate the time you're taking to contribute to Noora!
| return selectedOption.0 | ||
| } | ||
|
|
||
| private func runCancelable<T: Equatable>(options: [(T, String)]) -> T? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect this method to reuse the run method (or the other way around). Right now, we're duplicating a lot of code, making this piece harder to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! You're absolutely right about the duplication. I'll refactor runCancelable() to reuse the existing run() method internally rather than duplicating all that logic. I'll also add comprehensive tests to SingleChoicePromptTests to cover the cancel functionality. Should have the updates pushed shortly!
| collapseOnSelection: Bool, | ||
| filterMode: SingleChoicePromptFilterMode, | ||
| autoselectSingleChoice: Bool, | ||
| cancelKey: Character, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect us to provide a default, sensible choice, instead of forcing the developer to pick the cancel key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call on providing a default! I'll add a sensible default value (probably 'q') so developers don't have to specify it every time. Makes the API much more ergonomic.
- Eliminated code duplication by making runCancelable() reuse the existing run() implementation with an optional cancelKey parameter - Updated run() to support optional cancellation internally - Removed ~113 lines of duplicate code for easier maintenance - Added default cancelKey value of 'q' for better API ergonomics - Added comprehensive tests for cancel functionality: - Test cancellation when cancel key is pressed - Test cancel instruction rendering - Test non-cancelable behavior when cancelKey is nil - Updated all existing tests to include cancelKey parameter This refactoring makes the codebase more maintainable by having a single source of truth for the prompt logic, while maintaining full backwards compatibility.
Motivation
Many CLI tools allow users to cancel/go back using keyboard shortcuts (q, b, ESC) instead of navigating to a "Back" option in a list. This PR adds optional cancel key support to
singleChoicePrompt.Use Case
In our M4B audiobook editor, users rename chapters from a list. Currently, they must scroll to "← Back to main menu" and press Enter to cancel. With
cancelKey: "q", they can simply press 'q' from anywhere in the selection list.Changes
cancelKey: Character?property toSingleChoicePromptstructrunCancelable()methods that returnOptional<T>(nil on cancel)NoorableinterfaceNooraclass (both main methods and convenience extensions)Implementation Approach
Used the overload pattern to ensure zero breaking changes:
singleChoicePrompt()methods return non-optionalTsingleChoicePrompt(cancelKey:)methods return optionalT?Example Usage
Technical Details
[T]options andCaseIterablevariantsTesting
swift build)Related Patterns
Common in CLI tools: