4656 Commits

Author SHA1 Message Date
Johannes Altmanninger
6a0bb7d6de reader: when updating commandline, also update rendered highlighting
Whenever the command line changes, we redraw it with the previously computed
syntax highlighting. At the same time we start recomputing highlighting in
a background thread.

On some systems, the highlighting computation is slow, so the stale syntax
highlighting is visible.

The stale highlighting was computed for an old commandline.  When the user
had inserted or deleted some characters in the middle, then the highlighting
is wrong for the characters to the right.  This is because the characters
to the right have shifted but the highlighting hasn't.  Fix this by also
shifting highlighting.

This means that text that was alrady highlighted will use the same
highlighting until a new one is computed. Newly inserted text uses the color
left of the cursor.

This is implemented by giving editable_line_t ownership of the highlighting.
It is able to perfectly sync text and highlighting; they will invariably
have the same length.

Fixes #9180
2022-09-16 19:21:21 -05:00
Johannes Altmanninger
de353d3e04 reader: stop requiring edit_t to be an rvalue reference
While its true that we only ever call this with temporaries, there is no
fundamental reason for this restriction.  Taking by value is simpler and
more flexible. I think it does not change the generated code.

No functional change.
2022-09-16 19:21:21 -05:00
Johannes Altmanninger
be64c53888 reader: inline dangerous function
The idea for this function was that it stands as the one place that modifies
the text without push_edit. In practice I don't think it helps.

No functional change.
2022-09-16 19:21:21 -05:00
Johannes Altmanninger
8b4b24428c reader: make undo history private to editable_line_t
reader handles way too much state itself. Let's move the undo handling to
editable_line_t entirely.

No functional change.
2022-09-16 19:17:04 -05:00
Johannes Altmanninger
2b2f64c045 reader: move private members to the bottom
No functional change.
2022-09-16 19:17:04 -05:00
Johannes Altmanninger
0ffb0fb786 reader: move function definition out-of-line
Happily, clangd provides a code action to do this.

No functional change.
2022-09-16 19:17:04 -05:00
Johannes Altmanninger
b3a8e85b0f complete: use remove_if+erase instead of raw loop to remove leading decorators
In theory this does less work so we should generally use this style.
In practice it looks uglier so I'm not sure. Maybe wait for stdlib ranges...

No functional change.
2022-09-16 19:17:04 -05:00
Mahmoud Al-Qudsi
9cf56047fb Prevent anyone else from wasting time w/ sigqueue(2)
It turns out there *is* an obviously portable way... except it's
not-so-obviously not portable after all.

POSIX specifies that sigqueue(2) can be used to validate pid and signo
separately, returning EINVAL in the specific case of an invalid or unsupported
signal number. This would be perfect... if only it were actually implemented.
2022-09-16 18:53:05 -05:00
Mahmoud Al-Qudsi
67ac23c70e Fix signal starvation in readch_timed under WSLv1
It seems that the WSLv1 implementation of pselect(2) does not check for
undelivered signals after the temporary sigmask is un-applied from the thread in
question.
2022-09-16 18:26:49 -05:00
Mahmoud Al-Qudsi
f97650bf9a Fix stale references to getch() 2022-09-16 18:26:49 -05:00
Mahmoud Al-Qudsi
351500e42d Emit more specific error for incomplete escape sequences
This replaces "Invalid token ..." with "Incomplete escape sequence ..." for
bare \c, \u, \U, \x, and \X escapes.
2022-09-16 15:44:33 -05:00
Fabian Boehm
787ba6d951 path: Don't try to find empty commands
This would e.g. cause highlighting to be broken if you added an
executable file to $PATH
2022-09-14 18:18:08 +02:00
Fabian Boehm
cfecc4cc35 command_not_found: Add special error for ENOTDIR 2022-09-14 18:01:01 +02:00
Aaron Gyes
e927ad367f Add IWYU pragma
Fixes #9206
2022-09-13 06:56:52 -07:00
Aaron Gyes
168d74ab0e IWYU 2022-09-12 18:34:19 -07:00
Aaron Gyes
864bd4a9cb builtin bind: highlight output.
This highlights `bind` output, which is commands to reproduce the
current bind state, for interactive sessions ala builtin complete.
2022-09-12 15:33:07 -07:00
ridiculousfish
5cf0778207 Claim the tty unconditionally in reader_data_t::readline
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
2022-09-09 13:43:29 -07:00
ridiculousfish
331bb9024b clang-format reader.cpp
We had an errant newline incompatible with our format.
2022-09-09 11:35:06 -07:00
Fabian Boehm
24fd26ae6e Fix error for vararg functions with zero arguments 2022-09-09 18:52:45 +02:00
Fabian Boehm
c284c4ca99 Add length also for too-many/few-args error 2022-09-09 18:52:45 +02:00
Fabian Boehm
a3ee7da812 math: Add length to missing operator error 2022-09-09 18:52:45 +02:00
Fabian Boehm
52e065e479 math: Add error length
Like we now do for syntax errors, this marks the extent of the error.

Currently for unknown functions only, would be cool for division too
2022-09-09 18:52:45 +02:00
Fabian Boehm
5edba044a3 math: Give a proper error for division by zero
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.
2022-09-09 18:52:45 +02:00
Fabian Boehm
41c22d5e60 Add string shorten
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.
2022-09-09 18:49:57 +02:00
Aaron Gyes
08129537e8 timer.cpp: iwyu; update includes
after aaf50099f2362deb431ce85d546899fcd8b7b330
2022-08-30 23:56:33 -07:00
Aaron Gyes
c35b935e61 fallback.cpp: iwyu; update includes 2022-08-30 23:55:26 -07:00
Johannes Altmanninger
3b30d92b62 Commit transient edit when closing pager
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
2022-08-31 07:49:49 +02:00
Fabian Boehm
26285280a9 Remove some dead code 2022-08-27 20:33:39 +02:00
Fabian Boehm
b08490f051 Replace our use of strncpy
strncpy will fill the entire buffer with NUL.

In this case we have a 128 byte buffer and write "empty" - 5 bytes -
into it.

So now instead of writing 6 bytes it'll write 128 bytes. Especially
wasteful because we already did memset before
2022-08-27 17:47:18 +02:00
Fabian Boehm
227e1f6300 color: Use convert_digit
I can't believe how many "read this one hex digit" functions we have.
2022-08-27 11:41:29 +02:00
Fabian Boehm
5e0f5eff37 Remove wcsdup fallback
2a0e0d67214aa45493e2f7b1782f672164fb8bc4 removed the last use of it,
and in most cases we'd probably prefer to use a wcstring instead
2022-08-27 11:36:15 +02:00
Fabian Boehm
4dfcd4cb4e reader: Check bounds for color
This fixes a crash when you open the history pager and then do
history-token-search-backward (e.g. alt+. or alt-up).

It would sometimes crash because the `colors.at(i)` was an
out-of-bounds access.

Note: This might still leave the highlighting offset in some
cases (not quite sure why), but at least it doesn't *crash*, and the
search generally *works*.
2022-08-26 15:02:05 +02:00
Fabian Boehm
a42a651d0a Use color for $fish_color_valid_path if it exists
This otherwise threw away the color. Since that's just information
that is thrown away, let's just use it.

Fixes #9159.
2022-08-25 17:42:42 +02:00
Aaron Gyes
50d37527a9 Revert "I need to take a break. Fixup."
This reverts commit 3e556b984c7769a25b301871bc788daecb6d6d4e.

Revert "Further fix the issue and add the assert that'd have prevented it."

This reverts commit 056502001eb94f1fc001126738055f6605a6fc21.

Revert "Fix actual issue with allow_use_posix_spawn."

This reverts commit 85b9f3c71f703429e44290b76e6d9e87b58d31ce.

Revert "Stop using posix_spawn when it is not allowed"

This reverts commit 9c896e199080bd4e219507961975a0ac97ebc32d.

Revert "don't even set up a fish_use_posix_spawn handler if unsupported"

This reverts commit 8b14ac4a9c3d0e15f9a7bd16a7633a9d1452b6d9.
2022-08-22 14:11:52 -07:00
Aaron Gyes
3e556b984c I need to take a break. Fixup. 2022-08-22 13:55:44 -07:00
Aaron Gyes
056502001e Further fix the issue and add the assert that'd have prevented it.
Surprise: because FISH_USE_POSIX_SPAWN was from postfork.h, we
also were disabling things when we don't want to as well.
2022-08-22 13:53:41 -07:00
Aaron Gyes
85b9f3c71f Fix actual issue with allow_use_posix_spawn.
We were testing the function pointer, not evaluating the function.

This should be the proper fix. Thanks @ridiculousfish
2022-08-22 13:30:51 -07:00
ridiculousfish
9c896e1990 Stop using posix_spawn when it is not allowed
Commit 8b14ac4a9c3d0e15f9a7bd16a7633a9d1452b6d9 started using
posix_spawn even if allow_use_posix_spawn() returns false. Stop doing
that.

This may be reproduced with:

    ./docker/docker_run_tests.sh ./docker/centos7.Dockerfile

as centos7 has a too-old glibc.
2022-08-21 16:25:26 -07:00
ridiculousfish
aaf50099f2 Stop using a static vector for timers
This is thread unsafe. Just use a captured local variable instead.
2022-08-21 15:30:13 -07:00
ridiculousfish
3eae0a9b6a clang-format all C++ files
This mostly re-sorts headers that got desorted after the IWYU
application in 14d2a6d8ff.
2022-08-21 15:02:19 -07:00
ridiculousfish
c260c1259e Stop exporting kDefaultPath
This is used only within path.cpp; make it a static.
2022-08-21 14:43:28 -07:00
ridiculousfish
1d0c22b390 Remove unused 'vars' variable in path_get_path_core
This became unused deliberately in 40733ca25bd7511312b027.
2022-08-21 14:42:59 -07:00
Aaron Gyes
8b14ac4a9c don't even set up a fish_use_posix_spawn handler if unsupported
Also remove extern 'C' { gnu_get_libc_version }, it's no longer
used. allow_use_posix_spawn is determined true or false at
compile time.
2022-08-21 14:19:34 -07:00
Aaron Gyes
1198a05299 assert: identify the hot path
Does result in code that branches a little differently.
2022-08-21 05:55:34 -07:00
Aaron Gyes
14d2a6d8ff IWYU-guided #include rejiggering.
Let's hope this doesn't causes build failures for e.g. musl: I just
know it's good on macOS and our Linux CI.

It's been a long time.

One fix this brings, is I discovered we #include assert.h or cassert
in a lot of places. If those ever happen to be in a file that doesn't
include common.h, or we are before common.h gets included, we're
unawaringly working with the system 'assert' macro again, which
may get disabled for debug builds or at least has different
behavior on crash. We undef 'assert' and redefine it in common.h.

Those were all eliminated, except in one catch-22 spot for
maybe.h: it can't include common.h. A fix might be to
make a fish_assert.h that *usually* common.h exports.
2022-08-20 23:55:18 -07:00
Fabian Boehm
7988cff6bd Increase the string chunk size to increase performance
This is a *tiny* commit code-wise, but the explanation is a bit
longer.

When I made string read in chunks, I picked a chunk size from bash's
read, under the assumption that they had picked a good one.

It turns out, on the (linux) systems I've tested, that's simply not
true.

My tests show that a bigger chunk size of up to 4096 is better *across
the board*:

- It's better with very large inputs
- It's equal-to-slightly-better with small inputs
- It's equal-to-slightly-better even if we quit early

My test setup:

0. Create various fish builds with various sizes for
STRING_CHUNK_SIZE, name them "fish-$CHUNKSIZE".
1. Download the npm package names from
https://github.com/nice-registry/all-the-package-names/blob/master/names.json (I
used commit 87451ea77562a0b1b32550124e3ab4a657bf166c, so it's 46.8MB)
2. Extract the names so we get a line-based version:

```fish
jq '.[]' names.json | string trim -c '"' >/tmp/all
```

3. Create various sizes of random extracts:

```fish
for f in 10000 1000 500 50
    shuf /tmp/all | head -n $f > /tmp/$f
end
```

(the idea here is to defeat any form of pattern in the input).

4. Run benchmarks:

hyperfine -w 3 ./fish-{128,512,1024,2048,4096}"
    -c 'for i in (seq 1000)
            string match -re foot < $f
        end; true'"

(reduce the seq size for the larger files so you don't have to wait
for hours - the idea here is to have some time running string and not
just fish startup time)

This shows results pretty much like

```
Summary
'./fish-2048     -c 'for i in (seq 1000)
          string match -re foot < /tmp/500
      end; true'' ran
  1.01 ± 0.02 times faster than './fish-4096     -c 'for i in (seq 1000)
          string match -re foot < /tmp/500
      end; true''
  1.02 ± 0.03 times faster than './fish-1024     -c 'for i in (seq 1000)
          string match -re foot < /tmp/500
      end; true''
  1.08 ± 0.03 times faster than './fish-512     -c 'for i in (seq 1000)
          string match -re foot < /tmp/500
      end; true''
  1.47 ± 0.07 times faster than './fish-128     -c 'for i in (seq 1000)
          string match -re foot < /tmp/500
      end; true''
```

So we see that up to 1024 there's a difference, and after that the
returns are marginal. So we stick with 1024 because of the memory
trade-off.

----

Fun extra:

Comparisons with `grep` (GNU grep 3.7) are *weird*. Because you both
get

```
'./fish-4096 -c 'for i in (seq 100); string match -re foot < /tmp/500; end; true'' ran
11.65 ± 0.23 times faster than 'fish -c 'for i in (seq 100); command grep foot /tmp/500; end''
```

and

```
'fish -c 'for i in (seq 2); command grep foot /tmp/all; end'' ran
66.34 ± 3.00 times faster than './fish-4096 -c 'for i in (seq 2);
string match -re foot < /tmp/all; end; true''
100.05 ± 4.31 times faster than './fish-128 -c 'for i in (seq 2);
string match -re foot < /tmp/all; end; true''
```

Basically, if you *can* give grep a lot of work at once (~40MB in this
case), it'll churn through it like butter. But if you have to call it
a lot, string beats it by virtue of cheating.
2022-08-15 20:16:12 +02:00
Fabian Boehm
40733ca25b If relative path was used, use it
This was inadvertently changed in
ed78fd2a5f95ed72c0745e15f53fdea6fdde1f91

Fixes #9143
2022-08-15 20:01:50 +02:00
Aaron Gyes
2b2f772790 clarify "…variable is shadowed by the global variable of the same name"
Rephrase this to more explicitly indicate that the uvar actually
was successfully set. I believe the prior phrasing can leave some
ambiguity as far as wether set just failed with an error, whether it
has done anything or not.
2022-08-14 16:16:38 -07:00
Aaron Gyes
aacc71e585 builtin set: make error messages more consistent.
Now uses the same macro other builtins use for a missing -e arg,
and the error message show the short or long option as it was used.

e.g. before
    $ set -e
    set: Erase needs a variable name

after
    $ set --erase
    set: --erase: option requires an argument
    $ set -e
    set: -e: option requires an argument
2022-08-14 15:34:58 -07:00
ridiculousfish
2a0e0d6721 Remove the intern'd strings component
Intern'd strings were intended to be "shared" to reduce memory usage but
this optimization doesn't carry its weight. Remove it. No functional
change expected.
2022-08-13 12:51:36 -07:00