Commit graph

16421 commits

Author SHA1 Message Date
Aaron Gyes
4906c680c6 remove fish_indent wrapper
When this was introduced, we used fish_indent --ansi to format
the output of `builtin functions` for color output in `type`, etc.

We don't anymore.

Today it's not a potential showstopper if one launches a fish
session with a five year-old fish_indent in $PATH. We need not
go to lengths to try to make sure we run whatever is in the
build dir adjacent to the `fish` binary.
2022-10-30 22:07:15 -07:00
Aaron Gyes
02998aba76 git.fish: update general options
Adds a few options I see in my git manpage that were omitted:
-v, -h, -P, --config-env, --no-optional-locks, --list-cmds

Reword most general option descriptions
2022-10-30 15:26:38 -07:00
Fabian Boehm
9948bc2264 completions/apt: Add quotes
Simple way to make the apt completions spew:

function apt; end

on a system without an apt command installed. (even if it isn't
Darwin, because this uses test combiners!)

This is a thing some people do to avoid learning other package managers.

(of course our completions would probably be *wrong* still, but at least they
won't spew a `test` error)
2022-10-30 11:27:34 +01:00
Aaron Gyes
7c680af4e3 disable apt completions on macOS
macOS has a /usr/bin/apt that is some tool requiring Java,
abort the completions to avoid the confusing package manager
completions.
2022-10-29 11:28:37 -07:00
Mahmoud Al-Qudsi
8750f9ccb7 fixup! Reintroduce trivially copyable maybe_t impl
`git revert --no-commit` leaving the repo in a "middle of revert" state
tripped me up and my changes weren't included in the commit. Mea culpa.
2022-10-29 11:39:33 -05:00
Mahmoud Al-Qudsi
4f46abec9d Reintroduce trivially copyable maybe_t impl
This reverts commit 1c92d4c5db and
reintroduces support for trivially copyable `maybe_t` impls but with a
GCC version check to disable the optimization for GNU GCC compiler
versions 9 and below.

GCC 8.3.0 armhf builds seem to have a problem with the trivially
copyable `maybe_t` impl that introduces odd heisenbugs that cause the
tests to fail. GDB reveals that `maybe_t` function parameters received
in the callee differ from what was passed-in by the caller.

This behavior appears to be (but has not been confirmed as) a
platform-specific compiler bug. Under the same system (32-bit Debian 10
armhf), compiling with clang 7.0.1 does not result in any bugs and
causes all the tests to pass while compiling with GCC 10.2 under 32-bit
Debian 11 armhf also doesn't run into any problems, so just expand the
existing GCC version check that gates support for trivially copyable
`maybe_t` impls to encompass both the troublesome GCC 8 version and the
untested GCC 9 version.
2022-10-29 11:26:34 -05:00
Mahmoud Al-Qudsi
1c92d4c5db Revert "maybe_t: make maybe_t<T> trivially copyable if T is"
This reverts commit 9d303a74e3.
This reverts commit 0305c842e6.

9d303a7 broke 32-bit armhf builds for unknown reasons, specifically in
settings where a trivial copy of `maybe_t<int>` was performed. A caller
would pass a literal int in the place of a `maybe_t<int>` parameter and
the callee would see a populated `maybe_t` but with a value of `0`
rather than the actual value that was passed in. It was too painful to
debug to a resolution under qemu.
2022-10-29 10:12:41 -05:00
Fabian Boehm
8d7662335e function: Don't list empty function names and directories 2022-10-29 10:24:42 +02:00
Aaron Gyes
daf5e11179 Spelling fixes
Found with scspell
2022-10-28 20:10:09 -07:00
Mahmoud Al-Qudsi
2c0bbb0f82 Revert "cmake: Use pcre2 tag directly"
This reverts commit 2cc4437567.

As pointed out to me, it is safer to use the SHA directly to ~guarantee
the remote resource hasn't changed.
2022-10-28 17:59:08 -05:00
Mahmoud Al-Qudsi
c45fac84ee cmake: Use shallow clone for pcre2
This should speed up the clones and reduce unnecessary usage of both
bandwidth and disk space.
2022-10-28 14:04:14 -05:00
Mahmoud Al-Qudsi
2cc4437567 cmake: Use pcre2 tag directly 2022-10-28 14:04:14 -05:00
Aaron Gyes
a6e2e52eef apropos completions for macOS/BSD variants
This should show the correct options for macOS, NetBSD,
FreeBSD, OpenBSD, and DragonFly.
2022-10-28 11:07:35 -07:00
Aaron Gyes
c887e5dbde Fix apropos completions
The 'str' variable was apparently mistakenly removed by 49c5f96470.

Re-add it, and regex-escape it as well.

Allow completing on apropos <TAB> instaed of requiring an initial char.

Use __fish_apropos instead of apropos.

New regex to hopefully work on more platforms.

Explicitly use ^ instead of adding it at __fish_apropos
2022-10-28 10:01:35 -07:00
Aaron Gyes
054d0ac0ea git completions: undo mistaken set -f usage
and fix issue in __fish_git_needs_command
2022-10-28 01:14:45 -07:00
Aaron Gyes
b8dee05ad0 completions: remove functions that are never used
None of these __functions defined in completions are used or
referenced anywhere.

Found with:

function unused -a file search -d 'find unused functions'
  set -f (string replace -fr '^[\s]*function ([\w_]+).*' '$1' < $file)
  for cmd in $cmds
    printf %d\ %s\n (grep -r ".*$cmd.*" $search < $argv | count) $cmd
  end | string match '1 *'
end

for file in share/*/*.fish
  unused $file share && printf "in %s\n" $file
end
2022-10-27 23:25:44 -07:00
Aaron Gyes
f5711ad5ed git.fish: collapse repeat complete cmds, set -f, rm unneeded funcs
Get rid of functions:
__fish_git_diff_opt,
__fish__git_append_letters_nosep,
__fish_git_sort_keys

Use `set -f` inside blocks instead of `set -l foo` before blocks.

Two of these just printed out the argument\tdescription dictionaries
without providing any utility: only used once, just do it inline.

Collapse adjacent lines that look like
complete git -n '(blah)' -l option -d 'option help'
complete git -n '(blah)' -l option -a 'arg1' -d 'description 1'
complete git -n '(blah)' -l option -a 'arg2' -d 'description 2'
complete git -n '(blah)' -l option -a 'arg2' -d 'description 3'
...

into

complete git -n '(blah)' -l option -d 'option help' -a "
arg1\t'description 1'
arg2\t'description 2'
arg3\t'description 3'
..."

This sped up the source time about 10% by running complete
less.
2022-10-27 22:19:32 -07:00
Aaron Gyes
b7593a377a fish_key_reader: stop looping on SIGHUP
Using the machinery in reader.cpp rather than going back to
intalling our own handlerss

(see 89644911a1)

Fixes #9309
2022-10-27 17:17:05 -07:00
Johannes Altmanninger
0305c842e6 Fix build on CentOS 7
This fixes a regression in 9d303a74e (maybe_t: make maybe_t<T> trivially
copyable if T is, 2022-10-26). I subscribed to the launchpad repo now -.-
2022-10-27 09:28:52 +02:00
Aaron Gyes
efa2cf0cb6 Replace fallthrough comments with __fallthrough__
Defined in config.h
2022-10-26 21:02:48 -07:00
Aaron Gyes
df546e01f6 IWYU fixup 2022-10-26 20:04:04 -07:00
Aaron Gyes
92698dff48 Unallowed command subst error: add missing newline and simplify
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
2022-10-26 19:58:40 -07:00
Aaron Gyes
b2a4a50daf Run include-what-you-use 2022-10-26 19:58:40 -07:00
ridiculousfish
a4aaa4f59b Fix the Xenial build
The Xenial build was failing due to a missing default constructor
in maybe_t. Add it.
2022-10-26 14:19:01 -07:00
Mahmoud Al-Qudsi
06d9708d40 Add complete -k group order test
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.
2022-10-26 13:22:45 -05:00
Mahmoud Al-Qudsi
f7da014602 Optimize storage of completion entries
This is a salvage of the "no functional changes" part of #9221, and cherry-picks
storing completion entries in a vector instead of a linked list. The legacy
"reverse intuitive" group ordering is kept by iterating in reverse order.

Tests pass but don't actually cover group order, which needs another test.
2022-10-26 12:48:31 -05:00
Mahmoud Al-Qudsi
5ad0d95694 CHANGELOG: Add status current-commandline 2022-10-26 12:18:05 -05:00
Mahmoud Al-Qudsi
7133285c88 Move parser status vars to their own struct
Instead of using an enum + array, just use a struct and drop the getter and
setter methods from `parser_t`.
2022-10-26 12:15:02 -05:00
Mahmoud Al-Qudsi
125bcb8289 Add pexpect test for status current-commandline 2022-10-26 12:15:02 -05:00
Mahmoud Al-Qudsi
6ac18defd2 Add status current-commandline
Makes it possible to retrieve the currently executing command line as
opposed to the currently executing command (`status current-command`).

Closes #8905.
2022-10-26 12:15:02 -05:00
Mahmoud Al-Qudsi
e01eb2e615 Add proper way of storing value for status current-command
There should be no functional changes in this commit.

The global variable `$_` set in the parser variables by `reader.cpp` and
read by the `status` builtin was deprecated in fish 2.0 but kept around
internally because there's no good way to store/share/forward parser
variables.

A new enum is added that identifies the status variable and they are
stored in a private array in the parser. There is no need for
synchronization because they are only set during job init and never
thereafter. This is currently asserted via ASSERT_IS_MAIN_THREAD() but
that assert can be dropped in the interest of making the parser possible
to clone and use from worker threads.

The old `$_` global variable is still kept for backwards compatibility,
though it will be dropped in a future release.
2022-10-26 12:15:02 -05:00
Johannes Altmanninger
f637fb31b5 highlight: underline prefixes of valid paths only if at cursor
As the user is typing an argument, fish continually checks if the input is
the prefix of a valid file path. If yes, the input is underlined.

The same prefix-logic is used for all tokens on the command line, even for
"finished" tokens. This means we highlight any token that happens to be
a prefix of a valid file path. We actually want this to only apply to the
token that the user is currently typing.

Let's use the prefix-logic only for tokens adjacent to the cursor.  This should
better match user expectations (and reduce IO traffic). I don't think this is
the perfect criteria but I don't know how else we can determine if a token is
"unfinished".
2022-10-26 16:12:43 +02:00
Johannes Altmanninger
6667c9f50c highlighter: pass the cursor position to the highlighter
This allows the next commit to correct highlighting based on the cursor
position.
2022-10-26 16:11:00 +02:00
Johannes Altmanninger
861ac00a61 highlighter: underline valid "cd" arguments also if they come from CDPATH
When visiting the "cd" node, we mark invalid paths as error, but don't
underline valid paths.  This works fine most of the time because we later
underline paths (for any command, not just "cd").
However the latter check fails to honor CDPATH.  Let's correct that, which
also allows to simplify the logic.
2022-10-26 16:11:00 +02:00
Johannes Altmanninger
dfb0c00d72 highlighter: stop performing IO if canceled
The next commit wants to move the "Underline every valid path" logic into the
visit() methods. The logic currently polls the cancel checker before checking
each path. If that's valid, it should probably have the same behavior inside
visit(). Since we currently can't cancel an AST-visitation, the next best
thing seems to suspend all IO operations, the rest should be very fast anyway.

I'm not sure if the motivation is strong enough; a conceivable alternative
would be to stop using the cancel checker altogether for highlighting.
2022-10-26 16:11:00 +02:00
Johannes Altmanninger
9c6f46a808 highlighter: remove redundant check if we can do io
It's done a few lines above.
2022-10-26 16:09:02 +02:00
Johannes Altmanninger
acb47f70d2 history_file.cpp: remove an unused variable
Now that maybe_t<size_t> no longer has a user-defined destructor, the compiler
can better detect an unused variable of this type.
2022-10-26 16:09:02 +02:00
Johannes Altmanninger
9d303a74e3 maybe_t: make maybe_t<T> trivially copyable if T is
When passing a value of type maybe_t<size_t>, clangd complains:

    Parameter 'cursor' is passed by value and only copied once; consider
    moving it to avoid unnecessary copies (fix available)

We get this warning because maybe_t<size_t> is not trivially copyable
because it has a user-defined destructor and copy-constructor.  Let's remove
them if the contained type is trivially copyable, to avoid such warnings.
No functional change.
2022-10-26 16:09:02 +02:00
Johannes Altmanninger
1ce2961561 maybe_t: remove user-defined destructor
The destructor is equivalent to the compiler-generated one.  The user-defined
destructor prevents maybe_t<size_t> from bearing the predicate "trivially
copyable". Let's remove it. No functional change.
2022-10-26 14:54:33 +02:00
Johannes Altmanninger
45da77c5c5 Format some C++ files with clang-format 2022-10-26 14:53:06 +02:00
Mahmoud Al-Qudsi
ee62bee9cd CHANGELOG: Document eval overflow fix
[ci skip]
2022-10-25 13:51:46 -05:00
Mahmoud Al-Qudsi
2c870b305d Add eval recursion overflow regression test
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.
2022-10-25 13:40:21 -05:00
Mahmoud Al-Qudsi
3913b28153 Only retry failed pexpect tests under CI
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.
2022-10-25 13:40:21 -05:00
Mahmoud Al-Qudsi
21599a49ea Make CALL_STACK_LIMIT_EXCEEDED_ERR_MSG more generic
We're now using this when a stack overflow is detected during eval/substitution
loops, too.
2022-10-25 13:40:21 -05:00
Mahmoud Al-Qudsi
175caab583 Prevent stack overflow from eval/substitution recursion
It seems to have originally been thought that the only possible way a stack
overflow could happen is via function calls, but there are other possibilities.

Issue #9302 reports how `eval` can be abused to recursively execute a string
substitution ad infinitum, triggering a stack overflow in fish.

This patch extends the stack overflow check to also check the current
`eval_level` against a new constant `FISH_MAX_EVAL_DEPTH`, currently set to a
conservative but hopefully still fair limit of 500. For future reference, with
the default stack size for the main/foreground thread of 8 MiB, we actually have
room for a stack depth around 2800, but that's only with extremely minimal state
stored in each stack frame.

I'm not entirely sure why we don't check `eval_depth` regardless of block type;
it can't be for performance reasons since it's just a simple integer comparison
- and a ridiculously easily one for the branch predictor handle, at that - but
maybe it's to try and support non-recursive nested execution blocks of greater
than `FISH_MAX_STACK_DEPTH`? But even without recursion, the stack can still
overflow so may be we should just bump the limit up some (to 500 like the new
`FISH_MAX_EVAL_DEPTH`?) and check it all the time?

Closes #9302.
2022-10-25 13:40:21 -05:00
Fabian Boehm
14ecb63e40
completions/usermod: Fix subu/gid option spelling
It's "subuid", not "sub-uid".

Fixes #9303
2022-10-25 11:09:41 +02:00
Mahmoud Al-Qudsi
e7bf98adc1 Make block_t moveable
The presence of the explicit constructor (even though it did nothing) prevented
the compiler from generating a move constructor for `block_t`.
2022-10-24 22:06:30 -05:00
Mahmoud Al-Qudsi
84b53b4cae Significantly reduce size of block_t
A `block_t` instance is allocated for each live block type in memory when
executing a script or snippet of fish code. While many of the items in a
`block_t` class are specific to a particular type of block, the overhead of
`maybe_t<event_t>` that's unused except in the relatively extremely rare case of
an event block is more significant than the rest, given that 88 out of the 216
bytes of a `block_t` are set aside for this field that is rarely used.

This patch reorders the `block_t` members by order of decreasing alignment,
bringing down the size to 208 bytes, then changes `maybe_t<event_t>` to
`shared_ptr<event_t>` instead of allocating room for the event on the stack.
This brings down the runtime memory size of a `block_t` to 136 bytes for a 37%
reduction in size.

I would like to investigate using inheritance and virtual methods to have a
`block_t` only include the values that actually make sense for the block rather
than always allocating some sort of storage for them and then only sometimes
using it. In addition to further reducing the memory, I think this could also be
a safer and saner approach overall, as it would make it very clear when and
where we can expect each block_type_type_t-dependent member to be present and
hold a value.
2022-10-24 21:04:17 -05:00
Mahmoud Al-Qudsi
44c9c51841 Disable leak detection in test_autosuggest_suggest_special() under CI
This is a false positive as a result of disabling TLS support in LSAN due to an
incompatibility with newer versions of glibc.

Also remove the older workaround (because it didn't work).
2022-10-24 19:02:49 -05:00
Mahmoud Al-Qudsi
bfa172852f Add a workaround for intermittent LSAN crash under CI
LSAN seems to have an issue with glibc's TLS functionality that causes it to
intermittently crash with SIGSEGV when run virtualized, as it is in our CI.

Relevant GitHub issues:
* https://github.com/google/sanitizers/issues/1342
* https://github.com/google/sanitizers/issues/1409
2022-10-24 18:56:55 -05:00