This reimplements ridiculousfish/control_r which is a more future-proof
approach than #6686.
Pressing Control+R shows history in our pager and allows to search filter
commands with the pager search field.
On the surface, this works just like in other shells; though there are
some differences.
- Our pager shows multiple results at a time.
- Other shells allow to use up arrow/down arrow to select adjacent entries
in history. Shouldn't be hard to implement but the hidden state might
confuse users and it doesn't play well with up-or-search, so this is
left out.
Users might expect the history pager to use subsequence matching (fuzzy
matching) like the completion pager, however due to the history pager design it
uses substring matching. We could change this in future, however that means
we would also want to change the ordering from "reverse-chronological" to
"longest common subsequence" (e.g. what fuzzy finders do), because otherwise
a query "fis" might give this ordering:
fsck /dev/disk/by-partlabel/Linux\x20filesystem
fish
which is probably not what the user wants.
The pager shows only a small number of history items at a time. This is
because, as explained above, the history pager does not support subsequence
matching, so navigating it does not scale well.
Closes#602
The next patch wants to add state that should be reset when we clear the
pager, which will happen in this function.
This reverts commit b25b291d38.
No functional change.
The pager's rendering_needs_update() function detects some but not all
scenarios where a rendering is stale. In particular, it does not compare
the completion strings.
To make this work, we manually invalidate the pager rendering whenever we
update completion strings. The history pager needs the same functionality,
so let's move it into the pager.
No functional change.
This addresses code review feedback to not couple the purely visual
concept of cursor style with the logical concept of the selection size.
Instead this now uses a dedicated variable
`$fish_select_char_after_cursor` to determine whether to extend the
selection beyond the cursor:
* fish_select_char_after_cursor = 1 or unset -> extend selection
* all other cases -> place the selection end that the cursor
This fixes the handling of the right end of the selection. Currently the
right end is considered to be at the cursor position + 1. When using a
`block` or `underline` cursor this is arguably correct, because the
cursor has a width of 1 and spans from the current position to the next:
```
x x [x x x̲] x
```
This is incorrect though (or at least very unintuitive), when using a
`line` cursor:
```
x x [x x|x] x
```
This commit changes the strategy for determining the end of the
selection in the following way:
* If the current cursor as determined by `$fish_cursor_<bind_mode>` is
set to `line`, then a cursor width of `0` is assumed.
* In all other cases, including `block` and `underscore` as well as when
no value is set we retain the previous behavior of assuming a cursor
width of `1`.
```
x x [x x x̲] x
x x [x x|]x x
```
This change should not affect many users, because the selection is
probably used most by vi-mode users, who are also likely to use a
block cursor.
We use "c > 0" but we actually mean "c != 0". The former looks like the
other code path handles negative c. Yet if c is negative, our code would
print a single escaped byte (\xXY) which is wrong because a negative value
has "sizeof wchar_t" bytes which is at least 2.
I think on platforms with 16-bit wchar_t it's possible that we actually
get a negative value but I haven't checked.
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.
ESCAPE_ALL is not really a helpful name. Also it's the most common flag.
Let's make it the default so we can remove this unhelpful name.
While at it, let's add a default value for the flags argument, which helps
most callers.
The absence of ESCAPE_ALL makes it only escape nonprintable characters
(with some exceptions). We use this for displaying strings in the completion
pager as well as for the human-readable output of "set", "set -S", "bind"
and "functions".
No functional change.
When listing variables, "set" tries to escape variable names.
Since variable names cannot have special characters, this doesn't do anything.
The escaping is one of the few places that does not use ESCAPE_ALL. This has
complex behavior; let's alleviate the problem by getting rid of this call.
No functional change.
Or should we stop using it?
I'm fine with either always or never using auto-formatting but our current
way of using it only sometimes is confusing.
No functional change.
Almost all edits to our commandline are funneled through
reader_data_t::push_edit(). Notable exceptions are undo/redo (which move
across existing edits instead). Due to an oversight, undo/redo fail to
trigger commandline update hooks. Fix that.
Our behavior of triggering hooks only for the search field looks weird. I
reckon that the command line eventually catches up, but this means we trigger
some hooks redundantly. Once we figure that out we can remove the new function.
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.
If the completion pager renders as
foo1 bar1 baz1 qux1
foo2 bar2 baz2
foo3 bar3 baz3
and we go backwards from "foo1" (using left arrow), we'll end up at "baz3",
not "qux1". Pretty smart!
If however we go backwards once more, nothing happens.
The root cause is that there are two different kinds of selection indices:
the one before rendering (9/qux1) and the one after we cleverly subtract
the half-filled last column (8/baz3). The backwards movement ends up
decrementing the first, so it moves from 9 to 8 and nothing changes in
the rendering.
Fix this by using the selection index that we actually rendered.
There is another caller that relies on the old behavior of using the unrendered
selection index. Make it use a dedicated overload that does not depend on
the rendering.
Otherwise realpath would add the cwd, which would be broken if fish
ever cd'd.
We could add the original cwd, but even that isn't enough, because we
need *the parent's* idea of cwd and $PATH.
Or, alternatively, what we need is for the OS to give us the actual
path to ourselves.
get_executable_path says: "This needs to be realpath'd"
So how about we do that? The only other place we use it is fish.cpp,
and we realpath it there already.
See #9085
Our pager computes the selected completion based on its rendering. The number
of rows affect the selection, in particular when moving left from the top
left cell. This computation breaks if the number of rows is zero, which
happens in at least
two scenarios:
1. If the completion pager was not shown (as is the case for complete-or-search)
2. If the search field had filtered away every candidate but not anymore.
I believe in these scenarios the selected completion index is always 0,
so let's fix the selection for that case.
Probably too minor for a changelog entry.
Closes#9080
Posix allows this as an alternative with the same semantics for read.
Found in conjunction with #9067.
Should be no functional difference on other systems.
The wait status value, which we also use internally, is read by a
bunch of macros.
Unfortunately because we want to *create* such a value, and some
systems lack the "W_EXITCODE" macro to do that, we need to figure out
how it's encoded.
So we simply check a specific value, and assume the encoding from
that.
On Haiku the return status is in the lower byte, on other systems it's
typically the upper byte.
TODO: Test on musl (that's the other system without W_EXITCODE).
Fixes#9067
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.
These are non-POSIX extensions other test(1) utilities implement,
which compares the modification time of two files as proposed for
fish in #3589: testing if one file is newer than another file.
-ef is a common extension to test(1) which checks if two paths refer
to the same file, by comparing the dev and inode numbers.
As explained by the comment, this was dead code. If it were ever executed,
it would cause very weird behavior because it would make some completions
randomly affect others.
Let's just print a warning (maybe this is better than crashing?).
Previously, the search text is used to find out which part of the
updated command line should be highlighted during a history search. This
approach will cause the incorrect part to be highlighted when the line
contains multiple instances of the search text.
To address this, we have to find out exactly where to highlight, i.e.
the offset of the current token in the command line (0 if not a token
search) plus the offset of the search text in the match.
This function is supposed to return "the next directory". Because this
is imperfect, it only tries to.
Except it went to all the trouble of figuring out the type and then
just... returned it anyway.
This has nice speedups in globs with directory components like `*/` or
`**`. I have observed 1.1x to 2.0x.
We could also return when we know it's definitely a directory and then
skip a stat() later, but preliminary testing seemed to show that's not
worth much.
Take advantage of additional cleanup unlocked by this refactoring,
including eliminating unneeded error returns and simplifying some
control flow.
No user-visible behavior change expected here.
This switches builtin_string from using PCRE2 directly, to using the new re
component. This simplifies some code and removes redundancy.
No user-visible behavior change expected here.
