Previously, this message told the user to "set $BROWSER and try again". However,
when I first saw this error, I didn't know how I can set `BROWSER` in fish. Moreover,
I often see this error in situations when no browser will work. For instance, I might be
using fish over ssh, and I might either not know whether that system has a text-mode
browser installed or not want to use it.
A further improvement would be to report this message if a browser fails to start.
This concerns the behavior when running an external command from a key
binding. The history is:
Prior to 5f16a299a7, fish would run these external commands in shell
modes. This meant that fish would pick up any tty changes from external
commands (see #2114).
After 5f16a299a7, fish would save and restore its shell modes around
these external commands. This introduced a regression where anything the
user typed while a bound external command was executing would be echoed,
because external command mode has ECHO set in c_lflag. (This can be
reproed easily with `bind -q 'sleep 1'` and then pressing q and typing).
So 5f16a299a7 was reverted in fd9355966.
This commit partially reverts fd9355966. It has it both ways: external
commands are launched with shell modes, but/and shell modes are restored
after the external command completes. This allows commands to muck with
the tty, as long as they can handle getting shell modes; but it does not
enable ECHO mode so it fixes the regression found in #7770.
Fixes#7770. Fixes#2114 (for the third time!)
This partially reverts commit fd9355966e.
Unfortunately this causes input coming in while bind functions are
running to show up on screen.
Since the cure is worse than the disease let's just stop doing it.
My guess is this needs to *only* be done while running an external
command.
Fixes#7770
Reintroduces #2114
Partially reverts 5f16a299a7
This is broken on OpenBSD because it apparently doesn't have a /proc
we can query, so it just gives "fish".
Since it's unnecessary in this context just skip it.
This actually *worked* in my tests which confuses me.
It really shouldn't, `apropos -foo` will complain about "-o" not being
a valid option.
It should be `apropos -- -foo`.
Now, of course there are awful apropos implementations, so let's see
if someone complains
When executing “make test -jX” (with X > 1) to build and run tests in a
build directory, there is a race condition between the
serial_test_low_level target and the test_prep target (a dependency of
serial_test_fishscript and serial_test_interactive).
As far as I can tell, these events happen in a serial build scenario
(“make test” with the “Unix Makefiles” CMake generator):
1. The fish_tests binary is built and executed.
2. The test_prep target (a dependency of serial_test_fishscript)
cleans up test directories.
3. Tests in test.fish are executed.
In a parallel build scenario, this often happens:
1. Build of the fish_tests binary is started.
2. The test_prep target cleans up test directories.
3. Build of the fish_tests binary is finished.
4. Execution of the fish_tests binary starts.
5. Execution of the fish_tests binary finishes.
6. Tests in test.fish are executed.
However, if building the fish_tests binary is fast enough but not
instant (e.g. when using ccache), this can happen:
1. Build of the fish_tests binary is started.
2. Build of the fish_tests binary is finished.
3. Execution of the fish_tests binary starts.
4. The test_prep target cleans up test directories.
5. fish_tests tests that depend on said test directories may,
depending on timing, fail because they are wiped by test_prep.
Fix this by making test_prep a dependency of serial_test_low_level so
that test_prep can’t interfere with fish_tests execution.
For reasons unclear to me, fish enables bold mode unconditionally if
the background is set.
However, this called a background "set" if it wasn't exactly the
"normal" color, whereas set_color --print-colors would set a color
of *none*.
We have three special non-color colors:
- "normal"
- "reset"
- "none"
All of these specify some form of absence of background color, so all
of them should be checked.
Fixes#7805
* Rewrite the real file if history file is a symlink
When the history file is a symbolic link, `fish` used to overwrite
the link with a real file whenever it saved history. This makes
it follow the symlink and overwrite the real file instead.
The same issue was fixed for the `fish_variables` file in 622f2868e
from https://github.com/fish-shell/fish-shell/pull/7728.
This makes `fish_history` behave in the same way. The implementation
is nearly identical.
Since the tests for the two issues are so similar, I combined them
together and slightly expanded the older test.
This also addresses https://github.com/fish-shell/fish-shell/issues/7553.
* Add user-facing error when history renaming fails
Currently, when history file renaming fails, no message is shown to the
user. This happens, for instance, if the history file is a symlink
pointing to another filesystem.
This copies code (with a bit of variation, after reviewer comments) from
589eb34571/src/env_universal_common.cpp (L486-L491)
into `history.cpp`, so that a message is shown to the user.
* fixup! Rewrite the real file if history file is a symlink
Before now, we would be getting the terminal modes before config.fish,
then running config.fish without any of the term "stealing" and modes
copying. This meant that changes made to the terminal modes in there
were simply lost.
So, what we do is simply set the modes before config and then copy
them after, once.
Note that this does *not* turn off flow control again - if you turn it
on in config.fish that should work.
Fixes#7783.
f7e2e7d26b forbid any job exit events
from happening inside jobs that were themselves event handlers, but
that causes e.g.
```fish
function f --on-event fish_prompt
source (echo "echo hello world" | psub)
end
```
to not trigger psub's cleanup, so it leaves files in $TMPDIR behind.
This was hit by pyenv, because that still uses `source (thing |
psub)`.
Fixes#7792.
When `fish` is running in the Chrome OS Linux VM (Crostini),
both `help` and `fish_config` opened a "file not found"
page. That is because on Crostini, `BROWSER` is usually set to
`garcon-url-handler`, which opens URLs in the host OS Chrome
browser. That browser lacks access to the Linux file system.
This commit fixes these commands. `help` now opens the URL on
www.fishshell.com. `fish_config` now opens the URL for the
server it starts. Previously, it opened a local file that
redirects to the same URL.
In the case of `help`, the situation could be improved further
by starting a web server to serve help. I don't know of another
way to access `/share/fish` from outside the VM without user
intervention, and I think that might be a part of the security
model for the Crostini VM.
It's hard to write a test for this. I checked that `help math`,
`python2 webconfig.py`, and `python3 webconfig.py` work on my
machine running in Crostini.
This reverts commit e240d81ff8 and
introduces a more compatible method of finding newly added fish scripts
to syntax check.
`find -newer` is the original and is supported by everything under the
sun (including FreeBSD, NetBSD, Solaris, OpenIndiana, macOS 10.10, WSL,
and more), and if not, the tests will succeed anyway. `find -mnewer` was
added later around the time `find -cnewer` and co (which checks the
creation date rather than the modification date) was introduced, but
apparently the GNU version of coreutils never introduced the `-mnewer`
alias for `-newer`.
Yes, this is hacky and yes it would be ideal if the build system is the
one that picked which tests to run rather than the test itself picking.
But let's not pretend that our tests are idealogically ideal or pure
right now and until we fix the mess that is our CMake test integration
(e.g. use ctest and configure each test to be run separately with
configurable payloads, etc) eight seconds is still eight seconds, and
again, the CI isn't affected.
My find (GNU findutils 4.8.0) prints
> find: unknown predicate `-mnewer'
So we would have to test for support.
Also this is *super* hacky - tests aren't supposed to keep files
around, this is something you would do in the build system.
This reverts commit ddd0e28b4f.
This seems like a good idea, but there isn't anything we or anyone
else can *do* in this case. All we ever do is pile on additional
errors on the ignore pile, we can't handle any of them differently.
The command isn't a thing, so we check the next path.
The impetus for this is Cygwin apparently returning a wonderfully
useless 0, and it's not even the first one to do so.
Fixes#7785
Only check fish files that have been modified since the last time they
were checked. (This continues with the assumption that we are testing
for broken /usr/share fish scripts and not breakage of the fish parser,
which is covered by all the other tests.)
This saves 8 seconds on an NVMe disk under WSL. Won't affect integrity
of CI runs, which start with a blank slate each time.
While pid values may be reused, it is logical to assume that fish event
handlers coded against a particular job or process id mean just the job
that is currently referred to be any given pid/pgrp rather than in
perpetuity.
This trims the list of registered event handlers nice and early, and as
a bonus avoids the issue described in #7721.
The cleanup song-and-dance is extremely ugly due to the repeated locking
and unlocking of the event handler list.
Closes#7221.
This was a handler for various prompt variables that called a repaint.
Unfortunately, if you set one of those *inside* the prompt (a logical
place for it), this would lead to something like #7775.
So, because this isn't actually *useful* as far as I can see (how do
you set these variables in a way that you're not already inside a
prompt or about to draw a prompt? in a key binding?), we remove it,
like we removed the repaint from git's variable handlers.
Apparently the grep on FreeBSD doesn't do \s or \t. Since we're
looking for an actual tab, just give it an actual tab.
See https://builds.sr.ht/~faho/job/448496.
This `set -e` had a cartesian product that caused it to remove the
indexes separately, so the later indexes were off - removing the first
and then the second ends up removing the first and then the
old-*third* which is now the second.
Just quote the expansion so it runs in one go.
Fixes#7776
Because we removed repaint coalescing, currently setting any of the
git prompt variables in fish_prompt leads to a repaint loop (that
presumably aborts once it reaches the recursion limit).
Since repainting on these variables isn't really useful (when you
`set` them interactively you already get a new prompt), just remove
it.
There's two cases this "breaks":
- When you set a variable *after* the call to fish_git_prompt
- When you set a variable via a binding
In both of these it's not too much to expect an explicit "commandline
-f repaint", especially since for bindings that's already needed in
most cases, and setting a variable after using it isn't normal.
Fixes#7775.
It doesn't work on WSL, Solaris and Archlinux (and presumably that
means future versions of other linux distros).
In its current state I don't trust it enough to enable it anywhere by
default, especially since I'm not aware of an actual issue with the
named pipe (other than that the code is ugly).
Fixes#7774
