Prevent Ctrl-C from losing data when closing dijo

[charles-l]

Hey @NerdyPepper

So the idea behind this PR is to trap users who don't know how to quit with :q so they're forced to continue using dijo or reboot their machine. It's a strategy taken from vim.

JK :P

I actually wanted to prevent users from accidentally hitting Ctrl-C and losing their in-memory changes.

This change has two components in it:

• Makes the message parameter on the App structure writable in a different thread (e.g. a signal handler),
• and adds a handler to catch SIGINT and print a message rather than killing the application.

Now, here's the bit where I'm stuck -- I can't figure out how to capture Ctrl-C. For some reason, termion captures Ctrl-C, circumvents the SIGINT handler, and still quits the application. I have no idea why this is happening since it goes against everything I know about how process handling works on Linux. I've scoured the termion source, looking for code that stomps, ignores, or updates signal handler code, but can't find the root cause of why this is happening. I'd be interested in any ideas you have.

Granted, this functionality isn't critical, but I thought it would be a good idea to avoid data loss if a SIGINT does happen to be sent to the process.

Another potential solution to the problem is to just eliminate the save command, and make every write operation commit to the file (it's a small enough source file that this wouldn't be unreasonable). Anyways, feel free to reject this PR if it isn't going in a direction you think is useful, or if the code is just too non conventional (I don't write Rust on a daily basis, so I'm not sure if these changes stick out like a sore thumb).

[charles-l]

Oh, and if you want to test the functionality of this patch:

1. Start dijo
2. In a new terminal, run pkill -SIGINT dijo
3. Move the cursor in dijo (to force a screen redraw)
4. Observe the message telling you off for (virtually) hitting Ctrl-C -- use :q like a vimmer dang it

Of course, I'd like to make step 2 work for Ctrl-C in dijo...

[oppiliappan]

Haha, this is pretty cool!

One way to make Ctrl-C not quit in dijo is to catch the event in App::on_event, and make it display the same message.

[Ryman]

Granted, this functionality isn't critical, but I thought it would be a good idea to avoid data loss if a SIGINT does happen to be sent to the process.

Another potential solution to the problem is to just eliminate the save command, and make every write operation commit to the file (it's a small enough source file that this wouldn't be unreasonable).

👍 as a User I had no idea it wasn't autosaving and lost a few days of stuff it seems (had left it in a long running pane and only ctrl-c'd after a few days uptime when screensharing) 😓

[charles-l]

One way to make Ctrl-C not quit in dijo is to catch the event in App::on_event, and make it display the same message.

Hmm... I tried that (pushed the code that I added so someone can double check if I'm doing it right) but this doesn't seem to work either. I also tried panicking in the catch to ctrl-c, but I'm not seeing the panic message. Maybe it cleans up stdout so the panic doesn't print, but I'm definitely scratching my head on this one... between catching Ctrl-C in the on_event section and adding a signal handler, I don't know how it's circumventing it still.

Maybe I'll open a bug report in termion... when I disabled the termion initialization, the signal handler worked as expected. But I already dug through the termion source and there doesn't seem to be any magic handler code in there, and even if there was, my signal handler should overwrite it. So, once again, I'm very confused :P

[gyscos]

Note that by default, cursive binds Ctrl-C to Cursive::quit(). So if the backend traps the signal and returns a regular Ctrl-C event instead of aborting the process (termion does that by enabling the terminal's "raw mode"), then the cursive app will by default nicely exit and unwind everything (so it can at least clean up the terminal).

You can override it by first clearing the existing binding for this (Cursive::clear_global_callbacks()), and then adding a new one.

For example:

use cursive::views;

fn main() {
    let mut siv = cursive::default();

    siv.clear_global_callbacks(cursive::event::Event::CtrlChar('c'));

    siv.set_on_pre_event(cursive::event::Event::CtrlChar('c'), |s| {
        s.add_layer(
            views::Dialog::text("Do you want to quit?")
                .button("Yes", |s| s.quit())
                .button("No", |s| {
                    s.pop_layer();
                }),
        );
    });

    siv.add_layer(views::Dialog::text("Try pressing Ctrl-C!"));

    siv.run();
}

[charles-l]

@gyscos That worked! Thanks, man! I was really bothered that I couldn't figure it out.

Now that this part works, I'll finish up this PR and do a bit of cleanup on it.

[gyscos]

Note that you no longer need the Arc/RwLock I believe. Same for cloning the message.

Also, maybe Ctrl-C could have the same behavior as :q? That is, still exit the app, but at least save?

[charles-l]

Ah, good point -- I've removed the lock since it no longer uses the signal handler.

Also, maybe Ctrl-C could have the same behavior as :q? That is, still exit the app, but at least save?

But then it doesn't trap users and force them to continue using the app like Vim does ;)

I was just copying vim's functionality here -- I definitely could switch it to enabling quitting now that we can capture it and properly save there. I think vim disables ctrl-c more because of accidental keypresses that could come from shelling out or inserting weird crap in insert mode, so I guess it has less reason to be blocked in dijo.

[gyscos]

Vim actually uses Ctrl-C to abort any ongoing action (like running the highlighter, or autocomplete, ...), it doesn't quite try to "trap" the user on purpose 😛.

dijo already differs by saving on :q rather than :x.

Another simple solution would be to just save unconditionally after Cursive::run completes. This way, you don't even need to replace the Crtl-C hook, and :q can simply call Cursive::quit.

[oppiliappan]

@gyscos thanks for the help!

Another simple solution would be to just save unconditionally after Cursive::run completes. This way, you don't even need to replace the Crtl-C hook, and :q can simply call Cursive::quit.

Good point!

[charles-l]

Another simple solution would be to just save unconditionally after Cursive::run completes. This way, you don't even need to replace the Crtl-C hook, and :q can simply call Cursive::quit.

@gyscos ok -- I tried implementing this functionality, but it's not working because somethings panicking when I try to unwrap it using the code I've written. If this looks like the correct way to implement it to you, I can dig into it and try to figure out the root cause of the panic, but I feel like I'm working against the grain of the language and libraries right now. Is there a better way of doing this?

[gyscos]

Maybe just this?

s.call_on_name("Main", |app: &mut App| app.save_state());

[charles-l]

Ah, perfect (can't believe I missed call_on_name in the examples). This approach has so much less code than the original implementation. I'm a huge fan.

[epilys]

Just a comment, since you've changed your original code: it is not safe to use mutexes etc in a signal handler. Whenever you want to know a signal handler has been fired you employ the self pipe trick.

[charles-l]

@epilys ahh, I didn't realize that -- I assumed signal handlers ran in a different thread, rather than forcibly interrupting the control flow in the main thread without cleanup. That's pretty dangerous. TIL, thanks!

All the more reason to go with the second approach.

[charles-l]

@NerdyPepper any other changes you want me to make before you merge?

[oppiliappan]

Not really, no. This is good enough.

Thanks a bunch @charles-l!