Types like Environment are sometimes imported using their FQN and
sometimes via their re-export in the parent module; it seems better
to have one canonical name. I think we usually use the re-exports
already, so use them always.
It't not as trivial to enforce this in child modules (such as "mod
tests" which) but that's not a big loss.
While at it, make some other consistency improvements to imports.
This parameter is used to suppress certain verbose errors that are
expected during tests. It was awkwardly threaded through multiple call
sites. Just set it (test only) on Parser.
As described in
https://github.com/fish-shell/fish-shell/pull/9990#discussion_r1382494440,
prior to 77aeb6a2a8 (Port execution, 2023-10-08), "Parser" was
passed by mutable reference ("parser_t&"), even though operation
context was passed as "const operation_context_t &". This worked
because C++ doesn't propagate const to pointers by default (see
https://en.cppreference.com/cpp/experimental/propagate_const).
class operation_context_t {
std::shared_ptr<parser_t> parser;
...
};
So "*ctx->parser" was a "parser_t&", not "const parser_t&".
Rust has stricter const propagation rules which means that const
operation context can't simply hand out a non-const reference to parser.
To be able to port code without changing its structure,
77aeb6a2a8 passed "Parser" by shared reference, using interior
mutability (RefCell) to modify parser fields. This is a bit ugly
(c.f. https://doc.rust-lang.org/std/cell/index.html "interior mutability
is something of a last resort") and means that some borrowing conflicts
are not found at compile time but runtime.
Pass both parser and operation context by exclusive reference, and
remove the interior mutability wrappers from parser's fields.
Since "libdata" is no longer inside a "RefCell", add a "ScopedRefCell"
around "transient_commandline".
The downside is that "ScopeGuard" use can become more intrusive
when we pass "Parser" or "OperationContext" as context (especially
when we use "zelf" since we can't shadow "self"), see
* 2930466d53 (Introduce ScopedCell and ScopedRefCell, 2025-03-15)
* 29ae571afa (Make scoped_push nicer, 2024-12-28)
Avoid this in some cases, specifically when using "ScopedCell" or
"ScopedRefCell". Since "&mut Parser" prevents the "ScopeCell"'s
"ScopeGuard" from holding a shared reference, use an "Rc" to capture
a dynamically-checked reference to the Cell. We could also use raw
pointers instead.
Change "Completer::apply_var_assignments" to return a block ID, to
avoid the need to return a "zelf" "ScopeGuard". In future, we could
probably untangle completer and get away with returning a "ScopeGuard"
called "ctx".
Closes#12694
In C++ we can't have a field and method sharing a name,
but in Rust we can.
For some structs, most getters don't have a "get_", so it's weird
that some do. Remove the "get_" prefix where it's obvious enough.
While at it, give some related getters better names.
As of commit a296ee085c (Stop returning a value from ScopeGuarding::commit,
2025-03-15) "ScopeGuard::commit()" is equivalent to "drop()".
Let's use that instead.
Commit 3534c07584 (Adopt the new AST in parse_execution, 2020-07-03)
added to parse_execution_context_t::run_job_conjunction an early
return when any job in a job conjunction fails to launch. This causes
"nosuchcommand || echo hello" to not execute the continuation.
Fix this by restoring the previous behavior.
Fixes#12654
Move the functions for escaping and unescaping strings from
`src/common.rs` into `fish_common`. It might make sense to move them
into a dedicated crate at some point, but for now just move them to the
preexisting crate to unblock other extraction.
Closes#12625
Not reexporting means that imports have to change to directly import
from `fish_common`. This makes it easier to see which dependencies on
`src/common.rs` actually remain, which helps with identifying candidates
for extraction.
While at it, group some imports.
Part of #12625
To homogenize error reporting format, use a new Error struct. Currently this
is used for builtins and ensuring a common cmd/subcmd prefix.
Part of #12556
Complete the work started in
e78e3f16e (gettext: remove trailing newlines, 2026-01-30)
Now, there are no remaining trailing newlines in the localizable strings
in our Rust sources. A bit more work is still needed to get rid of a few
leading and trailing spaces, the goal being that for all localizable
strings `s` in our Rust sources, `s == s.trim()`.
Includes a bit of drive-by refactoring.
Part of #12405
Remove trailing newlines from msgids. Newlines do not need to be
localized, so translators should not have to care about them.
In addition to simplifying the jobs of translators using gettext, not
having trailing newlines also makes it easier to port localizations to
Fluent.
Closes#12399
Using `assert_eq!` instead of `assert!` has the advantage that when the
assertion fails, the debug representation of both sides will be shown,
which can provide more information about the failure than only seeing
that the assertion failed.
Part of #12336
This extracts the remaining code from `src/common.rs` which does not
depend on other parts of the main library crate. No functional changes.
Closes#12310
Commit 7996637db5 (Make fish immediately show color changes again,
2025-12-01) repaints unnecessarily when a local unexported color
variable changes. Also, it repaints when the change comes from
fish_prompt, causing an easy infinite loop. Same when changing TERM,
COLORTERM and others.
This feature is relevant when using a color-theme aware theme, so
try to keep it. Repaint only on global/universal changes.
Also ignore changes if already repainting fish prompt.
This change may be at odds with concurrent execution (parser should
not care about whether we are repainting) but that's intentional
because of 1. time constraints and 2. I'm not sure what the solution
will look like; we could use the event infrastructure. But a lot of
existing variable listeners don't use that.
Extract a context object we pass whenever we mutate the environment; While
at it, use it to pass EnvMode::USER, to reduce EnvMode responsibilities.
Fixes#12233
This command
echo $(/bin/echo -n 1; echo -n 2)
sometimes outputs "21" because we implement this as
let bufferfill = IoBufferfill::create_opts(...);
...
let eval_res = parser.eval_with(...);
let buffer = IoBufferfill::finish(bufferfill);
i.e. /bin/echo and builtin echo both output to the same buffer; the
builtin does inside parser.eval_with(), and the external process may
or may not output before that, depending on when the FD monitor thread
gets scheduled (to run item_callback).
(Unrelated to that we make sure to consume all available input in
"IoBufferfill::finish(bufferfill)" but that doesn't help with
ordering.)
Fix this by reading all available data from stdout after the child
process has exited.
This means we need to pass the BufferFill down to
process_mark_finished_children().
We don't need to do this for builtins like "fg" or "wait",
because commands that buffer output do not get job control, see
2ca66cff53 (Disable job control inside command substitutions,
2021-07-26).
We also don't need to do it when reaping from reader because there
should be no buffering(?).
fish still deviates from other shells in that it doesn't wait for
it's child's stdout to be closed, meaning that this will behave
non-deterministically.
fish -c '
echo -n $(
sh -c " ( for i in \$(seq 10000); do printf .; done ) & "
)
' | wc -c
We should fix that later.
Closes#12018
Having the prelude in wchar is not great. The wchar module was empty
except for the prelude, and its prelude included things from wutil.
Having a top-level prelude module in the main crate resolves this. It
allows us to completely remove the wchar module, and a top-level prelude
module makes more sense conceptually. Putting non-wchar things into the
prelude also becomes more sensible, if we ever want to do that.
Closes#12182
Another reduction in size of the main crate. Also allows other crates to
depend on the new wchar crate.
The original `src/wchar.rs` file is kept around for now to keep the
prelude imports working.
Part of #12182
"cargo test" captures stdout by default but not stderr.
So it's probably still useful to suppress test output like
in function 'recursive1'
in function 'recursive2'
[repeats many times]
This was done by should_suppress_stderr_for_tests() which has been
broken. Fix that, but only for the relevant cases instead of setting
a global.
As mentioned in 6896898769 (Add [lints] table to suppress lints
across all our crates, 2024-01-12), we can use workspace lints in
Cargo.toml now that we have MSRV >=1.74 and since we probably don't
support building without cargo.
This implies moving some lints from src/lib.rs to "workspace.lints".
While at it, address some of them insrtead.
This commit adds `style_edition = "2024"` as a rustfmt config setting.
All other changes are automatically generated by `cargo fmt`.
The 2024 style edition fixes several bugs and changes some defaults.
https://doc.rust-lang.org/edition-guide/rust-2024/rustfmt-style-edition.html
Most of the changes made to our code result from a different sorting
method for `use` statements, improved ability to split long lines, and
contraction of short trailing expressions into single-line expressions.
While our MSRV is still 1.70, we use more recent toolchains for
development, so we can already benefit from the improvements of the new
style edition. Formatting is not require for building fish, so builds
with Rust 1.70 are not affected by this change.
More context can be found at
https://github.com/fish-shell/fish-shell/issues/11630#issuecomment-3406937077Closes#11959
Length modifiers are useless. This simplifies the code a bit, results in
more consistency, and allows removing a few PO messages which only
differed in the use of length modifiers.
Closes#11878
Previously, for Processes which were BlockNodes, we stored the node separately
in the Process via an Option; just promote this to a real field of the
ProcessType::BlockNode.
No user visible changes expected.
This new wrapper type can be constructed via macros which invoke the
`gettext_extract` proc macro to extract the string literals for PO file
generation.
The type checking enabled by this wrapper should prevent trying to obtain
translations for a string for which none exist.
Because some strings (e.g. for completions) are not defined in Rust, but rather
in fish scripts, the `LocalizableString` type can also be constructed from
non-literals, in which case no extraction happens.
In such cases, it is the programmer's responsibility to only construct the type
for strings which are available for localization.
This approach is a replacement for the `cargo-expand`-based extraction.
When building with the `FISH_GETTEXT_EXTRACTION_FILE` environment variable set,
the `gettext_extract` proc macro will write the messages marked for extraction
to a file in the directory specified by the variable.
Updates to the po files:
- This is the result of running the `update_translations.fish` script using the
new proc_macro extraction. It finds additional messages compared to the
`cargo-expand` based approach.
- Messages IDs corresponding to paths are removed. The do not have localizations
in any language and localizing paths would not make sense. I have not
investigated how they made it into the po files in the first place.
- Some messages are reordered due to `msguniq` sorting differing from `sort`.
Remove docs about installing `cargo-expand`
These are no longer needed due to the switch to our extraction macro.
This is done in preparation for a proc macro which extracts strings which are
passed to `gettext`. Because the `concat!` macro would get expanded after the
proc macro, the proc macro would still see the `concat!`, which it cannot
handle.
