This concerns the problem of "injecting" fancy fish bits like job reaping
into the "common" input stuff which is also used by fish_key_reader.
Instead of providing a callback, make the input event queue a base class
with virtual functions. This allows for a richer interface and simplifies
some memory management issues.
select_wrapper_t wraps up the annoying bits of using select(): keeping
track of the max fd, passing null for boring parameters, and
constructing the timeout. Introduce a wrapper struct for this and
replace the existing uses of select() with the wrapper.
If fish launches a program and that program marks stdin as O_ASYNC, then
fish will start receiving SIGIO events on Mac. This occurs even though
the file descriptor itself does not have the O_ASYNC flag set.
SIGIO is reported as interrupting select which then breaks multiple-key
bindings, especially in vi-mode.
As the SIGIO based universal notifier is disabled, remove it and the
SIGIO handler itself. This allows fish to ignore properly ignore SIGIO.
Fixes#7853
Several functions including wgetopt and execve operate on null-terminated
arrays of nul-terminated pointers: a list of pointers to C strings where
the last pointer is null. Prior to this change, each process_t stored its
argv in such an array. This had two problems:
1. It was awkward to work with this type, instead of using std::vector,
etc.
2. The process's arguments would be rearranged by builtins which is
surprising
Our null terminated arrays were built around a fancy type that would copy
input strings and also generate an array of pointers to them, in one big
allocation.
Switch to a new model where we construct an array of pointers over
existing strings. So you can supply a `vector<string>` and now
`null_terminated_array_t` will just make a list of pointers to them. Now
processes can just store their argv in a familiar wcstring_list_t.
Prior to this change, builtins would take their arguments as `wchar_t **`.
This implies that the order of the arguments may be changed (which is
true, `wgetopter` does so) but also that the strings themselves may be
changed, which no builtin should do.
Switch them all to take `const wchar_t **` instead: now the arguments may
be rearranged but their contents may no longer be modified.
- Check for special characters *before* attempting to parse
- Also ignore lines with `{` and `*`
- Also skip lines with `<<` because that might be a heredoc (or a
- `<<<` herestring)
Fixes#7874.
Previously wbasename and wdirname wrapped the system-provided basename
and dirname. But these have thread-safety issues and some surprising
error conditions on Mac. Just reimplement these per the OpenGroup spec.
In particular these no longer trigger a null-dereference if the input
exceeds PATH_MAX.
Add some tests too.
This fixes#7837
Creating a file called "xfoo" could break the highlight tests because
we'd suddenly get a color with valid_path set to true.
So what we do is simply compare foreground/background and forced
underline, but only check for path validity if we're expecting a valid
path.
If we're not expecting a valid path, we don't fail whether it is there
or not.
This means that we can't check for a non-valid path, but we don't
currently do that anyway and we can just burn that bridge when we get
to it.
cc @siteshwar @krobelus, who both came across this
Similar to what fish_indent does. After typing "echo \" and hitting return,
the cursor will be indented.
A possible annoyance is that when you have multiple indented lines
echo 1 \
2 \
3 \
4 \
If you remove lines in the middle with Control-k, the lines below
the deleted one will start jumping around, as they are disconnected
from and reconnected to "echo".
If a variable is undefined, but it looks like it will be defined by the
current command line, assume the user knows what they are doing.
This should cover most real-world occurrences.
Closes#6654
In an interactive shell, typing "for x in (<RET>" would print an error:
fish: Expected end of the statement, but found a parse_token_type_t::tokenizer_error
Our tokenizer converts "(" into a special error token, hence this message.
Fix two cases by not reporting errors, but only if we allow parsing incomplete
input. I'm not really sure if this is necessary, but it's sufficient.
Fixes#7693
This may slightly improve performance by allowing the compiler greater
visibility into what is happing on top of not executing at runtime in
some hot paths, but more importantly, it gets rid of magic constants in a
few different places.
This introduces a new variable $fish_color_keyword that will be used
to highlight keywords. If it's not defined, we fall back on
$fish_color_command as before.
An issue here is that most of our keywords have this weird duality of
also being builtins *if* executed without an argument or with
`--help`.
This means that e.g.
if
is highlighted as a command until you start typing
if t
and then it turns keyword.
This concerns how fish prevents its own fds from interfering with
user-defined fd redirections, like `echo hi >&5`. fish has historically
done this by tracking all user defined redirections when running a job,
and ensuring that pipes are not assigned the same fds. However this is
annoying to pass around - it means that we have to thread user-defined
redirections into pipe creation.
Take a page from zsh and just ensure that all pipes we create have fds in
the "high range," which here means at least 10. The primary way to do this
is via the F_DUPFD_CLOEXEC syscall, which also sets CLOEXEC, so we aren't
invoking additional syscalls in the common case. This will free us from
having to track which fds are in user-defined redirections.
Previously we sometimes wanted to access an io_buffer_t to append to it
directly, but that's no longer true; all we really care about is its
separated_buffer_t. Make io_bufferfill_t::finish return the
separated_buffer directly, simplifying call sites. No user visible changes
expected here.
This concerns builtins writing to an io_buffer_t. io_buffer_t is how fish
captures output, especially in command substitutions:
set STUFF (string upper stuff)
Recall that io_buffer_t fills itself by reading from an fd (typically
connected to stdout of the command). However if our command is a builtin,
then we can write to the buffer directly.
Prior to this change, when a builtin anticipated writing to an
io_buffer_t, it would first write into an internal buffer, and then after
the builtin was finished, we would copy it to the io_buffer_t. This was
because we didn't have a polymorphic receiver for builtin output: we
always buffered it and then directed it to the io_buffer_t or file
descriptor or stdout or whatever.
Now that we have polymorphpic io_streams_t, we can notice ahead of time
that the builtin output is destined for an internal buffer and have it
just write directly to that buffer. This saves a buffering step, which is
a nice simplification.
This sometimes fails on github actions with ASAN. I am assuming that's
because the ctrl-c happens *before* the process has had a chance to
start.
So we do what we do and increase the delay.
Prior to this change, histories were immortal and allocated with either
unique_ptr or just leaked via new. But this can result in races in the
path detection test, as the destructor races with the pointer-captured
history. Switch to using shared_ptr.
When adding a command to history, we first expand its arguments to see
if any arguments are paths which refer to files. If so, we will only
autosuggest that command from history if the files are still valid. For
example, if the user runs `rm ./file.txt` then we will remember that
`./file.txt` referred to a file, and then only autosuggest that if the file
is present again.
Prior to this change we only performed simple expansion relative to the
working directory. This change extends it to variables and tilde
expansion. For example we will now apply the same hinting for
`rm ~/file.txt`
Fixes#7582
In preparation for fixing #7559, add a function poke_item to fd_monitor.
fd_monitor has a list of file descriptors, and invokes a callback when an
fd becomes readable. With this change, we assign each item a unique ID and
return it when the item is added; the ID may then be used to invoke the
callback explicitly.
The idea is that we can stop reading from the pipe associated with the
cmdsub when the job is finished, even if the pipe is still open.
Commands that start with a space should not be written to the history
file. Prior to this change, that was implemented by simply not adding them
to history. Items with leading spaces were simply dropped.
With this change, we add a 'history_persistence_mode_t' to
history_item_t, which tracks how the item persists. Items with leading
spaces are now marked as "ephemeral": they can be recovered via up arrow,
until the user runs another command, or types a space and hits return.
This matches zsh's HIST_IGNORE_SPACE feature.
Fixes#1383
A mildly interesting one is the call to test_wchar2utf8 with a non-null
pointer ("u1"/"dst") but 0 length. In this case we relied on malloc(0)
returning non-null which is not guaranteed.
src/fish_tests.cpp:1619:23: warning: Call to 'malloc' has an allocation
size of 0 bytes [clang-analyzer-optin.portability.UnixAPI]
mem = (char *)malloc(dlen);
^
test_wchar2utf8(w1, sizeof(w1) / sizeof(*w1), u1, 0, 0, 0,
"invalid params, dst is not NULL");
Before running a command, or before importing a command from bash history,
we perform error checking. As part of error checking we expand commands
including variables and globs. If the glob is very large, like `/**`, then
we could hang expanding it.
One fix would be to limit the amount of expansion from the glob, but
instead let's just not expand command globs when performing error checking.
Fixes#7407
This adds the ability to limit how many expansions are produced. For
example if $big contains 10 items, and is Cartesian-expanded as
$big$big$big$big... 10 times, we would naviely get 10^10 = 10 billion
results, which fish can't actually handle. Implement this in
completion_receiver_t, which now can return false to indicate an overflow.
The initial expansion limit 'k_default_expansion_limit' is set as 512k
items. There's no way for users to change this at present.
