These are another way to spell the same thing that doesn't match what
`bind` would print.
They're also not documented and tested thoroughly.
Since they are just small shortcuts and unreleased we can just remove
them.
Fixes#10845
The test case shows that the pager rendering is not quite right. It renders
'{\', leaving out the newline. This rendering is ambiguous.
Let's fix it by rendering \n as control picture, like we do for other control
characters in the pager.
Commit 5db0bd5 (Lock history file before reading it, 2024-10-09)
rewrites the history file in place instead of using rename().
By writing to the same file (with the same inode), it corrupts
our memory-mapped snapshot; mmap(3) says:
> It is unspecified whether modifications to the underlying object done
> after the MAP_PRIVATE mapping is established are visible through the
> MAP_PRIVATE mapping.
Revert it (it was misguided anyway).
Closes#10777Closes#10782
Our panic handler attempts a blocking read from stdin and only exits
after the user presses Enter.
This is unconventional behavior and might cause surprise but there is a
significant upside: crashes become more visible for terminals that don't
already detect crashes (see ecdc9ce1d (Install a panic handler to avoid
dropping crash stacktraces, 2024-03-24)).
As reported in 4d0aa2b5d (Fix panic handler, 2024-08-28), the panic handler
failed to exit fish if the panic happens on background threads. It would
only exit the background thread (like autosuggestion/highlight/history-pager
performer) itself. The fix was to abort the whole process.
Aborting has the additional upside of generating a coredump.
However since abort() skips stack unwinding, 4d0aa2b5d makes us no longer
restore the terminal on panic. In particular, if the terminal supports kitty
progressive enhancements, keys like ctrl-p will no longer work in say,
a Bash parent shell. So it broke 121680147 (Use RAII for restoring term
modes, 2024-03-24).
Fix this while still aborting to create coredumps. This means we can't use
RAII (for better or worse). The bad part is that we have to deal with added
complexity; we need to make sure that we set the AT_EXIT handler only after
all its inputs (like TERMINAL_MODE_ON_STARTUP) are initialized to a safe
value, but also before any damage has been done to the terminal. I guess we
can add a bunch of assertions.
Unfortunately, if a background thread panics, I haven't yet figured out how
to tell the main thread to do the blocking read. So the trick of "Press
Enter to exit", which allows users to attach a debugger doesn't yet work for
panics in background threads. We can probably figure that out later. Maybe
use pthread_kill(3)? Of course we still create coredumps, so that's fine.
As a temporary workaround, let's sleep for a bit so the user can at least
see that there is a crash & stacktrace.
One ugly bit here is that unit tests run AT_EXIT twice but it should be
idempotent.
Commit b00899179 (Don't indent multi-line quoted strings; do indent inside
(), 2024-04-28) made parse_util_compute_indents() crash on `echo "$()"'x`.
After recursively indenting the command substitution, we indent the "'x
suffix. We skip the quoted part by setting "done=2". Later we wrongly
index "self.indents[done..range.start+offset+1]" (= "self.indents[2..1]").
Fix this by making sure that "start >= done", thus not setting any indents
for the quoted suffix. There is no need to do so; only the first character
in each line needs an indent.
This was based on a misunderstanding.
On musl, 64-bit time_t on 32-bit architectures was introduced in version 1.2.0,
by introducing new symbols. The old symbols still exist, to allow programs compiled against older versions
to keep running on 1.2.0+, preserving ABI-compatibility. (see musl commit 38143339646a4ccce8afe298c34467767c899f51)
Programs compiled against 1.2.0+ will get the new symbols, and will therefore think time_t is 64-bit.
Unfortunately, rust's libc crate uses its own definition of these types, and does not check for musl version.
Currently, it includes the pre-1.2.0 32-bit type.
That means:
- If you run on a 32-bit system like i686
- ... and compile against a C-library other than libc
- ... and pass it a time_t-containing struct like timespec or stat
... you need to arrange for that library to be built against musl <1.2.0.
Or, as https://github.com/ericonr/rust-time64 says:
> Therefore, for "old" 32-bit targets (riscv32 is supposed to default to time64),
> any Rust code that interacts with C code built on musl after 1.2.0,
> using types based on time_t (arguably, the main ones are struct timespec and struct stat) in their interface,
> will be completely miscompiled.
However, while fish runs on i686 and compiles against pcre2, we do not pass pcre2 a time_t.
Our only uses of time_t are confined to interactions with libc, in which case with musl we would simply use the legacy ABI.
I have compiled an i686 fish against musl to confirm and can find no issue.
This reverts commit 55196ee2a0.
This reverts commit 4992f88966.
This reverts commit 46c8ba2c9f.
This reverts commit 3a9b4149da.
This reverts commit 5f9e9cbe74.
This reverts commit 338579b78c.
This reverts commit d19e5508d7.
This reverts commit b64045dc18.
Closes#10634
The "principal" parser is the one and only today; in the future we hope to
have multiple parsers to execute fish script in parallel.
Having a globally accessible "principle" parser is suspicious; now we can
get rid of it.
The "principal" environment stack was the one that was associated with the
"principal" parser and would dispatch changes like to TZ, etc.
This was always very suspicious, as a global; now we can remove it.
Prior to this commit, there was a stack of ReaderDatas, each one has a
reference to a Parser (same Parser in each, for now). However, the current
ReaderData is globally accessible. Because it holds a Parser, effectively
anything can run fish script; this also prevents us from making the Parser
&mut.
Split these up. Create ReaderData, which holds the data portion of the
reader machinery, and then create Reader which holds a ReaderData and a
Parser. Now `reader_current_data()` can only return the data itself; it
cannot execute fish script.
This results in some other nice simplifications.
This is a start on untangling input. Prior to this, a ReaderData and an
Inputter would communicate with each other; this is natural in C++ but
difficult in Rust because the Reader would own an Inputter and therefore
the Inputter could not easily reference the Reader. This was previously
"resolved" via unsafe code.
Fix this by collapsing Inputter into Reader. Now they're the same object!
Migrate Inputter's logic into a trait, so we get some modularity, and then
directly implement the remaining input methods on ReaderData.
There's no guarantee that a condition variable is stateful. The docs for
`Condvar::notify_one()` actually say the opposite:
> If there is a blocked thread on this condition variable, then it will be woken
> up from its call to wait or wait_timeout. Calls to notify_one are not buffered
> in any way.
This test was relying on the main loop obtaining the lock and entering the
condition variable sleep before the thread was scheduled and got around to
notifying the condition variable. If this non-deterministic behavior was not
upheld, the test would time out since it would obtain the lock (either before or
after the variable were updated) then call `condvar.wait()` *after* the variable
had been updated and the condvar signalled, but without (atomically or even at
all) checking to see if the desired wake precondition was fulfilled. As the
child thread had already run and the wake notification was NOT buffered, there
was nothing to wake the running thread.
There really wasn't any way to salvage the test as originally written, since the
write to `ctx.val` was not in any way linked to the acquire/release of the mutex
so regardless of whether or not the main thread obtained the mutex and checked
the value precondition before calling `condvar.wait()`, the child thread's write
could have happened after the check but before the wait() call. As such, the
test has been rewritten to use `wait_while()` but then also updated to bail in
case of a timeout instead of hanging indefinitely (since neither the `ctest`
runner nor the `cargo test` harness was timing out; `cargo test` would only
report that the test had exceeded 60 seconds but as long as it was not executed
with `cargo test -- -Z --ensure-time` (which is only available under nightly),
the test would not halt.
If this test were *intentionally* written to test the scenario that was timing
out, it should be written deterministically in such a way that the main loop
did not run until after it was guaranteed that the variable had been updated
(i.e. by looping until val became 5 or waiting for an AtomicBool indicating the
update had completed to be set), but I'm not sure what the benefit in that would
be since the docs actually guarantee the opposite behavior (the notified state
is explicitly not cached/buffered).
If we have fish code written with the assumption that condvar notifications
prior to *any* call to `Condvar::wait()` *are* buffered, then that code should
of course be revisited in light of this.
This updates is_windows_subsystem_for_linux() to take a WSL version to test for
(any, v1, or v2) and returns the boolean result depending on the system. I've
benchmarked and when running on regular Linux, this is still just as fast as the
previous binary check; it's only when it's WSL that this takes about 20ns
longer to figure out which variant.
Note that older WSLv2 kernels had a `-microsoft-standard` suffix while newer
ones appear to have a `-microsoft-standard-WSL2` suffix, so we make sure to test
for the least common denominator. (It doesn't matter to us, but note that newer
WSLv2 kernels have four dots in the version string!)
WSL workarounds pertaining to the default Windows terminal or executable
behavior of win32 binaries under a WSL shell are extended to WSLv2 while those
specific to oddities in kernel behavior are confined to WSLv1 only. (It
technically wouldn't hurt to extend them to WSLv2 but there's no good reason to
do so, either.)
`Parser` is a single-threaded `!Send`, `!Sync` type and does not need to use
`Arc` for anything. We were using it because that's all we had for the parser's
`EnvStack`, but though that is *technically* protected internally by a mutex
(shared with global EnvStack), there's nothing to say that other parsers with a
narrower scope/lifetime on other threads will be necessarily using the same
backing mutex.
We can safely marshal the existing `Arc<EnvStack>` we get from
`environment::principal()` into an `Rc<EnvStack>` since the underlying reference
is always valid. To prove this point, we could have PRINCIPAL_STACK be a static
`EnvStack` and have `environment::principal()` use `Arc::from_raw()` to turn
that into an `Arc<EnvStack>`, but there's no need to factorize this process.
The test_history_formats test was reading from build/tests/ which is an artifact
of the cmake test runner. The source code tests should not depend on the cmake
test harness at all, so this is changed to read from the original test source in
the ./tests/ directory instead.
FISH_BUILD_DIR (nominally, ./build) is created by cmake. If you only check out
the project via git and then run `cargo build`, this directory won't exist and
many of the tests will fail.
%ld expects a 4-byte parameter on 32-bit architectures and an 8-byte parameter
on 64-bit architectures, but we supplied are trying to supply a 64-bit parameter
that would overflow 32-bit storage.
Use %lld instead which expects a `long long` parameter, which should be 8-bytes
under both architectures.
See #10474
On a command with multiline quoted string like
begin
echo "line1
line2"
end
we actually indent line2 which seeems misleading because the indentation
changes the behavior when typed into a script.
This has become more prominent since commits
- a37629f86 (fish_clipboard_copy: indent multiline commands, 2024-04-13)
- 611a0572b (builtins type/functions: indent interactively-defined functions, 2024-04-12)
- 222673f33 (edit_command_buffer: send indented commandline to editor, 2024-04-12)
which add indentation to an exported commandline.
Never indent quoted strings, to make sure the rendering matches the semantics.
Note that we do need to indent the opening quote which is fine because
it's on the same line.
While at it, indent command substitutions recursively. That feature should
also be added to fish_indent's formatting mode (which is the default).
Fortunately the formatting mode already works fine with quoted strings;
it does not indent them. Not sure how that's done and whether indentation
can use the same logic.
This allows making something like
```fish
abbr --add gc --position anywhere --command git back 'reset --hard
HEAD^'
```
to expand "gc" to "reset --hard HEAD^", but only if the command is
git (including "command git gc" or "and git gc").
Fixes#9411
This is similar to f7dac82ed (Escape separators (colon and equals) to improve
completion, 2019-08-23) except we only escape : and = if they are the result of
file completions. This way we avoid issues with custom completions like dd.
This also means that it won't work for things like __fish_complete_suffix
[*] but that can be fixed later, once we can serialize the DONT_ESCAPE flag.
By moving the escaping step earlier, this causes some unit test changes
which should not result in actual behavior change.
See also #6099
[*]: The new \: and \= does not leak from "complete -C" because that command
unescapes its output -- unless --escape is given.
While it does need to store the string, we also need to use the string after
storing it, so we aren't getting any advantage from passing by value. Just pass
by reference to simplify the call sites.
When we read bytes like \xfc that don't produce a Unicode code point,
we encode them in a Unicode private use area.
This encoding should be transparent to the user.
We accidentally add it to uvar files as \uf6fc in this case. When reading
it back, read_unquoted_escape() will fail at the "fish_reserved_codepoint(c)"
check. This check is to avoid external input being misinterpreted
as one of our in-band signalling characters like ANY_CHAR (for *).
For encoded raw bytes, this check probably doesn't really matter in terms of
security because the only thing we do with these bytes is convert them back
to raw. So we could allow unescaping them at this point, thus supporting
old uvar files.
However that seems like the wrong direction. PUA encoding should never leak.
So let's instead make sure to serialize it as \xfc instead of \f6fc going
forward.
Fixes#10313
File names that have lots of spaces look quite ugly when inserted as
completions because every space will have a backslash.
Add an initial heuristic to decide when to use quotes instead of
backslash escapes.
Quote when
1. it's not an autosuggestion
2. we replace the token or insert a fresh one
3. we will add a space at the end
In future we could relax some of these requirements.
Requirement 2 means we don't quote when appending to an existing token.
Need to find a natural behavior here.
Re 3, if the completion adds no space, users will probably want to add more
characters, which looks a bit weird if the token has a trailing quote.
We could relax this requirement for directory completions, so «ls so»
completes to «ls 'some dir with spaces'/».
Closes#5433
This makes them more convenient to use interactively, similar to the existing
\c and \a versions. The resulting bind output keeps using the canonical
ctrl/alt version.
Not sure about s- because that's somewhat ambiguous, it could be "super".
