Commit Graph

27 Commits

Author SHA1 Message Date
Johannes Altmanninger
b89619330b Disable terminal protocols before cancellable operations
Some checks failed
make test / ubuntu (push) Waiting to run
make test / ubuntu-32bit-static-pcre2 (push) Waiting to run
make test / ubuntu-asan (push) Waiting to run
make test / macos (push) Waiting to run
Rust checks / rustfmt (push) Waiting to run
Rust checks / clippy (push) Waiting to run
Lock threads / lock (push) Has been cancelled
The [disambiguate flag](https://sw.kovidgoyal.net/kitty/keyboard-protocol/#disambiguate) means that:

> In particular, ctrl+c will no longer generate the SIGINT signal,
> but instead be delivered as a CSI u escape code.

so cancellation only works while we turn off disambiguation.

Today we turn it off while running external commands that want to
claim the TTY.  Also we do it (only as a workaround for this issue)
while expanding wildcards or while running builtin wait.

However there are other cases where we don't have a workaround,
like in trivial infinite loops or when opening a fifo.

Before we run "while true; end", we put the terminal back in ICANON
mode. This means it's line-buffered, so we won't be able to detect
if the user pressed ctrl-c.

Commit 8164855b7 (Disable terminal protocols throughout evaluation,
2024-04-02) had the right solution: simply disable terminal protocols
whenever we do computations that might take a long time.
eval_node() covers most of that; there are a few others.

As pointed out in #10494, the logic was fairly unsophisticated then:
it toggled terminal protocols many times.  The fix in 29f2da8d1
(Toggle terminal protocols lazily, 2024-05-16) went to the extreme
other end of only toggling protocols when absolutely necessary.

Back out part of that commit by toggling in eval_node() again,
fixing cancellation.  Fortunately, we can keep most of the benefits
of the lazy approach from 29f2da8d1: we toggle only 2 times instead
of 8 times for an empty prompt.

There are only two places left where we call signal_check_cancel()
without necessarily disabling the disambiguate flag
1. open_cloexec() we assume that the files we open outside eval_node()
   are never blocking fifos.
2. fire_delayed(). Judging by commit history, this check is not
   relevant for interactive sessions; we'll soon end up calling
   eval_node() anyway.

In future, we can leave bracketed paste, modifyOtherKeys and
application keypad mode turned on again, until we actually run an
external command.  We really only want to turn off the disambiguate
flag.

Since this is approach is overly complex, I plan to go with either
of these two alternatives in future:
- extend the kitty keyboard protocol to optionally support VINTR,
  VSTOP and friends.  Then we can drop most of these changes.
- poll stdin for ctrl-c. This promises a great simplification,
  because it implies that terminal ownership (term_steal/term_donate)
  will be perfectly synced with us enabling kitty keyboard protocol.
  This is because polling requires us to turn off ICANON.
  I started working on this change; I'm convinced it must work,
  but it's not finished yet. Note that this will also want to
  add stdin polling to builtin wait.

Closes #10864
2024-11-24 16:11:57 +01:00
Johannes Altmanninger
29f2da8d18 Toggle terminal protocols lazily
Closes #10494
2024-05-16 12:26:47 +02:00
Fabian Boehm
9e6a661c00 One more sleep 2024-05-08 16:35:00 +02:00
Fabian Boehm
29f9d3d843 tests/signals.py: Increase a timeout
10ms is *much* too short
2024-05-07 17:55:26 +02:00
ridiculousfish
2bbeed157b Further improvements to signals.py test
Get it passing again on macOS.
2024-05-06 10:26:32 -07:00
ridiculousfish
269b18532d Fix Ctrl-C signals test
Prior to this change, signals.py attempted to generate Ctrl-C (SIGINT) by
sending \x03 to stdin. But with the change to use the CSI U sequence, Ctrl-C no
longer generates SIGINT.

Switch to sending SIGINT directly. Also switch up some of the sleep constants so
that a sleep command can't be confused with another one.
2024-05-06 10:26:32 -07:00
Fabian Boehm
c43f7fbe9c tests: Add another sleep 2024-04-30 16:47:44 +02:00
Fabian Boehm
ac8b1db899 tests: More timeout 2024-04-25 21:36:31 +02:00
Fabian Boehm
16eeba8f65 pexpects: More timeouts 2024-04-23 21:59:40 +02:00
Johannes Altmanninger
bdd478bbd0 Disable focus reporting on non-tmux again for now
We sometimes leak ^[[I and ^[[O focus reporting events when run from VSCode's
"Run python file" button in the top right corner. To reproduce I installed
the ms-python extension set the VSCode default shell to fish and repeatedly
ran a script that does "time.sleep(1)". I believe VSCode synthesizes keys
and triggers a race condition.

We can probably fix this but I'm not sure when I'll get to it (given how
relatively unimportant this feature is).

So let's go back to the old behavior of only enabling focus reporting in tmux.

I believe that tmux is affected by the same VSCode issue (also on 3.7.1 I
think) but I haven't been able to get tmux to emit focus reporting sequences
yet.  Still, keep it to not regress cursor shape (#4788).  So far this is
the only motivation for focus reporting and I believe it is only relevant
for terminals that can split windows (though there are a bunch that do).

Closes #10448
2024-04-18 10:38:15 +02:00
Johannes Altmanninger
f062ad3ad6 Try to fix macOS CI by disabling fg.py, signals.py, torn_escapes.py
These work fine AFAICT, just not in CI.
2024-04-12 11:27:55 +02:00
Johannes Altmanninger
f285e85b0c Enable focus reporting only just before reading from stdin
Some terminals send the focus-in sequences ("^[I") whenever focus reporting is
enabled.  We enable focus reporting whenever we are finished running a command.
If we run two commands without reading in between, the focus sequences
will show up on the terminal.

Fix this by enabling focus-reporting as late as possible.

This fixes the problem with `^[I` showing up when running "cat" in
gnome-terminal https://github.com/fish-shell/fish-shell/issues/10411.

This begs the question if we should do the same for CSI u and bracketed paste.
It's difficult to answer that; let's hope we find motivating test cases.
If we enable CSI u too late, we might misinterpret key presses, so for now
we still enable those as early as possible.

Also, since we now read immediately after enabling focus events, we can get
rid of the hack where we defer enabling them until after the first prompt.
When I start a fresh terminal, the ^[I no longer shows up.
2024-04-06 11:22:19 +02:00
Fabian Boehm
3af849d739 tests/pexpect: Fix \d escape 2024-04-02 22:41:54 +02:00
Johannes Altmanninger
8bf8b10f68 Extended & human-friendly keys
See the changelog additions for user-visible changes.

Since we enable/disable terminal protocols whenever we pass terminal ownership,
tests can no longer run in parallel on the same terminal.

For the same reason, readline shortcuts in the gdb REPL will not work anymore.
As a remedy, use gdbserver, or lobby for CSI u support in libreadline.

Add sleep to some tests, otherwise they fall (both in CI and locally).

There are two weird failures on FreeBSD remaining, disable them for now
https://github.com/fish-shell/fish-shell/pull/10359/checks?check_run_id=23330096362

Design and implementation borrows heavily from Kakoune.

In future, we should try to implement more of the kitty progressive
enhancements.

Closes #10359
2024-04-02 14:35:16 +02:00
Fabian Boehm
ac9c5ed1b2 Retry open_cloexec for signals other than SIGINT
Fixes #10250
2024-01-25 11:14:31 +01:00
Fabian Boehm
946ecf235c Restyle fishscript and python 2023-06-01 18:20:19 +02:00
Fabian Boehm
48db9e6a3f tests/signals.py: Increase a sleep 2023-01-18 16:48:49 +01:00
Fabian Boehm
9e9c73e46a tests/signals.py: Kill leftover sleeps from python
This starts two sleep processes and expects them to be killed on
SIGHUP.

Unfortunately, if this ever fails the second run will also fail
because it'll see the old sleep still lying around (because it'll run
for 130 seconds).

So, what we do is:

1. Keep the pids for these specific sleeps
2. Check if any of them are still running (and only fail for them)
3. Kill them from python

Fixes #9152
2022-08-23 18:47:52 +02:00
Fabian Boehm
8f08fe80fd Restyle codebase
Not a lot of changes, tbh
2022-06-16 18:43:28 +02:00
ridiculousfish
9070ed8039 Correct the order of pkill arguments
On macOS, the tests would often fail because calls to `pkill` would "leak"
across tests: kill processes run by other tests. This is because on macOS,
the -P argument to pkill must come before the process name. On Linux it
doesn't matter.

This improves test reliability on Mac.
2021-09-04 13:34:52 -07:00
ridiculousfish
2ca66cff53 Disable job control inside command substitutions
This disables job control inside command substitutions. Prior to this
change, a cmdsub might get its own process group. This caused it to fail
to cancel loops properly. For example:

    while true ; echo (sleep 5) ; end

could not be control-C cancelled, because the signal would go to sleep,
and so the loop would continue on. The simplest way to fix this is to
match other shells and not use job control in cmdsubs.

Related is #1362
2021-08-18 22:20:03 +08:00
Fabian Homborg
689e96263e tests: Add yet another sleep 2020-09-27 09:22:45 +02:00
ridiculousfish
0379f21870 Correctly cancel on receipt of SIGHUP
When we receive SIGHUP, stop executing normal fish script (but allow
exit handlers to run).
2020-08-30 15:09:34 -07:00
Fabian Homborg
1c6953ce79 tests/signals: Add missing sys import
This was only used if it failed for sys.exit(1)
2020-08-12 18:38:15 +02:00
Fabian Homborg
a7305c7082 tests: Increase timeout for signals test 2020-08-10 22:03:51 +02:00
Fabian Homborg
149a0b98af Another formatting run
I really kinda hate how insistent clang-format is to have line
breaks *IFF THE LINE IS TOO LONG*.

Like... lemme just add a break if it looks better, will you?

But it is the style at this time, so we shall tie an onion to our
belt.
2020-06-24 20:43:56 +02:00
Fabian Homborg
93c3aaf5f4 Port signals test to pexpect 2020-06-13 15:38:54 +02:00