Our recursive create_dir() first calls stat() to check if the directory
already exists and then mkdir() trying to create it. If another (fish)
process creates the same directory after our stat() but before our
mkdir(), then our mkdir() fails with EEXIST. This error is spurious
if there is already a directory at this path (and permissions are
correct).
Let's switch to the stdlib version, which promises to solve this issue.
They currently do it by running mkdir() first and ask stat() later.
This implies that they will only return success even if we don't have
any of rwx permissions on the directory, but that was already a problem
before this change. We silently don't write history in that case..
Fixes#10813
All-whitespace autocompletions are invisible, no matter the cursor
shape. We do offer such autosuggestions after typing a command name
such as "fish". Since the autosuggestion is invisible it's probably
not useful. It also does no harm except when using a binding like
bind ctrl-g '
if commandline --showing-suggestion
commandline -f accept-autosuggestion
else
up-or-search
end'
where typing "fish<ctrl-g>" surprisingly does not perform a history
search. Fix this by detecting this specific case. In future we
could probably stop showing autosuggestions whenever they only
contain whitespace.
With BSD man, "PAGER=vim man man | cat" hangs because
[man](https://cgit.freebsd.org/src/tree/usr.bin/man/man.sh) wrongly
calls the pager even though stdout is not a terminal.
This hang manifests in places where we call apropos in a subshell,
such as in "complete -Ccar".
Let's work around this I guess. This should really be fixed upstream
because it's a problem in every app that wants to display man pages
but doesn't emulate a complete terminal.
Weirdly, the Apple derivative of man.sh uses WHATISPAGER instead
of MANPAGER.
Closes#10820
A release build is recommended to most users (to avoid occasional slowness)
whereas developers may prefer debug builds for shorter build times and more
accurate debug information.
There are more users of "make install" than developers, so I think the
default should be optimized for users, i.e. an optimized build. I think
that's in line with what most of our peer projects do.
Even if developers don't know about the -DCMAKE_BUILD_TYPE=Debug
trick, they will likely be able to iterate quickly by using "cargo
{build,check,clippy,test}" and rust-analyzer, all of which use a debug
configuration by default, irrespective of cmake. Granted, users will need
to use cmake to run system tests. If a task needs a lot of iterations,
one can always convert the system test to a script that can be run with
target/build/fish. For building & running all system tests, the release
build takes 30% longer, so not that much.
Here are my build/test times and binary sizes; with debug:
$ time ninja -C build-Debug/
________________________________________________________
Executed in 25.30 secs fish external
usr time 68.33 secs 676.00 micros 68.32 secs
sys time 11.34 secs 41.00 micros 11.34 secs
$ du -h build-Debug/fish
43M build-Debug/fish
$ time ninja -C build-Debug/ test
________________________________________________________
Executed in 193.96 secs fish external
usr time 182.84 secs 1.53 millis 182.83 secs
sys time 30.97 secs 0.00 millis 30.97 secs
with release
$ time ninja -C build-RelWithDebInfo/
________________________________________________________
Executed in 106.80 secs fish external
usr time 164.98 secs 631.00 micros 164.98 secs
sys time 11.62 secs 41.00 micros 11.62 secs
$ du -h build-RelWithDebInfo/fish
4.6M build-RelWithDebInfo/fish
$ time ninja -C build-RelWithDebInfo/ test
________________________________________________________
Executed in 249.87 secs fish external
usr time 260.25 secs 1.43 millis 260.25 secs
sys time 29.86 secs 0.00 millis 29.86 secs
Tangentially related, the numbers with "lto = true" deleted. This seems
like a nice compromise for a default but I don't know much about the other
benefits of lto.
$ time ninja -C build-RelWithDebInfo-thin-lto/
________________________________________________________
Executed in 35.50 secs fish external
usr time 196.93 secs 0.00 micros 196.93 secs
sys time 13.00 secs 969.00 micros 13.00 secs
$ du -h build-RelWithDebInfo-thin-lto/fish
5.5M build-RelWithDebInfo-thin-lto/fish
$ time ninja -C build-RelWithDebInfo-thin-lto/ test
________________________________________________________
Executed in 178.62 secs fish external
usr time 287.48 secs 976.00 micros 287.48 secs
sys time 28.75 secs 115.00 micros 28.75 secs
Alternative solution: have no default at all, and error out until the user
chooses a build type.
Currently the only difference between RelWithDebInfo and Release is that
the former adds -g (aka debuginfo=2) though it doesn't seem to make a lot
of difference in my testing.
Since build_tools/make_pkg.sh and debian/rules use RelWithDebInfo, let's be
consistent with those.
Some background: fish has some files which should be updated atomically:
specifically the history file and the universal variables file. If two fish
processes modified these in-place at the same time, then that could result
in interleaved writes and corrupted files.
To prevent this, fish uses the write-to-adjacent-file-then-rename to
atomically swap in a new file (history is slightly more complicated than
this, for performance, but this remains true). This avoids corruption.
However if two fish processes attempt this at the same time, then one
process will win the race and the data from the other process will be lost.
To prevent this, fish attempts to take an (advisory) lock on the target
file before beginning this process. This prevents data loss because only
one fish instance can replace the target file at once. (fish checks to
ensure it's locked the right file).
However some filesystems, particularly remote file systems, may have locks
which hang for a long time, preventing the user from using their shell.
This is far more serious than data loss, which is not catastrophic: losing
a history item or variable is not a major deal. So fish just attempts to
skip locks on remote filesystems.
Unfortunately Linux does not have a good API for checking if a filesystem
is remote: the best you can do is check the file system's magic number
against a hard-coded list. Today, the list is NFS_SUPER_MAGIC,
SMB_SUPER_MAGIC, SMB2_MAGIC_NUMBER, and CIFS_MAGIC_NUMBER.
Expand it to AFS_SUPER_MAGIC, CODA_SUPER_MAGIC, NCP_SUPER_MAGIC,
NFS_SUPER_MAGIC, OCFS2_SUPER_MAGIC, SMB_SUPER_MAGIC, SMB2_MAGIC_NUMBER,
CIFS_MAGIC_NUMBER, V9FS_MAGIC which is believed to be exhaustive.
ALSO include FUSE_SUPER_MAGIC: if the user's home directory is some FUSE
filesystem, that's kind of sus and the fewer tricks we try to pull, the
better.
As mentioned in 04c913427 (Limit command line rendering to $LINES
lines, 2024-10-25) our rendering breaks when the command line overflows
the screen and we have a pager search field.
Let's also apply the overflow logic in this case.
Note that the search field still works, it's just not visible.
In future we should maybe show a small search field (~4 lines) in
this case (removing 4 screen lines worth of command line). But again,
this is not really important.
If the first physical line in the command line overflows the screen,
the cursor will be wrong and we'll fail to clear the prompt without
a manual ctrl-l. Let's fix that, and also don't print the OSC 133
marker in this case.
Currently, when we are scrolled, the first line on the screen still
gets an indentation that would normally be filled by the prompt.
This happens even for soft-wrapped lines, so they might be
torn apart in weird ways here.
In future, we might paint the prompt here. If not, the current
behavior for soft-wrapped lines is debatable but its' not super
important to fix. The main goal is to first get rid of glitches in
these edge cases.
This test does "isolated-tmux send-keys Escape" to exit copy mode. When
EDITOR contains "vi", tmux will use Vi keybindings where Escape does
something else ("q" would exit copy mode).
Tests want to have predictable behavior so let's declare the default
emacs key bindings unconditionally.
Fixes#10812
This added link args to target macOS 10.9, but these arguments are not necessary
when building via the make_pkg.sh script, and this file is causing other
problems.
Let's provide a sensible default here. Use a line for "insert" and an
underline for "replace_one" mode. Neovim does the same, it feels pretty
slick.
As mentioned in #10806
As of the parent commit, __fish_vi_key_bindings_remove_handlers
should be working properly now, so this is no longer necessary That
function also cleans up other stuff like fish_cursor_end_mode, that
fish_default_key_bindings doesn't know anything about.
Also this fixes a spurious exit status of 4 in some scenarios.
fish_key_bindings may be set directly
or via fish_{default,vi}_key_bindings.
The latter use "set --no-event" to simplify their control
flow. This (24836f965 (Use set --no-event in the key binding
functions, 2023-01-10)) broke Vi mode cleanup, since Vi mode
uses a variable hook. Let's update this variable also when using
fish_{default,vi}_key_bindings. Another reason to keep this variable
in sync is to make the fish_key_bindings handlers working as expected.
Render the command line buffer only until the last line we can fit
on the screen.
If the cursor pushes the viewport such that neither the prompt nor
the first line of the command line buffer are visible, then we are
"scrolled". In this case we need to make sure to erase any leftover
prompt, so add a hack to disable the "shared_prefix" optimization
that tries to minimize redraws.
Down-arrow scrolls down only when on the last line, and up-arrow always
scrolls up as much as possible. This is somewhat unconventional;
probably we should change the up-arrow behavior but I guess it's a
good idea to show the prompt whenever possible. In future we could
solve that in a different way: we could keep the prompt visible even
if we're scrolled. This would work well because at least the left
prompt lives in a different column from the command line buffer.
However this assumption breaks when the first line in the command
line buffer is soft-wrapped, so keep this approach for now.
Note that we're still broken when complete-and-search or history-pager
try to draw a pager on top of an overfull screen. Will try to fix
this later.
Closes#7296
It's a 9-char CSI and we've read 3 (`<ESC>[T`), so we need to read six more.
Verified against the previous C++ codebase and couldn't find a reason for the
change to consuming 10 chars in a `git blame` run.
Commit ba67d20b7 (Refresh TTY timestamps after nextd/prevd, 2024-10-13)
wasn't quite right because it also needs to fix it for arbitrary commands.
While at it, do this only when needed:
1. It seems to be only relevant for multiline prompts.
Note that we can wait until after evaluation to check if the prompt is
multiline, because repaint events go through the queue, see 5ba21cd29
(Send repaint requests through the input queue again, 2024-04-19).
2. When the binding doesn't execute any external command, we probably don't
need to fix up whatever the user printed. If they actually wanted to show
output and print another prompt, they should currently use "__fish_echo",
to properly support multiline prompts. Bindings should produce no other
output. What distinguishes external programs is that they can trigger this
issue even if they don't produce any output that remains visible in fish,
namely by using the terminal's alternate screen.
Would be nice if we could get rid of __fish_echo; I'm not yet sure how.
Fixes#10800
A side effect of cd9e50c2c (completions/set: Complete variables of all scopes
when setting, 2024-10-03) is that
HOME=$(mktemp -d) fish
fish_config choose ayu\ Light
set -S fish_color_
gives only completions that have the "Universal variable" description even
though most colors are also defined in the global scope which usually takes
precedence.
Fix this by reordering the completions. (The last-added completion is shown
first which is very surprising, we should change that).
This is not perfect; if the user has already specified `-U`, then we should
probably not show description of the global version. But that's still
worth the trade that this commit makes. Finally, the description could show
something like "Defined in universal and global scope" etc.
The test case shows that the pager rendering is not quite right. It renders
'{\', leaving out the newline. This rendering is ambiguous.
Let's fix it by rendering \n as control picture, like we do for other control
characters in the pager.
Given
$ echo {\
C
where C is the cursor.
Completions have prefix "{\\\n".
Since \n has a wcwidth of -1, this line always fails
let prefix_len = usize::try_from(fish_wcswidth(&self.prefix));
This triggers uncovers a regression in 43e2d7b48 (Port pager.cpp, 2023-12-02),
where we end up computing comp_width=0 for all completions.
Fix this. Test in the next commit.
The C++ version added the prefix width only if the completion had a valid
width. That seems wrong, let's do it always (if the prefix width is valid).
Completion on ": {*," used to work but nowadays our attempt to wildcard-expand
it fails with a syntax error and we do nothing. This behavior probably only
makes sense for the overflow case, so do that.
On a German keyboard, with a German keymap, and this ~/.wezterm.lua
local wezterm = require 'wezterm'
local config = wezterm.config_builder()
config.enable_kitty_keyboard = true
return config
when I press shift+# (which is single quote)
WezTerm sends the CSI u encoding shift-'.
Because of this, we completely disable kitty progressive enhancements and
modifyOtherKeys on WezTerm.
It makes no sense for every single app to work around WezTerm violating the
protocol. All these workarounds just create unnecessary version dependencies.
Also our workaround is brittle; it breaks as soon as you're inside something
like SSH.
Least importantly, the workarond prevents users of English keyboard layouts
to easily use the new features.
Since it seems so easy to work around by settting "enable_kitty_keyboard = false",
and most importantly, since that's the default, it seems better to remove
the workaround to simplify the world.
See #10663
