This is in regards to a comment on 290d07a833, which resulted in 46c967903d.
Those commits handled the default path when it is unset on startup.
DEFAULT_PATH is used when PATH is unset at runtime as far as I can tell.
As far as I can tell this has had the non-overidding ordering behavior since inception
(or at least 17 years ago ea998b03f2).
We don't change anything about compilation-setup, we just immediately jump to
Rust, making the eventual final swap to a Rust entrypoint very easy.
There are some string-usage and format-string differences that are generally
quite messy.
In CMake this used a `version` file in the CARGO_MANIFEST_DIR, but
relying on that is problematic due to change-detection, as if we add
`cargo-rerun-if-changed:version`, cargo would rerun every time if the file does
not exist, since cargo would expect the file to be generated by the
build-script. We could generate it, but that relies on the output of `git
describe`, whose dependencies we can only limit to anything in the
`.git`-folder, again causing unnecessary build-script runs.
Instead, this reads the `FISH_BUILD_VERSION`-env-variable at compile time
instead of the `version`-file, and falls back to calling git-describe through
the `git_version`-proc-macro. We thus do not need to deal with extraneous
build-script running.
C++ main used getopt (no w!), which appears to internally print
error-messages. The Rust version will use `wgetopter_t`, and therefore needs to
print this itself.
- It is currently never set, but will be set once `main` is ported
- `should_suppress_stderr_for_tests` used to be PROGRAM_NAME !=
TESTS_PROGRAM_NAME, but the equivalent C++ code was
`!std::wcscmp(program_name, TESTS_PROGRAM_NAME)`, and `wcsmp` returns
zero if they are equal, thus is equivalent to `==` in Rust
Similar to `time`, except that one is more common as a command.
Note that this will also allow `builtin and`, which is somewhat
useless, but then it is also useless outside of a pipeline.
Addition to #9985
This allows e.g. `foo | command time`, while still rejecting `foo | time`.
(this should really be done in the ast itself, but tbh most of
parse_util kinda should)
Fixes#9985
- "1.6.0" now supports formatting let-else statements which we use liberally,
and appears to have some fixes in regards to long-indented-lines with macros
like `wgettext_ft!`
- This commit updates the formatting so that devs with the latest stable don't
see random format-fixes upon running `cargo fmt`
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.
- Make CMake use the correct target-path
- Make build.rs use the correct target dir
Workspaces place it in the project root by default, the alternative to making
this change is to add a `.cargo/config.toml` file with
```toml
[build]
target-dir = "fish-rust/target"
```
Which I think is unnecessary, as we likely want to use the new location anyways.
- This allows running `cargo fmt/clippy/test/etc` from root
- Ideally the root should be the fish-rust package instead of being virtual, but
that requires changed to CMake/Corrosion. This change should instead be
completely compatible with our existing setup.
- This also means we will only have on `Cargo.lock` for all current and future
crates.
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.
