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.

Sign up for GitHub

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

Jump to bottom

Add 'interact_text_opt' and 'interact_text_on_opt' for 'Input' prompts #278

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add 'interact_text_opt' and 'interact_text_on_opt' for 'Input' prompts #278

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4f6e627
Add 'interact_text_opt' and 'interact_text_on_opt' for 'Input' prompts
tbergerd Sep 10, 2023
05f6a3b
Remove dead code and unnecessary 'Debug' trait bound on Input's 'inte…
tbergerd Sep 10, 2023
08630fc
Mark Input::interact_text as inline as well
tbergerd Sep 10, 2023
3154935
Fix small inaccuracies in documentation
tbergerd Sep 10, 2023
b8f842a
Correctly clear 'Input' prompt on cancellation
tbergerd Sep 11, 2023
1ca3639
Merge branch 'console-rs:master' into input_opt
tbergerd Sep 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 1 addition & 13 deletions src/prompts/input.rs
View file
Open in desktop
@@ -1,6 +1,5 @@
use std::{
cmp::Ordering,
fmt::Debug,
io, iter,
str::FromStr,
sync::{Arc, Mutex},
Expand Down Expand Up @@ -281,7 +280,7 @@ where
impl<T> Input<'_, T>
where
T: Clone + ToString + FromStr,
<T as FromStr>::Err: Debug + ToString,
<T as FromStr>::Err: ToString,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing this bound?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bound was only required by the call to unwrap (in the dead code I removed in the same commit : 05f6a3b)
I figured I might as well remove it since it seems unnecessary (interact and interact_on didn't have this constraint).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it's definitely worth fixing. I suggest making a separate PR for this and removal of the redundant check.

{
/// Enables the user to enter a printable ascii sequence and returns the result.
///
Expand Down Expand Up @@ -339,11 +338,6 @@ where
},
)?;

// Read input by keystroke so that we can suppress ascii control characters
if !term.features().is_attended() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note : removed this because the check is already performed at the beginning of the function (and returns an error instead which makes more sense)

return Ok(Some("".to_owned().parse::<T>().unwrap()));
}

let mut chars: Vec<char> = Vec::new();
let mut position = 0;
#[cfg(feature = "history")]
Expand Down Expand Up @@ -661,13 +655,7 @@ where
}
}
}
}

impl<T> Input<'_, T>
where
T: Clone + ToString + FromStr,
<T as FromStr>::Err: ToString,
{
/// Enables user interaction and returns the result.
///
/// Allows any characters as input, including e.g arrow keys.
Expand Down