WIP: move from console to crossterm #213
Conversation
…nfirmation prompt (which will come next)
… the Term type. Instead, crossterm uses just a io::Write trait object. Doesn't compile yet; many terminal methods are still missing. In the next changes, I'll slowly add them to TermThemeRenderer instead or replace calls by their crossterm equivalent. (The buffered example is not adjusted yet; this will happen in the very end.)
The password prompt needs it, and it allows removing the Term accessor.
They are superfluous, now that TermThemeRenderer::clear also clears the current line.
This required adding another few helper methods to theme.rs, but apart from that went quite smoothly. The result is essentially untested; that's for later.
…ing errors remaining.
General things of note - As crossterm operates on any io::Write trait object instead of a dedicated Term struct, we use the static terminal::size() method to obtain the terminal size, which returns an option. - We conciously only support clearing an u16 number of lines, since that's crossterm only allows moving the cursor by that many lines. - We conciously manually clear the last N lines instead of the entire terminal since the confirmation prompt expects it so. This change was manually tested, by running the `confirm` example with various inputs. The confirmation prompt behaves the same as before this change.
- create a new constant for the default terminal size, as that's now used in three different places - make clear_last_lines a static method, operating on any dyn io::Write, now just a TermThemeRenderer -- paging also needs it. - also extract clear_current_line to that level
…tance. Use an Arc<Mutex<dyn io::Write>> instead of a &mut dyn io::Write. - an arc allows sharing the terminal, a mutex allows mutating it while shared To avoid lots of boilerplate with .lock().unwrap(), introduce a wrapper type. Make the remaining prompts compile; almost all examples compile now. The sole exception is the buffered example, which uses console's buffered_stderr type.
- {show,hide}_cursor can be removed now,
- flush() can be reduced to one single occurrence.
These were needed before we wrapped our terminal in an Arc.
Replaces lots of boilerplate with Term::new(Arc::new(Mutex::new(_))).
|
I have solved the borrowing issue above; with the latest push, all prompts compile now. (The one exception is the buffered example, which I haven't ported. cargo fmt, cargo clippy and cargo test also pass.) I haven't tested the prompts any further. |
|
FWIW, this PR is not going to go to waste. We just need to implement the testing framework is all. |
|
That's great to hear. Having testing infrastructure would also accelerate my progress with ironing out the remaining bugs. |
|
I have been making some good progress in figuring out how to do unit testing for TUIs. I should have something soon. |
|
@pksunkara This sounds exciting! Any news since March? (I've seen there are merge conflicts on this PR. I'll resolve them when there is a clearer story forward - which, as I understand it, is blocked on figuring out a better unit testing story.) |
|
I would like the part with switching to Hopefully it can make Windows experience better. |
|
Agreed. I have some work locally that I will look to push. @Gordon01 if you are interested, you can also take charge of it later. |
|
I do not want to do this merge without considering what this means. There are a lot of projects using console and dialoguer in one way or another and this switch would have to be considered properly first. In general all of these crates (dialoguer included) are quite buggy as a hole and have frequent regressions, I'm not convinced that this move will significantly improve on this. |
|
To make this more actionable: I rather see a fork of this library out of this organization to be based on crossterm than to make such a major departure here. |
Agree. Wasn't familiar with the organization of the project but now I see.
Generally, they work well. But definitely needs a few touches. Especially for Windows, ha-ha.
@pksunkara great! Thank you for your effort and quick merge of my patches.
|
|
@pksunkara I believe, this PR may now be closed as this crate would stay on It's also listed as a 0.11 milestone which is definitely not true now) |
|
Before anyone goes ahead and forks |
|
I definitely wouldn't do it, since we now fixed the most annoying bugs of non-ascii characters support. @pksunkara can this be closed now? |
I've started moving this crate to crossterm as hinted here and figured I'd share my progress.
Scope of task. TTBOMK, there are three parts to this migration, corresponding to the different console types used:
console::Keybycrossterm::KeyCodeconsole::Termby acrosstermequivalentconsole::{style, Style, StyledObject}by crossterm equivalentsThe first item is tedious, but pretty straightforward; all necessary changes are mechanical. Replacing console::Term is more subtle; I've introduced bugs in the porting process more than once (which manual testing has caught). I haven't looked deeply into the third part yet. console's and crossterm's machinery around terminal styling are somewhat different; I haven't wrapped my head around that. I'd leave that for another PR.
Testing. I mostly used manual testing to verify my changes.
cargo test,cargo clippyandcargo fmtpasscargo run --example confirmand likewiseStatus.
console::Keybycrossterm::KeyCodeconsole::Termby acrosstermequivalentbuffered.rsexample: usescrossterm::Term::buffered_stderrconsole::{style, Style, StyledObject}by crossterm equivalents: not started