On
a;
we don't expand the abbreviation because the cursor is right of semicolon,
not on the command token. Fix this by making sure that we call expand-abbr
with the cursor on the semicolon which is the end of the command token.
(Now that our bind command execution order is less surprising, this is doable.)
This means that we need to fix the cursor after successfully expanding
an abbreviation. Do this by setting the position explicitly even when no
--set-position is in effect.
An earlier version of this patch used
bind space self-insert backward-char expand-abbr or forward-char
The problem with that (as a failing test shows) was that given "abbr m
myabbr", after typing "m space ctrl-z", the cursor would be after the "m",
not after the space. The second space removes the space, not changing the
cursor position, which is weird. I initially tried to fix this by adding
a hack to the undo group logic, to always restore the cursor position from
when begin-undo-group was used.
bind space self-insert begin-undo-group backward-char expand-abbr end-undo-group or forward-char
However this made test_torn_escapes.py fail for mysterious reasons.
I believe this is because that test registers and triggers a SIGUSR1 handler;
since the signal handler will rearrange char events, that probably messes
with the undo group guards.
I resorted to adding a tailor-made readline cmd. We could probably remove
it and give the new behavior to expand-abbr, not sure.
Fixes#9730
File names that have lots of spaces look quite ugly when inserted as
completions because every space will have a backslash.
Add an initial heuristic to decide when to use quotes instead of
backslash escapes.
Quote when
1. it's not an autosuggestion
2. we replace the token or insert a fresh one
3. we will add a space at the end
In future we could relax some of these requirements.
Requirement 2 means we don't quote when appending to an existing token.
Need to find a natural behavior here.
Re 3, if the completion adds no space, users will probably want to add more
characters, which looks a bit weird if the token has a trailing quote.
We could relax this requirement for directory completions, so «ls so»
completes to «ls 'some dir with spaces'/».
Closes#5433
I think commit 8386088b3 (Update commandline state changes eagerly as well,
2024-04-11) broke the alt-s binding.
This is because we update the commandline state snapshot (which is consumed
by builtin commandline and others) only at key points. This seems like a
dubious optimization. With the new streamlined bind execution semantics,
this doesn't really work anymore; any shell command can run any number of
commands like "commandline -i foo" which should synchronize.
Do the simple thing of calculating the snapshot whenever needed.
Commit c3cd68dda (Process shell commands from bindings like regular char
events, 2024-03-02) mentions a "weird ordering difference".
The issue is that "commandline -f foo" goes through the input
queue while other commands are executed directly.
For example
bind ctrl-g "commandline -f end-of-line; commandline -i x"
is executed in the wrong order. Fix that.
This doesn't yet work for "commandline -f exit" but that can be fixed easily.
It's hard to imagine anyone would rely on the existing behavior. "commandline
-f" in bindings is mostly used for repainting the commandline.
If a binding was input starting with "\e", it's usually a raw control sequence.
Today we display the canonical version like:
bind --preset alt-\[,1,\;,5,C foo
even if the input is
bind --preset \e\[1\;5C foo
Make it look like the input again. This looks more familiar and less
surprising (especially since we canonicalize CSI to "alt-[").
Except that we use the \x01 representation instead of \ca because the
"control" part can be confusing. We're inside an escape sequence so it seems
highly unlikely that an ASCII control character actually comes from the user
holding the control key.
The downside is that this hides the canonical version; it might be surprising
that a raw-escape-sequence binding can be erased using the new syntax and
vice versa.
Some terminals send the focus-in sequences ("^[I") whenever focus reporting is
enabled. We enable focus reporting whenever we are finished running a command.
If we run two commands without reading in between, the focus sequences
will show up on the terminal.
Fix this by enabling focus-reporting as late as possible.
This fixes the problem with `^[I` showing up when running "cat" in
gnome-terminal https://github.com/fish-shell/fish-shell/issues/10411.
This begs the question if we should do the same for CSI u and bracketed paste.
It's difficult to answer that; let's hope we find motivating test cases.
If we enable CSI u too late, we might misinterpret key presses, so for now
we still enable those as early as possible.
Also, since we now read immediately after enabling focus events, we can get
rid of the hack where we defer enabling them until after the first prompt.
When I start a fresh terminal, the ^[I no longer shows up.
See the changelog additions for user-visible changes.
Since we enable/disable terminal protocols whenever we pass terminal ownership,
tests can no longer run in parallel on the same terminal.
For the same reason, readline shortcuts in the gdb REPL will not work anymore.
As a remedy, use gdbserver, or lobby for CSI u support in libreadline.
Add sleep to some tests, otherwise they fall (both in CI and locally).
There are two weird failures on FreeBSD remaining, disable them for now
https://github.com/fish-shell/fish-shell/pull/10359/checks?check_run_id=23330096362
Design and implementation borrows heavily from Kakoune.
In future, we should try to implement more of the kitty progressive
enhancements.
Closes#10359
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
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
It appears that the shift-delete key escape sequence is not being generated
because there's no mapping for it in screen-256color, causing the test to fail.
Switch to using f1 for the test.
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.
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.
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.
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.
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.
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
The C++ code implicitly relied on wrapping behavior.
There are probably more cases like this. Maybe we should disable
"overflow-checks" in release mode.
This would crash from the highlighter for something like
`PATH={$PATH[echo " "`
The underlying cause is that we use "char_at" which panics on
overread.
So instead this implements try_char_at and then just returns None.
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.
This was an issue with "--no-execute", which has no variables and
therefore no $HOME:
```fish
fish --no-execute /path/to/file
```
would say the error is in `~/path/to/file`.
Instead, since this is just for a message, we simply return the
filename without doing the replacement.
Fixes#10171
These printed "Unknown error while evaluating command substitution".
Now they print something like
```
fish: for: status: cannot overwrite read-only variable
for status in foo; end
^~~~~^
in command substitution
fish: Invalid arguments
echo (for status in foo; end)
^~~~~~~~~~~~~~~~~~~~~~~^
```
for `echo (for status in foo; end)`
This is, of course, still not *great*. Mostly the `fish: Invalid
arguments` is basically entirely redundant.
An alternative is to simply skip the error message, but that requires some
more scaffolding (describe_with_prefix adds some error messages on its
own, so we can't simply say "don't add the prefix if we don't have a
message")
(cherry picked from commit 1b5eec2af6)
This fixes the following deadlock. The C++ functions path_get_config and
path_get_data lazily determine paths and then cache those in a C++ static
variable. The path determination requires inspecting the environment stack.
If these functions are first called while the environment stack is locked
(in this case, when fetching the $history variable) we can get a deadlock.
The fix is to call them eagerly during env_init. This can be removed once
the corresponding C++ functions are removed.
This issue caused fish_config to fail to report colors and themes.
Add a test.
This makes it so
```fish
if -e foo
# do something
end
```
complains about `-e` not being a command instead of `end` being used
outside of an if-block.
That means both that `-e` could now be used as a command name (it
already can outside of `if`!) *and* that we get a better error!
The only way to get `if` to be a decorated statement now is to use `if
-h` or `if --help` specifically (with a literal option).
The same goes for switch, while and begin.
It would be possible, alternatively, to disallow `if -e` and point
towards using `test` instead, but the "unknown command" message should
already point towards using `test` more than pointing at the
"end" (that might be quite far away).
This was already supposed to handle `--foo=bar<TAB>` cases, except it
printed the `--foo=` again, causing fish to take that as part of the
token.
See #9538 for a similar thing with __fish_complete_directories.
Fixes#10011
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.
This used to print all codepoints outside of the ASCII range (i.e.
above 0x80) in \uXXXX or \UYYYYYYYY notation.
That's quite awkward, considering that this is about keys that are
being pressed, and many keyboards have actual symbols for these on
them - I have an "ö" key, so I would like to use `bind ö` and not
`bind \u00F6`. So we go by iswgraph.
On a slightly different note, `\e` was written as `\c[ (or \e)`. I do
not believe anyone really uses `\c[` (the `[` would need to
be escaped!), and it's confusing and unnecessary to even mention that.
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
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 used expect_re with a regex ending in `.*`, followed by an
`expect_prompt`.
This meant that, depending on the timing, the regex could swallow the
prompt marker, which caused extremely confusing output like
>Testing file pexpects/generic.py:Failed to match pattern: prompt 14
> ...
> OUTPUT +1.33 ms (Line 70): \rprompt 13>functions\r\nN_, abbr,
> alias, bg, cd, [SNIP], up-or-search, vared, wait\r\n⏎
> \r⏎ \r\rprompt 14>
Yeah - it shows that "prompt 14" was in the output and it can't find
"prompt 14".
I could reproduce the failure locally when running the tests
repeatedly. I got one after 17 attempts and so far haven't been able
to reproduce it with this change applied.
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.
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
```
We could end up overflowing if we print out something that's a multiple of the
chunk size, which would then finish printing in the chunk-printing, but not
break out early.
This makes `fish -c begin` fail with a status of 127 - it already
printed a syntax error so that was weird. (127 was the status for
syntax errors when piping to fish, so we stay consistent with that)
We allow multiple `-c` commands, and this will return the regular
status if the last `-c` succeeded.
This is fundamentally an extremely weird situation but this is the
simple targeted fix - we did nothing, unsuccessfully, so we should
fail.
Things to consider in future:
1. Return something better than 127 - that's the status for "unknown
command"!
2. Fail after a `-c` failed, potentially even checking all of them
before executing the first?
Fixes#9888
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
This caused math to assert out because it never wrote into the buffer.
Now, presumably it wrote somewhere but I don't know where, so fixing
this seems like a good idea.
Fixes#9735.
Most of it is duplicated, hence untested.
Functions like mbrtowc are not exposed by the libc crate, so declare them
ourselves.
Since we don't know the definition of C macros, add two big hacks to make
this work:
1. Replace MB_LEN_MAX and mbstate_t with values (resp types) that should
be large enough for any implementation.
2. Detect the definition of MB_CUR_MAX in the build script. This requires
more changes for each new libc. We could also use this approach for 1.
Additionally, this commit brings a small behavior change to
read_unquoted_escape(): we cannot decode surrogate code points like \UDE01
into a Rust char, so use � (\UFFFD, replacement character) instead.
Previously, we added such code points to a wcstring; looks like they were
ignored when printed.
Otherwise this would complete
`git --exec-path=foo`, by running `complete -C"'' --exec-path=foo"`,
which would print "--exec-path=foo", and so it would end as
`git --exec-path=--exec-path=foo` because the "replaces token" bit was
lost.
I'm not sure how to solve it cleanly - maybe an additional option to
`complete`?
Anyway, for now this
Fixes#9538.
Another from the "why are we asserting instead of doing something
sensible" department.
The alternative is to make exit() and return() compute their own exit
code, but tbh I don't want any *other* builtin to hit this either?
Fixes#9659
The test passes but only if executed on its own. It's not the most perfect test,
but I can basically never get `make test` to pass under WSL while that's not the
case on all my other machines.
Keeps the location of original function definition, and also stores
where it was copied. `functions` and `type` show both locations,
instead of none. It also retains the line numbers in the stack trace.
By default, fish does not complete files that have leading dots, unless the
wildcard itself has a leading dot. However this also affected completions;
for example `git add` would not offer `.gitlab-ci.yml` because it has a
leading dot.
Relax this for custom completions. Default file expansion still
suppresses leading dots, but now custom completions can create
leading-dot completions and they will be offered.
Fixes#3707.
This translated ctrl-k to "\v", which is a "vertical tab", and ctrl-l
to "\f" and ctrl-g to "\a".
There is no "vertical tab" or "alarm" or "\f" *key*, so these
shouldn't be translated. Just drop these and call them `\ck` and such.
(vertical tab specifically is utterly useless and I would be okay with
dropping it entirely, I have never seen it used anywhere)
