Currently, when the history pager is selected, fish always one undo away from the original search term. If the search has no matches, the fish replaces the search term with itself (resulting in no visible change, but it is still treated as a transient edit). However, fish ignores undo operations where the replacement is "" by "", leading to an unnecessary undo. For example:
1. Type 'test'.
2. Do ctrl-c.
3. Open the history pager.
4. Make so there are no matches.
5. Exit - you will undo back to 'test'.
This commit also ensures that if you select a pager element that does not change the content of the commandline, it will not be added to the undo history.
Currently, if we return to the present, fish thinks that it still has a transient edit. This results in an unnecessary undo when performing a backward search. For example:
1. Type ': '.
2. Do a backward token search.
3. Do a forward token search.
4. Do another backward token search - this will result in the undo of ': '.
Currently, the search always restarts when moving from the present.
For example:
1. Do a few backward searches.
2. Use PageDown to return to the present.
3. Do another backward search.
4. Now, PageUp does nothing because the search was restarted, even if we maintained search mode.
Every reader gets their own wait handle which is wrong and not actually
needed - it's a singleton. We should probaly make it global. Let's
do an intermediate solution for now -- not much time this weekend ;).
Fixes#11110
There is no guarantee that Write::write() will write the entirety of the
provided buffer in one go, regardless of how short it is, because that depends
on the semantics of the underlying write handle (even if it was a
single byte, though in this case that would be because of an interrupt).
The only case I'm aware of that would guarantee a single Write::write() call
would suffice is when writing into a high-level memory-backed buffer, and we
can't make that guarantee (now or in perpetuity).
This is consistent with what we do for highlighting history search,
see d7354880e3 (Fix regression causing crash in token history search,
2025-01-27). In future, we should try to find a better fix (and a
better test).
Fixes the other problem described in #11096
The two terminals Midnight Commander and dvtm are special in that
they filter requests (or perhaps responses) like
printf "\x1b[0c"
and don't implement the response themselves -- so we never get
one. Let's work around that until we can fix it.
Disable the kitty protocol in mc for now (to keep the code simple),
though we could certainly re-enable it.
Fixes 64859fc242 (Blocking wait for responses to startup queries, 2025-01-25).
Today we might
1. enable modifyOtherKeys
2. get a reply indicating the kitty keyboard protocol is supported
3. because of 2, we never turn off modifyOtherKeys again
Let's get rid of this weird issue by enabling either modifyOtherKeys
or the kitty enhancements only after we know whether the kitty protocol
is supported.
This means we need to call terminal_protocols_enable_ifn() before every
call to readch() until the querying is done. Fortunately, this is
already in place in read_normal_chars(); there are other places that
call readch() but none of those is executed until querying has completed.
At startup we query for
- the cursor position (CSI 6 n)
- kitty keyboard protocol support (CSI ? u)
- terminfo capabilities via XTGETTCAP
Since we don't wait for responses, those can leak into child processes.
Some child processes like fzf cannot decode DCS replies. Plug the
leak by ending each round of querying by asking for the Primary Device
Attribute, and resume input processing only after a response has been
received, (or ctrl-c as an escape hatch).
This is a nice simplification. Tested with the lowest common
denominator (putty, Terminal.app and st).
Fixes#11079
The st terminal wrongly parses CSI ? u as DECRC. A fix has been
proposed upstream. Let's also work around it I guess (not to mention
that querying in the first place is also sort of a workaround).
This reverts commit ebdc3a0393.
Not discussed, includes a new thing that queries the terminal for the client OS
when what is really needed is just a `uname` - which would also work on Terminal.app.
Unlike other builtins, "{" is a separate token, not a keyword-string
token.
Allow the left brace token as command string; produce it when parsing
"{ -h"/"{ --help" (and nowhere else). By using a decorated statement,
we reuse logic for redirections etc.
Other syntax elements like "and" are in the builtin list, which
- adds highlighting logic
- adds it to "builtin --names"
- makes it runnable as builtin
(e.g. "builtin '{'" would hypothetically print the man page)
These don't seem very important (highlighting for '{' needs to match
'}' anyway).
Additionally, making it a real builtin would mean that we'd need to
deactivate a few places that unescape "{" to BRACE_BEGIN.
Let's not add it to the built in list. Instead, simply synthesize
builtin_generic in the right spot.
I'm assuming we want "{ -h" to print help, but '"{" -h' to run an
external command, since the latter is historical behavior. This works
naturally with the above fake builtin approach which never tries to
unescape the left brace.
If a child program crashes with some text rendered below the cursor,
we fail to clear that text. For example run vim, "pkill -9 vim" and
observe that scrollback-push fails to clean up the leftover text.
Fix that.
libc::getpid and getpgrp *cannot fail*, so an unsafe declaration here
is just noise.
I mean sure, if your libc is broken and these fail, but at that point
I'm comfortable declaring your computer a platypus that we do not need
to support.
Sorry, commit 51adba6ee0 (Restore autosuggestion after corrected
typo, 2025-01-10) was pushed too early. One issue is that it saves
autosuggestions also when we edit in the middle, where we can't
restore it. We'd still restore it in some cases, even though it
doesn't apply. This breaks invariants that may cause various problems
when interacting with the autosuggestion.
Fix it by only saving the autosuggestion when we will be able to
restore it correctly.
For better or worse, backward-token completely skips over operators
like > & |.
forward-token is (accidentally?) inconsistent with that. Fix that.
Skipping over those tokens might be wrong weird. Maybe not for
redirections since they are tighly coupled to their target. Maybe we
can improve this in future.
When pasting and executing a full line, the trailing newline character
will be included in history.
I usually manually delete the newline before executing, but sometimes
I forget. When I recall my (typically single-line) commands, it's
surprising that the cursor is on the blank second line.
The newline doesn't seem useful. Let's remove it automagically.
I wonder if anyone will be thrown off by this smart behavior.
In future, we can make this space trimming configurable, similar to
fish_should_add_to_history.
As mentioned in
https://github.com/fish-shell/fish-shell/pull/11045#discussion_r1915994998,
we need to refresh TTY timestamps to avoid timing-based issues.
For some context see
git log --grep='[Rr]efresh.* TTY'
Make things more consistent again. I don't know if all of these are
absolutely necessary, hoping to find out later (and consolidate this
logic in outputter).
And leave the old behavior under the name "cancel-commandline".
This renames "cancel-commandline-traditional" back to
"cancel-commandline", so the old name triggers the old behavior.
Fixes#10935
Commit 4f3d6427ce (Fix regression causing crash in "commandline -j",
2025-01-12) wasn't quite right; it mishandles the edge case where
the current process has no token, fix that.
If you don't care about file paths containing '=' or ':', you can
stop reading now.
In tokens like
env var=/some/path
PATH=/bin:/usr/local/b
file completion starts after the last separator (#2178).
Commit db365b5ef8 (Do not treat \: or \= as file completion anchor,
2024-04-19) allowed to override this behavior by escaping separators,
matching Bash.
Commit e97a4fab71 (Escape : and = in file completions, 2024-04-19)
adds this escaping automatically (also matching Bash).
The automatic escaping can be jarring and confusing, because separators
have basically no special meaning in the tokenizer; the escaping is
purely a hint to the completion engine, and often unnecessary.
For "/path/to/some:file", we compute completions for "file" and for
"/path/to/some:file". Usually the former already matches nothing,
meaning that escaping isn't necessary.
e97a4fab71 refers us to f7dac82ed6 (Escape separators (colon and
equals) to improve completion, 2019-08-23) for the original motivation:
$ ls /sys/bus/acpi/devices/d<tab>
$ ls /sys/bus/acpi/devices/device:
device:00/ device:0a/ …
Before automatic escaping, this scenario would suggest all files from
$PWD in addition to the expected completions shown above.
Since this seems to be mainly about the case where the suffix after
the separator is empty,
Let's remove the automatic escaping and add a heuristic to skip suffix
completions if:
1. the suffix is empty, to specifically address the above case.
2. the whole token completes to at least one appending completion.
This makes sure that "git log -- :!:" still gets completions.
(Not sure about the appending requirement)
This heuristic is probably too conservative; we can relax it later
should we hit this again.
Since this reverts most of e97a4fab71, we address the code clone
pointed out in 421ce13be6 (Fix replacing completions spuriously quoting
~, 2024-12-06). Note that e97a4fab71 quietly fixed completions for
variable overrides with brackets.
a=bracket[
But it did so in a pretty intrusive way, forcing a lot of completions
to become replacing. Let's move this logic to a more appropriate place.
---
Additionally, we could sort every whole-token completion before every
suffix-completion. That would probably improve the situation further,
but by itself it wouldn't address the immediate issue.
Closes#11027
Commit 3fcc6482cb (Fix parse_util_process_extent including too much
on the left, 2024-12-24) changed the process extent based on the
observation that "A\n\n\nB" comprises three tokens with ranges 0..1,
1..2 and 4..5. Prior to that commit, the second process extent was
2..5, which seems a bit weird because it includes newlines.
Weirdness aside, the real reason for changing it was this snippet in
the autosuggestion performer, where we compute the process extent
around cursor, and check if the line at process start matches the
cached search string.
// Search history for a matching item unless this line is not a continuation line or quoted.
if range_of_line_at_cursor(
&command_line,
parse_util_process_extent(&command_line, cursor_pos, None).start,
) == search_string_range
Given "A\n\n\nB" and cursor_pos=1 commit 3fcc6482cb changed the output
from 2..5 to 4..5. This brings problems:
1. leading spaces will not be included (which is probably
inconsequential but still ugly).
2. the specified cursor position is not included in the given range.
We could paper over 2 by computing min(cursor_pos)
but that would leave 1.
For now let's revert and solve the autosuggestion issue in a less
brittle way.
Backspace signals that the user is not happy with the commandline,
and by extension the autosuggestion.
For this reason, backspace suppresses autosuggestions until the next
text insertion.
However if I
1. type something that has an autosuggestion
2. type *one* wrong letter (removing the autosuggestion)
3. type backspace
backspace does not visibly suppress any autosuggestion but rhater
signal that the user wants to go back to the previous state of the
commandline, which does have an autosuggestion.
Enable this scenario by caching the autosuggestion when it's
invalidated. On certain edits that make the cached autosuggestion
valid again, restore it from the cache. Currently, only do this up
to a single backspace. Could extend that in future.
This implementation is really bad.. but it's a start.
Weirdly, it does not restore the cache on undo; but that's
inconsequential because undo doesn't suppress autosuggestion as
of today.
Closes#3549
