A large portion of time was spent constructing strings and passing
them to debug(). Turn debug into a macro so that the strings are only
constructed if they're going to be printed.
A recent discussion involving whether `can_be_encoded()` was broken
caused me to notice that we are inconsistent about whether Unicode code
points are specified using `\xXXXX` or `\uXXXX` notation. Which is
harmless but silly and potentially confusing.
The type cached_esc_sequences_t caches escape sequences, and is tasked
with finding an escape sequence that prefixes a given string. Before
this fix, it did so by storing the lengths of cached escape sequences,
and searching for substrings of that length. The new implementation
instead stores all cached escape sequences in a sorted vector, and uses
binary search to find the shortest escape sequence that is a prefix of
the input. This is a substantial simplification that also reduces
allocations.
This eliminates the "missing" notion of env_var_t. Instead
env_get returns a maybe_t<env_var_t>, which forces callers to
handle the possibility that the variable is missing.
No longer using RAII wrappers around pthread_mutex_t and pthread_cond_t
in favor of the C++11 std::mutex, std::recursive_mutex, and
std::condition_variable data types.
Internally fish should store vars as a vector of elements. The current
flat string representation is a holdover from when the code was written
in C.
Fixes#4200
Newer versions of GCC and Clang are not satisfied by a cast to void,
this fix is adapted from glibc's solution.
New wrapper function ignore_result should be used when a function with
explicit _unused_attribute_ wrapper is called whose result will not be
handled.
Make the `env_var_t::missing_var()` object a singleton rather than a
dynamically constructed object. This requires some discipline in its use
since C++ doesn't directly support immutable objects. But it is slightly
more efficient and helps identify code that incorrectly mutates `env_var_t`
objects that should not be modified.
It's bugged me forever that the scope is the second arg to `env_get()`
but not `env_set()`. And since I'll be introducing some helper functions
that wrap `env_set()` now is a good time to change the order of its
arguments.
My previous change to eliminate `class var_entry_t` caused me to notice
that `env_get()` turned a set but empty var into a missing var. Which
is wrong. Fixing that brought to light several other pieces of code that
were wrong as a consequence of the aforementioned bug.
Another step to fixing issue #4200.
This is the first step to implementing issue #4200 is to stop subclassing
env_var_t from wcstring. Not too surprisingly doing this identified
several places that were incorrectly treating env_var_t and wcstring as
interchangeable types. I'm not talking about those places that passed
an env_var_t instance to a function that takes a wcstring. I'm talking
about doing things like assigning the former to the latter type, relying
on the implicit conversion, and thus losing information.
We also rename `env_get_string()` to `env_get()` for symmetry with
`env_set()` and to make it clear the function does not return a string.
This silences warnings from the compiler about ignoring return value of
‘ssize_t write(int, const void*, size_t)’, declared with attribute
warn_unused_result [-Wunused-result].
This is the first step in implementing a better abstraction for handling
fish script vars in the C++ code. It implements a new function (with two
signatures) to provide a standard method for construct the flag string
representation of a fish script array.
Partial fix for #4200
We need a way to encode arbitrary strings into valid fish variable
names. It would also be nice if we could convert strings to valid URLs
without using the slow and hard to understand `__fish_urlencode` function.
In particular, eliminating the need to manipulate the locale.
Fixes#4150
This is the first step in addressing issue #3965. It renames some of the
functions involved in validating variable and function names to clarify
their purpose. It also augments the documentation to make the rules for
such identifiers clearly documented.
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.
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.
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.
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
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
Emitting warnings about EPIPE errors when writing to stdout or stderr is
more annoying than helpful. So suppress that specific warning message.
Fixes#2516
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
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'.
