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
After commit 6dd6a57c60, 3 remaining
builtins were affected by uint8_t overflow: `exit`, `return`, and
`functions --query`.
This commit:
- Moves the overflow check from `builtin_set_query` to `builtin_run`.
- Removes a conflicting int -> uint8_t conversion in `builtin_return`.
- Adds tests for the 3 remaining affected builtins.
- Simplifies the wording for the documentation for `set --query`.
- Does not change documentation for `functions --query`, because it does
not state the exit code in its API.
- Updates the CHANGELOG to reflect the change to all builtins.
This was lost in
6bdbe732e40c2e325aa15fcf0f28ad0dedb3a551..c7160d7cb4970c2a03df34547f357721cb5e88db.
Note that we only print a term-support flog message for now, the
warning seems a bit much.
Fixes#7709.
Prior to this fix, if stdin were explicitly closed, then builtins would
silently fail. For example:
count <&-
would just fail with status 1. Remove this limitation and allow each
builtin to handle a closed stdin how it sees fit.
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
Prior to this change, if you pipe a builtin to another process, it would
be buffered. With this fix the builtin will write directly to the pipe if
safe (that is, if the other end of the pipe is owned by some external
process that has been launched).
Most builtins do not produce a lot of output so this is somewhat tricky to
reproduce, but it can be done like so:
bash -c 'for i in {1..500}; do echo $i ; sleep .5; done' |
string match --regex '[02468]' |
cat
Here 'string match' is filtering out numbers which contain no even digits.
With this change, the numbers are printed as they come, instead of
buffering all the output.
Note that bcfc54fdaa fixed this for the case where the
builtin outputs to stdout directly. This fix extends it to all pipelines
that include only one fish internal process.
Add compile-time checks to ensure list of string subcommands, builtins,
and electric variables are kept in asciibetical order to facilitate
binary search lookups.
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.
These functions are called in the event queue hot path every time an
input event takes place. If we could guarantee a maximum length of
non-char (i.e. readline) events in the queue, we could use
`event_queue_peeker_t` with a fixed storage size of, e.g., 32 events,
but I'm not sure what a reasonable number would in fact be, so I'm just
changing these to use a thread-local vector that will re-use its
previous heap allocation in subsequent invocations rather than thrashing
the heap.
The lookups are executed on all input events, so they are worth
optimizing.
Cache the list of names, use binary search to get a function code from a
name, and stop enumerating mappings after `has_function` and `has_command`
have been determined.
builtin_set_query returns the number of missing variables. Because the
return value passed to the shell is an 8-bit unsigned integer, if the
number of missing variables is a multiple of 256, it would overflow to 0.
This commit saturates the return value at 255 if there are more than 255
missing variables.
builtin_test stashes some variables in statics, to support
the `test -t` expression. However this will cause conflicts with
concurrent execution, where we may want to run two `test` expressions at
once. Do the grunt work of threading the data into all places it needs
to go.
fish isn't quite sure what to do if the user specifies an fd redirection
for builtins. For example `source <&5` could potentially just read from
an arbitrary file descriptor internal to fish, like the history file.
fish has some lame code that tries to detect these, but got the sense
wrong. Fix it so that fd redirections for builtins are restricted to
range 0 through 2.
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.
The iothread pool has a feature where, if the thread is emptied, some
threads will choose to wait around in case new work appears, up to a
certain amount of time (500 msec). This prevents thrashing where new
threads are rapidly created and destroyed as the user types. This is
implemented via `std::condition_variable::wait_for`. However this function
is not properly instrumented under Thread Sanitizer (see
https://github.com/google/sanitizers/issues/1259) so TSan reports false
positives. Just disable this feature under TSan.
fd_monitor_t allows observing a collection of fds. It also has its own
fd, which it uses to awaken itself when there are changes. Switch to
using fd_event_signaller_t instead of a pipe; this reduces the number of
file descriptors and is more efficient under Linux.
queues use std::deque under the hood which is more expensive than a vector.
We always consume the entire queue so there is no advantage to use deque here.
Just use a vector.
Replace the complicated implementation which shared a condition variable, with
one which just uses std::future<void>. This may allocate more condition
variables but is much simpler.
Fish was previously oblivious to the existence of mouse-tracking ANSI
escapes; this was mostly OK because they're disabled by default and we
don't enable them, but if a TUI application that turned on mouse
reporting crashed or exited without turning mouse reporting off, fish
would be left in an unusable state as all mouse reporting CSI sequences
would be posted to the prompt.
This can be tested by executing `printf '\x1b[?1003h'` at the prompt,
then clicking with any mouse button anywhere within the terminal window.
Previously, this would have resulted in seeming garbage being spewed to
the prompt; now, fish detects the mouse tracking CSIs posted to stdin by
the terminal emulator and a) ignores them to prevent invalid input, as
well as b) posts the CSI needed to disable future mouse tracking events
from being emitted on subsequent mouse interactions (until re-enabled).
Note that since we respond to a mouse tracking CSI rather than
pre-emptively disable mouse reporting, we do not need to do any sort of
feature detection to determine whether or not the terminal supports
mouse reporting (otherwise, if it didn't support it and we posted the
CSI anyway, we'd end up with exactly the kind of cruft posted to the
prompt that we're trying to avoid).
Fixes#4873
This is a stack-allocating utility class to peek up to N
characters/events out of an `event_queue_t` object. The need for a
hard-coded maximum peek length N at each call site is to avoid any heap
allocation, as this would be called in a hot path on every input event.
This allows directly inserting multiple characters/events in one go at
the front of the input queue, instead of needing to add them one-by-one
in reverse order.
In addition to improving performance in case of fragmented dequeue
allocation, this also is less error prone since a dev need not remember
to use reverse iterators when looping over a vector of peeked events.
Under non-Linux builds, binary_semaphore is implemented with a
self-pipe. When TSan is active we mark the pipe as non-blocking as TSan
cannot interrupt read (but can interrupt select). However we weren't
properly testing for EAGAIN leading to an assertion failure.
Allow looping on EAGAIN.
io_buffer_t is a buffer that fills itself by reading from a file
descriptor (typically a pipe). When the file descriptor is widowed, the
operation completes, and it reports completion by marking a
`std::promise<void>`. The "main thread" waits for this by waiting on the
promise's future. However TSan was reporting that the future's destructor
races with its promise's wait method. It's not obvious if this is valid,
but we can fix it by keeping the promise alive until the io_buffer_t is
deallocated.
This fixes the TSan issues reported under
`complete_background_fillthread_and_take_buffer` for #7681 (but there
are other unresolved issues).
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.
fds.h will centralize logic around working with file descriptors. In
particular it will be the new home for logic around moving fds to high
unused values, replacing the "avoid conflicts" logic.
This needs to be rewritten, I'm pretty sure we have like 6 of these
kinds of ad-hoc "is this quoted" things lying around.
But for now, at least don't just check if the *previous* character was
a backslash.
Fixes#7685.
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.
While the user waits at the prompt, fish is waiting in select(), on stdin.
The sigio based universal notifier interrupts select() by arranging for a
signal to be delivered, which causes select() to return with EINTR.
However we weren't polling the notifier at that point so we would not
notice uvar changes, until we got some real input.
I didn't notice this when testing, because my testing was changing fish
prompt colors which updated the prompt for other reasons.
Fixes#7671.
Now that we have multiple clients of count_preceding_backslashes, factor
it out from fish_indent into wcstringutil.h, and then use the shared
implementation.
This goes to a separate file because that makes option parsing easier
and allows profiling both at the same time.
The "normal" profile now contains only the profile data of the actual
run, which is much more useful - you can now profile a function by
running
fish -C 'source /path/to/thing' --profile /tmp/thefunction.prof -c 'thefunction'
and won't need to filter out extraneous information.
Expansion parses slices like "$PATH[1..2]", but so does "set" when assigning
"set PATH[1..2] . .". Commit be06f842a ("Allow to omit indices in index
range expansions") forgot the latter.
Just like OPOST this just breaks output for anything not prepared for
it. Fish itself might work with it (and #4505 recommends it), but external commands are broken.
You'll see output like
foo
⏎
from `echo foo`.
Fixes#4873.
Continuation of #7133.
If given a windows path like `F:\foo`, this currently ends up
assert()ing in path_normalize_for_cd.
Instead, since these paths violate a bunch of assumptions we make, we
reject them and fall back on getting $PWD via getcwd() (which should
give us a nice proper unixy path).
Fixes#7636.
This isn't tested because it would require a system where a windowsy
path passes paths_are_same_file, and on the unix systems we run our
tests that's impossible as far as I can tell?
This used to print a literal DEL character in the output for `bind`,
which wouldn't actually show up and made it hard to figure out what
the key was.
So we just escape it back to how we actually used it - `\x7f`.
Fixes#7631.
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.
These are a foreground and a background color. Now I see the point in
not naming them "foreground_color" and "background_color", but at
least "fg" and "bg" should do, right?
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.
A weird interaction between grouped short options and our weird option
parsing that puts unknown options back:
```
echo "-n foo"
```
would see the `-n`, turn off printing newlines, interpret the " " as
another grouped short option, see that there is no short option for
space and put the entire token back on the arguments pile.
So it would print "-n foo" *without a newline*.
Fix this by keeping an old state of the options around and reverting
it when putting options back.
The alternative is *probably* to forbid the " " short option in
wgetopt, then check if an option group contains it and error out, but
this should only really be a problem in `echo` because that is,
AFAICT, the only thing that puts the options back.
Fixes#7614
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
This removes the 100 msec timeout from io_buffer_t. We no longer need to
periodically wake up to check if a command substitution is finished,
because we get explicitly poked when that happens.
