(or use the correct specifiers for the type if we can.)
These are hard to track down because we can't get compile-time
warnings for the wprintf family of in libc like is possible for
the narrow versions.
- Introduce BUILTIN_ERR_COMBO2_EXCLUSIVE
- Distill generally more terse, unambiguous error descriptions.
Remember English is not everyone's language.
- Do not capitalize sentence fragments
- Use the modality where problem input is in a %s: prefix, then
is explained.
- Do not address the user (the "You cannot do ..." kraderism)
- Spell out 'arguments' rather than 'args' for consistency
- Mention 'function' as a scope
Watching for exit events is rare, so check if we have any exit events
before actually emitting them. This saves about 2% of time in
external_cmds benchmark.
This untangles some of the complicated logic and loops around posting
job exit events, and invoking the fish_job_summary function. No
functional change here (hopefully).
Prior to this change, job_t::is_stopped() returned true if there were
zero running processes in the job. This meant that completed jobs were
reported as stopped. Stop doing this, it's a footgun.
Exited processes generate event_t::process_exit if they exit with a
nonzero status. Prior to this change, to avoid sending duplicate events,
we would clear the status. This is ugly since we're lying about the
process exit status. Use a real flag to prevent sending duplicate
notifications.
This adds a variable, $fish_autosuggestion_enabled.
When set to 0, it will turn off autosuggestions/highlighting.
Setting it to anything else will enable it (which also
means this remains enabled by default).
Commit ec3d3a481 (Support "$(cmd)" command substitution without line
splitting, 2021-07-02) started treating an input string like
"a$()b" as if it were "a"$()"b". Yet, we do not actually insert the
virtual quotes. Instead we just adapted the definition of when quotes
are closed - hence the changes to quote_end().
parse_util_locate_cmdsubst_range() is aware
of the changes to quote_end() but some of its
callers like parse_util_detect_errors_in_argument() and
highlighter_t::color_as_argument() are not. They split strings at
command substitution boundaries without handling the special quoting
rules. (Only the expansion logic did it right.)
Fix this by handling the special quoting rules inside
parse_util_locate_cmdsubst_range(). This is a bit hacky since it
makes it harder for callers to process some substrings in between
command substitutions, but that's okay because current callers only
care about what's inside the command substitutions.
Fixes#8394
For some reason on a current glibc 2.33, the configure check fails.
The man page says we'd have to define XOPEN_SOURCE>=700, but I don't
want to do that since it changes a bunch of other things, and it
didn't work in my tests.
So we just force it, since we know it works (since glibc 2.3).
This is a performance difference of ~20% for printf, so it's a
reasonably big deal.
Since #4376, for-loops would set the loop variable outside, so it
stays valid.
They did this by doing the equivalent of
```fish
set -l foo $foo
for foo in 1 2 3
```
And that first imaginary `set -l` would also fire a set-event.
Since there's no use for it and the variable isn't actually set, we
remove it.
Fixes#8384.
This was meant to trigger the wcstring_list_t overload by constructing one with `{norm_dir}`. Older gcc can't figure out what to do.
So instead we use the wcstring overload for now.
This used to construct a vector, which was then passed down and filled
with a new event_t each go around the loop. That's useless - we fire
one event here, and it's simply the variable event.
This reduces the overhead of a for-loop by ~10%:
```fish
for i in (seq 100000)
true
end
```
runs in about 90% of the time now.
This reverts commit 61cd05efb0.
It is true that we detect break and continue errors statically, but they can
still be invoked dynamically, example:
set sneaky break
$sneaky # dynamically breaks from the loop
or just `eval break`.
A followup commit will add tests for this.
function_info_t was the "mutable bits" of a function, like its
description. But we have eliminated all of those, so we can eliminate
the class.
No functional change here.
This was already printed by builtin_missing_argument/unknown_option.
Since we need more control (because we add our own errors in other
places), teach builtin_unknown_option to suppress the trailer, like
missing_argument already could.
And then use it.
Fixes#8368.
Now looks like
```
Error: Wrong color in test at index 8-11 in text (expected 0x6, actual 0x2):
command echo abc foo &
^^^^
```
instead of repeating the error for every character that is wrong.
This introduces a new variable, $fish_color_option, that can be used
to highlight options differently.
Options are tokens starting with `-`, but only up to (and including!)
the first `--`.
Fixes#8292.
__GLIBC_PREREQ is the preferred way to conditionally enable features
based on glibc versions. Use it to avoid expensive parsing and
locale sensitivity. See #8204
We want to enable posix_spawn only for glibc >= 2.24, so we check
gnu_get_libc_version() at runtime. This returns a string with the
version number.
Because it's a version number it's spelt with a "." and never a ",",
but we interpret it as a float. This is iffy to begin with, but simple
enough. Only when the locale uses a ",", things break - it'll read it
as "2" and fail the check, which absolutely *tanks* performance on WSL1.
I'm unsure if this gives the proper runtime glibc version - it might,
whereas __GLIBC_MINOR__ and such definitely would not.
So fix the immediate problem by at least using a c locale - this is
already masked by 8dc3982408, but better
safe than sorry.
In most cases, like math, we want C-semantics for floating point
numbers. In particular "." needs to be the decimal separator.
Instead, we pay the price in printf, which is currently the sole place
to output in locale-specific numbers and attempt to read them and
C-style ones.
Similar to `test`, `_` is so likely to at least slow down if not
break all things catastrophically that it ought not be allowed as a
function name. Fixes#8342
This just compares two longs as strings on the go.
We can simply
1. ignore leading zeroes - they have no influence on the value
2. compare the digits char-by-char
3. keep the comparison for the first differing digit
4. if one number is longer than the other, that is larger
5. if the numbers have the same length, the one larger in the first
differing digit is larger
This makes this comparison quite a bit faster, which makes globs in
directories with numbered files up to 20% faster.
Note that, for historical reasons, this still ignores whitespace right
after the numbers!
1ab81ab90d removed one usage of iswdigit()
but there are others; more importantly, the knowledge that iswdigit() is
slow isn't preserved anywhere apart from the git history, so there's
nothing to prevent its use from creeping back into the codebase.
Another alternative is to blacklist iswdigit() (shadow it with a
function of the same name that throws a static_assert) but if we're
going to shadow it anyway, might as well make it useful.
- Only check for HAVE_CLOCK_GETTIME and HAVE_FUTIMENS on Linux, since
they are only used to implement a Linux-specific workaround related
to mtime precision.
- Make sure that hack is limited to Linux builds
- HAVE_SYS_SYSCTL_H was unused, but we should have been using it
- HAVE_TERMIOS_H was unused, remove it
The only functional change is that unix machines with clock_gettime
and futimens will not bother with a Linux-specific hack, and won't
waste time checking for either during cmake configuration either.
For some godforsaken reason it's slow on glibc
Like, actually, this manages to somehow make "echo **" 10% faster now?
The spec says this matches 0 through 9 always, so this is safe. We
also use this logic in a variety of other places already.
This allows us to skip re-wcs2stringing the base_dir again and again
by simply using the fd. It's about 10% faster in my testing.
fstatat is defined by POSIX, so it should be available everywhere.
Also for the glob version, because this is just a performance thing.
Makes `echo **` 20% faster - 100ms to 80ms for the fish repo.
This also applies to the future `path` builtin.
Still not a speed demon, but this is a very very easy win.
Now we probably gotta do globbing all in string instead of wcs2stringing ourselves to death.
Like the $status commit, this would add the offset to already existing
errors, so
```fish
(foo)
(bar)
something
```
would see the "(foo)" error, store the correct error location, then
see the "(bar)" error, and *add the offset of (bar)* to the "(foo)"
error location.
Solve this by making a new error list and appending it to the existing
ones.
There's a few other ways to solve this, including:
- Stopping after the first error (we only display the first anyway, I
think?)
- Making it so the source location has an "absolute" flag that shows
the offset has already been added (but do we ever need to add two offsets?)
I went with the simpler fix.
This would break the location of any prior errors without doing
anything of value.
E.g.
```fish
echo foo | exec grep # this exec is not allowed!
$status
somethingelse # The error might be found here!
```
Would apply the offset of `$status` to the offset of `exec`, locating
the error for `exec` somewhere after $status!
Allows the compiler to know our bespoke assert functions
are cold paths. This would normally occur somehow for real assert().
Assembly does appear it will save some branches.
Also don't worry about NDEBUG
(This doesn't matter because we rolled our own assert functions.
Thanks @zanchey.)
Just guess anew when it's not set.
(this still uses the value of $fish_emoji_width, but clamped to 1 or 2
- we could also guess if it's an unusable value, but that's a
different issue and tbh this variable is becoming less and less useful
as time moves on and things move to the new widths by default)
Fixes#8274.
This makes it so we treat backspaces as width -1, but never go below a
0 total width when talking about *lines*, like in screen or string
length --visible.
Fixes#8277.
OpenBSD has a posix_spawn implementation which fails to return ENOEXEC
on a shebangless script, causing us to fail the shebangless tests.
Disable posix_spawn on OpenBSD.
OpenBSD's mmap is famously unsychronized with file IO. In theory fsync
and msync can be used to synchronize but I was unable to get it to work.
Just don't use mmap for history on OpenBSD. This fixes the history merge
tests.
When getting the hostname to construct the legacy uvar path, if the
hostname is empty, we will create a path pointing at a directory. On
BSDs this path can be successfully open'd and we will produce errors
about invalid uvar files.
The "linear" wildcard_match actually contained a bug that compared two
strings on every iteration, causing this to be much slower than
necessary. Fix this.
To broadcast a uvar change on Linux, we write to a named pipe, wait a bit,
and then read it back. While the pipe is readable, fish will enter a "polling
mode" where it will check for uvar changes every N msec, until the pipe is no
longer readable. If the pipe stays readable for too long (5 seconds), fish
will try to drain it; this may happen if broadcasting instance of fish is
killed before it can read back its data.
In #8209 we have a case where fish is launched in the background to set a
uvar, and then immediately exits, leaving data on the pipe. This means that
we are perpetually in a polling mode until we hit that timeout. Reduce the
timeout to 1 second and the polling interval to 10 msec.
This improves #8209; it doesn't fix it fully but I think it's the best we can
do absent some other IPC mechanism.
Now that we removed EROTTEN which had the same error code as EPERM,
we can give a less confusing error in case a user has not allowed
their terminal access to a directory.
See #8264
When cd is passed a broken symlink, this changes the error message from
"no such directory" to "broken symbolic link". This scenario probably
won't happen very often since completion won't suggest broken symlinks
but it can't hurt to give a good error.
Fish used to do this until 7ac5932. This logic used to be in
path_get_cdpath, however, that is only used for highlighting, so we
don't need error messages there. Changing cd is enough.
Reword from "rotten" to "broken" since that's what file(1) uses.
Clean-up leftovers from old "rotten" code (nomen est omen).
See #8264
This currently changes builtin realpath with the "-s" option:
builtin realpath -s ///tmp
previously would print "///tmp", now it prints "/tmp".
The only thing "allow_leading_double_slashes" does is allow *two*
slashes.
This is important for `path match`, to be introduced in #8265.
Tmux has support for wrapping arbitrary escape sequences inside
```
\ePtmux;\e%s\e\\
```
Since this ends like the screen title escape, we just reuse that.
Characteristically, this is basically undocumented, but we already use
it in e.g. fish_vi_cursor.
The default matching logic for fish_tests was prefix based, so when we
were running `history` we were also running all history tests. This
causes the test to fail for an unknown reason.
Even though we are using CMake's ctest for testing, we still define our
own `make test` target rather than use its default for many reasons:
* CMake doesn't run tests in-proc or even add each tests as an
individual node in the ninja dependency tree, instead it just bundles
all tests into a target called `test` that always just shells out to
`ctest`, so there are no build-related benefits to not doing that
ourselves.
* CMake devs insist that it is appropriate for `make test` to never
depend on `make all`, i.e. running `make test` does not require any
of the binaries to be built before testing.
* The only way to have a test depend on a binary is to add a fake test
with a name like "build_fish" that executes CMake recursively to
build the `fish` target.
* It is not possible to set top-level CTest options/settings such as
CTEST_PARALLEL_LEVEL from within the CMake configuration file.
* Circling back to the point about individual tests not being actual
Makefile targets, CMake does not offer any way to execute a named
test via the `make`/`ninja`/whatever interface; the only way to
manually invoke test `foo` is to to manually run `ctest` and specify
a regex matching `foo` as an argument, e.g. `ctest -R ^foo$`... which
is really crazy.
With this patch, it is now possible to execute any single test by name,
by invoking the build directly, e.g. to run the `universal.fish` check:
`cmake --build build --target universal.fish` or
`ninja -C build universal.fish`. Unfortunately, this is not integrated
into the Makefile wrapper, so `make universal.fish` won't work (although
this can potentially be hacked around).
Instead of compiling `fish_tests.cpp` dynamically with weakly-linked
symbols and asking it to print the list of all available tests, we
use a magic string `#define`'d as a no-op to allow CMake to regex search
for matching test groups. This speeds up configuration somewhat (by not
compiling anything), but more importantly, it's much less brittle and
doesn't involve and linker dark magic.
There's of course still no getting around the fact that it's really ugly.
We have a *lot* of color sequences to try and tparm is slow (on the
whole, when you do this thousands of times).
So let's just check colors last, which makes everything else (which is
comparatively nothing) faster, while barely impacting
colors (benchmarking confirms no measurable difference).
Fixes#8253.
Fixes#8232.
Note that this needed to have expect_prompt used in the pexpect test -
we might want to add a "catchup" there so you can just ignore the
prompt counter for a bit and pick it back up later.
* Remove safe_strerror, safe_perror and safe_append
This no longer works on new glibcs because they removed sys_errlist.
So just hardcode the relevant errno messages (and phrase them better).
Fixes#4183.
Co-authored-by: Johannes Altmanninger <aclopte@gmail.com>
The clang warning for pending_signals_t was about the operator=
return type being wrong (misc-unconventional-assign-operator).
Signed-off-by: Rosen Penev <rosenp@gmail.com>
We don't want to convert the input to a "wcstring &" because
"stage_variables" needs to have the same type as other stages, so we
can use it in a loop. Communicate that to clang-tidy.
We also don't want to take "wcstring &&". As the Google style guide
states, it's not really beneficial here, and it potentially hurts
readability because it's a relatively obscure feature.
The rest of our code contains a bunch of && parameters. We might
want to get rid of some of them.
Closes#8227
clang-tidy wrongly sees an std::move to a const ref parameter and
believes it to be pointless. The copy constructor however is deleted.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
This disables job control inside command substitutions. Prior to this
change, a cmdsub might get its own process group. This caused it to fail
to cancel loops properly. For example:
while true ; echo (sleep 5) ; end
could not be control-C cancelled, because the signal would go to sleep,
and so the loop would continue on. The simplest way to fix this is to
match other shells and not use job control in cmdsubs.
Related is #1362
The previous layout confused me for a minute as it suggested it was
possible for `pipe_next_read` to be moved twice (once in the first
conditional block, then again when the deferred process conditional
called `continue` - if and only if the deferred process *was* the last
process in the job. This patch clarifies that can't be the case.
`pipe_next_read` is moved in the body of the loop, and not
re-initialized the last go around. However, we call
`pipe_next_read.close()` after the loop, which is undefined behavior (as
it's been moved).
Best case scenario, the compiler passed the address of our copy of the
struct to `exec_process_in_job` and beyond, it went out of scope there,
the value of `fd` was set to closed (minus one), and we explicitly call
`.close()` again, in which case it does nothing.
Worst case scenario, the compiler re-uses the storage for the now-moved
struct for something else and our call to `.close()` ends up closing
some other value of `fd` (valid or invalid) and things break.
Aside from the fact that we obviously don't need to close it since it's
not assigned for the last process in the job, it's a RAII object so we
don't have to worry about manually closing it in the first place.
`escape_code_length()` was converted from returning a `size_t` to
returning a `maybe_t<size_t>` but that subtly broke all existing call
sites by forcing all input to go through the slow path of assuming a
zero-length escape sequence was found.
This is because all callers predicated their next action on what amounts
to `if (escape_code_length(...))` which would correctly skip the slow
path when `escape_code_length` returned zero, but after the conversion
to `maybe_t` contained not `maybe_t::none()` but rather
`maybe_t::some(0)` due to coercion of the result from the `size_t` local
`esc_seq_len` to the `maybe_t<size_t>` return value - which, when
coerced to a boolean returns *true* for `maybe_t::some(0)` rather than
false.
The regression was introduced in 7ad855a844
and did not ship in any released versions so no harm, no foul.
This is required for the usage of placement new. Not an issue for fish
as it gets picked up from elsewhere, but it lets one use it in a C++
test directly this way.
* commandline: Add --is-valid option to query whether it's syntactically complete
This means querying when the commandline is in a state that it could
be executed. Because our `execute` bind function also inserts a
newline if it isn't.
One case that's not handled right now: `execute` also expands
abbreviations, those can technically make the commandline invalid
again.
Unfortunately we have no real way to *check* without doing the
replacement.
Also since abbreviations are only available in command position when
you _execute_ them the commandline will most likely be valid.
This is enough to make transient prompts work:
```fish
function reset-transient --on-event fish_postexec
set -g TRANSIENT 0
end
function maybe_execute
if commandline --is-valid
set -g TRANSIENT 1
commandline -f repaint
else
set -g TRANSIENT 0
end
commandline -f execute
end
bind \r maybe_execute
```
and then in `fish_prompt` react to $TRANSIENT being set to 1.
Because we are, ultimately, interested in how many cells a string
occupies, we *have* to handle carriage return (`\r`) and line
feed (`\n`).
A carriage return sets the current tally to 0, and only the longest
tally is kept. The idea here is that the last position is the same as
the last position of the longest string. So:
abcdef\r123
ends up looking like
123def
which is the same width as abcdef, 6.
A line feed meanwhile means we flush the current tally and start a new
one. Every line is printed separately, even if it's given as one.
That's because, well, counting the width over multiple lines
doesn't *help*.
As a sidenote: This is necessarily imperfect, because, while we may
know the width of the terminal ($COLUMNS), we don't know the current
cursor position. So we can only give the width, and the user can then
figure something out on their own.
But for the common case of figuring out how wide the prompt is, this
should do.
Without escapes.
The new option is a bit cheesy, but "width" isn't as expressive and
requires an argument.
Maybe we want "pad" to also require --visible?
* Add `set --function`
This makes the function's scope available, even inside of blocks. Outside of blocks it's the toplevel local scope.
This removes the need to declare variables locally before use, and will probably end up being the main way variables get set.
E.g.:
```fish
set -l thing
if condition
set thing one
else
set thing two
end
```
could be written as
```fish
if condition
set -f thing one
else
set -f thing two
end
```
Note: Many scripts shipped with fish use workarounds like `and`/`or`
instead of `if`, so it isn't easy to find good examples.
Also, if there isn't an else-branch in that above, just with
```fish
if condition
set -f thing one
end
```
that means something different from setting it before! Now, if
`condition` isn't true, it would use a global (or universal) variable of
te same name!
Some more interesting parts:
Because it *is* a local scope, setting a variable `-f` and
`-l` in the toplevel of a function ends up the same:
```fish
function foo2
set -l foo bar
set -f foo baz # modifies the *same* variable!
end
```
but setting it locally inside a block creates a new local variable
that shadows the function-scoped variable:
```fish
function foo3
set -f foo bar
begin
set -l foo banana
# $foo is banana
end
# $foo is bar again
end
```
This is how local variables already work. "Local" is actually "block-scoped".
Also `set --show` will only show the closest local scope, so it won't
show a shadowed function-level variable. Again, this is how local
variables already work, and could be done as a separate change.
As a fun tidbit, functions with --no-scope-shadowing can now use this to set variables in the calling function. That's probably okay given that it's already an escape hatch (but to be clear: if it turns out to problematic I reserve the right to remove it).
Fixes#565
Fixes some regressions from 35ca42413 ("Simplify some parse_util functions").
The tmux tests are not beautiful but I find them easy to write.
Probably a pexpect test would also be enough here?
The names in the implementation differed from those in the header, but
the header names were definitely better (because they correlated across
function calls).
This doesn't work.
The real thing that tells if something is read-only is
electric_var_t::readonly().
This wasn't used, and we provide no way to make a variable read-only,
which makes this an unnecessary footgun.
for PWD in foo; true; end
prints:
>..src/parse_execution.cpp:461: end_execution_reason_t parse_execution_context_t::run_for_statement(const ast::for_header_t&, const ast::job_list_t&): Assertion `retval == ENV_OK' failed.
because this used the wrong way to see if something is read-only.
env_var_t::read_only() is basically broken.
It doesn't work for $PWD, as best as I can tell no variable is
read-only except for a hardcoded list of some of the electric ones.
So we should probably remove the entire read_only and
setting_read_only mechanism.
This breaks in comma-using locales (like my own de_DE.UTF-8), because
it still uses the locale-dependent strtod, which will then refuse to
read
1234.567
Using strtod_l (not in POSIX, I think?) might help, but might also be
a lot slower. Let's revert this for now and figure out if that is
workable.
This reverts commit fba86fb821.
fish_wcstod had a "fast path" which looked for all digits, otherwise
falling back to wcstod_l. However we now pass the C locale to wcstod_l,
so it is safe to extend the fast path to all ASCII characters.
In practice math parsing would pass strings here like "123 + 456" and
the space and + were knocking us off the fast path. benchmarks/math.fish
goes from 2.3 to 1.4 seconds with this change.
is_block is a field which supports 'status is-block', and also controls
whether notifications get posted. However there is no reason to store
this as a distinct field since it is trivially computed from the block
list. Stop storing it. No functional changes in this commit.
Through a mechanism I don't entirely understand, $PWD is sometimes
writable (so that `cd` can change it) and sometimes not.
In this case we ended up with it writable, which is wrong.
See #8179.
This didn't do all the syntax checks, so something like
fish -c 'echo foo; and $status'
complained of a missing command `0` (i.e. $status), and
fish -c 'echo foo | exec grep'
hit an assert!
So we do what read_ni does, parse each command into an ast, run
parse_util_detect_errors on it if it worked and then eval the ast.
It is possible to do this neater by modifying parser::eval, but I
can't find where.
This is slightly unclean. Even tho it would otherwise be syntactically
valid, using $status as a command is very very very likely to be an
error, like
if not $status
We have reports of this surprisingly regularly, including #2773.
Because $status can only ever be a value from 0 to 255, it is also
very unlikely to be an actual command, and that command is very
unlikely to do what you want.
So we simply point the user towards the "conditions" help section,
that should explain things.
This means, if we repaint with a shorter prompt, we won't overwrite the longer parts.
This reintroduces #8002, but that's a much rarer usecase - having a prompt that fills the entire screen,
in certain terminals.
This reverts commit d3ceba107e.
Fixes#8163.
This is opt-in through a new feature flag "ampersand-nobg-in-token".
When this flag and "qmark-noglob" are enabled, this command no longer
needs quoting:
curl https://example.com/thing?foo=bar&duran=duran
Compared to the previous approach e1570a4 ("Let '&' only separate as
the first char of a word"), this has some advantages:
1. "&&" and "&>" are no longer affected. They are still special, even
if used between tokens without spaces, like "echo bar&>foo".
Maybe this is not really *better*, but it avoids risking to annoy
users by breaking the old variant.
2. "&" is still special if at the end of a token, like in "sleep 1&".
Word movement is not affected by the semantics change, so Alt-F and
friends still stop at every "&".
Currently, if a "return" is given outside of a function, we'd just
throw an error.
That always struck me as a bit weird, given that scripts can also
return a value.
So simply let "return" outside also exit the script, kinda like "exit"
does.
However, unlike "exit" it doesn't quit an interactive shell - it seems
weird to have "return" do that as well. It sets $status, so it can be
used to quickly set that, in case you want to test something.
Today the reader exposes its internals directly, e.g. to the commandline
builtin. This is of course not thread safe. For example in concurrent
execution, running `commandline` twice in separate threads would cause a
race and likely a crash.
Fix this by factoring all the commandline state into a new type
'commandline_state_t'. Make it a singleton (there is only one command
line
after all) and protect it with a lock.
No user visible change here.
No functional change here; this migrates the fix ensuring that history
items are available in the builtin interactive read command into the
reader itself, in preparation for removing reader_get_history().
This adds a hack to the parser. Given a command
echo "x$()y z"
we virtually insert double quotes before and after the command
substitution, so the command internally looks like
echo "x"$()"y z"
This hack allows to reuse the existing logic for handling (recursive)
command substitutions.
This makes the quoting syntax more complex; external highlighters
should consider adding this if possible.
The upside (more Bash compatibility) seems worth it.
Closes#159
When the user presses control-C, fish marks a cancellation signal which
prevents fish script from running, allowing it to properly unwind.
Prior to this commit, the signal was cleared in the reader. However this
missed the case where a binding would set $fish_bind_mode which would
trigger event handlers: the event handlers would be skipped because of
the cancellation flag was still set. This is similar to #6937.
Let's clear the flag earlier, as soon as we it's set, in inputter_t.
Fixes#8125.
* string: Allow `collect --no-empty` to avoid empty ellision
Currently we still have that issue where
test -n (thing | string collect)
can return true if `thing` doesn't print anything, because the
collected argument will still be removed.
So, what we do is allow `--no-empty` to be used, in which case we
print one empty argument.
This means
test -n (thing | string collect -n)
can now be safely used.
"no-empty" isn't the best name for this flag, but string's design
really incentivizes reusing names, and it's not *terrible*.
* Switch to `--allow-empty`
`--no-empty` does the exact opposite for `string split` and split0.
Since `-a`/`--allow-empty` already exists, use it.
This reverts commit b56b230076.
which somehow made us miss repaints on uvar notifications.
The commit was a workaround for a polling bug which was later properly
fixed by 7c5b8b855 ("Use the uvar notifier pipe timestamp to avoid
excessive polling"), so it's no longer necessary.
Add a system test. If I had a better understanding of the bug I could
probably write a better test.
Fixes#8088
We used to warn about PATH and CDPATH that are not valid directories,
but only if they contain colons.
However, the warning was a false positive because we would split
those values by colons anyway. So there is nothing left we want to
warn about.
Fixes#8095
FISH_USE_POSIX_SPAWN is always defined, thanks to the line
#define FISH_USE_POSIX_SPAWN HAVE_SPAWN_H
So replace #ifdef with #if to fix compilation on platforms lacking
spawn.h. Also make the spawn.h inclusion condition consistent across
files.
The sort routine was using the address of the **function pointer**
`signal(int signal)` rather than the union payload of the same name.
Perhaps one of the two should be renamed.
Prior to this change, if you were to type `./fish_indent` it woul dbe
colored as valid, because the path
`$fish_functions_path/./fish_indent.fish` is a real file. However of
course this is not actually executed as a function. Teach
function_exists to return false for function names which are invalid.
This only accepted "--background=". Really what we need to do is do an
actual getopt, but that wants a null-terminated array and is tightly
coupled to set_color.
Fixes#8053.
* Try to set LC_CTYPE to something UTF-8 capable
When fish is started with LC_CTYPE=C (even just effectively, often via
LC_ALL=C!), it's basically broken. There's no way to handle non-ASCII
characters with a C locale unless we want to write our
locale-independent replacements for all of the system functions.
Since we're not going to do that, let's try to find *some locale* for
LC_CTYPE.
We already do that in __fish_setlocale, but that's
- a bit of a weird thing that reads unstandardized system
configuration files
- allows setting locale to C explicitly
So it's still easily possible to end up in a broken configuration.
Now, the issue with this is that there is (AFAICT) no portable way to
get a list of all allowed locales and C.UTF-8 is not standardized, so
we have no one locale to fall back on and are forced to try a few. The
list we have here is quite arbitrary, but it's a start.
Python does something similar and only tries C.UTF-8, C.utf8 and
"UTF-8".
Once C.UTF-8 is (hopefully) standardized, that will just start
working (tm).
Note that we do not *export* the fixed LC_CTYPE variable, so external
programs still have to deal with the C locale, but we have no real
business messing with the user's environment.
To turn it off: $fish_allow_singlebyte_locale, if set to something true (like "1"),
will re-run the locale initialization and skip the bit where we force
LC_CTYPE to be utf8-capable.
This is mainly used in our tests, but might also be useful if people
are trying to do something weird.
This concerns the behavior of posix_spawn for shebangless scripts. At some
point, glibc started executing them using `sh`, which is desirable for
fish's shebangless support (see #7802). On glibcs without that behavior
the shebangless test fails. So this change disables posix_spawn on older
glibcs.
It's not easy to figure out when that happened but it definitely happens
in glibc 2.28, and does not happen in glibc 2.17. Presumably the new
behavior is present in glibc 2.24 (see BZ#23264) so that's the cutoff:
posix_spawn is no longer allowed on glibc < 2.24.
This fixes the noshebang test failures on Ubuntu Xenial and Centos 7.
See discussion at bottom of #8021.
Now that `$last_pid` is never fish's pid, we no longer need to force
jobs to run in their own pgroup. Restore the job control behavior to
what it was prior, so that signals may be delivered properly in
non-interactive mode.
This reverts commit 3255999794
Prior to this change, a function with an on-job-exit event handler must be
added with the pgid of the job. But sometimes the pgid of the job is fish
itself (if job control is disabled) and the previous commit made last_pid
an actual pid from the job, instead of its pgroup.
Switch on-job-exit to accept any pid from the job (except fish itself).
This allows it to be used directly with $last_pid, except that it now
works if job control is off. This is implemented by "resolving" the pid to
the internal job id at the point the event handler is added.
Also switch to passing the last pid of the job, rather than its pgroup.
This aligns better with $last_pid.
When a job is placed in the background, fish will set the `$last_pid`
variable. Prior to this change, `$last_pid` was set to the process group
leader of the job. However this caussed problems when the job ran in
fish's process group, because then fish itself would be the process group
leader and commands like `wait` would not work.
Switch `$last_pid` to be the actual last pid of the pipeline. This brings
it in line with the `$!` variable from zsh and bash.
This is technically a breaking change, but it is unlikely to cause
problems, because `$last_pid` was already rather broken.
Fixes#5036Fixes#5832Fixes#7721
These asserts require a recursive template instantiation and are currently
checked for every file that pulls in common.h. Place them in a .cpp file so
they are only checked once, hopefully improving compile time.
It is possible to run a function when a process exits via `function
--on-process-exit`, or when a job exits via `function --on-job-exits`.
Internally these were distinguished by the pid in the event: if it was
positive, then it was a process exit. If negative, it represents a pgid
and is a job exit. If zero, it fires for both jobs and processes, which is
pretty weird.
Switch to tracking these explicitly. Separate out the --on-process-exit
and --on-job-exit event types into separate types. Stop negating pgids as
well.
This passed the wchar_t* to outputter::writestr(), which then had to
do a wcslen on it, when it already has a perfectly cromulent
wcstring overload.
Just use that one.
This makes the right prompt position independent of the width of the
commandline, which prevents staircase effects. That means, with "X"
standing in as a character that the terminal and fish disagree on:
```
> echo X rightprompt
```
will stay like that instead of creating a staircase like
```
> echo X rightpromp
t> echo X rightpromp
pt> echo X
```
and so on.
The cursor still won't be *correct*, but it will be wrong in a less
annoying way.
If the user has a multi-line prompt, we will emit a clr_eol on every
line except the last (see #7404). Prior to this change we would emit
clr_eol after the line, but in some terminals, if the line extended the
width of the tty, the last character would be deleted. Switch to
emitting clr_eol first; now the last character will not be cut off.
Fixes#8002
In preparation for using wait handles in --on-process-exit events, factor
wait handles into their own wait handle store. Also switch them to
per-process instead of per-job, which is a simplification.
This crashed on Fedora with the rpm packages, but not when building
from source, so some compiler option triggers it.
But the root cause is us running `text.front()` on an empty string,
which isn't something you should do.
Fixes#8009.
This switches builtin_wait from waiting on jobs in the active job list, to
waiting on the wait handles. The wait handles may be either derived from
the job list itself, or from saved wait handles from jobs that exited in
the background.
Fixes#7210
This is preparing to address the problem where fish cannot wait on a
reaped job, because it only looks at the active job list. Introduce the
idea of a "wait handle," which is a thing that `wait` can use to check if
a job is finished. A job may produce its wait handle on demand, and
parser_t will save the wait handle from wait-able jobs at the point they
are reaped.
This change merely introduces the idea; the next change makes builtin_wait
start using it.
In rare cases this may cause the universal variable file to drop
an update, if two happen at the same time and HOME is on an nfs mount.
But this is considered better than hanging if nfs is lockless.
Fixes#7968.
env_universal_t locking discipline is now managed by env.cpp.
That is, the shared instance of env_universal_t is managed by a lock.
We no longer need to have an internal lock, so remove it.
Previously an instance of env_universal_t had to be created with a file
path. Switch to allowing it to be created as empty, and later initialized
with the file path. This will help simplify the case where universal
variables are not used; they may simply be not initialized and so just
appear empty.
This avoids using locks for the history file if the file appears to be on
a remote file system, like NFS. This is to avoid hangs if the filesystem
does not support locking.
If locking is not enabled, then in rare cases, history items may be
dropped if multiple sessions try to write to the history file at once.
This is thought to be better than hanging. Hopefully the recent change to
require a trailing newline will avoid propagating partial items.
Prior to this fix, an escaped character like \x41 (hex for ascii A)
was interpreted the same was as A, so that $\x41 would be the same
as $A. Fix this by inserting an INTERNAL_SEPARATOR before these escapes,
so that we no longer treat it as part of the variable name.
This also affects brackets; don't treat echo $foo\1331\135 the same as
echo $foo[1].
Fixes#7969
In the named pipe notifier, notifications are broadcast by writing to the
pipe, waiting briefly, and then reading it back. When clients see the pipe
as readable, they report the uvars as potentially changed and fish will
sync against the uvar file.
Prior to this change, we synced repeatedly when the pipe was readable. But
we can do somewhat better by also checking the named pipe's timestamp (via
fstat). If the pipe has not changed, then we can skip the sync even if
there is currently data lingering on it.
With this change we should sync against the variable file less often
(typically once or twice per write); in the next change we refactor this
logic so it's easier to follow.
This removes the "did_visit" message because it doesn't really add
anything.
For example:
```
ast-construction: make job_list 0x55a6d19729f0
ast-construction: make job_conjunction 0x55a6d1971c00
ast-construction: will_visit job_conjunction 0x55a6d1971c00
ast-construction: will_visit job 0x55a6d1971c18
ast-construction: variable_assignment_list size: 0
ast-construction: will_visit statement 0x55a6d1971c48
ast-construction: make decorated_statement 0x55a6d1972650
ast-construction: will_visit decorated_statement 0x55a6d1972650
ast-construction: make argument_or_redirection 0x55a6d1968310
ast-construction: will_visit argument_or_redirection 0x55a6d1968310
ast-construction: make argument 0x55a6d197b0b0
ast-construction: did_visit argument_or_redirection 0x55a6d1968310
ast-construction: argument_or_redirection_list size: 1
ast-construction: did_visit decorated_statement 0x55a6d1972650
ast-construction: did_visit statement 0x55a6d1971c48
ast-construction: job_continuation_list size: 0
ast-construction: did_visit job 0x55a6d1971c18
ast-construction: job_conjunction_continuation_list size: 0
ast-construction: did_visit job_conjunction 0x55a6d1971c00
ast-construction: job_list size: 1
```
those "did_visit" messages all correspond to "will_visit" ones. They
are effectively block delimiters like `end` or `}`.
If we remove them it turns into:
```
ast-construction: make job_list 0x55a6d19729f0
ast-construction: make job_conjunction 0x55a6d1971c00
ast-construction: will_visit job_conjunction 0x55a6d1971c00
ast-construction: will_visit job 0x55a6d1971c18
ast-construction: variable_assignment_list size: 0
ast-construction: will_visit statement 0x55a6d1971c48
ast-construction: make decorated_statement 0x55a6d1972650
ast-construction: will_visit decorated_statement 0x55a6d1972650
ast-construction: make argument_or_redirection 0x55a6d1968310
ast-construction: will_visit argument_or_redirection 0x55a6d1968310
ast-construction: make argument 0x55a6d197b0b0
ast-construction: argument_or_redirection_list size: 1
ast-construction: job_continuation_list size: 0
ast-construction: job_conjunction_continuation_list size: 0
ast-construction: job_list size: 1
```
Which is still unambiguous because of the indentation.
(this is still *super verbose* and we might want to remove it from the
`*` "all" debug category and only allow turning it on explicitly)
This simply checks if the parser requested exit after running any
binding scripts (in read_normal_chars).
I think this means we no longer need the `exit` bind function.
Fixes#7967.
Reproducible at least on Linux, where the "named pipe" universal
variable notifier is used:
rm -rf build/test/xdg_config
XDG_CONFIG_HOME=build/test/xdg_config ./build/fish -c "xterm -e ./build/fish"
The child fish reacts to keyboard input with a noticeable initial
delay. This is because the universal variable file is polled over
a million times, even when I immediately press Control-D. This polling
prevents readb() from handling keyboard input.
Before commit 939aba02d ("Refactor input_common.cpp:readb"), readb()
reacted to keyboard input even when there were universal variable
notifications. Restore this behavior, but make sure to call the
universal variable notifier after the new "prepare_to_select" logic.
Maybe the problem is in the notifier but the old behavior was sane.
Fixes the problems described in
7a556ec6f2 (commitcomment-49773677)
Adding "-d uvars-file" to the reproducesr shows that we are checking
the uvar file repeatedly:
uvar-file: universal log sync
uvar-file: universal log sync elided based on fast stat()
uvar-file: universal log no modifications
This only uses the functions fish ships with, but still doesn't allow
any *customization*, which is the point of no-config.
This makes it a lot more usable, given that the actual normal prompt
and things are there.
This still doesn't set any colors, because we don't run
__fish_config_interactive because we don't read config.fish (any
config.fish), because that would run the snippets.
