These tests are unreliable in CI when running with address sanitiation
enabled, resulting in intermittent CI failures.
Disable them to get rid of the many false positives to reduce annoyance
and to avoid desensitization regarding failures of the asan CI job.
Suggested in
https://github.com/fish-shell/fish-shell/pull/12132#issuecomment-3605639954Closes#12142Closes#12132Closes#12126
This lint is intended to make it harder to unintentionally cast a
function to an int. We do want to store function pointers in a usize
here, so add the intermediate cast suggested by the lint.
Closes#12131
Buggy programs parsing "fish_indent --ansi" output break because
we wemit SGR0 after the final newline. I suppose this is weird,
and there's no need to wait until after the \n to emit SGR0 so let's
move it before that.
Closes#12096
We should isolate conditionally-compiled code as much as possible,
to allow the compiler to do more work, and to simplify code.
Do that for the embedding definition for __fish_build_paths, like we
already do for "Docs". Duplicate the workaround we use to set the
embedded struct to a "virtually empty" directory. Don't set it to
"$FISH_RESOLVED_BUILD_DIR", because that directory can contain >50k
files, and in debug mode listing files (even if all are excluded)
would take a second.
Ref: https://github.com/fish-shell/fish-shell/pull/12103#issuecomment-3592280477
This should also fix#12120.
Multiple gettext-extraction proc macro instances can run at the same
time due to Rust's compilation model. In the previous implementation,
where every instance appended to the same file, this has resulted in
corruption of the file. This was reported and discussed in
https://github.com/fish-shell/fish-shell/pull/11928#discussion_r2488047964
for the equivalent macro for Fluent message ID extraction. The
underlying problem is the same.
The best way we have found to avoid such race condition is to write each
entry to a new file, and concatenate them together before using them.
It's not a beautiful approach, but it should be fairly robust and
portable.
Closes#12125
This eliminates the proc-macro panic occasionally observed in CI,
instead ignoring paths which cannot be canonicalized.
See #12120.
While this does not address the underlying issue of why the proc-macro
fail happens, it should result in builds no longer failing, and since
they failed in the case where no embedding is desired, there should be
no issue with ignoring failed path canonicalization, since we do not
want to embed anything in these cases.
The old approach does not properly protect from concurrent tests
accessing the same tempdir. Fix this by moving to the new tempfile
functionality. This also eliminates the need to explicitly create and
delete the directory. There is also no longer a reason to specify names
for the temporary directories, since name collisions are avoided using
more robust means, and there is little benefit to having the names in
the directory name.
Closes#12030
The `mkstemp` function opens files without setting `O_CLOEXEC`. We could
manually set this using `fnctl` once the file is opened, but that has
the issue of introducing race conditions. If fish `exec`s in another
thread before the `fnctl` call completes, the file would be left open.
One way of mitigating this is `mkostemp`, but that function is not
available on all systems fish supports, so we can't rely on it.
Instead, build our own tempfile creation logic which uses the `rand`
crate for getting entropy and relies on Rust's stdlib for the rest.
The stdlib functions we use set `O_CLOEXEC` by default.
For directory creation we keep using `mkdtemp`, since there we don't
open anything. We could replace this by extending our custom logic a
bit, which would allow us to drop the `nix` dependency for our
`tempfile` crate, but since the code is simpler as it is now and we need
nix in fish's main crate, there is no need to modify the directory
creation code.
Part of #12030
`SmallRng` was chosen in part due to limitation of old macOS versions.
This is no longer relevant, since the affected versions are not
supported by Rust anymore.
Switch everything which does not need fixed sequences based on a seed to
`ThreadRng`, which has better cryptographic properties and is
occasionally reseeded. Performance differences should not matter much,
since initialization of `ThreadRng` is not that expensive and it's
generation speed is easily fast enough for our purposes.
In some cases, like tests and the `random` builtin, we want a PRNG which
consistently produces the same sequence of values for a given seed.
`ThreadRng` does not do this, since it is occasionally reseeded
automatically. In these cases, we keep using `SmallRng`.
Part of #12030
Function names containing `gen` have been deprecated to avoid conflicts
with the Rust 2024 keyword, so this commit also switches to the new
names.
Part of #12030
Since we changed our MSRV to 1.85, macOS < 10.12 is no longer supported,
so the comment removed here is no longer relevant.
The workaround for old macOS was introduced in
9337c20c2 (Stop using the getrandom feature of the rand crate, 2024-10-07)
We might want to reconsider which RNGs and other features of the `rand`
crate we use, but there is no immediate need to do so, since the current
approach seems to have worked well enough.
Part of #12030
As discussed in #12112, this is a false friend (the libc logb()
does something else), and without keyword arguments or at least
function overloading, this is hard to read.
Better use "/ log(base)" trick.
The old handling of Unicode escape sequences seems partially obsolete
and unnecessarily complicated. For our purposes, Rust's u32 to char
parsing should be exactly what we want.
Due to fish treating certain code points specially, we need to check
if the provided character is among the special chars, and if so we need
to encode it using our PUA scheme. This was not done in the old code,
but is now.
Add tests for this.
Fixes#12081
Rework the error message for invalid code points.
The old message about invalid code points not yet being supported seems
odd. I don't think we should support this, so stop implying that we
might do so in the future.
In the new code, indicating that a Unicode character is
out of range is also not ideal, since the range is not contiguous.
E.g. `\uD800` is invalid, but \U0010FFFF is valid.
Refrain from referring to a "range" and instead just state that the
character is invalid.
Move formatting of the escape sequence into Rust's `format!` to simplify
porting to Fluent.
Closes#12118
While it's not necessary to rebuild the proc macro for extraction when
the env var controlling the output location changes, this way everything
using the macro will automatically be rebuilt when this env var changes,
making it impossible to forget adding this to other `build.rs` files.
For now, keeping the rebuild instructions in fish's main crate's
`build.rs` would be fine as well, but if we start breaking this crate
into smaller parts, it would become annoying to add the rebuild command
in every crate depending on gettext extraction.
Closes#12107
There was a mismatch between the extraction done from
`build_tools/check.sh` and the one done in
`build_tools/fish_xgettext.fish`, resulting in messages guarded by
default features being extracted by the former but not the latter.
This brings them in sync.
Ideally, we would enable all features for extraction, but compiling with
`--all-features` is broken and manually keeping the features lists
updated is tedious and error prone, so we'll settle on only using
default features for now.
Closes#12103
Improve separation of concerns by handling catalog lookup in the
`gettext_impl` module, while taking care of string interning and string
type conversion in the outer `gettext` function as before.
This improves isolation, meaning we no longer have to expose the
catalogs from `gettext_impl` to its parent module.
It also helps with readability.
Part of #12102
Modify the `str_enum!` macro to be able to handle multiple strings per
enum variant. This means that no separate step is required for defining
the enum. Instead definition of the enum and implementation of the
string conversion functions is now combined into a single macro
invocation, eliminating code duplication, as well as making it easier to
add and identify aliases for an enum variant.
Closes#12101
This seems to be a left-over detail from the C++ version. Setting the
first element to 0 does not have any effect, since that's the default,
and setting it to 1 seems to serve no purpose in the current code.
Part of #12101
There is no difference in how these two variants are treated. Keeping
the old command name around for compatibility is nice, but it does not
need to have its own enum variant.
Part of #12101
The -n flag adds warnings about preexisting problems because we don't
check it in CI, so let's not recommend that.
Not everyone uses cmake but it's still the source of truth for docs,
so mention the cmake incantation. Though it seems like the direct
sphinx-build invocation works just fine, so keep it first.
Commit 0893134543 (Added .editorconfig file (#3332) (#3313),
2016-08-25) trimmed trailing whitespace for Markdown file (which do
have significant trailing whitespace) but ReStructuredText does not,
and none of our Markdown files cares about this, so let's clean up
whitespace always.
This message is no longer used since we unconditionally embed files now.
The `#[allow(dead_code)]` annotation was needed to avoid warnings while
ensuring that message extraction was not broken.
Closes#12099
Completions for "foo bar=baz" are sourced from
1. file paths (and custom completions) where "bar=baz" is a subsequence.
2. file paths where "baz" is a subsequence.
I'm not sure if there is ever a case where 1 is non-empty and we
still want to show 2.
Bravely include this property in our completion ranking, and only
show the best ones.
This enables us to get rid of the hack added by b6c249be0c (Back out
"Escape : and = in file completions", 2025-01-10).
Closes#5363
This is another case where we can don't need to use complicated
formatting specifiers. There is also no need to use `wgettext_fmt` for
messages which don't contain any localizable parts.
The instances using `sprintf` could be kept as is, but I think it's
better to keep the width computation at the place where it can sensibly
be done, even though it is not done correctly at the moment because it
does not use grapheme widths.
Removing complicated conversion specifiers from localized messages helps
with the transition to Fluent.
Closes#12119
We already compute the length of the substring we want to print in Rust.
Passing that length as the precision to let printf formatting limit the
length is brittle, as it requires that the same semantics for "length"
are used.
Simplifying conversion specifiers also makes the transition to Fluent
easier.
Part of #12119
