-
Notifications
You must be signed in to change notification settings - Fork 4
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
🚧 fix: inputs get messed up when a line overflows #36
base: main
Are you sure you want to change the base?
Conversation
// calculate potential overflow of lines overflowing terminal width | ||
let overflow = output | ||
.lines() | ||
.map(|l| { |
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.
looks good to me. I feel like maybe with "fold" or "reduce" you could make it a tiny bit more concise but honestly I'd probably just do it the way you did
src/confirm.rs
Outdated
@@ -193,7 +195,15 @@ impl<'a> Confirm<'a> { | |||
Ok(std::str::from_utf8(out.as_slice()).unwrap().to_string()) | |||
} | |||
|
|||
fn set_height(&mut self, output: String) -> io::Result<()> { | |||
let height = common::get_height(&self.term, output); |
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.
let height = common::get_height(&self.term, output); | |
let (h, o) = common::get_height(&self.term, output); |
this looks like the right approach to me—definitely tough to review this kind of code though |
overflowing lines need to be calculated seperately of height so we can move the cursor accordingly before clearing. for
selects
this is much more involved since capacity of options also need to be calculated based on title,description and help which all might be overflowing.