fish will print messages for some jobs when they exit abnormally, such as
with SIGABRT. If a job exits abnormally inside the prompt, then (prior to
this commit) fish would print the message and re-trigger the prompt, which
could result in an infinite loop. This has existed for a very long time.
Fix it by reaping jobs after running the prompt, and NOT triggering a
redraw based on that reaping. We still print the message but the prompt is
not executed.
Add a test.
Fixes#9796
This built on my test system, might be version differences.
(it's also not enough to make it *work*, but a necessary step)
This reverts commit 6fded249cd.
Commit 29dc30711 (Insert some completions with quotes instead of
backslashes, 2024-04-13) wrongly copmletes
$ cat ~/space
to
$ cat '~/path with spaces'
Today completions can be either replacing or appending. We never quote
(but backslash-escape) appending completions (unless they "append"
to an empty token). We always quote replacing completions. The
assumption in this part of the code is that replacing completions
can be quoted without changing meaning.
This assumption is wrong for tildes. For the backslash-escaping code
path, we take care of this edge case via a special DONT_ESCAPE_TILDES
flag. However that flag does not take effect when using quotes for
escaping. Fix that.
Unfortunately, e97a4fab7 (Escape : and = in file completions,
2024-04-19) introduced a (hopefully temporary) code clone in
escape_separators, which made added an extra step to debugging here.
When built with the default "installable" feature, the data files (share/) are
included in the fish binary itself.
Run `fish --install` or `fish --install=noconfirm` (for
non-interactive use) to install fish's data files into ~/.local/share/fish/install
To figure out if the data files are out of date, we write the current version
to a file on install, and read it on start.
CMake disables the default features so nothing changes for that, but this allows installing via `cargo install`,
and even making a static binary that you can then just upload and have extract itself.
We set $__fish_help_dir to empty for installable builds, because we do not have
a way to generate html docs (because we need fish_indent for highlighting).
The man pages are found via $__fish_data_dir/man
This now allows:
- Same argument (`random 5 5`)
- Swapped ends (`random 10 2`)
- One possibility (`random 0 5 4`)
This makes it easier to use with numbers generated elsewhere instead
of hard-coded, so you don't need to check as much before running it.
Fixes#10879
We don't really care if the process has a custom handler installed, we
can just set it to default.
The one we check is SIGHUP, which may be given to us via `nohup`.
This saves ~30 syscalls *per process* we spawn, so:
```fish
for f in (seq 1000)
command true
end
```
has ~30000 fewer rt_sigaction calls. These take up about ~30% of the
total time spent in syscalls according to strace.
We could also compute this set once at startup and then reuse it.
exists_no_autoload() wrongly thinks that tombstoned functions can be
autoloaded; fix that.
While at-it replace the use of get_props() with something simpler.
Co-authored-by: Himadri Bhattacharjee
Closes#10873
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
Moving the "make empty ToLowercase iterator" logic to within the
`unwrap_or_else()` instead of always generating it brings most of the speedup;
unrolling the recursive call brings in the rest.
Using `c.is_uppercase()` instead of getting the iterator and checking if the
first (and only) lowercase letter of the sequence is the same as the original
input is 5-8x faster (measured via criterion against `/usr/share/dict/words`).
(Additional benefit of forcibly inlining the now iterator-based comparison not
taken into account; this necessitated changing from a closure to a local
function as the inline attribute on closures is not yet supported with the
stable compiler toolchain.)
This is still suboptimal because we are allocating a vector of indices to be
removed (but allocation-free in the normal case of no duplicates) but
significantly better than the previous version of the code that duplicated the
strings (which are larger and spread out all over the heap).
The ideal code (similar to what we had in the C++ version, iirc) would look like
this, but it's not allowed because the borrow checker hates you:
```
fn unique_in_place_illegal(comps: &mut Vec<Completion>) {
let mut seen = HashSet::with_capacity(comps.len());
let mut idx = 0;
while idx < comps.len() {
if !seen.insert(&comps[idx].completion) {
comps.remove(idx);
continue;
}
idx += 1;
}
}
```
If a semicolon-delimited list of CSI parameters contained an (invalid) long
sequence of ascii numeric characters, the original code would keep multiplying
by ten and adding the most recent ones field until the `params[count][subcount]`
u32 value overflowed.
This was found via automated fuzz testing of the `try_readch()` routine against
a corpus of some proper/valid CSI escapes.
This lets us call into the entirety of the prior `readch()` with an exhaustible
input stream without panicking on the `unreachable!()` call. The previous
functionality is kept under the old name by calling `try_readch()` with the
`blocking` parameter set to `true` (100% same behavior as before).
While the `try_readch(false)` entrypoint isn't used directly by the current fish
codebase, it is required in order to automate input reader tests without the
overhead and complexity of running the test harness in a tty emulator emulator
like pexpect or tmux, which moreover necessitates out-of-process testing – which
is incompatible with most perf-guided testing harnesses.
I hope to be able to upstream harness integrations using this entry point in the
near future.
`print_help` is a hacky-wacky function used to support the `--help` command
of `fish_key_reader` and others. The Rust version panics on an error; fix
that and make it print more useful help messages.
I'm guessing this was missed in the port because there were comments referencing
using a hash set to perform the deduplication but there was no hashset. (The
TODO was added later.)
This caught an incorrect description for process/job exit handlers for ANY_PID
(now removed) which has been replaced with a message stating the handler is for
any process exit event.
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
ctrl-r ctrl-s ctrl-s
Attemps to go before the beginning and asserts out. Instead refuse to
do that.
(there's some weirdness where it can reduce the pager to the first
entry if you keep pressing, which I haven't found yet, but that's better than *crashing*)
These are another way to spell the same thing that doesn't match what
`bind` would print.
They're also not documented and tested thoroughly.
Since they are just small shortcuts and unreleased we can just remove
them.
Fixes#10845
Given "printf %18446744073709551616s", we parse the number only in
the printf crate, which tells us that we overflowed somwhere (but
not where exactly).
We were previously printing the internal `INVALID_PID` value (since removed),
which was a meaningless `-2` constant, when there was no pgid associated with a
job.
This PR changes that to `-` to indicate no pgid available, which I prefer over
something like `0` or `-1`, but will cause problems for code that is hardcoded
to convert this field to an integral value.
