As reported in
https://matrix.to/#/!YLTeaulxSDauOOxBoR:matrix.org/$yD_Lutaftf6ytk617kjw5vC-k_OgHRQxIiSRv89uBMI
it's weird that command name completions shadow abbreviation name
completions, given that the abbreviation will the one that will be
executed in the typical cases. Let's put abbreviation completions first;
unique_completions_retaining_order() will take care of removing any command
completions we add later.
When a command like "long-running-command &" exits, the resulting SIGCHLD
is queued in the topic monitor. We do not process this signal immediately
but only after e.g. the next command has finished. Only then do we reap the
child process.
Some terminals, such as Terminal.app, refuse to close when there are unreaped
processes associated with the terminal -- as in, having the same session ID,
see setsid(3).
In future, we might want to reap proactively.
For now, apply an isolated workaround: instead of taking care of a child
process, double-fork to create an orphaned process. Since the orphan will
be reaped by PID 1, we can eventually close Terminal.app without it asking
for confirmation.
/bin/sh -c '( "$@" ) >/dev/null 2>&1 &' -- cmd arg1 arg2
This fix confines the problem to the period during which a background process
is running. To complete the fix, we would need to call setsid to detach the
background process from a controlling terminal. That seems to be desirable
however macOS does provide a setsid utility.
setsid cmd arg1 arg2 >/dev/null 2>&1
Fixes#11181
As of 303af07, iTerm2 3.5.11 on two different machines has two different
behaviors. For unknown reasons, when pressing alt-right fish_key_reader
shows "\e\[1\;9C" on one machine and "\e\[1\;3C" on another.
Feels like iTerm2 interprets modifyOtherKeys differently, depending on
configuration.
We don't want to risk asking for the kitty
keyboard protocol until iTerm2 3.5.12 (see
https://github.com/fish-shell/fish-shell/issues/11004#issuecomment-2571494782).
So let's work around around this weirdness by adding back the legacy
bindings removed in c0bcd817ba (Remove obsolete bindings, 2024-04-28) and
plan to remove them in a few years.
Note that fish_key_reader still reports this as "left", which already has
a different binding, but it looks like literal matches of legacy sequences
have precedence.
Fixes the problem described in
https://github.com/fish-shell/fish-shell/issues/11192#issuecomment-2692247060Closes#11192
Before 798527d79a (completions: fix double evaluation of tokenized commandline, 2024-01-06)
git-foo completions did something like
set -l subcommand_args (commandline -opc)
complete -C "git-foo $subcommand_args "
As mentioned in 368017905e (builtin commandline: -x for expanded tokens,
supplanting -o, 2024-01-06), the "-o" option is bad
because it produces a weird intermediate, half-expanded state.
The immediate goal of 798527d79a was to make sure we do not do any
more expansion on top of this. To that end, it changed the above to
"\$subcommand_args". The meaning is more or less the same[^*] but crucially,
the recursive completion invocation does not see through the variable,
which breaks some completions.
Fix this with the same approach as in 6b5ad163d3 (Fix double expansion of
tokenized command line, 2025-01-19).
[^*]: It wasn't semantically correct before or after -- this was later
corrected by 29f35d6cdf (completion: adopt commandline -x replacing deprecated
-o, 2024-01-22)).
Closes#11205
Given
$ cat ~/.config/kitty/kitty.conf
notify_on_cmd_finish unfocused 0.1 command notify-send "job finished with status: %s" %c
kitty will send a notification whenever a long-running (>.1s) foreground
command finishes while kitty is not focused.
The %c placeholder will be replaced by the commandline.
This is passed via the OSC 133 command start marker, kitty's fish shell
integration.
That integration has been disabled for fish 4.0.0 because it's no longer
necessary since fish already prints OSC 133. But we missed the parameter for
the command string. Fix it. (It's debatable whether the shell or the terminal
should provide this feature but I think we should fix this regression?)
Closes#11203
See https://github.com/kovidgoyal/kitty/issues/8385#issuecomment-2692659161
Something like
write!(f, "foo{}bar", ...)
seems to call f.write_str() thrice.
Splitting a single OSC 133 command into three calls to write(3) might result in
odd situations if one of them fails. Let's try to do it in one in most cases.
Add a new buffered output type that can be used with write!(). This is
somewhat redundant given that we have scoped_buffer(). While at it, remove
the confused error handling. This doesn't fail unless we are OOM (and this
new type makes that more obvious).
This was broken for 4.0 because it used `{}` command grouping.
Instead just do one of the things the fish_git_prompt does.
(the default isn't usable here because it gets the sha from elsewhere)
(cherry picked from commit e925eccad2)
Fix the accidental "git branch" output leaking while making sure we support:
1. unborn branch, where HEAD does not exist (`git init`)
2. detached head (`git checkout --detach`)
Notably computing the branch name should be independent of computing
a diff against HEAD.
In scenario 1 there is a branch but no HEAD,
while in scenario 2 it's the other way round.
Hence we need a separate check to see if we're in a git repo.
"git rev-parse" seems to work. Not sure what's best pracitce.
Also remove the ahead/behind logic, it was broken because it misspelled
@{upstream}.
Fixes#11179
As reported in
https://matrix.to/#/!YLTeaulxSDauOOxBoR:matrix.org/$CLuoHTdvcRj_8-HBBq0p-lmGWeix5khEtKEDxN2Ulfo
Running
fish -C '
fzf_key_bindings
echo fish_vi_key_bindings >>~/.config/fish/config.fish
fzf-history-widget
'
and pressing "enter" will add escape sequences like "[2 q" (cursor shape)
to fish's command line.
This is because fzf-history-widget binds "enter" to a filter
that happens to be a fish script:
set -lx FZF_DEFAULT_OPTS \
... \
"--bind='enter:become:string replace -a -- \n\t \n {2..} | string collect'" \
'--with-shell='(status fish-path)\\ -c)
The above ~/.config/fish/config.fish (redundantly) runs "fish_vi_key_bindings"
even in *noninteractive* shells, then "fish_vi_cursor" will print cursor
sequences in its "fish_exit" handler. The sequence is not printed to the
terminal but to fzf which doesn't parse CSI commands.
This is a regression introduced by a5dfa84f73 (fish_vi_cursor: skip if stdin
is not a tty, 2023-11-14). That commit wanted "fish -c read" to be able to
use Vi cursor. This is a noninteractive shell, but inside "read" we are
"effectively interactive". However "status is-interactive" does not tell
us that.
Let's use a more contained fix to make sure that we print escape sequences only
if either fish is interactive, or if we are evaluating an interactive read.
In general, "fish -c read" is prone to configuration errors, since we
recommend gating configuration (for bind etc) on "status is-interactive"
which will not run here.
Historically, up-arrow search matches have been highlighted by
1. using the usual foreground (from syntax highlighting)
2. using the background from $fish_color_search_match
Commit 9af6a64fd2 (Fix bad contrast in search match highlighting, 2024-04-15)
broke this by also applying the foreground from $fish_color_search_match.
As reported on gitter, there is a meaningful scenario where the foreground
from syntax highlighting should not be overwritten:
set fish_color_search_match --reverse
this copies the foreground from syntax highlighting to the background.
Since commit 9af6a64fd2 overwrites the foreground highlight, the resulting
background will be monocolored (black in my case) instead of whatever is
the syntax-colored foreground.
FWIW the reversed foreground will always be monocolored, because we have
always done 2.
Let's unbreak this scenario by using the foreground from
fish_color_search_match only if it's explicitly set (like we do since
9af6a64fd2).
This is hacky because an empty color is normally the same as "normal", but
it gets us closer to historical behavior. In future we should try to come
up with a better approach to color blending/transparency.
Konsole has a bug: it does not recognize file:://$hostname/path as directory.
When we send that via OSC 7, that breaks Konsole's "Open Folder With"
context menu entry.
OSC 7 producers are strongly encouraged to set a non-empty hostname, but
it's not clear if consumers are supposed to accept an empty hostname (see
https://gitlab.freedesktop.org/terminal-wg/specifications/-/issues/20).
I think it should be fine; implementations should treat it as local path.
Let's work around the Konsole bug by omitting the hostname for now. This
may not be fully correct when using a remote desktop tool to access a
system running Konsole but I guess that's unlikely and understandable.
We're using KONSOLE_VERSION, so it the workaround should not leak into SSH
sessions where a hostname component is important.
Closes#11198
Proposed upstream fix https://invent.kde.org/frameworks/kio/-/merge_requests/1820
As reported in b5736c5535 (Extend iTerm CSI u workaround to < 3.5.12,
2025-02-20), iTerm 3.5.12 has resolved our issues related to the kitty
keyboard protocol. Enable it here too, matching the release branch.
The flag to gate this is set for versions of iTerm that don't have sufficient
support for the kitty keyboard protocol. CSI u is (more or less) the encoding
used by that protocol. Let's name things accordingly. My bad.
Commit 2d234bb676 (Only request keyboard protocols once we know if kitty kbd
is supported, 2025-01-26) queries support for the kitty protocol and acted
upon it after having read next character.
Unfortunately this meant that we don't turn on the kitty protocol until after
we read a character -- since the CSI ? u response does not generate a char
event. Let's query for primary DA additionally, matching what fish does.
In future, we should do a timed wait as well, to avoid terminal responses
leaking when there we exit fish_key_reader too quickly.
Whenever we add logic to print a control sequence that we hadn't printed
before, there is a nonzero risk that a terminal mishandles it.
Terminal-specific workarounds cause pain but are probably better than not
being able to use any new commands provided by terminals.
There is no universal way to identify a terminal. Device attributes (primary
through tertiary) typically get spoofed responses, likely not good enough
for working around bugs in specific versions of a terminal.
The de-facto standard for the terminal name and version is XTVERSION.
It's usually specific to the terminal, except for something like VTE-based
terminals, where we get this (which seems good enough also)
printf '\x1b[>0q'; cat
^[P>|VTE(7803)^[\
Of course querying for XTVERSION can trigger terminal bugs just as well. Let's
start querying for it now -- even without a concrete use case -- to increase
the chance we can use it during crunch time when we don't want to test
anymore. (We typically discover buggy terminals only very late in the release
cycle, most prominently after a release).
When we enable/disable terminal protocols,
we use atomic operations because of issues like
1. halfway through enabling, we might be interrupted by a signal handler.
2. our SIGTERM handler runs the (idempotent) disabling sequences,
so the operations must be async-signal safe.
The flags to keep track of whether things like kitty keyboard protocol are enabled
are "mirrored" between the enabling and disabling logic:
- the enabling logic marks it as enabled *before* enabling anything
- the disabling logic marks it as disabled *after* everything has been disabled
This ensures that we are well-behaved in issue 1; we will always (perhaps
redundantly) disable the kitty keyboard protocol.
We forgot to use the same ordering for bracketed paste.
If we get SIGTERM after this line
BRACKETED_PASTE.store(false, Ordering::Release);
we might exit with bracketed paste still turned on.
Code blocks are often written like
$ echo hello world
hello world
The "$ " is widely understood to introduce a shell command. It's often
easier to copy the whole line than copying everything after "$ ".
This gets more pronounced when there are multiple commands without interleaved
output (either due to omission or the rule of silence). Copying the whole
code block is the most natural first step.
You could argue that this is a presentation issue - the dollar prefix
should be rendered but not copied to clipboard. But in my experience there
are many cases where there is no HTML or Javascript that would allow the
copy-to-clipboard functionality to strip the prefixes.
The "$ " prefix is almost never useful when pasting; strip it automatically.
Privileged commands use "# " as prefix which overlaps with comments, so do
not strip that until we can disambiguate (another potential reason not to
do that would be safety but it's unclear if that really matters).
Add the new logic to the commandline builtin, because we don't know about the
AST in fish script. (Technically, the tokenizer already knows whether a "$
" is in command position and at the beginning of a line, but we don't
have that either (yet).)
Maybe we should move the rest of __fish_paste over as well. I'm not sure what
difference that would make; for one, pasting could no longer be cancelled
by ctrl-c (in theory), which seems like a good direction?
Comments by macOS users have shown that, apparently, on that platform
this isn't wanted.
The functions are there for people to use,
but we need more time to figure out if and how we're going to bind
these by default.
For example, we could change these bindings depending on the OS in future.
This reverts most of commit 6af96a81a8.
Fixes#10926
See #11107
(cherry picked from commit 378f452eaa)
This can happen if your filesystem on macOS has xattrs, so the newly
created dirs will also have them and `ls` will print an "@" indicator.
Fixes#11137
(cherry picked from commit 414293521e)
The majority of users will be going straight from 3.7 to 4.0. The 4.0 notes
should reflect this transition, rather than the changes that were only in 4.0b1.
iTerm has a bug where it'll send Option-Left as Left instead of the
proper Alt-Left. This was reported upstream and fixed in
480f059bce
which is contained in the 3.5.12-beta2 tag, so let's assume that fixes
it.
Fixes#11025
(not necessary in 4.1)
parse_util_lineno() returns 1-based line numbers but
parse_util_get_offset_from_line() expects zero based line offsets.
Fixes#11162
(cherry picked from commit afbdb9f268)
Commit 4f536d6a9b (Update commandline state snapshot lazily,
2024-04-13) add an optimization to update the search field only if
necessary. The optimization accidentally prevents us from resetting
the search field.
Fixes#11161
(cherry picked from commit 72f2433120)
