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)
Commit 3b30d92b6 (Commit transient edit when closing pager, 2022-08-31)
inadvertently introduced two regressions to history search:
1. It made Escape keeps the selected history entry,
instead of restoring the commandline before history search.
2. It made history search commands add undo entries.
Fix both of this issues.
Inadvertently broken in a2d816710f,
this made `cd .` no longer offer `cd ../` (same for general file completions
like `ls .`, which only offers dotfiles)
This means cleaning out old universal variables is now just:
```fish
abbr --erase (abbr --list)
```
which makes upgrading much easier.
Note that this erases the currently defined variable and/or any
universal. It doesn't stop at the former because that makes it *easy*
to remove the universals (no running `abbr --erase` twice), and it
doesn't care about globals because, well, they would be gone on
restart anyway.
Fixes#9468.
This now means `abbr --add` has two modes:
```fish
abbr --add name --function foo --regex regex
```
```fish
abbr --add name --regex regex replacement
```
This is because `--function` was seen to be confusing as a boolean flag.
Example output from a Cirrus bionic-asan-clang run:
```
fish: Unknown command: man
/tmp/cirrus-ci-build/share/functions/__fish_man_page.fish (line 30):
if man "$maincmd" &>/dev/null
^~^
in function '__fish_man_page'
�
[I] prompt 9>echo TEXT
[I] prompt 9>echo TEXThrAi
[I] prompt 9>echo TEXThrAi
TEXThrAi
```
Yes, this detected escape, waiting *300ms* and then "h" as being below
the escape timeout of 120ms.
Unfortunately print_hints was true *by default* - so for all builtins
that didn't pass it it would now be false instead.
This resulted in the trailer missing, which includes the line number
and context. So if you ran a script that includes `bind -M` the error
message would now just be "bind: -M: option requires an argument",
with no indication as to where.
This reverts commit 8a50d47a46.
This would print
```
abbr -a -- dotdot --regex ^\\.\\.+\$ --function multicd
```
which expands "dotdot" to "--regex ^\\.\\.+\$...".
Instead, we move the name to right before the replacement, and move
the `--` before that:
```
abbr -a --regex ^\\.\\.+\$ --function -- dotdot multicd
```
It might be possible to improve that, but this at least round-trips.
Historical behavior is to stop option parsing at the first non-option argument.
Since we have added more options, it seemed impractical to keep that behavior.
However people are using options in their abbr expansions ("abbr e emacs
-nw"). To support this, we ignore options. However, we only ignore them
if they are not valid "abbr" options. Let's ignore all options in the
expansion definition, which is a small price to pay to keep most existing
configurations working.
Fixes#9410
This does not fix other cases which used to work, like
abbr x -unknown
Those are hopefully not used by anyone, so I don't think we need to maintain
support for that.
Also default the marker to '%'. So you may write:
abbr -a L --position anywhere --set-cursor "% | less"
or set an explicit marker:
abbr -a L --position anywhere --set-cursor=! "! | less"
This renames abbreviation triggers from `--trigger-on entry` and
`--trigger-on exec` to `--on-space` and `--on-enter`. These names are less
precise, as abbreviations trigger on any character that terminates a word
or any key binding that triggers exec, but they're also more human friendly
and that's a better tradeoff.
set-cursor enables abbreviations to specify the cursor location after
expansion, by passing in a string which is expected to be found in the
expansion. For example you may create an abbreviation like `L!`:
abbr L! --position anywhere --set-cursor ! "! | less"
and the cursor will be positioned where the "!" is after expansion, with
the "| less" appearing to its right.
This adds support for the `--function` option of abbreviations, so that the
expansion of an abbreviation may be generated dynamically via a fish
function.
Prior to this change, abbreviations were stored as fish variables, often
universal. However we intend to add additional features to abbreviations
which would be very awkward to shoe-horn into variables.
Re-implement abbreviations using a builtin, managing them internally.
Existing abbreviations stored in universal variables are still imported,
for compatibility. However new abbreviations will need to be added to a
function. A follow-up commit will add it.
Now that abbr is a built-in, remove the abbr function; but leave the
abbr.fish file so that stale files from past installs do not override
the abbr builtin.
We have had multiple crashes for relative CDPATH entries. Commit 5e274066e
(Always return absolute path in path_get_cdpath, 2019-10-17) tried to fix
all of them but it failed to do justice to its title. Let's fix this to
actually return absolute paths, always. Take care to to normalize the path
because it is used for autosuggestions. The normalization is mostly relevant
for CDPATH=. (the default) but it doesn't hurt others.
Closes#9407
Inside a comment we offer plain file completions (or command completions if
the comment is in command position). However these completions are broken
because they don't consider any of the surrounding characters. For example
with a command line
echo # comment
^ cursor
we suggest file completions and insert them as
echo # comsomefile ment
Providing completions inside comments does not seem useful and it can be
misleading. Let's remove the completions; this should communicate better that
we are in a free-form comment that's not subject to fish syntax.
Closes#9320
This fixes#9321
IEEE Std 1003.1-2017 Issue 6 added optional error condition
[EINVAL] for if no conversion could be performed.
Switch back to wcstoimax/wcstoumax: do not work around the old FreeBSD
8 issue.
Add a test for printf '%d %d' 1 2 3
Like the pexpect-based pager compeltions test `complete-group-order.py`, but for
the `complete` builtin. Verifies the same sort/dedup rules that apply to the
pager are also applied to the output of `complete` and asserts the sort behavior
for multiple `complete -k` calls for the same command and with the same (or with
both passing) preconditions.
This addresses a long-standing TODO where `complete -C` output isn't
deduplicated.
With this patch, the same deduplication and sort procedure that is run on actual
pager completions is also executed for `complete -C` completions (with a `-C`
payload specified).
This makes it possible to use `complete -C` to test what completions will
actually be generated by the completions pager instead of it displaying
something completely divorced from reality, improving the productivity of fish
completions developers.
Note that completions that wouldn't be shown in the pager are also omitted from
the results, e.g. `test/buildroot/` and `test/fish_expand_test/` are omitted
from the check matches in `checks/complete_directories.fish` because even if
they were generated, the pager wouldn't have shown them. This again makes
reasoning about and debugging completions much easier and more sane.
Fixes ommitted newline char shown after complete -n'(foo)'
Also axes the 'contains syntax errors' line before the error.
Update tests
before
> complete -n'(foo)'
complete: Condition '(foo)' contained a syntax error
complete: Command substitutions not allowed⏎
after
> complete -n'(foo)'
complete: -n '(foo)': command substitutions not allowed here
Ensure that multiple `-k` completions intermixed with one or more non-`-k`
completions are produced in the expected order with the order of all completions
in a single `-k` completion respected, non-`-k` completions correctly sorted and
interspersed, and the results of multiple `-k` completions in the
reverse-intuitive order (with chronologically later completions coming before
chronologically earlier `-k` counterparts), as per #9221.
This particular variant must be executed as a pexpect test since it relies on
the interactive-only `$history` to trigger the recursion. Note that recursion is
possible via other means (e.g. reading/writing a file), the usage of history
here is just one such example.
A false negative while testing locally should be a rare thing, and individual
pexpect tests already take too long in case of a non-match making for a painful
edit-test loop.
Up to now, in normal locales \x was essentially the same as \X, except
that it errored if given a value > 0x7f.
That's kind of annoying and useless.
A subtle change is that `\xHH` now represents the character (if any)
encoded by the byte value "HH", so even for values <= 0x7f if that's
not the same as the ASCII value we would diverge.
I do not believe anyone has ever run fish on a system where that
distinction matters. It isn't a thing for UTF-8, it isn't a thing for
ASCII, it isn't a thing for UTF-16, it isn't a thing for any extended
ASCII scheme - ISO8859-X, it isn't a thing for SHIFT-JIS.
I am reasonably certain we are making that same assumption in other
places.
Fixes#1352
We forgot to decode (i.e. turn into nice wchar_t codepoints)
"byte_literal" escape sequences. This meant that e.g.
```fish
string match ö \Xc3\Xb6
math 5 \X2b 5
```
didn't work, but `math 5 \x2b 5` did, and would print the wonderful
error:
```
math: Error: Missing operator
'5 + 5'
^
```
So, instead, we decode eagerly.
Prior to 1811a2d, the return value for negative return codes was UB and I'd
witnessed both expected cases like -256 mapping to a $status of 0 and unexpected
cases like a return value of -1 mapping to a $status of 0. As such, this doesn't
test just one fixed return value but the entire range from negative multiples of
256 all the way down (rather, up!) to -1.
...for improved cross-platform support.
Following up on the work in c90ac7b. There was one more test that had mktemp in
the littlecheck "shebang" and this also removes a now-unnecessary `env` prefix.
All usages of `mktemp` must go through the (fish-only) `mktemp` test function
that abstracts over the differences across multiple platforms/flavors.
Tests can be easily run individually via `ninja -C build test_xxx` and there
isn't a good reason to randomly manually override $HOME and $XDG_CONFIG_HOME for
a test here and a test there.
If it's absolutely necessary, littlecheck.py should be extended to support a
`%temp` variable initialized to a temporary directory and that can be used
instead of calling out to the platform-provided `mktemp` via a subshell.
For unknown reasons, the i686 launchpad builders fail on this date,
but apparently not the others.
Let's just remove it, we've tested dates older than the epoch, this is
slightly redundant.
As discussed in #9221, a bug in the autocomplete that was fixed in 66391922
caused completions to be incorrectly suppressed. The dropped test/check was
inadvertently relying on the buggy behavior and expected a git invocation to
generate no completions but there are, in fact, completions now that the bug has
been resolved.
cc @faho: I'm not sure if you want to replace this with a different check that
actually doesn't yield any completions or if you're happy with it just being
dropped.
This fails on old Ubuntu with:
> touch: invalid date format ‘190112112040.39’
Because we don't actually need the seconds here, we just use minute
resolution. It's fine.
Also use `path mtime`, because that's a portable way to get the mtime.
I forgot `stat` is non-portable. There's no great way to portably get a
machine-readable representation of stat(2) for a file. I don't want to ship our
own lstat(2) wrapper executable just for this test and don't want to fork out to
python or perl for this either - I just wanted to get the tests to pass under
WSL :'(
Anyway, just give up and make it skip just for WSL. If another OS fails this
test in the future, the comments and existing workaround will make it easy to
figure out what the problem is and what needs to be done. We'll cross that
bridge when we get there.
It turns out that not all systems print an unsigned integer as the output of
`stat -c %Y xxx` and the leading `-` can be misinterpreted as a parameter to
`string match`.
There's no guarantee (nor requirement) that the filesystem support pre-epoch
modification dates. If it doesn't, the `test` tests were failing to get the
expected results.
Skip the test if it seems the fs doesn't support pre-epoch timestamps
(determined by pre-epoch mt of `oldest` evaluating to 0 or the unix epoch).
* Replace ";" with "\n" in alias-generated functions
This can let us add a "#" in our aliases to make
them ignore additional arguments.
* Update changelog about aliases that ignore arguments
* Update test for alias.fish
This is now compliant with the aliases that can
ignore arguments.
When fish runs with job control enabled, it transfers ownership of the
tty to a child process, and then reclaims the tty after the process
exits. If job control is disabled then fish does not transfer or reclaim
the tty.
It may happen that the child process creates a pgroup and then transfers
the tty to it. In that case fish will not attempt to reclaim the tty, as
fish did not transfer it. Then when fish reads from stdin it will
receive SIGTTIN instead of data.
Fix this by unconditionally claiming the tty in readline().
Fixes#9181
This errored out *later* because the result was infinite or NaN, but
it didn't actually stop evaluation.
I'm not sure if there is a way to get floating point math to turn an
infinity back into something that doesn't depend on a literal
infinity, but division by zero conceptually isn't a thing we can
support.
There's entire branches of maths dedicated to figuring out what
dividing by "basically zero" means and we don't have to get into it.
This is essentially the inverse of `string pad`.
Where that adds characters to get up to the specified width,
this adds an ellipsis to a string if it goes over a specific maximum width.
The char can be given, but defaults to our ellipsis string.
("…" if the locale can handle it and "..." otherwise)
If the ellipsis string is empty, it just truncates.
For arguments given via argv, it goes line-by-line,
because otherwise length makes no sense.
If "--no-newline" is given, it adds an ellipsis instead and removes all subsequent lines.
Like pad and `length --visible`, it goes by visible width,
skipping recognized escape sequences, as those have no influence on width.
The default target width is the shortest of the given widths that is non-zero.
If the ellipsis is already wider than the target width,
we truncate instead. This is safer overall, so we don't e.g. move into a new line.
This is especially important given our default ellipsis might be width 3.
When selecting items in the pager, only the latest of those items is kept
in the edit history, as so-called transient edit. Each new transient edit
evicts any old transient edit (via undo).
If the pager is closed by a command that performs another transient edit
(like history-token-search-backward) we thus inadvertently undo (= remove)
the token inserted by the pager. Fix this by closing a transient edit
session when closing the pager. Token search will start its own session.
Fixes#9160
This starts two sleep processes and expects them to be killed on
SIGHUP.
Unfortunately, if this ever fails the second run will also fail
because it'll see the old sleep still lying around (because it'll run
for 130 seconds).
So, what we do is:
1. Keep the pids for these specific sleeps
2. Check if any of them are still running (and only fail for them)
3. Kill them from python
Fixes#9152
This checked specifically for "| and" and "a=b" and then just gave the
error without a caret at all.
E.g. for a /tmp/broken.fish that contains
```fish
echo foo
echo foo | and cat
```
This would print:
```
/tmp/broken.fish (line 3): The 'and' command can not be used in a pipeline
warning: Error while reading file /tmp/broken.fish
```
without any indication other than the line number as to the location
of the error.
Now we do
```
/tmp/broken.fish (line 3): The 'and' command can not be used in a pipeline
echo foo | and cat
^~^
warning: Error while reading file /tmp/broken.fish
```
Another nice one:
```
fish --no-config -c 'echo notprinted; echo foo; a=b'
```
failed to give the error message!
(Note: Is it really a "warning" if we failed to read the one file we
wer told to?)
We should check if we should either centralize these error messages
completely, or always pass them and remove this "code" system, because
it's only used in some cases.
This skipped printing a "^" line if the start or length of the error
was longer than the source.
That seems like the correc thing at first glance, however it means
that the caret line isn't skipped *if the file goes on*.
So, for example
```fish
echo "$abc["
```
by itself, in a file or via `fish -c`, would not print an error, but
```fish
echo "$abc["
true
```
would. That's not a great way to print errors.
So instead we just.. imagine the start was at most at the end.
The underlying issue why `echo "$abc["` causes this is that `wcstol`
didn't move the end pointer for the index value (because there is no
number there). I'd fix this, but apparently some of
our recursive variable calls absolutely rely on this position value.
This stops us from loading the completions for e.g. `./foo` if there
is no `foo` in path.
This is because the completion scripts will call an unqualified `foo`,
and then error out.
This of course means if the script would work because it never calls
the command, we still don't load it.
Pathed completions via `complete --path` should be unaffected because
they aren't autoloaded anyway.
Workaround for #3117Fixes#9133
This was misguidedly "fixed" in
9e08609f85, which made printf error out
with any "-"-prefixed words as the first argument.
Note: This means currently `printf --help` doesn't print the help.
This also matches `echo`, and we currently don't have anything to make
a literal `--help` execute a builtin help except for keywords. Oh well.
Fixes#9132
This used to be kept, so e.g. testing it with
fish_read_limit=5 echo (string repeat -n 10 a)
would cause the prompt and such to error as well.
Also there was no good way to get back to the default value
afterwards.
* string repeat: Don't allocate repeated string all at once
This used to allocate one string and fill it with the necessary
repetitions, which could be a very very large string.
Now, it instead uses one buffer and fills it to a chunk size,
and then writes that.
This fixes:
1. We no longer crash with too large max/count values. Before they
caused a bad_alloc because we tried to fill all RAM.
2. We no longer fill all RAM if given a big-but-not-too-big value. You
could've caused fish to eat *most* of your RAM here.
3. It can start writing almost immediately, instead of waiting
potentially minutes to start.
Performance is about the same to slightly faster overall.
The previous fix was reverted because it broke another scenario. Add tests
for both scenarios.
The first test exposes another problem: autosuggestions are sometimes not
recomputed after selecting the first completion with Tab Tab. Fix that too.
Since the fix for #3892, this escaping style escapes
\n to \\n
as well as
\\ to \\\\
\' to \\'
I believe these two are the only printable characters that are escaped with
ESCAPE_NO_PRINTABLES.
The rationale is probably to keep the encoding unambiguous and reversible.
However that doesn't justify escaping the single quote. Probably this was
an accident, so let's revert that part.
This has the nice effect that single quotes will no longer be escaped
when rendered in the completion pager (which is consistent with other
special characters). Try it:
complete : -a "aaa\'\; aaaa\'\;" -f
Also this makes the error output of builtin bind consistent:
$ bind -e --preset \;
$ bind -e --preset \'
$ bind \;
bind: No binding found for sequence “;”
$ bind \'
bind: No binding found for sequence “'”
the last line is clearly better than the old version:
bind: No binding found for sequence “\'”
In general, the fact that ESCAPE_NO_PRINTABLES escapes the (printable)
backslash is weird but I guess it's fine because it looks more consistent to
users, even though the result is an undocumented subset of the fish language.
command_line_has_transient_edit tracks the actual command line, not the
pager search field. We accidentally reset it after modifying the search field
which causes unexpected behavior - the commandline added by the completion
pager remains even after I press Escape.
This was an inadvertent change from
cc632d6ae9.
Because we used wgetcwd directly before, we always got the "physical"
resolved $PWD.
There's an argument to be made to use the logical $PWD here as well
but I prefer not to make changes lik that in a random commit without
good reason.
This can be used to print the modification time, like `stat` with some
options.
The reason is that `stat` has caused us a number of portability
headaches:
1. It's not available everywhere by default
2. The versions are quite different
For instance, with GNU stat it's `stat -c '%Y'`, with macOS it's `stat
-f %m`.
So now checking a cache file can be done just with builtins.
This adds a line to `set --show`s output like
```
$PATH: originally inherited as |/home/alfa/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/var/lib/flatpak/exports/bin|
```
to help with debugging.
Note that this means keeping an additional copy of the original
environment around. At most this would be one ARG_MAX's worth, which
is about 2M.
This is sort of slow because it's called hundreds of times.
We used to have a cache, introduced in ad9b4290e, but it was removed
in fee5a9125a because it had
false-positives.
So what we do, because the issue is that this is called hundreds of
times per-commandline, we cache it keyed on the commandline.
This speeds up `complete -C'git sta'` by a factor of 2.3x.
It's still useful without, for instance to implement a command that
takes no options, or to check min-args or max-args.
(technically no optspecs, no min/max args and --ignore-unknown does
nothing, but that's a very specific error that we don't need to forbid)
Fixes#9006
Commit ad9b4290e optimized git completions by adding a completion that would
run on every completion request, which allows to precompute data used by
other completion entries. Unfortunately, the completion entry is not run
when the commandline contains a flag like `git -C`. If we didn't
already load git.fish, we'd error. Additionally, we got false positive
completions for `git diff -c`.
So this hack was a very bad idea. We should optimize in another way.
This is simply an error in test setup. There's a limit to how far we
can isolate them from the system.
(it's possible new cmake versions close fds automatically since I
can't reproduce the original issue via `ninja test` or `make test`)
Fixes#9017
This lacks the tmux-256color terminfo entry, leading to spurious
warnings like
warning: Could not set up terminal. <= no check matches
warning: TERM environment variable set to \'tmux-256color\'. <= no check matches
warning: Check that this terminal type is supported on this system. <= no check matches
warning: Using fallback terminal type \'ansi\'. <= no check matches
Git's pathspec system is kind of annoying:
> A pathspec that begins with a colon : has special meaning. In the short form, the leading colon : is followed by zero or more "magic signature" letters (which optionally is terminated by another colon :), and the remainder is the pattern to match against the path. The "magic signature" consists of ASCII symbols that are neither alphanumeric, glob, regex special characters nor colon. The optional colon that terminates the "magic signature" can be omitted if the pattern begins with a character that does not belong to "magic signature" symbol set and is not a colon.
So if we complete `:/foo`, that "works" because "f" is alphanumeric
and so the "/" is the only magic character here.
If, however the filename starts with a magic character, that's used as
a magic signature.
So we do what the docs say and terminate the magic signature after the
"/" (which means "from the repo root").
Fixes#9004
This makes it so
1. The informative status can work without showing untracked
files (previously it was disabled if bash.showUntrackedFiles was
false)
2. If untrackedfiles isn't explicitly enabled, we use -uno, so git
doesn't have to scan all the files.
In a large repository (like the FreeBSD ports repo), this can improve
performance by a factor of 5 or up.
In b0084c3fc4, we refactored out event handlers get removed. But this
also caused us to remove "one-shot" handlers even if they have not yet
been fired. Fix this.
When switching this to use `git status`, I neglected to use the
correct definition of what a "dirty" and a "staged" change is.
So this now showed already staged files still as "dirty".
Fixes#8986
This makes it so `complete -c foo -n test1 -n test2` registers *both*
conditions, and when it comes time to check the candidate, tries both,
in that order. If any fails it stops, if all succeed the completion is offered.
The reason for this is that it helps with caching - we have a
condition cache, but conditions like
```fish
test (count (commandline -opc)) -ge 2; and contains -- (commandline -opc)[2] length
test (count (commandline -opc)) -ge 2; and contains -- (commandline -opc)[2] sub
```
defeats it pretty easily, because the cache only looks at the entire
script as a string - it can't tell that the first `test` is the same
in both.
So this means we separate it into
```fish
complete -f -c string -n "test (count (commandline -opc)) -ge 2; and contains -- (commandline -opc)[2] length" -s V -l visible -d "Use the visible width, excluding escape sequences"
+complete -f -c string -n "test (count (commandline -opc)) -ge 2" -n "contains -- (commandline -opc)[2] length" -s V -l visible -d "Use the visible width, excluding escape sequences"
```
which allows the `test` to be cached.
In tests, this improves performance for the string completions by 30%
by reducing all the redundant `test` calls.
The `git` completions can also greatly benefit from this.
This adds a path builtin to deal with paths.
It offers the following subcommands:
filter to go through a list of paths and only print the ones that pass some filter - exist, are a directory, have read permission, ...
is as a shortcut for filter -q to only return true if one of the paths passed the filter
basename, dirname and extension to print certain parts of the path
change-extension to change the extension to a different one (as a string operation)
normalize and resolve to canonicalize the paths in various flavors
sort to sort paths, also only using the basename or dirname as a key
The definition of "extension" here was carefully considered and should line up with how extensions are actually used - ~/.bashrc doesn't have an extension, but ~/.conf.d does (".d").
These subcommands all compose well - they can read from arguments or stdin (like string), they can use null-delimited input or output (input is autodetected - if a NULL happens in the first PATH_MAX bytes it switches automatically).
It is both a failglob exception (so like set if a glob passed to it fails it just doesn't get any arguments for it instead of triggering an error), and passes output to command substitution buffers explicitly split (like string split0) so newlines are easy to handle.
This would still remove non-existent paths, which isn't a strict
inversion and contradicts the docs.
Currently, to only allow paths that exist but don't pass a type check,
you'd have to filter twice:
path filter -Z foo bar | path filter -vfz
If a shortcut for this becomes necessary we can add it later.
This is now added to the two commands that definitely deal with
relative paths.
It doesn't work for e.g. `path basename`, because after removing the
dirname prepending a "./" doesn't refer to the same file, and the
basename is also expected to not contain any slashes.
Because we now count the extension including the ".", we print an
empty entry.
This makes e.g.
```fish
set -l base (path change-extension '' $somefile)
set -l ext (path extension $somefile)
echo $base$ext
```
reconstruct the filename, and makes it easier to deal with files with
no extension.
This means "../" components are cancelled out even after non-existent
paths or files.
(the alternative is to error out, but being able to say `path resolve
/path/to/file/../../` over `path resolve (path dirname
/path/to/file)/../../` seems worth it?)
Yeah, the macOS tests fail because it's started in /private/var... with a
$PWD of /var.... So resolve canonicalizes the path, which makes it no
longer match $PWD.
Simply use pwd -P
This just goes back until it finds an existent path, resolves that,
and adds the normalized rest on top.
So if you try
/bin/foo/bar////../baz
and /bin exists as a symlink to /usr/bin, it would resolve that, and
normalize the rest, giving
/usr/bin/foo/baz
(note: We might want to add this to realpath as well?)
This includes the "." in what `path extension` prints.
This allows distinguishing between an empty extension (just `.`) and a
non-existent extension (no `.` at all).
This adds a "path" builtin that can handle paths.
Implemented so far:
- "path filter PATHS", filters paths according to existence and optionally type and permissions
- "path base" and "path dir", run basename and dirname, respectively
- "path extension PATHS", prints the extension, if any
- "path strip-extension", prints the path without the extension
- "path normalize PATHS", normalizes paths - removing "/./" components
- and such.
- "path real", does realpath - i.e. normalizing *and* link resolution.
Some of these - base, dir, {strip-,}extension and normalize operate on the paths only as strings, so they handle nonexistent paths. filter and real ignore any nonexistent paths.
All output is split explicitly, so paths with newlines in them are
handled correctly. Alternatively, all subcommands have a "--null-input"/"-z" and "--null-output"/"-Z" option to handle null-terminated input and create null-terminated output. So
find . -print0 | path base -z
prints the basename of all files in the current directory,
recursively.
With "-Z" it also prints it null-separated.
(if stdout is going to a command substitution, we probably want to
skip this)
All subcommands also have a "-q"/"--quiet" flag that tells them to skip output. They return true "when something happened". For match/filter that's when a file passed, for "base"/"dir"/"extension"/"strip-extension" that's when something about the path *changed*.
Filtering
---------
`filter` supports all the file*types* `test` has - "dir", "file", "link", "block"..., as well as the permissions - "read", "write", "exec" and things like "suid".
It is missing the tty check and the check for the file being non-empty. The former is best done via `isatty`, the latter I don't think I've ever seen used.
There currently is no way to only get "real" files, i.e. ignore links pointing to files.
Examples
--------
> path real /bin///sh
/usr/bin/bash
> path extension foo.mp4
mp4
> path extension ~/.config
(nothing, because ".config" isn't an extension.)
This teaches `--on-signal SIGINT` (and by extension `trap cmd SIGINT`)
to work properly in scripts, not just interactively. Note any such
function will suppress the default behavior of exiting. Do this for
SIGTERM as well.
Like `set` and `read` before it, `eval` can be used to set variables,
and so it can't be shadowed by a function without loss of
functionality.
So this forbids it.
Incidentally, this means we will no longer try to autoload an
`eval.fish` file that's left over from an old version, which would
have helped with #8963.
Previously, running `fish_add_path /foo /foo` would result in /foo
being added to $PATH twice.
Now we check that it hasn't already been given, so we skip the
second (and any further) occurence.
This *might* be a bit faster running under TSAN, otherwise it takes >
400 seconds on Github Actions.
If this doesn't work we need to disable it for TSAN.
Curses variables like `enter_italics_mode` are secretly defined to
dereference through the `cur_term` variable. Be sure we do not read or
write these curses variables if cur_term is NULL. See #8873, #8875.
Add a regression test.
To recap, this means `&` in the middle of a word no longer
backgrounds.
So:
```fish
echo foo&bar # prints foo&bar
echo foo& bar # backgrounds an echo that prints "foo" and runs "bar"
```
This can no longer be changed. If "no-stderr-nocaret" is in
$fish_features it will simply be ignored.
The "^" redirection that was deprecated in fish 3.0 is now gone for good.
Note: For testing reasons, it can still be set _internally_ by running
"feature_flags_t::set". We simply shouldn't do that.
Prior to this change, if you tab-completed a token with a wildcard (glob), we
would invoke ordinary completions. Instead, expand the wildcard, replacing
the wildcard with the result of expansions. If the wildcard fails to expand,
flash the command line to signal an error and do not modify it.
Example:
> touch file(seq 4)
> echo file*<tab>
becomes:
> echo file1 file2 file3 file4
whereas before the tab would have just added a space.
Some things to note:
1. If the expansion would produce more than 256 items, we flash the command
line and do nothing, since it would make the commandline overfull.
2. The wildcard token can be brought back through Undo (ctrl-Z).
3. This only kicks in if the wildcard is in the "path component
containing the cursor." If the wildcard is in a previous component,
we continue using completions as normal.
Fixes#954.
When you do
```fish
set foo-bar baz
```
"foo-baz" isn't usable as a variable *name*. When you just say the
"variable" is invalid that could also be interpreted to be a special
type of variable or something.
String tokens are subdivided by command substitutions. Some syntax errors
can occur in the gap between two command substitutions. Make the caret point
to the start of that gap, instead of the token start.
When expanding command substitutions, we use a naïve way of detecting whether
the cmdsub has the optional leading dollar. We check if the last character was
a dollar, which breaks if it's an escaped dollar. We wrongly expand
\$(echo "") to the empty string. Fix this by checking if the dollar was escaped.
The parse_util_* functions have a bunch of output parameters. We should
return a parameter bag instead (I think I tried once and failed).
Given
set var a
echo "$var$(echo b)"
the double-quoted string is expanded right-to-left, so we construct an
intermediate "$varb". Since the variable "varb" is undefined, this wrongly
expands to the empty string (should be "ab"). Fix this by isolating the
expanded command substitution internally. We do the same when handling
unquoted command substitutions.
Fixes#8849
The read test is now failing on GitHub actions even though it passes on
my Mac. It may be due to differences in dd between these two
environments. Stop using dd and just use head.
The read.fish check has a test where it limits the amount of data passed to
`read` to 8192 bytes, and verifies that fish reads exactly that amount.
This check occasionally fails on the OBS builds; it's very hard to repro a
failure locally, but I finally did it.
The amount of data written is limited via `yes` and `dd`:
yes $line | dd bs=1024 count=(math "$fish_read_limit / 1024")
The bug is that `dd` outputs a fixed number of "blocks" where a block
corresponds to a single read. As `yes` and `dd` are running concurrently,
it may happen that `dd` performs a short read; this then counts as a single
block. So `dd` may output less than the desired amount of data.
This can be verified by removing the 2>/dev/null redirection; on a
successful run dd reports `8+0 records out`, on a failed run it reports
`7+1 records out` because one of the records was short.
Fix this by using `fullblock` so that dd will no longer count a short read
as a single block. `head` would probably be a simpler tool to use but we'll
do this for now.
Happily it's not a fish bug. No need to relnote it.
This was already apparently supposed to work, but didn't because we
just overrode errno again.
This now means that, if a correctly named candidate exists, we don't
start the command-not-found handler.
See #8804
This used to call exec_subshell, which has two issues:
1. It creates a command substitution block which shows up in a stack
trace
2. It does much more work than necessary
This removes a useless "in command substitution" from an error message
in an autoloaded file, and it speeds up autoloading a bit (not
measurable in actual benchmarks, but microbenchmarks are 2x).
Cancellation groups were meant to reflect the following idea: if you ran a
simple block:
begin
cmd1
cmd2
end
then under job control, cmd1 and cmd2 would get separate groups; however if
either exits due to SIGINT or SIGQUIT we also want to propagate that to the
outer block. So the outermost block and its interior jobs would share a
cancellation group. However this is more complex than necessary; it's
sufficient for the execution context to just store an int internally.
This ought not to affect anything user-visible.
* Implement fish_wcstod_underscores
* Add fish_wcstod_underscores unit tests
* Switch to using fish_wcstod_underscores in tinyexpr
* Add tests for math builtin underscore separator functionality
* Add documentation for underscore separators for math builtin
* Add a changelog entry for underscore numeric separators
We can't always read in chunks because we often can't bear to
overread:
```fish
echo foo\nbar | begin
read -l foo
read -l bar
end
```
needs to have the first read read `foo` and the second read `bar`. So
here we can only read one byte at a time.
However, when we are directly redirected:
```fish
echo foo | read foo
```
we can, because the data is only for us anyway. The stream will be
closed after, so anything not read just goes away. Nobody else is
there to read.
This dramatically speeds up `read` of long lines through a pipe. How
much depends on the length of the line.
With lines of 5000 characters it's about 15x, with lines of 50
characters about 2x, lines of 5 characters about 1.07x.
See #8542.
This is the simple fix - if we have no valid digit, we have nothing to
return. So instead of returning a NULL, we return an error.
This is already the case for invalid octal escapes (like `\777`).
Fixes#8545
This reverts commits:
2d9e51b43ed1d9f147ec346ce8081b
The box drawing because it's entangled with the rest and we don't
currently use this anywhere I know of. Nor was it gated on terminfo,
so it could have broken things, for subjectively little gain.
Fixes#8727.
A history search ends when you move the cursor, but the commandline inserted by
history search is still marked as transient. This means that the next history
search will clear the transient commandline. This means we are dropping an undo
point, for example:
echo 11
echo 1
echo autosuggestion
echo^P # commandline is "echo 1"
^A # stop history search
^P # commandline is "echo 11"
^Z # Bug: commandline goes back to "echo", but it should be "echo 1"
In the worst case, we are switching from line-search to token-search (see
the attached test case). Clearing the transient edit means the line is gone
and only the token is left on the command line.
Say the user has a multi-char binding (typically an escape sequence), and a
signal arrives partway through the binding. The signal has an event handler
which enques some readline event, for example, `repaint`. Prior to this
change, the readline event would cause the multi-char binding to fail. This
would cause bits of the escape sequence to be printed to the screen.
Fix this by noticing when a sequence was "interrupted" by a non-char event,
and then rotating a sequence of such interruptions to the front of the
queue.
Fixes#8628
Today, a command like "var=val status " has custom completions
because we skip over the var=val variable override when detecting
the command token.
However if the custom completions read the commandline state (via
"commandline -opc") they do see they variable override, which breaks
them, most likely. Try "a=b git ".
For completions of wrapped commands, we already set a transient
commandline. Do the same for commands with leading variable overrides;
then git completions for "a=b git " will think the commandline is
"git ".
`read` allows specifying the initial command line text. This was
text got accidentally ignored starting in a32248277f. Fix this
regression and add a test.
Fixes#8633
Previously, when we got an unknown option with --ignore-unknown, we
would increment woptind but still try to read the same contents.
This means in e.g.
```
argparse -i h -- -ooo -h
```
The `-h` would also be skipped as an option, because after the first
`-o` getopt reads the other two `-o` and skips that many options.
This could be handled more extensively in wgetopt, but the simpler fix
is to just skip to the next argv entry once we have an unknown option
- there's nothing more we can do with it anyway!
Additionally, document this and clearly explain that we currently
don't transform the option.
Fixes#8637
fish_git_prompt may run certain git commands which may invoke certain
external programs as specified `.git/config`. Prevent this by suppressing
certain git config options.
This affects the caret position. In an expression like
123 456
we previously reported:
123 456
^ missing operator
Now we do:
123 456
^ missing operator
We do it on the first space, which should be acceptable.
(no need for a changelog entry, we have already ignored #8511)