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
confirm: properly clear the terminal in case of a multi-line prompt string (take 2) #175
base: master
Are you sure you want to change the base?
Conversation
pub fn clear(&mut self) -> io::Result<()> { | ||
// We must clear the current line first: `clear_last_lines` places the cursor on the first deleted line, | ||
// whereas we want to have it on the current line. | ||
self.term.clear_line()?; |
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.
Hmm... And this didn't effect the other prompts from having issues? I thought the next statement was the one which clears everything.
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.
Well, clear_last_lines()
only clears the line before the current line. I would expect the current line to also be cleared, so this seems like an oversight to me. I just pushed a commit clarifying this a bit.
TBH, I'm a bit surprised the other prompts were not hit by this. I haven't checked why... but a quick spot-check with the selection and multi-selection prompt shows no issues.
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.
Which is why i think the issue is actually in rendering the prompts themselves. Looks like the pattern for rendering other prompts is different to rendering the confirm prompt.
This was a simple oversight in the code. Make sure to preserve the cursor at the beginning of the current line.
…input prompt. With TermThemeRenderer.clear() also clearing the current line, they have become superfluous.
I just squashed changes and reworded the commit message. Basically, I think this fixes two bugs/defects:
Why did none of the other prompts notice this before?
|
I just pushed an additional commit which removes these now-superfluous calls. |
This a revised version of #166, see discussion there.
Fixes #165.