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.
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
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).
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
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
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'.
Tests that exercise error paths may result in output to
stderr. This may make it look like the test failed when it did
not. Introduce should_suppress_stderr_for_tests() to suppress
this output so the test output looks clean.
This commit fixes a bug which causes that
fish -c ')'; echo $status
("Illegal command name" error) returns 0. This is inconsistent with
e.g. when trying to run non-existent command:
fish -c 'invalid-command'; echo $status
("Unknown command" error) which correctly returns 127.
A new status code,
STATUS_ILLEGAL_CMD = 123
is introduced - which is returned whenever the 'Illegal command name *'
message is printed.
This commit also adds a test which checks if valid commands return 0,
while commands with illegal name return status code 123.
Fixes#3606.
The `test` builtin currently has unexpected behavior with respect to
expressions such as `'' -eq 0`. That currently evaluates to true with a
return status of zero. This change addresses that oddity while also
ensuring that other unusual strings (e.g., numbers with leading and
trailing whitespace) are handled consistently.
Fixes#3346
The existing code is inconsistent, and in a couple of cases wrong, about
dealing with strings that are not valid ints. For example, there are
locations that call wcstol() and check errno without first setting errno
to zero. Normalize the code to a consistent pattern. This is mostly to
deal with inconsistencies between BSD, GNU, and other UNIXes.
This does make some syntax more liberal. For example `echo $PATH[1 .. 3]`
is now valid due to uniformly allowing leading and trailing whitespace
around numbers. Whereas prior to this change you would get a "Invalid
index value" error. Contrast this with `echo $PATH[ 1.. 3 ]` which was
valid and still is.
My previous change removed one place where is_wchar_ucs2() was used and
replaced it with compile time tests. This change does the same for the
other uses.
On Cygwin there are two narrowing conversions at line 931 in
src/fish_tests.cpp due to the code assuming a wchar_t is four bytes.
Obviously that's wrong but only became an issue with the pending change to
switch to C++11. The problematic values aren't actually used on Windows
because the tests that would use them are bypassed if is_wchar_ucs2()
returns true. This change predicates that code on a compile time rather
than a run time test.
Earlier lint cleanups overlooked a couple of modules because on macOS at
the moment oclint ignores them. I noticed this when I ran `make lint-all`
on Ubuntu.
When performing fuzzy completion, if a directory segment is
valid, then don't consider it for a fuzzy match even if
the literal match produces no results.
Fixes#3211
Prior to this fix, when performing completions, we would prepend
the wildcard to the resulting files. When doing fuzzy completions,
we would take some wildcard segment, attempt to locate it in the
final completion, and then replace it with our fuzzy-matched directory.
With this fix, we pass along the "resolved so far" path, and prepend
that instead of doing "surgery" on the completion. This simplifies the
logic.
Fixes#3185
