fix: ctrl-c doesn't restore cursor #75
Conversation
roele
force-pushed
the
issues/74
branch
6 times, most recently
from
26bda9a to
baa0df0
Compare
| @@ -153,6 +154,8 @@ impl<'a> Input<'a> { | |||
| /// This function will block until the user submits the input. If the user cancels the input, | |||
| /// an error of type `io::ErrorKind::Interrupted` is returned. | |||
| pub fn run(mut self) -> io::Result<String> { | |||
| let ctrlc_handle = ctrlc::show_cursor_after_ctrlc(&self.term)?; | |||
There was a problem hiding this comment.
this approach/try with Input seems to work more or less but having to close the handle is a bit cumbersome though. it seems a bit safer than using unsafe operations to register/unregister. signal_hooks also probably won't work on Windows as per docs.
@jdx any ideas?
There was a problem hiding this comment.
can you even hide the cursor to begin with on windows?
There was a problem hiding this comment.
I'll test real quick on my windows laptop
There was a problem hiding this comment.
so in main demand currently does hide the cursor on windows and it doesn't reappear just like in unix, so whatever we do we should fix it for windows too.
This pr doesn't compile on windows at least (maybe on unix too, didn't check)
error[E0599]: no method named `write` found for struct `HANDLE` in the current scope
--> src\ctrlc.rs:50:39
|
19 | / lazy_static! {
20 | | static ref HANDLE: RwLock<Option<Handle>> = RwLock::new(None);
21 | | }
| |_- method `write` not found for this struct
...
50 | let mut handle_guard = HANDLE.write().unwrap();
| ^^^^^ method not found in `HANDLE`
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following traits define an item `write`, perhaps you need to implement one of them:
candidate #1: `Hasher`
candidate #2: `std::io::Write`
There was a problem hiding this comment.
Windows support for signals seems hopeless at this point. The ctrlc crate goes a long way to make it happen but theres no way to unregister for our use-case. For now i guess we have to accept this as a known issue.
| let handle_guard = HANDLE.read().unwrap(); | ||
| return Ok(handle_guard.clone()); | ||
| } | ||
| INIT.store(true, Ordering::Relaxed); |
There was a problem hiding this comment.
if I'm reading this right, we will trap sigint the first time demand is run but if a CLI made multiple prompts it seems we would not trap it the second time
There was a problem hiding this comment.
It should ensure we can only register a handler once.
How would multiple prompts work? We can only render one at a time... if the run method exits the handler should be dropped which should enable you to register another one for another input.
There was a problem hiding this comment.
ok I tested this and it does seem to work as you described.
As far as the multiple prompts things, I handle this with a mutex in mise: https://github.com/jdx/mise/blob/0f9dae377eaa8a527c099310944e75bac60bc3a3/src/ui/prompt.rs#L13
but we should probably put that in here since it never makes sense to show 2 prompts at the same time
roele
force-pushed
the
issues/74
branch
2 times, most recently
from
832cb65 to
0204fdf
Compare
roele
force-pushed
the
issues/74
branch
4 times, most recently
from
187d444 to
02c5545
Compare
roele
force-pushed
the
issues/74
branch
4 times, most recently
from
a1b1210 to
0f29119
Compare
|
you may want to do something like this if you can't get windows to work: https://github.com/jdx/mise/blob/127e1646f80fba36a2b2a1b7956d6d8e77f83c5f/src/ui/mod.rs#L3 |
roele
force-pushed
the
issues/74
branch
2 times, most recently
from
750e3a3 to
3db3c03
Compare
|
@jdx thanks for the hint, thought i could get it to work within the same file but your approach with a specific stub seems much easier to work with |
roele
force-pushed
the
issues/74
branch
2 times, most recently
from
a227e5c to
1eb2e05
Compare
Fixes #74