Note: This *requires* an argument after the format string:
```rust
FLOGF!(debug, "foo");
```
won't compile. I think that's okay, because in that case you should
just use FLOG.
An alternative is to make it skip the sprintf.
"FLOGF!" is supposed to treat its first argument as a format
string (but doesn't because that part isn't implemented currently).
That means running something like
```rust
FLOGF!(term_support, "curses var", var_name, "=", value);
```
That would rightly just print "curses var", ignoring the other
arguments.
By contrast, FLOG! is the literal "just join these as a string"
version.
This was "function", needs to be "function*s*".
It was only an issue in the option parsing because we set cmd there
again instead of passing it. Maybe these should just be file-level constants?
This is an alternative to the very common pattern of
```rust
streams.err.append(output);
streams.err.append1('\n');
```
Which has negative performance implications, see https://github.com/fish-shell/fish-shell/pull/9229
It takes `Into<WString>` to hopefully avoid allocating anew when the argument is
a WString with leftover capacity
This removes some spurious unsafe and some imports.
Note: We don't use it in `test`, because that can be asked to check
arbitrary file descriptors, while this only checks stdout specifically.
Turns out doing `==` on Enums with values will do a deep comparison,
including the values.
So EventDescription::Signal(SIGTERM) is !=
EventDescription::Signal(SIGWINCH).
That's not what we want here, so this does a bit of a roundabout thing.
The `impl<T> Hash for &T` hashes the string itself[^1].
It is unclear if that is actually faster than just calling `keyfunc` multiple times (they should all be linear).
For context, Rust by default uses SipHash 1-3 db1b1919ba
An alternative would be to store it as raw pointers aka `*const T`, which have a cheaper hash impl.
That has a more complicated implementation + removes lifetimes.
This commit rather removes the premature optimization.
[^1]: Source: https://doc.rust-lang.org/std/ptr/fn.hash.html
- The Err-variants will be used by e.g. wildcard, so might as well change it
now.
- `create_directory` should now not infinitely loop until it fails with an
error message that isn't `EAGAIN`
Padding with an unprintable character is now disallowed, like it was for other
zero-length characters.
`string shorten` now ignores escape sequences and non-printable characters
when calculating the visible width of the ellipsis used (except for `\b`,
which is treated as a width of -1).
Previously `fish_wcswidth` returned a length of -1 when the ellipsis-str
contained any non-printable character, causing the command to poentially
print a larger width than expected.
This also fixes an integer overflows in `string shorten`'s
`max` and `max2`, when the cumulative sum of character widths turned negative
(e.g. with any non-printable characters, or `\b` after the changes above).
The overflow potentially caused strings containing non-printable characters
to be truncated.
This adds test that verify the fixed behaviour.
- Add test to verify piped string replace exit code
Ensure fields parsing error messages are the same.
Note: C++ relied upon the value of the parsed value even when `errno` was set,
that is defined behaviour we should not rely on, and cannot easilt be replicated from Rust.
Therefore the Rust version will change the following error behaviour from:
```shell
> string split --fields=a "" abc
string split: Invalid fields value 'a'
> string split --fields=1a "" abc
string split: 1a: invalid integer
```
To:
```shell
> string split --fields=a "" abc
string split: a: invalid integer
> string split --fields=1a "" abc
string split: 1a: invalid integer
```
Empty hash maps muck around with TLS. Per code review, use a boxed slice
of a tuple instead. This has the nice benefit of printing inherited vars
in sorted order.
This adopts the new function store, replacing the C++ version.
It also reimplements builtin_function in Rust, as these was too coupled to
the function store to handle in a separate commit.
DirIter had a serious bug where it would crash on an invalid path. Make it more
robust and rationalize its error handling. Move it into its own module and add
tests.
Prior to this change, we had a silly wrapper type EventDescription which wrapped
EventType, which actually described the event.
Remove this wrapper and rename EventType to EventDescription (since it describes
more than just the type of event).
The RETURN_IN_ORDER argparse mode (enabled via leading '-') causes non-options
(i.e. positionals) to be returned intermixed with options in the original order,
instead of being permuted to the end. Such positionals are identified via the
option sentinel of char code 1. Use a real named constant for this return,
rather than weird stuff like '\u{1}'
This also allows scoped feature tests that makes testing feature flags thread-safe.
As in you can guarantee that the test actually has the correct feature flag
value, regardless of which other tests are running in parallell.
This also cleans up and removes unnecessary usage of FFI-oriented `feature_metadata_t`,
which is only used from Rust code after `builtins/status` was ported.
Note this is slightly incomplete - the FD is not moved into the parser, and so
will be freed at the end of each directory change. The FD saved in the parser is
never actually used in existing code, so this doesn't break anything, but will
need to be corrected once the parser is ported.
This shaves about 9 seconds off of the runtime, and makes the test
deterministic.
We do not touch the test_convert test because there is a known failure and we
need to track it down before making it deterministic.
Get some stuff out of the common module, which is growing large.
Also migrate the tests into "native" Rust tests so they will run in parallel.
We have to use an explicit setlocale() call to get a multibyte locale, for the
"crazy" tests.
Prior to this commit, FLOG used the ffi bridge to get the output fd. Invert
this: have fish set the output fd within main. This allows FLOG to be used in
pure Rust tests.
Two small fixes:
1. ParsedSourceRef, if present, should not be None; express that in the type.
2. ParsedSourceRef is intended to be shareable across threads; make it so.
Use as_wstr() instead of from_ffi() in a few places to avoid an allocation,
and make job_control_t work in &wstr instead of &str to reduce complexity at
the call sites.
- Using an option makes it much clearer that the check for empty args is
redundant.
- Also prefer implementing TryFrom only for &str, to not hide the string
conversion and allocation happening.
This was present in the C++ version for command, though never for type.
Checking over all elements of PATH can be slow on some platforms eg
WSL2, so only do that when used with `--all`.
Based on discussion in
https://github.com/fish-shell/fish-shell/pull/9856
This restores the status quo where builtins are like external commands
in that they can't see anything after a 0x00, because that's the c-style
string terminator.
* Make NULs work for builtins
This switches from passing a c-string to output_stream_t::append to
passing a proper string.
That means a builtin that prints a NUL no longer crashes with "thread '' panicked
at 'String contained intermediate NUL character: ".
Instead, it will actually handle the NUL, even as an argument.
That means something like
`echo foo\x00bar` will now actually print a NUL instead of truncating
after the `foo` because we passed c-strings around everywhere.
The former is *necessary* for e.g. `string`, the latter is a change
that on the whole makes dealing with NULs easier, but it is a
behavioral change.
To restore the c-string behavior we would have to truncate arguments
at NUL.
See #9739.
* Use AsRef instead of trait bound
Prior to this change, parser_t exposed an environment_t, and Rust had to go
through that. But because we have implemented Environment in Rust, it is
better to just expose the native Environment from parser_t. Make that
change and update call sites.
The writembs macro was ported from C++, which attempted to detect when a NULL
termcap was used. However we have never gotten a bug report from this. Bravely
remove it.
The outputter code has a lot of checks that string capabilities are non-empty;
just enforce that at the curses layer so we can remove those checks.
Also remove some types and traits, replacing them with simple functions.
Per code review, we think that tparm does nothing when there are no parameters,
and it is safe to remove it, even though this is a break from C++. This
simplifies some code.
This makes some simplifications to scoped_push and ScopeGuard:
1. ScopeGuard no longer uses ManuallyDrop; the memory management is now
trivial and no longer requires `unsafe`.
2. The functions `cancel` and `rollback` have been removed, as
these were unused. They can be added back later if needed.
3. `scoped_push` has been simplified in both signature and implementation.
4. `Projection` is no longer required and has been removed.
Also add some tests.
