This switches function execution from the function's source code to
its stored node and pstree. This means we no longer have to re-parse
the function every time we execute it.
The idea is that we can return the shared pointer directly, avoiding
lots of annoying little getter functions that each need to take locks.
It also helps to pull together the data structures used to initialize
functions versus store them.
This concerns block nodes with redirections, like
begin ... end | grep ...
Prior to this fix, we passed in a pointer to the node. Switch to passing
in the tnode and parsed source ref. This improves type safety and better
aligns with the function-node plans.
Prior to this fix, functions stored a string representation of their
contents. Switch them to storing a parsed source reference and the
tnode of the contents. This is part of an effort to avoid reparsing
a function's contents every time it executes.
Add a fish-specific wrapper around std::mutex that records whether it is
locked in a bool. This is to make ASSERT_IS_LOCKED() simpler (it can just
check the boolean instead of relying on try_lock) which will make Coverity
Scan happier.
Some details: Coverity Scan was complaining about an apparent double-unlock
because it's unaware of the semantics of try_lock(). Specifically fish
asserts that a lock is locked by asserting that try_lock fails; if it
succeeds fish prints an error and then unlocks the lock (so as not to leave
it locked). This unlock is of course correct, but it confused Coverity Scan.
Use wcstring/string instead of a character array. The variable
`term_env` was not being freed before the function exited.
Fixes defect 7520324 in coverity scan.
keepalive processes are typically killed by the main shell process.
However if the main shell exits the keepalive may linger. In WSL
keepalives are used more often, and the lingering keepalives are both
leaks and prevent the tests from finishing.
Have keepalives poll for their parent process ID and exit when it
changes, so they can clean themselves up. The polling frequency can be
low.
Have WSL use a keepalive whenever the first process is external.
This works around the fact that WSL prohibits setting an exited
process as the group leader.
When the pager wants to use the full screen to show many options, it reserves
space at the top to see the command. Previously it pretended the command was a
prompt and engaged the prompt layout mechanism to compute these lines. Instead
let's juts count newlines since escape sequences within commands are very rare.
There were several issues with the way that the include tests for curses.h
were being done that were ultimately causing fish to use the headers from
ncurses but link against curses on platforms that provide an actual
libcurses.so that isn't just a symlink to libncurses.so
In particular, the old code was first testing for curses's cureses.h and then
falling back to libncurses's implementation of the same - but that logic was
reversed when it came to including term.h, in which case it was testing for
the ncurses term.h and falling back to the curses.h header. Long story short,
while cmake will link against libcurses.so if both libcurses.so and
libncurses.so are present (unless CURSES_NEED_NCURSES evaluates to TRUE, but
that makes ncurses a hard requirement), but we were brining in some of the
defines from the ncurses headers, causing SIGSEGV panics when fish ultimately
tried to access variables that weren't exported or were mapped to undefined
areas of memory in the other library.
Additionally it is an error to include termios.h prior to including the plain
Jane curses.h (not ncurses/curses.h), causing errors about unimplemented types
SGTTY/chtype. So far as I can tell, both curses.h and ncurses/curses.h pull in
termios.h themselves so it shouldn't even be necessary to manually include it,
but I have just moved its #include below that of curses.h
This special cases expansion of $history variables, so that slicing
history no longer needs to construct the entire history array. Speedup
is around 100x in my test.
Fixes#4650
Prior to this fix, if the user typed normal characters while the
completion pager was shown, it would begin searching. This feature was
not well liked, so we are going to instead just append the characters as
normal and disable paging. Control-S can be used to toggle the search
field.
Fixes#2249
try_get_child() was taking the address of a reference; clang was thereby
assuming it could not be null and so was dropping the null check. Ensure
we do not dereference a null pointer.
Fixes#4678
This was a symbol that represented either an argument or a redirection.
This was only used as part of argument_or_redirection_list.
It's simpler to just have these types be alternatives in the list type.
Some dotfile users like to add directories to PATH that point at
non-existent directories (because those directories exist on other
machines). Stop warning in that case, unless those directories contain
a colon, in which case it's probably a user error.
Changed cd completion to differentiate between cd autosuggest and cd tab
completion. cd autosuggest will find deepest unique hierarchy and cd tab
completion will not.
Issue #4402
This untangles the CMake versioning issues (I hope) as discussed in #4626.
Note most of the advice found on the Internet about how to inject git
versions into CMake is just wrong.
The behavior we want is to unconditionally run the script
build_tools/git_version_gen.sh at build time (i.e. when you invoke ninja or
make, and not when you invoke cmake, which is build system generation time).
This script is careful to only update the FISH-BUILD-VERSION-FILE if the
contents have changed, to avoid spurious rebuilding dependencies of
FISH-BUILD-VERSION-FILE. Assuming the git version hasn't changed, the script
will run, but not update FISH-BUILD-VERSION-FILE, and therefore
fish_version.o will not have to be rebuilt.
This might normally rebuild more than is necessary even if the timestamp is
not updated, because ninja computes the dependency chain ahead of time. But
Ninja also supports the 'restat' option for just this case, and CMake is rad
and exposes this via BYPRODUCTS. So mark FISH-BUILD-VERSION-FILE as a
byproduct and make the script always update a dummy file
(fish-build-version-witness.txt). Note this is the use case for which
BYPRODUCTS is designed.
We also have fish_version.cpp #include "FISH-BUILD-VERSION-FILE", and do a
semi-silly thing and make FISH-BUILD-VERSION-FILE valid C++ (so there's just
one version file). This means we have to filter out the quotes in other
cases..
This reverts commit 25839b8c36.
This was an attempt to simplify the version generation, but it
computed the version at build sytem generation time rather than
at build time, requiring another run of CMake to update it.
Correctly generate FISH_BUILD_VERSION for use in fish_version.h/cpp and
fish.pc to allow `fish --version` and `echo $version` to work again.
Not needing the same convoluted measures used by Makefile builds to
prevent the regeneration of the fish version file when it hasn't
changed.
Purposely created a new `cmake_git_version_gen.sh` file so that the old
`git_version_gen.sh` remains compatible with the existing Makefile build
script. Same reason why `fish.pc.in` was not modified to use a lowercase
variable name to match the CMAKE variable of the same name.
Closes#4626
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.
This adds a new class arg_iterator_t which encapsulates decisions about
whether to read arguments from stdin or argv. It also migrates the
unread bytes buffer from a static variable to an instance variable.
Profiling with callgrind revealed that about 60% of the time in a `something | string match` call
was actually spent in `string_get_arg_stdin()`,
because it was calling `read` one byte at a time.
This makes it read in chunks similar to builtin read.
This increases performance for `getent hosts | string match -v '0.0.0.0*'` from about 300ms to about 30ms (i.e. 90%).
At that point it's _actually_ quicker than `grep`.
To improve performance even more, we'd have to cut down on str2wcstring.
Fixes#4604.
Prior to this fix, a "bare variable" in math like 'x + 1' would be
looked up in the environment, i.e. equivalent to '$x + 1'. This appears
to have been done for performance. However this breaks the orthogonality
of fish; performance is not a sufficient justification to give math this
level of built-in power, especially because the performance of math is
not a bottleneck. The implementation is also ugly.
Remove this feature so that variables must be prefixed with the dollar
sign and undergo normal variable expansion. Reading 'git grep' output
does not show any uses of this in fish functions or completions.
Also added to changelog.
Fixes#4393
For some reason on Solaris the previous code was refusing to compile
with an error (regarding the declaration of stdout in the opts struct)
error: declaration of ‘__iob’ as array of references
The obvious guess that it had something to do with the name of the
variable in question proved true; renaming it from `stdout` to
`opts.stdout` allows the build to go through.
macOS 10.5 and earlier do not support the convention of returning
a dynamically allocated string, plus this seems like an unnecessary
malloc. Always allocate a buffer for realpath() to write into.
It seems that `parse_cmd_opts` does not correctly handle no arguments,
and so argc was being decremented to -1 causing uninitialized memory
access when argv[0] was dereferenced at a later point.
No longer using `-` to indicate reading to stdout. Use lack of arguments
as stdout indicator. This prevents mixing of variables with stdout
reading and makes it clear that stdout may not be mixed with delimiters
or array mode.
Added an option to read to stdout via `read -`. While it may seem
useless at first blush, it lets you do things like include
mysql -p(read --silent) ...
Without needing to save to a local variable and then echo it back.
Kicks in when `-` is provided as the variable name to read to. This is
in keeping with the de facto syntax for reading/writing from/to
stdin/stdout instead of a file in, e.g., tar, cat, and other standard
unix utilities.
This silences binding errors due to keys not found in the current
termcap config in the default fish bindings.
Closes#4188, #4431, and obviates the original fix for #1155
It was necessary to re-implement builtin_bind as a class in order to
avoid passing around the options array from function to function and
as adding an opts parameter to `get_terminfo_sequence` would require
otps to be passed to all other builtin_bind_ functions so they could, in
turn, pass it to `get_terminfo_sequence`.
On powerpc64 (big-endian platform) one test failed as:
Testing file printf.in ... fail
Output differs for file printf.in. Diff follows:
--- printf.tmp.out 2017-10-02 18:14:17.740000000 -0700
+++ printf.out 2017-10-02 18:11:59.370000000 -0700
@@ -1,5 +1,5 @@
Hello 1 2 3.000000 4.000000 5 6
-a B 0 18446744073709551615
+a B 10 18446744073709551615
It happens due to roughly the following code:
swprintf(..., L"%o", (long long)8);
Here mismatch happens between "%o" (requires 32-bit value)
and 'long long' (requires 64-bit value).
The fix turns it effectively to:
swprintf(..., L"%llo", (long long)8);
as it was previously done for 'x', 'd' and other int-like types.
Makes tests pass on powerpc64.
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
The POSIX standard specifies that a buffer should be supplied to
getcwd(), not doing so is undefined (or rather, platform-defined)
behavior. This was causing the getcwd errors on illumos (though not seen
on Solaris 11) reported in #3340Closes#3340
Took care of remaining issues preventing fish from building on Solaris.
Mainly caused by some assumptions that certain defines are POSIX when
they are not (`NAME_MAX`).
Moved `NAME_MAX` defines to common.h - for some reason, it was being
defined in a cpp file (`env_universal_common.cpp`) even though it is used
in multiple source files.
Now compiles on Solaris 11 with GNU Make. Still some warnings because
fish was written with GNU getopt in mind and the Solaris version doesn't
use `const char *` but rather just `char *` for getopt values, but it
builds nevertheless.
Assuming this closes#3340
We had pid_status defined as a pid_t instance, which was fine since on
most platforms pid_t is an alias for int. However, that is not
universally the case and waitpid takes an int *, not a pid_t *.
A completion may have zero length; in this case the length of the
prefix was omitted and the completion was not visible. Correct the
calculation to account for zero-width completions.
Fixes#4424
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.
This flag was only documented for a few weeks before being renamed
`--show-time` and has been deprecated for a long time. Fish 3.0 is a good
opportunity to remove it.
Instead of treating the search term as a literal string to be matched
treat it as a glob. This allows the user to get a more useful set of
results by using the `*` glob character in the search term.
Partial fix for #3136
* Implement `history search --reverse`
It should be possible to have `history search` output ordered oldest to
newest like nearly every other shell including bash, ksh, zsh, and csh.
We can't make this the default because too many people expect the
current behavior. This simply makes it possible for people to define
their own abbreviations or functions that provide behavior they are
likely used to if they are transitioning to fish from another shell.
This also fixes a bug in the `history` function with respect to how it
handles the `-n` / `--max` flag.
Fixes#4354
* Fix comment for format_history_record()
As discussed in #3805, this patch disables assigning fish to its own
process group at startup. This was trialled in #4349 alongside other
pgrp fixes which introduced additional problems, but this particular fix
seems to be OK.
Fixes#3805 and works around Microsoft/BashOnWindows#1653
* Hoist `for` loop control var to enclosing scope
It should be possible to reference the last value assigned to a `for`
loop control var when the loop terminates. This makes it easier to detect
if we broke out of the loop among other things. This change makes fish
`for` loops behave like most other shells.
Fixes#1935
* Remove redundant line
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.
maybe_t is an implementation of the Maybe/Optional type, allowing
for an optional value to be stored. This will enable a more
principled approach for functions that return values or failure,
such as env_get.
This commit backs out certain optimizations around setting environment
variables, and replaces them with move semantics. env_set accepts a
list, by value, permitting callers to use std::move to transfer
ownership.
Commit f872f25f introduced a freed memory access regression on line 460
of env.cpp, where an environment variable was converted to a temporary
string, the .c_str() address of which was stored while the string
temporary was destroyed.
This commit keeps a reference to the original string lying around so
that the c_str() pointer does not point to freed memory.
Valgrind warns that the sometimes uninitialized sigaction.sa_flags field
is sometimes used when passed to the signal handler.
This patch explicitly zeros out the sigaction.sa_flags field at creation
time.
cherry-picked from krader1961/fish-shell commit b69df4fe72
Fixes#4353 (regression in indexing of history contents) and introduces
new unit tests to catch bad $history indexing in the future.
This implements an LRU cache of recently seen math expressions. When
executing math inside loops and the like this can provide a 33% decrease
in the time to execute the `math` command.
We need our `math` builtin to behave like `bc` with respect to rounding
floating point values to integer to avoid breaking to many existing
uses. So when scale is zero round down to the nearest integer.
Another change for #3157.
The MuParser supports the concept of multiple expressions separated by
commas. This implements support for that so that you can do things like
this:
set results (math '1+1, 4*2, 9^2')
This is the second baby step in resolving #3157. Implement a bare minimum
builtin `math` command. This is solely to ensure that fish can be built
and run in the Travis build environments. This is okay since anyone running
`builtin math` today is already getting an error response.
Also, more work is needed to support bare var references, multiple result
values, etc.
Using a read-only variable like `status` as a for loop control variable
has never worked. But without this change you simply get non-sensical
behavior that leaves you scratching your head in puzzlement. This change
replaces the non-sensical behavior with an explicit error message.
Fixes#4342
Recent changes to switch to unordered sets/maps can cause the order in
which items are returned to be non-deterministic. This change ensures
that the argparse "Mutually exclusive flags" error message to be
deterministic with respect to the order of the interpolated values.
