feat: add --interactive option to prompt for each change #1119
Conversation
Bnyro
force-pushed
the
prompt-to-write
branch
from
c83b305 to
ffd6238
Compare
| Ok(()) | ||
| } | ||
|
|
||
| fn select_fix(typo: &typos::Typo<'_>) -> Option<usize> { |
There was a problem hiding this comment.
When I run this locally, I get weird output on the right item
There was a problem hiding this comment.
Something still isn't right
And if I ctrl-c,. my cursor no longer shows up
There was a problem hiding this comment.
I can reproduce the issue with the cursor (but thought something in my setup is broken, apparently it's not my setup), I've really no idea what causes it. I've tried the inquire library as well because I thought something is wrong with the dialoguer library but that one worked even worse...
There was a problem hiding this comment.
Oh well, apparently we have to handle Ctrl+C issues ourselves as per console-rs/dialoguer#294.
There was a problem hiding this comment.
It seems like the available cli libraries for prompts are not really that mature yet.
Bnyro
force-pushed
the
prompt-to-write
branch
2 times, most recently
from
5c2fec3 to
b0bdd7e
Compare
Bnyro
changed the title
Bnyro
force-pushed
the
prompt-to-write
branch
from
b0bdd7e to
13b0a28
Compare
|
Thanks for your review so far, I'm way more comfortable with the code now than before the review. |
Bnyro
force-pushed
the
prompt-to-write
branch
3 times, most recently
from
b4cabe8 to
5fc0015
Compare
Bnyro
force-pushed
the
prompt-to-write
branch
from
5fc0015 to
7f632ac
Compare
|
I've added a custom I've no idea why the position of the prompt is weird sometimes. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
crates/typos-cli/src/file.rs
Outdated
| .default(true) | ||
| .show_default(true) | ||
| .interact(); | ||
| return confirmation.map(|_| 0).ok(); |
Check failure
Code scanning / clippy
unneeded `return` statement
crates/typos-cli/src/file.rs
Outdated
| return match selection { | ||
| Ok(selection) => { | ||
| if selection != 0 { | ||
| Some(selection - 1) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| Err(_) => None, | ||
| }; |
Check failure
Code scanning / clippy
unneeded `return` statement
| @@ -32,6 +32,13 @@ fn run() -> proc_exit::ExitResult { | |||
|
|
|||
| init_logging(args.verbose.log_level()); | |||
|
|
|||
| // https://github.com/console-rs/dialoguer/issues/294 | |||
| ctrlc::set_handler(move || { | |||
| let _ = console::Term::stdout().show_cursor(); | |||
There was a problem hiding this comment.
I now see the cursor but I can't see anything I type
There was a problem hiding this comment.
That's strange, for me everything behaves normal after that change.
| // https://github.com/console-rs/dialoguer/issues/294 | ||
| ctrlc::set_handler(move || { | ||
| let _ = console::Term::stdout().show_cursor(); | ||
| std::process::exit(0); |
There was a problem hiding this comment.
Is exit(0) the correct way of exiting on ctrl-c to mirror behavior before this change?
There was a problem hiding this comment.
You're right, the previous exit code for ctrl-c was 130, and according to my quick research 130 is preferred over 0 when ctrl-c killed the process via SIGTERM.
For me, its always weird for the first prompt. This would be a blocker for merging |
I see, but unfortunately I don't see any way to fix this except for trying other libraries? |
|
How about we switch to showing prompts that require hitting |
That could fix the issue for the If I do however run this on different source code folders, for some everything works normally, and for some it behaves as described above, which doesn't make a lot of sense to me. The only two considerable crates I found are the |
Bnyro
force-pushed
the
prompt-to-write
branch
from
7f632ac to
a843d91
Compare
Bnyro
force-pushed
the
prompt-to-write
branch
from
a843d91 to
b435635
Compare
The code is probably far from perfect, I'm hoping for some help at improving the code quality in the review :)
closes #397