Currently, we expand command-abbrs (those with `--position command`) after `if`, but not after `command` or `builtin` or `time`:
```fish
abbr --add gc "git checkout"
```
will expand as `if gc` but not as `command gc`.
This was explicitly tested, but I have no idea why it shouldn't be?
The incorrect order of operations was being used since && binds tighter than ||
in rust (as with most sane languages).
Under Linux, EAGAIN == EWOULDBLOCK so this would always succeed in the case of a
non-blocking fd without making the call to make_fd_nonblocking().
Comparing to the 3.7.0 C++ code, it looks like this was an oversight introduced
in the migration to rust.
In particular, this allows restoring the terminal on crashes, which is
feasible now that we have the panic handler. Since std::process::exit() skips
destructors, we need to reshuffle some code. The "exit_without_destructors"
semantics (which std::process::exit() als has) was mostly necessary for C++
since Rust leaks global variables by default.
When fish crashes due to a panic, the terminal window is closed. Some
terminals keep the window around when the crash is due to a fatal signal,
but today we don't exit via fatal signal on panic.
There is the option to set «panic = "abort"» in Cargo.toml, which
would give us coredumps but also worse stacktraces on stderr.
More importantly it means that we don't unwind, so destructors are skipped
I don't think we want that because we should use destructors to
restore the terminal state.
On crash in interactive fish, read one more line before exiting, so the
stack trace is always visible.
In future, we should move this "read one line before exiting" logic to where
we call "panic!", so I can attach a debugger and see the stacktrace.
Today fish_cursor_selection_mode controls whether selection mode includes
the cursor. Since it's by default only used for Vi mode, perhaps use it to
also decide whether it should be allowed to select one-past the last character.
Not allowing to select to select one-past the last character is much nicer
in Vi mode. Unfortunately Vi mode sometimes needs to temporarily select
past end (using forward-single-char and such), so reset fish_cursor_selection_mode
for the duration of the binding.
Also fix other things like cursor placement after yank/yank-pop.
Closes#10286Closes#3299
A long standing issue is that bindings cannot mix special input functions
and shell commands. For example,
bind x end-of-line "commandline -i x"
silently does nothing. Instead we have to do lift everything to shell commands
bind x "commandline -f end-of-line; commandline -i x"
for no good reason.
Additionally, there is a weird ordering difference between special input
functions and shell commands. Special input functions are pushed into the
the queue whereas shell commands are executed immediately.
This weird ordering means that the above "bind x" still doesn't work as
expected, because "commandline -i" is processed before "end-of-line".
Finally, this is all implemented via weird hack to allow recursive use of
a mutable reference to the reader state.
Fix all of this by processing shell commands the same as both special input
functions and regular chars. Hopefully this doesn't break anything.
Fixes#8186Fixes#10360Closes#9398
Multiline search strings are weirdly broken (inserting control characters
in the command line) and probably not very useful anyway.
On the other hand I often want to compose a multi-line command
from single-line commands I ran previously.
Let's support this case by limiting the initial search string to the current
line; and replace only that line.
Alternatively this could operate on jobs (that is, replace a surrounding
"foo | bar") instead of using line boundaries.
This is resistant to misuse by including O_DIRECTORY in the open flags and it is
a separate function from {w,}open_cloexec() in preparation for making that one
return a `File` instead of an `OwnedFd`.
`intersects()` is "any of" while `contains()` is "all of" and while it makes no
difference when testing a single bit, I believe `contains()` is less brittle
for future maintenance and updates as its meaning is clearer.
</pedantic>
More work in prep for having wopen_cloexec() return `File` directly.
This eliminates checking for an invalid fd and makes both ownership and
mutability clear (some more operations that involve changes to the underlying
state of the fd now require `&mut File` instead of just a `RawFd`).
Code that clearly does not use non-blocking IO is ported to use
`Write::write_all()` directly instead of our rusty port of the `write_loop()`
function (which handles EAGAIN/EWOULDBLOCK in addition to EINTR, while
`write_all()` only handles the latter).
This is a step towards converting `wopen_cloexec()` to return `File` instead of
`OwnedFd`/`AutocloseFd`.¹
In addition to letting us use native standard library functions instead of
unsafe libc calls, we gain additional semantic safety because `File` operations
that manipulate the state of the fd (e.g. `File::seek()`) require a `&mut`
reference to the `File`, whereas using `RawFd` or `OwnedFd` everywhere leaves us
in a position where it's not clear whether or not other references to the same
fd will manipulate its underlying state.
¹ We actually wouldn't even need `wopen_cloexec()` at all (just a widechar
wrapper) as Rust's native `File::open()`/`File::create()` functionality uses
`FD_CLOEXEC` internally.
* builtin/test: Split Token enum into 2-level hierarchy
* builtin/test: Rearrange the Token enum hierarchy
* builtin/test: Separate Token into Unary and Binary
* builtin/test: import IsOkAnd polyfill
* builtin/test: Rename enum variants one more time
I was able to trigger this by flipping around the history pager.
Since the only applicable caller here already stops if it gets None,
just don't assert.
I don't think the existing logic is correct, as the comment says, our internal
state is only matched if we *actually* wrote out the file. But if we ran into an
error, it doesn't match, does it?
The incompatible_msrv one is a false positive because we have polyfills for
is_some_and() and is_ok_or() which are Rust 1.74. I'm not yet sure how to
communicate that to Clippy.
Call fish_should_add_to_history to see if a command should be saved
If it returns 0, it will be saved, if it returns anything else, it
will be ephemeral.
It gets the right-trimmed text as the argument.
If it doesn't exist, we do the historical behavior of checking for a
leading space.
That means you can now turn that off by defining a
`fish_should_add_to_history` that just doesn't check it.
documentation based on #9298
The existing logic did not work because:
- Path::new("/foo/bar").ends_with("/bar") does not return true.
- PathBuf::shrink_to() only (potentially) reallocates the backing
storage, and won't have an effect on the stored value.
As mentioned in the comment the historical behavior is because pressing unknown
control characters like Ctrl+4 inserts confusing characters, so let's back
out that part of b77d1d0e2 (Stop crashing on invalid Unicode input, 2024-02-27).
We still have the code for rendering control characters, for pasted text,
or text recalled from history. It is unclear whether we should strip those.
Some terminals already strip control characters from pasted text -- but not
all of them: see https://codeberg.org/dnkl/foot/pulls/312 for example which
has a follow up called "Don't strip HT when pasting in non-bracketed mode".
Don't force the internal use of `RefCell<T>`, let the caller place that into
`MainThread<>` manually. This lets us remove the reference to `MainThread<>`
from the definition of `Screen` again and reduces the number of
`assert_is_main_thread()` calls.
Fairly straightforward, with the only unfortunate part of this being that
`Screen` isn't as pure and now encodes the facte that we use it with
main-thread-only stdout `Outputter`.
The regex struct is pretty large at 560 bytes, with the entire
Abbreviation being 664 bytes.
If it's an "Option<Regex>", any abbr gets to pay the price. Boxing it
means abbrs without a regex are over 500 bytes smaller.
IfStatement is 680 bytes, much larger than the other
variants (SwitchStatement is next at 232). An enum is as large as its
largest variant, so this saves a bunch, especially since
DecoratedStatement is much more likely than IfStatement.
This will speed up the no-execute benchmark by 1.07x.
Unlike C++, Rust requires "char" to be a valid Unicode code point. As a
workaround, we take the raw (probably UTF-8-encoded) input and convert each
input byte to a char representation from the private use area (see commit
3b15e995e (str2wcs: encode invalid Unicode characters in the private use
area, 2023-04-01)). We convert back whenever we output the string, which
is correct as long as the encoding didn't change since the data was input.
We also need to convert keyboard input; do that.
Quick testing shows that our reader drops PUA characters. Since this patch
converts both invalid Unicode input as well as PUA input into a safe PUA
representation, there's no longer a reason to not add PUA characters to
the commandline, so let's do that to restore traditional behavior.
Render them as � (REPLACEMENT CHARACTER); unfortunately we show one per
input byte instead of one per code point. To fix this we probably need our
own char type.
While at it, remove some special cases that try to prevent insertion of
control characters. I don't think they are necessary. Could be wrong..
This makes it so code like
```fish
echo foo
echo bar
```
is collapsed into
```fish
echo foo
echo bar
```
One empty line is allowed, more is overkill.
We could also allow more than one for e.g. function endings.
We don't need to know that it tried these five before finally getting
one, the list is *right there*.
It is also very unlikely that someone has "xterm" or "ansi" but not "xterm-256color"
For xterm-256color, we don't warn *at all* because we have that one hardcoded.
This allows us to get the terminfo information without linking against curses.
That means we can get by without a bunch of awkward C-API trickery.
There is no global "cur_term" kept by a library for us that we need to invalidate.
Note that it still requires a "unhashed terminfo database", and I don't know how well it handles termcap.
I am not actually sure if there are systems that *can't* have terminfo, everything I looked at
has the ncurses terminfo available to install at least.
We still don't support tabs but as of the parent commit, there are no more
weird glitches, so it should be fine to recall those lines?
This reverts commit cc0e366037.
Inserting Tab or Backspace characters causes weird glitches. Sometimes it's
useful to paste tabs as part of a code block.
Render tabs as "␉" and so on for other ASCII control characters, see
https://unicode-table.com/en/blocks/control-pictures/. This fixes the
width-related glitches.
You can see it in action by inserting some control characters into the
command line:
set chars
for x in (seq 1 0x1F)
set -a chars (printf "%02x\\\\x%02x" $x $x)
end
eval set chars $chars
commandline -i "echo '" $chars
Fixes#6923Fixes#5274Closes#7295
We could extend this approach to display a fallback symbol for every unknown
nonprintable character, not just ASCII control characters.
In future we might want to support tab properly.
This reserved 64, which is *gigantic*.
Over all of share/**.fish, 75% of lists are empty, 99.97% are 16
elements or fewer.
Reducing this to 16 reduces memory usage for a gigantic example
script (git.fish pasted a bunch of times for a total of almost 100k
lines) by ~10% and speeds up "--no-execute" time by the same amount.
For smaller scripts it's less noticeable simply because parse time
matters less.
There are other options, like creating the vec ::with_capacity, or
using 8 instead of 16, or even letting the vec just grow
naturally (rust's vec currently grows from 0 to 4 and then doubles,
which isn't terrible for this use), but the point is that 64 is
wasteful and never comes out on top, always in the last two places
comparing a bunch of choices.
Commit e5b34d5cd (Suppress autosuggesting during backspacing like browsers do,
2012-02-06) disabled autosuggestion when backspacing. Autosuggestions are
re-enabled whenever we insert anything in the command line. Undo uses a
different code path to insert into the command line, which does not re-enable
autosuggestion.
Fix that.
Also re-enable autosuggestion when undo erases from the command line.
This seems like the simplest approach. It's not clear if there's a better
behavior; browsers don't agree on one in any case.
This is the last remnant of the old percent expansion.
It has the downsides of it, in that it is annoying to combine with
anything:
```fish
echo %self/foo
```
prints "%self/foo", not fish's pid.
We have introduced $fish_pid in 3.0, which is much easier to use -
just like a variable, because it is one.
If you need backwards-compatibility for < 3.0, you can use the
following shim:
```fish
set -q fish_pid
or set -g fish_pid %self
```
So we introduce a feature-flag called "remove-percent-self" to turn it
off.
"%self" will simply not be special, e.g. `echo %self` will print
"%self".
This stops you from doing e.g.
```fish
set pager command less
echo foo | $pager
```
Currently, it would run the command *builtin*, which can only do
`--search` and similar, and would most likely end up printing its own
help.
That means it very very likely won't work, and the code is misguided -
it is trying to defeat function resolution in a way that won't do what
the author wants it to.
The alternative would be to make the command *builtin* execute the
command, *but*
1. That would require rearchitecting and rewriting a bunch of it and
the parser
2. It would be a large footgun, in that `set EDITOR command foo` will
only ever work inside fish, but $EDITOR is also used outside.
I don't want to add a feature that we would immediately have to discourage.
Currently, if you enter `echo` and press up-arrow, it might select
e.g. `echo foo`.
You can then enter text, making it `echo foobar` and press up-arrow
again, but the search string is *still* `echo`.
Many *other* input functions will end history search, including e.g.
expand-abbr, so pressing space by default will already end it.
So this ends the history search once you input something.
Incidentally this allows suggestions to work in this case, so it
Fixes#10287
Note that autosuggestions have been disabled while history search is
active since a08450bcb6, I'm not sure
it's actually *needed*, so it would also be possible to enable it in
that case.
But since this is already awkward (history search is *active* but with
the old search string) and I'm not sure if e.g. suggestions during
history search would be too busy, let's do this first.
This reduces the test time by ~33% on my system (23s to 15s)
Given that it takes ~180-240s on Github Actions, if we get a reduction
like that we can save over a minute.
We only use this
1. if we have localeconv_l
2. to get the decimal point / thousands separator for numbers
So we can ignore all this and directly create a purely LC_NUMERIC locale.
This *was* more useful when we were in C++ and the printing functions
all relied on locale, but we only use this in printf and that only
extracts the number stuff.
Commit b768b9d3f (Use fuzzy subsequence completion for options names as well,
2024-01-27) allowed completing "oa" to "--foobar", which is a false positive,
especially because it hides other valid completions of non-option arguments.
Let's at least require a leading dash again before completing option names.
The lines of code I commented on in #10254 were meant to serve only as examples
of the changes I was requesting, not the only instances.
Also just use `Mode::from_bits_truncate()` instead of unsafe or unwrapping since
we know the modes are correct.
* Fix build on NetBSD
Notably:
1. A typo in `f_flag` vs `f_flags` - this was probably never tested
2. Some pointless name differences - `st_mtimensec` vs
`st_mtime_nsec`
3. The big one: This said that LC_GLOBAL_LOCALE() was -1 "everywhere".
Well, not on NetBSD.
* ifdef for macos
Version 2.1.0 introduced subsequence matching for completions but as the
changelog entry mentions, "This feature [...] is not yet implemented for
options (like ``--foobar``)". Add it. Seems like a strict improvement,
pretty much.
.unwrap() is in effect an assert(). If it is applied mistakenly, the
program crashes and there isn't a good error.
I would like it to be used as a last resort. In these cases there are
nicer ways to do it that handle a missing result properly.
Issue #10194 reports Cobra completions do
set -l args (commandline -opc)
eval $args[1] __complete $args[2..] (commandline -ct | string escape)
The intent behind "eval" is to expand variables and tildes in "$args".
Fair enough. Several of our own completions do the same, see the next commit.
The problem with "commandline -o" + "eval" is that the former already
removes quotes that are relevant for "eval". This becomes a problem if $args
contains quoted () or {}, for example this command will wrongly execute a
command substituion:
git --work-tree='(launch-missiles)' <TAB>
It is possible to escape the string the tokens before running eval, but
then there will be no expansion of variables etc. The problem is that
"commandline -o" only unescapes tokens so they end up in a weird state
somewhere in-between what the user typed and the expanded version.
Remove the need for "eval" by introducing "commandline -x" which expands
things like variables and braces. This enables custom completion scripts to
be aware of shell variables without eval, see the added test for completions
to "make -C $var/some/dir ".
This means that essentially all third party scripts should migrate from
"commandline -o" to "commandline -x". For example
set -l tokens
if commandline -x >/dev/null 2>&1
set tokens (commandline -xpc)
else
set tokens (commandline -opc)
end
Since this is mainly used for completions, the expansion skips command
substitutions. They are passed through as-is (instead of cancelling or
expanding to nothing) to make custom completion scripts work reasonably well
in the common case. Of course there are cases where we would want to expand
command substitutions here, so I'm not sure.
Move all qmark tests to `scoped_test()` sections with explicitly set feature
flags. We already test the default qmark behavior in the functionality tests.
It seems the logic for calculating the cursor position was not ported correctly,
because the correct place to insert it is at the cursor_pos regardless of
range.start, going by the parameters submitted to the function and the expected
result.
This allows running a fish built from `cargo build` *and* built via
cmake.
In future, we should make this an optional thing that's removed from
installed builds.
NCurses headers contain this conditional "#define cur_term":
print "#elif @cf_cv_enable_reentrant@"
print "NCURSES_WRAPPED_VAR(TERMINAL *, cur_term);"
print "#define cur_term NCURSES_PUBLIC_VAR(cur_term())"
print "#else"
OpenSUSE Tumbleweed uses this configuration option; For reentrancy, cur_term
is a function. If the NCurses autoconf variable @NCURSES_WRAP_PREFIX@
is not changed from its default, the function is called _nc_cur_term.
I'm not sure if we have a need to support non-default @NCURSES_WRAP_PREFIX@
but if we do there are various ways;
- search for the symbol with the cur_term suffix
- figure out the prefix based on the local curses installation,
for example by looking at the header files.
Fixes#10243
This is run in the current locale, without resetting to en_US.UTF-8
like our integration tests do.
So if you want to check for a specific message you need to check the
localized version.
This was previously limited to Linux predicated on the existence
of certain headers, but Rust just exposes those functions unconditionally. So
remove the check and just perform the mtime hack on Linux and Android.
Make sure to also look for the error part that occurs after the last format
specifier.
Still not great because it won't fail if there's unexpected output at the
beginning or end of the string.
Commit 5f849d0 changed control-C to print an inverted ^C and then a newline.
The original motivation was
> In bash if you type something and press ctrl-c then the content of the line
> is preserved and the cursor is moved to a new line. In fish the ctrl-c just
> clears the line. For me the behaviour of bash is a bit better, because it
> allows me to type something then press ctrl-c and I have the typed string
> in the log for further reference.
This sounds like a valid use case in some scenarios but I think that most
abandoned commands are noise. After all, the user erased them. Also, now that
we have undo that can be used to get back a limited set of canceled commands.
I believe the original motivation for existing behavior (in other shells) was
that TERM=dumb does not support erasing characters. Similarly, other shells
like to leave behind other artifacts, for example when using tab-completion
or in their interactive menus but we generally don't.
Control-C is the obvious way to quickly clear a multi-line commandline.
IPython does the same. For the other behavior we have Alt-# although that's
probably not very well-known.
Restore the old Control-C behavior of simply clearing the command line.
Our unused __fish_cancel_commandline still prints the ^C. For folks who
have explicitly bound ^C to that, it's probably better to keep the existing
behavior, so let's leave this one.
Previous attempt at #4713 fizzled.
Closes#10213
This function is a hotspot, but it has inefficient codegen:
1. For whatever reason, the chars() iterator of wstr is slower
than that of a slice. Use the slice.
2. Unnecessary overflow checks were preventing vectorization.
Switch to a more optimized implementation.
This improves aliases benchmark time by about 9%.
Since none of the compiles(xxx) calls are to particularly complex code, we can
just use `rsconf` directly to test for the presence of the symbols or headers as
needed.
Note that it seems at least some of the previous detection was not working
correctly; in particular HAVE_PIPE2 was evaluating to false on my WSL install
where pipe2(2) was available (caught because it revealed some compilation errors
in that conditional compilation path after porting).
I kept the cfg names and the tests themselves mostly as-is, though we might want
to change that to conform with the rust convention of lowercase cfg names and
decide whether we want to prefix all these with have_, fish_, or nothing at all.
Also the posix_spawn() test should probably check for the symbol `posix_spawn()`
rather than the header `spawn.h` since we don't use it via the header but rather
via the symbol (but in reality they're almost certainly going to give the same
result).
NB: I only encountered this when rewriting the cfg detection, which means that
the previous detection wasn't correct since I have pipe2 on Linux but didn't run
into this build error before.
I had originally created a safe `set_locale()` wrapper and clippy-disallowed
`libc::setlocale()` but almost all our uses of `libc::setlocale()` are in a loop
where it makes much more sense to just obtain the lock outright then call
`setlocale()` repeatedly rather than lock it in the wrapper function each time.
No need to use cfg_attr and have to worry about syncing the preconditions for
the cfg_attr with the preconditions for where `slice_contains_slice()` is used
in the codebase, just mark it as `allow(unused)` with a comment.
Use Rust for executables
Drops the C++ entry points and restructures the Rust package into a
library and three binary crates.
Renames the fish-rust package to fish.
At least on Ubuntu, "fish_indent" is built before "fish".
Make sure export CURSES_LIBRARY_LIST to all binaries to make sure
that "cached-curses-libnames" is populated.
Closes#10198
Keep running tests serially to avoid breaking assumptions.
I think many of these tests can run in parallel and/or don't need test_init().
Use the safe variant everywhere, to get it done faster.
This would misname `\e\x7F` as "backspace":
bind -k backspace 'do something'
bind \e\x7F 'do something'
because it would check if there was any key *in there*.
This was probably meant for continuous mode, but it simply doesn't
work right. It's preferable to not give a key when one would work over
giving one when it's not correct.
