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
PR #6777 changed all the keys to uppercase, but many Vi commands are case
sensitive.
PR #7908 changed the "u" binding but the documentation still had the old
meaning.
