Commit graph

31 commits

Author SHA1 Message Date
Peter Ammon
5d10bc6a02
Fix/suppress some clippies
needless_lifetimes is annoying; supress that. Fix some spacing that clippy was
complaining about.
2024-11-30 12:21:30 -08:00
Johannes Altmanninger
b89619330b Disable terminal protocols before cancellable operations
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
Mahmoud Al-Qudsi
4061ef7137 Remove unnecessary Pid::get() calls 2024-11-14 13:02:03 -06:00
Mahmoud Al-Qudsi
fc47d9fa1d Use strongly typed Pid for job control 2024-11-14 13:02:03 -06:00
Mahmoud Al-Qudsi
95ac51101e Use Option<Pid> instead of Option<pid_t>
Statically assert that the interior value is both positive and non-zero.
2024-11-14 13:02:03 -06:00
Mahmoud Al-Qudsi
3307672998 Use type safety for pid values
The previous approach of "treat this field as an `Option<NonZeroU32>` and
remember to check `p.has_pid()` before accessing it" was a mix of C++ and rust
conventions and led to some bugs or incorrect behaviors.

* `jobs -p` would previously print both the (correct) external pid and the
  (incorrect) internal value of `0` if a backgrounded command contained a
  fish function (e.g. `function foo; end; cat | foo &; jobs`)
* Updating/calculating job cpu time and usage was incorrectly including all of
  fish's cpu usage/time for each function/builtin member of the job pipeline.

Closes #10832
2024-11-14 13:02:03 -06:00
Mahmoud Al-Qudsi
33a170d614 Replace INVALID_PID constant with Option<NonZeroU32>
If we end up using this in more places, we can create a `Pid` newtype.
Note that while the constant is no longer used in code, its previous value of -2
is still printed by `jobs` when no pgid is associated with a job. I will open a
PR to change this to something else, likely either `0` or `-`.
2024-11-04 16:19:22 -06:00
Johannes Altmanninger
2dbaf10c36 Also refresh TTY timestamps after external commands from bindings
Commit ba67d20b7 (Refresh TTY timestamps after nextd/prevd, 2024-10-13)
wasn't quite right because it also needs to fix it for arbitrary commands.

While at it, do this only when needed:
1. It seems to be only relevant for multiline prompts.
   Note that we can wait until after evaluation to check if the prompt is
   multiline, because repaint events go through the queue, see 5ba21cd29
   (Send repaint requests through the input queue again, 2024-04-19).
2. When the binding doesn't execute any external command, we probably don't
   need to fix up whatever the user printed. If they actually wanted to show
   output and print another prompt, they should currently use  "__fish_echo",
   to properly support multiline prompts. Bindings should produce no other
   output. What distinguishes external programs is that they can trigger this
   issue even if they don't  produce any output that remains visible in fish,
   namely by using the terminal's alternate screen.
   Would be nice if we could get rid of __fish_echo; I'm not yet sure how.

Fixes #10800
2024-10-21 12:13:00 +02:00
Johannes Altmanninger
30cba03bf9 Make SIGTERM handler async-signal-safe again 2024-10-21 09:30:47 +02:00
Johannes Altmanninger
15b08cbcab Make import style less noisy 2024-09-01 14:05:48 +02:00
Mahmoud Al-Qudsi
d90d924c8c Remove parser library_data_pod_t ffi workaround
We don't need to separate POD fields from the main parser libdata any more.
2024-06-02 20:27:44 -05:00
Johannes Altmanninger
d40d2b786f Work around wants_terminal not begin set inside eval
On this binding we fail to disable CSI u

    bind c-t '
        begin
            set -lx FZF_DEFAULT_OPTS --height 40% --bind=ctrl-z:ignore
            eval fzf | while read -l r; echo read $r; end
        end
    '

because for "fzf", ParseExecutionContext::setup_group() returns early with the
parent process group (which should be fish's own) , hence "wants_terminal"
is false. This seems questionable, I don't think the eval should make a
difference here.

For now, don't touch it; use the more accurate way of detecting whether
a process may read keyboard input. In many of such cases "wants_terminal"
is false, like

    echo (echo 1\n2\n3 | fzf)

Fixes #10504
2024-05-18 20:55:06 +02:00
Johannes Altmanninger
29f2da8d18 Toggle terminal protocols lazily
Closes #10494
2024-05-16 12:26:47 +02:00
Mahmoud Al-Qudsi
5f8f799cf7 Replace C++ doc \return with "Return"
quick_replace '\\\\return(s)? ' 'Return$1 ' src/

Filtered to only lines beginning with //
2024-05-06 14:59:36 -05:00
Mahmoud Al-Qudsi
589639a87d Replace C++-style \p with Markdown backticks
quick_replace '\\\\p ([a-zA-Z0-9_]+)' '`$1`' src/

Filtered to only lines beginning with //
2024-05-06 14:59:23 -05:00
Anurag Singh
4fa8d95b98 Move push_timer to measure command substitution timing too 2024-05-03 09:32:26 +02:00
Johannes Altmanninger
fd61bad946 Further simplify terminal_protocols scoping
Remove the last non scoped place where we disable protocols (just before
exec(1)); it's not necessary with the current approach because we always
disable inside eval.
There is an edge case where we don't:

    fish -ic "exec bash"

leaving bash with CSI u enabled.  Disable that also in -ic mode where we
don't have a reader.

In future we should use the same approach for restore_term_mode() but I'm
not sure which one is better.
2024-04-21 21:32:44 +02:00
Johannes Altmanninger
8164855b70 Disable terminal protocols throughout evaluation
Test changes are very hacky, will cleanup later.

Closes #10408
2024-04-02 21:25:47 +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
Mahmoud Al-Qudsi
0ca199ef98 Change wopen_cloexec() to return File 2024-03-23 01:34:23 -05:00
Fabian Boehm
b03e727531 Remove unnecessary formatting 2024-03-09 12:06:24 +01:00
Fabian Boehm
46afbea72b exec: Pass some cstrs as cstr instead of converting to ptr and back 2024-02-16 19:40:41 +01:00
ridiculousfish
5021639db1 Correct some comments and duplicative error messages
If we fail to create a pipe, we will report that fact in multiple places; remove
some redundant error reporting.
2024-02-11 12:16:58 -08:00
PolyMeilex
b9ba9e57e8 Use nix & Results 2024-02-11 11:40:27 -08:00
PolyMeilex
971d774e67 Use OwnedFd in AutoClosePipes 2024-02-11 11:40:27 -08:00
PolyMeilex
6ef8125c96 Return OwnedFd from open_cloexec 2024-01-27 20:42:13 +01:00
PolyMeilex
2512849ece Use nix OFlag for open_cloexec 2024-01-27 20:42:13 +01:00
PolyMeilex
6915aeb44c Use nix mode for open_cloexec 2024-01-27 20:42:13 +01:00
PolyMeilex
23301e4895 Return Result from wopen_cloexec 2024-01-27 20:42:13 +01:00
Fabian Boehm
09cd7c7ad9 Remove widestring-suffix uses
This removes both the `#[widestrs]` annotation as well as all `"foo"L`
suffixes, and does a `cargo fmt` run on the result
2024-01-13 08:52:54 +01:00
Johannes Altmanninger
3ae20bdba0 Move fish-rust to project root 2024-01-13 03:58:33 +01:00
Renamed from fish-rust/src/exec.rs (Browse further)