Repository navigation
Conversation
Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
| .force(force) | ||
| .default(false) | ||
| .prompt()? | ||
| { |
There was a problem hiding this comment.
Note: I changed printing behavior here. Now in non-interactive mode we still print the database tree info and the prompt (followed by "skipping confirmation due to --yes").
That just seems clearer and better to me? Totally fine to revert it though.
| // Note that this includes EOF | ||
| return Ok(default); | ||
| } | ||
| Ok(input == "y" || input == "yes") |
There was a problem hiding this comment.
note: this is the previous behavior, but since we now have an optional default, I could see an argument for requiring a clear yes or no, and ambiguous input causing a re-prompt.
I'm happy to update that here/now if we think it's worth it.
Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
| std::io::stdin().read_line(&mut input)?; | ||
| let input = input.trim().to_lowercase(); | ||
| if input.is_empty() { | ||
| // Note that this includes EOF |
There was a problem hiding this comment.
I kinda think that we should fail on EOF, since non-interactive contexts should be using force and not hitting this, but this is the previous behavior.
Thoughts?
There was a problem hiding this comment.
I agree, I probably wouldn't return Ok(default) here I would return an Err
Signed-off-by: Zeke Foppa <196249+bfops@users.noreply.github.com>
Description of Changes
#6337 wants to add a new bool param to the shared
y_or_nfunction (for the default value). It made the invocations look ugly, and made alllll the call sites change, so factored it into its own PR with a builder pattern that is much more readable imo.This PR replaces
y_or_nwithYesOrNo::new(message).force(...).default(...).prompt(). EOF now returns an error instead of using the default; pressing Enter still uses the configured default.API and ABI breaking changes
This changes how our CLI prompts respond to EOF (it is now an error instead of answering "no")
Rollback safety impact
n/a
Expected complexity level and risk
1
Testing
Existing smoketests have been updated.