Restore terminal modes after running key bindings with external commands

This concerns the behavior when running an external command from a key
binding. The history is:

Prior to 5f16a299a728, fish would run these external commands in shell
modes. This meant that fish would pick up any tty changes from external
commands (see #2114).

After 5f16a299a728, fish would save and restore its shell modes around
these external commands. This introduced a regression where anything the
user typed while a bound external command was executing would be echoed,
because external command mode has ECHO set in c_lflag. (This can be
reproed easily with `bind -q 'sleep 1'` and then pressing q and typing).
So 5f16a299a728 was reverted in fd9355966.

This commit partially reverts fd9355966. It has it both ways: external
commands are launched with shell modes, but/and shell modes are restored
after the external command completes. This allows commands to muck with
the tty, as long as they can handle getting shell modes; but it does not
enable ECHO mode so it fixes the regression found in #7770.

Fixes #7770. Fixes #2114 (for the third time!)

This partially reverts commit fd9355966ed4392b77137e522e7fb4b28503d674.
This commit is contained in:
ridiculousfish 2021-03-10 21:02:32 -08:00
parent 7e8b8345e7
commit 3cb105adbd
2 changed files with 19 additions and 16 deletions

View File

@ -2827,20 +2827,25 @@ struct readline_loop_state_t {
/// Run a sequence of commands from an input binding.
void reader_data_t::run_input_command_scripts(const wcstring_list_t &cmds) {
// Need to donate/steal the tty - see #2114.
// Unfortunately this causes us to enable ECHO,
// which means if input arrives while we're running a bind function
// it will turn up on screen, see #7770.
//
// What needs to happen is to tell the parser to acquire the terminal
// when it's running an external command, but that's a lot more involved.
// term_donate();
auto last_statuses = parser().get_last_statuses();
for (const wcstring &cmd : cmds) {
parser().eval(cmd, io_chain_t{});
}
parser().set_last_statuses(std::move(last_statuses));
// term_steal();
// Restore tty to shell modes.
// Some input commands will take over the tty - see #2114 for an example where vim is invoked
// from a key binding. However we do NOT want to invoke term_donate(), because that will enable
// ECHO mode, causing a race between new input and restoring the mode (#7770). So we leave the
// tty alone, run the commands in shell mode, and then restore shell modes.
int res;
do {
res = tcsetattr(STDIN_FILENO, TCSANOW, &shell_modes);
} while (res < 0 && errno == EINTR);
if (res < 0) {
wperror(L"tcsetattr");
}
termsize_container_t::shared().invalidate_tty();
}
/// Read normal characters, inserting them into the command line.

View File

@ -92,13 +92,11 @@ send("\x12") # ctrl-r, placing fth in foreground
expect_str("SIGCONT")
# Do it again.
# FIXME: Unfortunately the fix for #2114 had to be reverted because of #7770,
# so this is broken.
# send("\x1A")
# expect_str("SIGTSTP")
# sleep(0.1)
# send("\x12")
# expect_str("SIGCONT")
send("\x1A")
expect_str("SIGTSTP")
sleep(0.1)
send("\x12")
expect_str("SIGCONT")
# End fth by sending it anything.
send("\x12")