The recent change to improve the behavior of the `bg` command (commit
3edb7d538) resulted in the `send_to_bg()` no longer using the `name`
parameter it was given. This rectifies that lint warning by removing
that parameter as it never served a useful purpose.
Switch from null terminated arrays to `wcstring_list_t` for lists of
special env var names. Rename `list_contains_string` to `contains` and
modify the latter interface to not rely on a `#define`.
Rename `list_contains_string()` to `contains()` and eliminate the
current variadic implementation. Update all callers of the removed
version to use the string list version.
There are at least three env vars describing a sequence of paths to be
searched where an empty path element is implicitly equivalent to ".".
This change converts the implicit "." to explicit whenever the variable
is imported or set. This makes the variable much easier to use in fish
scripts.
Fixes#3914
- Error out if anything that is not a PID is given
- Otherwise background all matching existing jobs, even if not all
PIDs exist (but print a message for the non-existing ones)
Fixes#3909.
A call to default_vars_path() takes the environment variable
lock while the uvars lock is held. Ensure that doesn't happen by
deferring the uvars lock to later in the function.
cached_esc_sequences_t::find_entry was constructing a wcstring
from a c string, using lengths longer than the length of the cstring.
Detected with asan.
In the process of fixing the issue I decided it didn't make sense to
have two, incompatible, ways of converting variable strings to arrays.
Especially since the one I'm removing does not return empty array elements.
Fixes#2106
This is the next step in determining whether we can disable blocking
signals without a good reason to do so. This makes not blocking signals
the default behavior. If someone finds a problem they can add this to
their ~/config/fish/config.fish file:
set FISH_NO_SIGNAL_BLOCK 0
Alternatively set that env var before starting fish. I won't be surprised
if people report problems. Till now we have relied on people opting in
to this behavior to tell us whether it causes problems. This makes the
experimental behavior the default that has to be opted out of. This will
give us a lot more confidence this change doesn't cause problems before
the next minor release.
Note that there are still a few places where we force blocking of
signals. Primarily to keep SIGTSTP from interfering with the shell in
response to manipulating the controlling tty. Bash is more selective
in the signals it blocks around the problematic syscalls (c.f., its
`git_terminal_to()` function). However, I don't see any value in that
refinement.
There should be just one place that calls `setupterm()`. While refactoring
the code I also decided to not make initializing the curses subsystem a
fatal error. We now try two fallback terminal names ("ansi" and "dumb")
and if those can't be used we still end up with a usable shell.
Fixes#3850
Before defining fallback functions of wcsdup(), wcscasecmp() and
wcsncasecmp(), use the std:: namespace functions instead if they exist.
0019c12af3 fixed the build on Solaris 10, but broke it on Solaris 11.
This reverts commit e30f3fee88.
Not sure why I didn't notice this before merging it but the change I'm
reverting makes it impossible to start a login shell.
This is the next step in determining whether we can disable blocking
signals without a good reason to do so. This makes not blocking signals
the default behavior. If someone finds a problem they can add this to
their ~/config/fish/config.fish file:
set FISH_NO_SIGNAL_BLOCK 0
Alternatively set that env var before starting fish. I won't be surprised
if people report problems. Till now we have relied on people opting in
to this behavior to tell us whether it causes problems. This makes the
experimental behavior the default that has to be opted out of. This will
give is a lot more confidence this change doesn't cause major problems
prior to the next minor release.
The previous change neglected to consider that numbers too large for the
long long datatype will result in calling strerror(ERANGE) whose return
value can vary depending on the platform. Which breaks the unit test.
The primary pupose of this change is to make OpenSUSE builds happy by
adding a `DIE()` call so its build toolchain knows we won't fall off the
end of function `selection_direction_is_cardinal()`.
I recently upgraded the software on my macOS server and was dismayed to
see that cppcheck reported a huge number of format string errors due to
mismatches between the format string and its arguments from calls to
`assert()`. It turns out they are due to the macOS header using `%lu`
for the line number which is obviously wrong since it is using the C
preprocessor `__LINE__` symbol which evaluates to a signed int.
I also noticed that the macOS implementation writes to stdout, rather
than stderr. It also uses `printf()` which can be a problem on some
platforms if the stream is already in wide mode which is the normal case
for fish.
So implement our own `assert()` implementation. This also eliminates
double-negative warnings that we get from some of our calls to
`assert()` on some platforms by oclint.
Also reimplement the `DIE()` macro in terms of our internal
implementation.
Rewrite `assert(0 && msg)` statements to `DIE(msg)` for clarity and to
eliminate oclint warnings about constant expressions.
Fixes#3276, albeit not in the fashion I originally envisioned.
I'm starting to wonder if IWYU is worth the effort. Nonetheless, this
makes it lint clean on macOS and reduces the number of warnings on
FreeBSD and Linux.
This puts a hard upper bound of 10 MiB on the amount of data that read
will consume. This is to avoid having the shell consume an unreasonable
amount of memory, possibly causing the system to enter a OOM condition,
if the user does something non-sensical.
Fixes#3712
* color: make brgrey really grey
The 0055 value is actually 0x2d which isn't 0x55 mentioned further and is probably a typo
* color.cpp: reformat color table
Tidy color table up and also fix hex number case for grey color. This should ease spotting errors like one from previous commit.
We now are stingier with taking history file locks - if the lock
is held too long we may just break it. But the current file save
architecture holds the lock for the duration of the save. It also
has some not-quite-right checks that can cause spurious failures in
the history stress test.
Reimplement the history save to retry. Rather than holding the lock,
rewrite the file to a temporary location and then take the lock. If
the history file has changed, start all over.
This is going to be slower under contention, but the advantage is that
the lock is only held for a brief period (stat + rename) rather than
across calls to write().
Some updated logic also fixes spurious failures that were easy to observe
when tsan was enabled. These failures were due to failing to check if the
file at the path was the same file we opened.
The next step is to move the history file saving to a background thread
to reduce the chances of it impacting user's typing.
Allow retrying, fix an issue where we trip over our own changes
by thinking the file has changed when we are responsible for changing
it, and improve some commenting
This class is used to accumulate data to be written to the history
file. It has some dubious optimizations around trying to track an
offset separately from the size of the buffer. After some investigation
these aren't helping, vector behaves fine on its own. So just make
this a simple wrapper around vector.
Cache the escape sequences we've seen when checking for those which
don't take any visual space when writing the prompt or similar strings.
This reduces the cost of determining the true cost of such strings by a
full order of magnitude if they include lots of such escape sequences.
Periodically sort the cached escape sequence lengths based on feedback
from cache hits so that we're always checking for the most likely
sequence lengths first.
Also cache the prompt layouts to avoid doing the calculations if the
prompt doesn't change.
Fixes#3793
Currently, if bind is run with --mode but not --sets-mode, the
binding gets an implicit --sets-mode equivalent to the mode. This
is usually unobservable but it may matter if the mode is changed
by some internal part of the binding (e.g. set fish_bind_mode...)
then that setting will be lost after the binding is complete.
This was an old experiment to compile scripts directly into the
shell itself, reducing the amount of I/O performed at startup.
It has not been used for a long time. Time to remove it.
Commit ab189a75 introduced a regression where we stop breaking out
of loops in response to a child death via a signal. Fix that regression.
Also introduces a test to help ensure we don't regress in the future.
Fixes#3780
The block stack is now sound, and no longer needs this ancient
cleanup logic, which tried to account for cases where blocks
were pushed but never popped.
Currently the block stack is just a vector of pointers.
Clients must manually use new() to allocate a block, and then
transfer ownership to the stack (so must NOT delete it).
Give the parser itself responsibility for allocating blocks too,
so that it takes over both allocation and deletion. Use unique_ptr
to make deletion less error-prone.
Previously we would try to walk all the blocks (from within the
signal handler!) and mark them as skipped. Stop doing that, it's
wildly unsafe.
Also rationalize how the skip flag is set per block. Remove places
that shouldn't set it (e.g. break and continue shouldn't set skip
on the loop block).
read_in_chunks does not clear the intermediate string 'str'
between iterations, so every chunk has every other chunk prepended
to it.
A secondary issue is that it calls str2wcstring() on an intermediate
chunk, which may split multi-byte sequences. This needs to be deferred
to the end.
Test added. Fixes#3756
This implements a way to use the `functions` command to perform
introspection to learn about the characteristics of a function. Such as
where it came from.
Fixes#3295
If the kernel reports a size of zero for the rows or columns (i.e., what
`stty -a` reports) fall back to the `COLUMNS` and `LINES` variables. If
the resulting values are not reasonable fallback to using 80x24.
Fixes#3740
Refactor `builtin_read()` to split the code that does the actual reading
into separate functions. This introduces the `read_in_chunks()` function
but in this change it is just a clone of `read_one_char_at_a_time()`. It
will be modified to actually read in chunks in the next change.
Partial fix for #2007
The shell was doing a log of signal blocking/unblocking that hurts
performance and can be avoided. This reduced the elapsed time for a
simple benchmark by 25%.
Partial fix for #2007
We should never use stdio functions that use stdout implicitly. Saving a
few characters isn't worth the inconsistency. Too, using the forms such
as `fwprintf()` which take an explicit stream makes it easier to find
the places we write to stdout versus stderr.
Fixes#3728
If the tty has been closed (i.e., become invalid) the `ttyname()`
function will return NULL. Passing that NULL to `strstr()` can crash
fish which means it won't kill its child processes and exit cleanly.
Another fix for #3644
mkostemp is not available on some older versions of macOS. In order
for our built binaries to run on them, mkostemp must be weak-linked.
On other systems, we use the autoconf check.
Introduce a function fish_mkstemp_cloexec which uses mkostemp if
it was detected and is available at runtime, else falls back to
mkstemp. This isolates some logic that is currently duplicated in
two places.
See #3138 for more on weak linking.
In order to use C++11 with the standard macOS Xcode toolset,
we must use libc++. This in turn requires using 10.7 as our
MIN_REQUIRED in the availability macros, which in turn marks
certain wide-character functions as strong symbols (since they
were introduced in 10.7).
Redeclare them as weak, so that we can run on 10.6 without link
errors. See #3138 for more.
GNU systems don't allow mixing narrow and wide IO, so some of these
messages were lost since 1621fa43d8.
stderr is also the more logical place for error output to end up.
Fixes#3704.
Emitting warnings about EPIPE errors when writing to stdout or stderr is
more annoying than helpful. So suppress that specific warning message.
Fixes#2516
A third-party plugin noticed that using `$CMD_DURATION` in the prompt
causes problems when combined with the recent changes to tighten up
parsing of strings meant to be integer values. This fixes the problem by
ensuring the var is defined before the first interactive command is run.
See https://github.com/fisherman/dartfish/issues/7
It was pointed out that the previous change to alert people to the fact
their completion scripts were using flags that are no longer valid
resulted in way too many warnings. This limits the warning to one per
session.
Fixes#3640
On some platforms, notably GNU libc, you cannot mix narrow and wide
stdio functions on a stream like stdout or stderr. Doing so will drop
the output of one or the other. This change makes all output to the
stderr stream consistently use the wide forms.
This change also converts some fprintf(stderr,...) calls to debug()
calls where appropriate.
Fixes#3692
* Add italics and dim modifier to set_color
* update documentation for set_color
* add reverse mode to set_color
* Use standout mode as fallback for reverse mode
* Apply patch from @Darkshadow2 adding additional modes
I noticed that universal variable tests were failing on Cygwin and
Dragonfly BSD. The failures were because we are attempting to verify the
correct behavior of mechanisms that are known to be broken on those
platforms. There are still uvar test failures on those platforms with
this change but they are due to actual problems rather than bugs in the
tests.
Fixes#3587
Using `\e` is clearer and shorter than `\x1b`. It's also consistent with how
we write related control chars; e.g., we don't write `\x0a` we write '\n'.
Update our implementation of the PROMPT_SP heuristic to match current
zsh behavior. This makes it behave better on terminals like ConEmu and
the native MS Windows console which automatically insert a newline when
writing to the last column of the line.
Fixes#789
There are several places that use writestr() which should instead be
using fwprintf() or equivalent. Also, clarify the documentation for why
writestr() and writechr() exist so they aren't used inappropriately
again.
Fixes#3657
The complete builtin had once -A / --authoritative and -u /
--unauthoritative switches which indicated whether all possibilities for
completion are specified and would cause an error if the completion was
authoritative and an unknown option was encountered.
This feature was functionally removed during one of the past parser
rewritings, but -A and -u still remained in parts of the code and
command completions, although having no effect.
This commit removes the leftovers and prints an warning whenever user
tries to run the complete command with -A / -u / --authoritative /
--unauthoritative switches.
Fixes#3640.
Commit 8d27f81a to change how background jobs are handled (killed rather
than left running) when the shell is exited did not correctly handle
the nested interactive context created by the `breakpoint` command. This
fixes that mistake. Now any background jobs that already existed, or were
created within the `breakpoint` context, are left running when exiting
that context.
Fish is not consistent with other shells like bash and zsh when exiting
an interactive shell with background jobs. While it is true that fish
explicitly claims no compatibility with POSIX 1003.1 this is an area
where deviation from the established practice adds negative value.
The reason for the current behavior seems to be due to two users who did
not understand why interactive shells managed background jobs as they
did and were not aware of tools like `nohup` or `disown`. See issue
There is also a fairly significant bug present due to a misunderstanding of
what a true value from `reader_exit_forced()` means. This change corrects
that misunderstanding.
Fixes#3497
The recent discussion around allowing the user to change various termios
(i.e., stty) settings reminded me that there are places in our code
where we assume the interrupt key is [ctrl-C]. That's a bad assumption.
Instead use the actual value reported to us by the kernel.
This also makes the fkr program friendlier by always reporting when a
signal was received, not just when run with -d2, and prompting the user
to press the INTR or EOF key a second time to exit.
If acquiring a lock on the history or uvar file takes more than 250 ms
disable locking of the file. On systems with broken remote file system
locking it can cause tens of seconds delay after running each command
which can make the shell borderline unusable.
This also changes history file locking to use flock() rather than
fcntl() to be consistent with uvar file locking. It also implements the
250 ms time limit before giving up on locking.
Fixes#685
If an interactive shell has its tty invalidated attempts to write to
stdout or stderr can trigger this bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=20632
Avoid that by reopening the stdio streams on /dev/null if we're getting
an ENOTTY error when trying to do things like give or take ownership of
the tty.
This includes some unrelated style cleanups but including them seems
reasonable.
Fixes#3644