The previous approach of "treat this field as an `Option<NonZeroU32>` and
remember to check `p.has_pid()` before accessing it" was a mix of C++ and rust
conventions and led to some bugs or incorrect behaviors.
* `jobs -p` would previously print both the (correct) external pid and the
(incorrect) internal value of `0` if a backgrounded command contained a
fish function (e.g. `function foo; end; cat | foo &; jobs`)
* Updating/calculating job cpu time and usage was incorrectly including all of
fish's cpu usage/time for each function/builtin member of the job pipeline.
Closes#10832
ctrl-r ctrl-s ctrl-s
Attemps to go before the beginning and asserts out. Instead refuse to
do that.
(there's some weirdness where it can reduce the pager to the first
entry if you keep pressing, which I haven't found yet, but that's better than *crashing*)
CMake Warning (dev) at cmake/Tests.cmake:56 (add_custom_command):
Exactly one of PRE_BUILD, PRE_LINK, or POST_BUILD must be given. Assuming
POST_BUILD to preserve backward compatibility.
Policy CMP0175 is not set: add_custom_command() rejects invalid arguments.
Run "cmake --help-policy CMP0175" for policy details. Use the cmake_policy
command to set the policy and suppress this warning.
So we just keep it the same.
These are another way to spell the same thing that doesn't match what
`bind` would print.
They're also not documented and tested thoroughly.
Since they are just small shortcuts and unreleased we can just remove
them.
Fixes#10845
This has the side effect of changing the order of completions for a bare `git
diff` to show modified files before revisions; previously they came at the very
end after all revisions, stashes, local branches, remote branches, and tags.
That seems sensible to me?
As I understand the completions file, it seems to me that the intention was for
`git diff src/` to only show modified files to begin with it
previously/currently shows them all, so we might want to add a `-n 'not ...'`
condition for `git diff` to prevent that.
fish by default shows a git-aware prompt. Recall that on macOS, there are
two hazards we must avoid:
1. The command `/usr/bin/git` is installed by default. This command is not
actually git; instead it's a stub which pops open a dialog proposing to
install Xcode command line tools. Not a good experience.
2. Even after installing these tools, the first run of any `git` or other
command may be quite slow, because it's now a stub which invokes `xcrun`
which needs to populate a cache on a fresh boot. Another bad experience.
We previously attempted to fix this by having `xcrun` print out its cache
path and check if there's a file there. This worked because `xcrun` only
lazily created that file. However, this no longer works: `xcrun` now
eagerly creates the file, and only lazily populates it. Thus we think git
is ready, when it is not.
(This can be reproduced by running `xcrun --kill-cache` and then running
the default fish shell prompt - it will be slow).
Change the fix in the following way: using sh, run `/usr/bin/git --version;
touch /tmp/__fish_git_ready` in the background. Then detect the presence of
/tmp/__fish_git_ready as a mark that git is ready.
Fixes#10535
Given "printf %18446744073709551616s", we parse the number only in
the printf crate, which tells us that we overflowed somwhere (but
not where exactly).
We were previously printing the internal `INVALID_PID` value (since removed),
which was a meaningless `-2` constant, when there was no pgid associated with a
job.
This PR changes that to `-` to indicate no pgid available, which I prefer over
something like `0` or `-1`, but will cause problems for code that is hardcoded
to convert this field to an integral value.
Just like we already fix terminal modes if a command left them broken,
having an invisible cursor makes the terminal hard to use and so we
fix it.
We can't really use cnorm/cursor_normal because that often includes
other gunk like making the cursor blink, but it turns out every
terminfo entry agrees on the sequence to make the cursor visible, so
we hardcode it.
Fixes#10834
This is nicer when you use fish over ssh, and that system does not
have a browser. But the system where your terminal is has one, and so
now you can just click the link.
If we end up using this in more places, we can create a `Pid` newtype.
Note that while the constant is no longer used in code, its previous value of -2
is still printed by `jobs` when no pgid is associated with a job. I will open a
PR to change this to something else, likely either `0` or `-`.
If we try to memory map the history file, and we get back ENODEV meaning that
the underlying device does not support memory mapping, then treat that as a hint
that the filesystem is remote and disable history locking.
As of 04c913427 (Limit command line rendering to $LINES lines,
2024-10-25), we only render a part of the command line. This removes
valuable information from scrollback.
The reasons for the limit were
1. to enable redrawing the commandline (can't do that if part of it
is off-screen).
2. if the cursor is at the beginning of the command-line, we can't
really render the off-screen suffix (unless we can tell the terminal
to scroll back after doing that).
Fortunately these don't matter for the very last rendering of a
command line. Let's render the entire command just before executing,
fixing the scrollback for executed commands.
In future, we should fix it also for pre-execution renderings. This
needs a terminal command to clear part of the scrollback. Can't find
anything on https://invisible-island.net/xterm/ctlseqs/ctlseqs.html
There is "Erase Saved Lines" but that deletes the entire scrollback.
See the discussion in #10827
Since f89909ae3 (Also handle overflown screens if editing pager search
field, 2024-10-27), cursor_arr is never None after the loop.
Assert that by unwrapping.
qa.sh
alt-e restores the cursor position received from the editor, moving by
one character at a time. This can be super slow on large commandlines,
even on release builds. Let's fix that by setting the coordinates
directly.
This happens when using alt-e to edit the command buffer,
adding some lines, leaving the cursor at the end
and quitting the editor without saving.
Let's avoid the noisy error that has sort of bad rendering (would
need __fish_echo).
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.
