Clean up inconsistent code sending a Ctrl-C

[egmontkob]

Is there an existing issue for this?

• I have searched the existing issues

Midnight Commander version and build configuration

git

Operating system

.

Is this issue reproducible using the latest version of Midnight Commander?

• I confirm the issue is still reproducible with the latest version of Midnight Commander

How to reproduce

In src/subshell/common.c there are 3 places where we send Ctrl-C (write_all(..., "\003", 1)) to the subshell. Those three look all inconsistent. They should be made consistent and extracted to a helper method, if possible.

In all three of them, fish is handled as an exception, with no explanation. The explanation I believe is: fish doesn't repaint the prompt, and in turn doesn't reexecute its prompt command on a Ctrl-C, so we can't do a SIGCHLD-based synchronization to know when fish has finished handling this keypress.

https://github.com/MidnightCommander/mc/blob/4.8.33/src/subshell/common.c#L927

Even though with fish we don't send anything, we still set subshell_state = ACTIVE then.

Why don't we send Ctrl-C for fish if we do for other shells?

Also we call feed_subshell() recursively, pretty fishy [no pun intended].

https://github.com/MidnightCommander/mc/blob/4.8.33/src/subshell/common.c#L1402

We do send Ctrl-C to fish but don't process its response. This is okay-ish (we can't call feed_subshell() as that would wait for SIGCHLD which doesn't happen with fish), still, we'd need to read and discard fish's output, will that be done at some point later, and will the tty line surely not get clogged until then?

https://github.com/MidnightCommander/mc/blob/4.8.33/src/subshell/common.c#L1713

Here we again (like in the first case) don't send Ctrl-C to fish, but do we achieve whatever we achieve in other shells by that Ctrl-C?

Expected behavior

.

Actual behavior

.

Additional context

No response

[zyv]

I think you might find some answers in the following commit, which belongs to #4114, specifically search for "003" and check the comments above:

ff0fc17

Later, a special case for fish has been added in e89713a, which belongs to #4198.

[krobelus]

We do send Ctrl-C to fish

Once fish removes the mc-specific workarounds mentioned in #4837,
this might actually pose a problem,
because fish stops waiting for the terminal's response once it registers a ctrl-c.

I guess fish could disable that behavior for the initial query, but it shouldn't be just to work around mc.

The explanation I believe is: fish doesn't repaint the prompt, and in turn doesn't reexecute its prompt command on a Ctrl-C, so we can't do a SIGCHLD-based synchronization to know when fish has finished handling this keypress.

sounds about right.
To force a repaint of the prompt in fish, run commandline -f repaint
(if triggered from a binding, a shorter version also works, for example bind ctrl-g repaint and then send ctrl-g)

[egmontkob]

Once fish removes the mc-specific workarounds

Let's really hope that fish doesn't remove them before mc is fixed (and has a stable release tarball) to no longer rely on that workaround.

[krobelus]

> Once fish removes the mc-specific workarounds Let's really hope that fish doesn't remove them _before_ mc is fixed (and has a stable release tarball) to no longer rely on that workaround.

Sure. FWIW, for a previous one, we made mc advertise that it no longer needs the workaround, by setting the "__mc_kitty_keyboard" shell variable in an "--init-command", so we don't need to remove the workaround for old mc to be able to test new mc. We can do the same for #4837, e.g. set "__mc_primary_da". I once sketched a way to opt-out of all terminal-specific workarounds (fish-shell/fish-shell@4523b28) which might be useful for debugging eventually