When "self.paste_is_buffering()" is true, "parse_escape_sequence()" explicitly
returns "None" instead of "Some(Escape)". This is irrelevant because this
return value is never read, as long as "self.paste_is_buffering()" remains
true until "parse_escape_sequence()" returns, because the caller will return
early in that case. Paste buffering only ends if we actually read a complete
escape sequence (for ending bracketed paste).
Remove this extra branch.
(cherry picked from commit e5fdd77b09)
The new key notation canonicalizes aggressively, e.g. these two bindings
clash:
bind ctrl-shift-a something
bind shift-ctrl-a something else
This means that key events generally match at most one active binding that
uses the new syntax.
The exception -- two coexisting new-syntax binds that match the same key
event -- was added by commit 50a6e486a5 (Allow explicit shift modifier for
non-ASCII letters, fix capslock behavior, 2025-03-30):
bind ctrl-A 'echo A'
bind ctrl-shift-a 'echo shift-a'
The precedence was determined by definition order.
This doesn't seem very useful.
A following patch wants to resolve#11520 by matching "ctrl-ц" events against
"ctrl-w" bindings. It would be surprising if a "ctrl-w" binding shadowed a
"ctrl-ц" one based on something as subtle as definition order. Additionally,
definition order semantics (which is an unintended cause of the implementation)
is not really obvious. Reverse definition order would make more sense.
Remove the ambiguity by always giving precedence to bindings that use
explicit shift.
Unrelated to this, as established in 50a6e486a5, explicit shift is still
recommended for bicameral letters but not typically for others -- e.g. alt-+
is typically preferred over alt-shift-= because the former also works on a
German keyboard.
See #11520
(cherry picked from commit 08c8afcb12)
We canonicalize "ctrl-shift-i" to "ctrl-I".
Both when deciphering this notation (as given to builtin bind),
and when receiving it as a key event ("\e[105;73;6u")
This has problems:
A. Our bind notation canonicalization only works for 26 English letters.
For example, "ctrl-shift-ä" is not supported -- only "ctrl-Ä" is.
We could try to fix that but this depends on the keyboard layout.
For example "bind alt-shift-=" and "bind alt-+" are equivalent on a "us"
layout but not on a "de" layout.
B. While capslock is on, the key event won't include a shifted key ("73" here).
This is due a quirk in the kitty keyboard protocol[^1]. This means that
fish_key_reader's canonicalization doesn't work (unless we call toupper()
ourselves).
I think we want to support both notations.
It's recommended to match all of these (in this order) when pressing
"ctrl-shift-i".
1. bind ctrl-shift-i do-something
2. bind ctrl-shift-I do-something
3. bind ctrl-I do-something
4. bind ctrl-i do-something
Support 1 and 3 for now, allowing both bindings to coexist. No priorities
for now. This solves problem A, and -- if we take care to use the explicit
shift notation -- problem B.
For keys that are not affected by capslock, problem B does not apply. In this
case, recommend the shifted notation ("alt-+" instead of "alt-shift-=")
since that seems more intuitive.
Though if we prioritized "alt-shift-=" over "alt-+" as per the recommendation,
that's an argument against the shifted key.
Example output for some key events:
$ fish_key_reader -cV
# decoded from: \e\[61:43\;4u
bind alt-+ 'do something' # recommended notation
bind alt-shift-= 'do something'
# decoded from: \e\[61:43\;68u
bind alt-+ 'do something' # recommended notation
bind alt-shift-= 'do something'
# decoded from: \e\[105:73\;6u
bind ctrl-I 'do something'
bind ctrl-shift-i 'do something' # recommended notation
# decoded from: \e\[105\;70u
bind ctrl-shift-i 'do something'
Due to the capslock quirk, the last one has only one matching representation
since there is no shifted key. We could decide to match ctrl-shift-i events
(that don't have a shifted key) to ctrl-I bindings (for ASCII letters), as
before this patch. But that case is very rare, it should only happen when
capslock is on, so it's probably not even a breaking change.
The other way round is supported -- we do match ctrl-I events (typically
with shifted key) to ctrl-shift-i bindings (but only for ASCII letters).
This is mainly for backwards compatibility.
Also note that, bindings without other modifiers currently need to use the
shifted key (like "Ä", not "shift-ä"), since we still get a legacy encoding,
until we request "Report all keys as escape codes".
[^1]: <https://github.com/kovidgoyal/kitty/issues/8493>
(cherry picked from commit 50a6e486a5)
This notation doesn't make sense, use either A or shift-a. We accept it
for ASCII letters only -- things like "bind shift-!" or "bind shift-Ä"
do not work as of today, we don't tolerate extra shift modifiers yet.
So let's remove it for consistency.
Note that the next commit will allow the shift-A notation again, but it will
not match shift-a events.
(cherry picked from commit 7f25d865a9)
Switch to fish_wcstoul because we want the constant to be unsigned.
It's u32 because most callers of function_key() want that.
(cherry picked from commit e9d1cdfe87)
Commit 109ef88831 (Add menu and printscreen keys, 2025-01-01)
accidentally broke an assumption by inverting f1..f12. Fix that.
Fixes#11098
(cherry picked from commit d2b2c5286a)
These aren't typically used in the terminal but they are present on
many keyboards.
Also reorganize the named key constants a bit. Between F500 and
ENCODE_DIRECT_BASE (F600) we have space for 256 named keys.
(cherry picked from commit 109ef88831)
We parse "\e\e[x" as alt-modified "Invalid" key. Due to this extra
modifier, we accidentally add it to the input queue, instead of
dropping this invalid key.
We don't really want to try to extract some valid keys from this
invalid sequence, see also the parent commit.
This allows us to remove misplaced validation that was added by
e8e91c97a6 (fish_key_reader: ignore sentinel key, 2024-04-02) but
later obsoleted by 66c6e89f98 (Don't add collateral sentinel key to
input queue, 2024-04-03).
(cherry picked from commit 84f19a931d)
This situation can be triggered in practice inside a terminal like tmux
3.5 by running
tmux new-session fish -C 'sleep 2' -d reader -o log-file
and typing "alt-escape x"
The log shows that we drop treat this as alt-[ and drop the x on the floor.
reader: Read char alt-\[ -- Key { modifiers: Modifiers { ctrl: false,
alt: true, shift: false }, codepoint: '[' } -- [27, 91, 120]
This input ("\e[x") is ambiguous.
It looks like it could mean "alt-[,x". However that conflicts with a
potential future CSI code, so it makes no sense to try to support this.
Returning "None" from parse_csi() causes this weird behavior of
returning "alt-[" and dropping the rest of the parsed sequence.
This is too easy; it has even crept into a bunch of places
where the input sequence is actually valid like "VT200 button released"
but where we definitely don't want to report any key.
Fix the default: report no key for all unknown sequences and
intentionally-suppressed sequences. Treat it at "alt-[" only when
there is no input byte available, which is more or less unambiguous,
hence a strong enough signal that this is a actually "alt-[".
(cherry picked from commit 3201cb9f01)
This used to get all the interfaces and ssids when the completions
were loaded. That's obviously wrong, given that ssids especially can, you know, change
(cherry picked from commit 9116c61736)
cherry-picking since this easy to trigger
(seen again in https://github.com/fish-shell/fish-shell/pull/11549)
As mentioned in the previous few commits and in #11535, running
"set fish_complete_path ..." and "complete -C 'git ...'" may result in
"share/completions/git.fish" being loaded multiple times.
This is usually fine because fish internally erases all cached completions
whenever fish_complete_path changes.
Unfortunately there is at least global variable that grows each time git.fish
is sourced. This doesn't make a functional difference but it does slow
down completions. Fix that by resetting the variable at load time.
(cherry picked from commit 4b5650ee4f)
Commit 5918bca1eb (Make "complete -e" prevent completion autoloading,
2024-08-24) makes "complete -e foo" add a tombstone for "foo", meaning we
will never again load completions for "foo".
Due to an oversight, the same tombstone is added when we clear cached
completions after changing "fish_complete_path", preventing completions from
being loaded in that case. Fix this by restoring the old behavior unless
the user actually used "complete -e".
(cherry picked from commit a7c04890c9)
When two fish processes rewrite the uvar file concurrent, they rely on the
uvar file's mtime (queried after taking a lock, if locking is supported) to
tell us whether their view of the uvar file is still up-to-date. If it is,
they proceed to move it into place atomically via rename().
Since the observable mtime only updates on every OS clock tick, we call
futimens() manually to force-update that, to make sure that -- unless both
fish conincide on the same *nanosecond* -- other fish will notice that the
file changed.
Unfortunately, commit 77aeb6a2a8 (Port execution, 2023-10-08) accidentally
made us call futimens() only if clock_gettime() failed, instead of when
it succeeded. This means that we need to wait for the next clock tick to
observe a change in mtime.
Any resulting false negatives might have caused us to drop universal variable updates.
Reported in https://github.com/fish-shell/fish-shell/pull/11492#discussion_r2098948362
See #10300
(cherry picked from commit 8617964d4d)
When locking the uvar file, we retry whenever flock() fails with EINTR
(e.g. due to ctrl-c).
But not when locking the history file. This seems wrong; all other libc
functions in the "history_file" code path do retry.
Fix that. In future we should extract a function.
Note that there are other inconsistencies; flock_uvar_file() does not
shy away from remote file systems and does not respect ABANDONED_LOCKING.
This means that empirically probably neither are necessary; let's make things
consistent in future.
See https://github.com/fish-shell/fish-shell/pull/11492#discussion_r2095096200
Might help #10300
(cherry picked from commit 4d84e68dd4)
WHen "status current-command" is called outside a function it always returns
"fish". An extra newline crept in, fix that.
Fixes 77aeb6a2a8 (Port execution, 2023-10-08).
Fixes#11503
(cherry picked from commit e26b585ce5)
Another regression from d51f669647 (Vi mode: avoid placing cursor beyond last
character, 2024-02-14) "Unfortunately Vi mode sometimes needs to temporarily
select past end". So do the replace_one mode bindings which were forgotten.
Fix this.
This surfaces a tricky problem: when we use something like
bind '' self-insert some-command
When key event "x" matches this generic binding, we insert both "self-insert"
and "some-command" at the front of the queue, and do *not* consume "x",
since the binding is empty.
Since there is a command (that might call "exit"), we insert a check-exit
event too, after "self-insert some-command" but _before_ "x".
The check-exit event makes "self-insert" do nothing. I don't think there's a
good reason for this; self-insert can only be triggered by a key event that
maps to self-insert; so there must always be a real key available for it to
consume. A "commandline -f self-insert" is a nop. Skip check-exit here.
Fixes#11484
(cherry picked from commit 107e4d11de)
Commit b00899179f (Don't indent multi-line quoted strings; do indent inside
(), 2024-04-28) changed how we compute indents for string tokens with command
substitutions:
echo "begin
not indented
end $(
begin
indented
end)"(
begin
indented
end
)
For the leading quoted part of the string, we compute indentation only for
the first character (the opening quote), see 4c43819d32 (Fix crash indenting
quoted suffix after command substitution, 2024-09-28).
The command substitutions, we do indent as usual.
To implement the above, we need to separate quoted from non-quoted
parts. This logic crashes when indent_string_part() is wrongly passed
is_double_quoted=true.
This is because, given the string "$()"$(), parse_util_locate_cmdsub calls
quote_end() at index 4 (the second quote). This is wrong because that function
should only be called at opening quotes; this is a closing quote. The opening
quote is virtual here. Hack around this.
Fixes#11444
(cherry picked from commit 48704dc612)
Commit df3b0bd89f (Fix commandline state for custom completions with variable
overrides, 2022-01-26) made us push a transient command line for custom
completions based on a tautological null-pointer check ("var_assignments").
Commit 77aeb6a2a8 (Port execution, 2023-10-08) turned the null pointer into
a reference and replaced the check with "!ad.var_assignments.is_empty()".
This broke scenarios that relied on the transient commandline. In particular
the attached test cases rely on the transient commandline implicitly placing
the cursor at the end, irrespective of the cursor in the actual commandline.
I'm not sure if there is an easy way to identify these scenarios.
Let's restore historical behavior by always pushing the transient command line.
Fixes#11423
(cherry picked from commit 97641c7bf6)
Commit f4503af037 (Make alt-{b,f} move in directory history if commandline is
empty, 2025-01-06) had the intentional side effect of making alt-{left,right}
(move in directory history) work in Terminal.app and Ghostty without other,
less reliable workarounds.
That commit says "that [workaround] alone should not be the reason for
this change."; maybe this was wrong.
Extend the workaround to Vi mode. The intention here is to provide
alt-{left,right} in Vi mode. This also adds alt-{b,f} which is odd but
mostly harmless (?) because those don't do anything else in Vi mode.
It might be confusing when studying "bind" output but that one already has
almost 400 lines for Vi mode.
Closes#11479
(cherry picked from commit 3081d0157b)
Specifically, the width and precision format specifiers are interpreted as
referring to the width of the grapheme clusters rather than the byte count of
the string. Note that grapheme clusters can differ in width.
If a precision is specified for a string, meaning its "maximum number of
characters", we consider this to limit the width displayed.
If there is a grapheme cluster whose width is greater than 1,
it might not be possible to get precisely the desired width.
In such cases, this last grapheme cluster is excluded from the output.
Note that the definitions used here are not consistent with the `string length`
builtin at the moment, but this has already been the case.
(cherry picked from commit 09eae92888)
As mentioned in
https://github.com/fish-shell/fish-shell/pull/9688#discussion_r1155089596,
commit b77d1d0e2b (Stop crashing on invalid Unicode input, 2024-02-27), Rust's
char type doesn't support arbitrary 32-bit values. Out-of-range Unicode
codepoints would cause crashes. That commit addressed this by converting
the encoded bytes (e.g. UTF-8) to special private-use-area characters that
fish knows about. It didn't bother to update the code path in builtin read
that relies on mbrtowc as well.
Fix that. Move and rename parse_codepoint() and rename/reorder its input/output
parameters.
Fixes#11383
(cherry picked from commit d9ba27f58f)
This also changes the single-byte locale code path to treat keyboard input
like "\x1ba" as alt-a instead of "escape,a". I can't off-hand reproduce
a problem with "LC_ALL=C fish_key_reader", I guess we always use a UTF-8
locale if available?
(cherry picked from commit b061178606)
This part of the code could use some love; when we happen to clear the
selected text, we should end the selection.
But that's not how it works today. This is fine for Vi mode, because Vi
mode never deletes in visual mode.
Let's fix the crash for now.
Fixes#11367
(cherry picked from commit af3b49bf9c)
With
bind ctrl-r 'sleep 1' history-pager
typing ctrl-r,escape crashes fish in the history pager completion callback,
because the history pager has already been closed.
Prior to 55fd43d86c (Port reader, 2023-12-22), the completion callback
would not crash open a pager -- which causes weird races with the
user input.
Apparently this crash as been triggered by running "playwright",
and -- while that's running typing ctrl-r ligh escape.
Those key strokes were received while the kitty keyboard protocol
was active, possibly a race.
Fixes#11355
(cherry picked from commit c94e30293a)
Our versions look like
4.0.0
4.0b1
4.0.1-535-abfef-dirty
But packagers may want to add more information here, and we don't
really care. Note that we no longer ever set the version to "unknown"
since 5abd0e46f5.
Supersedes #11173
(cherry picked from commit 411a396fa9)
This uses jj's dynamic completions when possible.
This avoids an annoying problem. After 04a4e5c4, jj's dynamic
completions (see the second paragraph of
<https://jj-vcs.github.io/jj/latest/install-and-setup/#command-line-completion>)
do not work very well in fish if the user puts `COMPLETE=fish jj |
source` in their `~/.config/fish/config.fish`. When the user types `jj
<TAB>`, they are instead overridden by fish's built-in non-dynamic
completions.
The difference is subtle. One problem I saw is that `jj new <TAB>` works
as expected (and shows revisions) while `jj new -A <TAB>` becomes broken
(and shows files).
If the user puts `COMPLETE=fish jj | source` in
`~/.config/fish/completions/jj.fish` there is no problem. However, users
might be confused if they run `COMPLETE=fish jj | source` or put it in
their config and it works in a broken fashion. I certainly was.
Meanwhile, I checked that if the user has `jj completion fish | source`
in their `config.fish`, executing `COMPLETE=fish jj
__this_command_does_not_exist | source` afterwards still works
correctly.
Let me know if there's a better approach to this problem.
(cherry picked from commit 932010cd04)
The commands 'close', 'resize', and 'status' each take 'name' as their solo argument.
Signed-off-by: memchr <memchr@proton.me>
(cherry picked from commit 5012bcb976)
