12 Commits

Author SHA1 Message Date
Johannes Altmanninger
190d367bc4 Use globals for color variables, react to light/dark mode
Implicitly-universal variables have some downsides:
- It's surprising that "set fish_color_normal ..."
  and "set fish_key_bindings fish_vi_key_bindings" propagate to other
  shells and persist, especially since all other variables (and other
  shells) would use the global scope.
- they don't play well with tracking configuration in Git.
- we don't know how to roll out updates to the default theme (which is
  problematic since can look bad depending on terminal background
  color scheme).

It's sort of possible to use only globals and unset universal variables
(because fish only sets them at first startup), but that requires
knowledge of fish internals; I don't think many people do that.

So:
- Set all color variables that are not already set as globals.
  - To enable this do the following, once, after upgrading:
    copy any existing universal color variables to globals, and:
    - if existing universal color variables exactly match
      the previous default theme, and pretend they didn't exist.
    - else migrate the universals to ~/.config/fish/conf.d/fish_frozen_theme.fish,
      which is a less surprising way of persisting this.
    - either way, delete all universals to do the right thing for most users.
- Make sure that webconfig's "Set Theme" continues to:
  - instantly update all running shells
    - This is achieved by a new universal variable (but only for
      notifying shells, so this doesn't actually need to be persisted).
      In future, we could use any other IPC mechanism such as "kill -SIGUSR1"
      or if we go for a new feature, "varsave" or "set --broadcast", see
      https://github.com/fish-shell/fish-shell/issues/7317#issuecomment-701165897
      https://github.com/fish-shell/fish-shell/pull/8455#discussion_r757837137.
  - persist the theme updates, completely overriding any previous theme.
    Use the same "fish_frozen_theme.fish" snippet as for migration (see above).
    It's not meant to be edited directly. If people want flexibility
    the should delete it.
    It could be a universal variable instead of a conf snippet file;
    but I figured that the separate file looks nicer
    (we can have better comments etc.)
- Ask the terminal whether it's using dark or light mode, and use an
  optimized default. Add dark/light variants to themes,
  and the "unknown" variant for the default theme.
  Other themes don't need the "unknown" variant;
  webconfig already has a background color in context,
  and CLI can require the user to specify variant explicitly if
  terminal doesn't advertise colors.
- Every variable that is set as part of fish's default behavior
  gets a "--label=default" tacked onto it.

  This is to allow our fish_terminal_color_theme event handler to
  know which variables it is allowed to update. It's also necessary
  until we revert 7e3fac561d (Query terminal only just before reading
  from it, 2025-09-25) because since commit, we need to wait until
  the first reader_push() to get query results.  By this time, the
  user's config.fish may already have set variables.

  If the user sets variables via either webconfig, "fish_config theme
  {choose,save}", or directly via "set fish_color_...", they'd almost
  always remove this label.
- For consistency, make default fish_key_bindings global
  (note that, for better or worse, fish_add_path still remains as
  one place that implicitly sets universal variables, but it's not
  something we inject by default)
- Have "fish_config theme choose" and webconfig equivalents reset
  all color variables. This makes much more sense than keeping a
  hardcoded subset of "known colors"; and now that we don't really
  expect to be deleting universals this way, it's actually possible
  to make this change without much fear.

Should have split this into two commits (the changelog entries are
intertwined though).

Closes #11580
Closes #11435
Closes #7317
Ref: https://github.com/fish-shell/fish-shell/issues/12096#issuecomment-3632065704
2025-12-14 17:03:03 +01:00
ridiculousfish
b4fc5160ba Set FISH_TEST_NO_CURSOR_POSITION_QUERY in pexpect tests
This was causing query timeouts with high parallelism in the tests.
2025-11-08 18:28:41 -08:00
Johannes Altmanninger
a772470b76 Multi-line autosuggestions
Unlike other shells, fish tries to make it easy to work with multiline
commands. Arguably, it's often better to use a full text editor but
the shell can feel more convenient.

Spreading long commands into multiple lines can improve readability,
especially when there is some semantic grouping (loops, pipelines,
command substitutions, quoted parts). Note that in Unix shell, every
quoted string can span multiple lines, like Python's triple quotes,
so the barrier to writing a multiline command is quite low.

However these commands are not autosuggested. From
1c4e5cadf2 (commitcomment-150853293)

> the reason we don't offer multi-line autosuggestion is that they
> can cause the command line to "jump" to make room for the second
> and third lines, if you're at the bottom of your terminal.

This jumping (as done by nushell for example) might be surprising,
especially since there is no limit on the height of a command.

Let's maybe avoid this jumping by rendering only however many lines
from the autosuggestion can fit on the screen without scrolling.

The truncation is hinted at by a single ellipsis ("…") after the
last suggested character, just like when a single-line autosuggestion
is truncated. (We might want to use something else in future.)

To implement this, query for the cursor position after every command,
so we know the y-position of the shell prompt within the terminal
window (whose height we already know).

Also, after we register a terminal window resize, query for the cursor
position before doing anything else (until we od #12004, only height
changes are relevant), to prevent this scenario:

	1. move prompt to bottom of terminal
	2. reduce terminal height
	3. increase terminal height
	4. type a command that triggers a multi-line autosuggestion
	5. observe that it would fail to truncate properly

As a refresher: when we fail to receive a query response, we always
wait for 2 seconds, except if the initial query had also failed,
see b907bc775a (Use a low TTY query timeout only if first query
failed, 2025-09-25).

If the terminal does not support cursor position report (which is
unlikely), show at most 1 line worth of autosuggestion.  Note that
either way, we don't skip multiline commands anymore.  This might make
the behavior worse on such terminals, which are probably not important
enough.  Alternatively, we could use no limit for such terminals,
that's probably the better fallback behavior. The only reason I didn't
do that yet is to stay a little bit closer to historical behavior.

Storing the prompt's position simplifies scrollback-push and the mouse
click handler, which no longer need to query.  Move some associated
code to the screen module.

Technically we don't need to query for cursor position if the previous
command was empty. But for now we do, trading a potential optimization
for andother simplification.

Disable this feature in pexpect tests for now, since those are still
missing some terminal emulation features.
2025-11-08 21:18:25 +01:00
Johannes Altmanninger
b964072c11 Move scrollback-push feature detection to fish script
A lot of terminals support CSI Ps S.  Currently we only allow them
to use scrollback-up if they advertise it via XTGETTCAP.  This seems
surprising; it's better to make visible in fish script  whether this
is supposed to be working.  The canonical place is in "bind ctrl-l"
output.

The downside here is that we need to expose something that's rarely
useful. But the namespace pollution is not so bad, and this gives
users a nice paper trail instead of having to look in the source code.
2025-09-27 14:22:18 +02:00
Johannes Altmanninger
e9ae143bab Fix cursor position reports being ignored
When we receive a cursor position report, we only store the result;
we'll act on it only when we receive the primary DA reply.  Make sure
we don't discard the query state until then.

Fixes 06ede39ec9 (Degrade gracefully when failing to receive cursor
position report, 2025-09-23)
2025-09-26 12:10:04 +02:00
Johannes Altmanninger
ef7bd4f13a Filter all control sequences but colors and cursor movement in tests
There are more escape sequences that can affect terminal state; let's allow
only the ones that are known-safe.
2025-04-11 15:11:22 +02:00
Peter Ammon
f2dde229aa Revert changes to restore cursor position after undo
This feature is nice and desirable, but it was implemented in a intrusive way
by modifying the sequence of bytes we emit when running a command; this in
turn requires changing a bunch of tests.

This sequence hasn't changed in decades and the consequences of changing it
are hard to predict, given that it is likely terminal dependent; we've
already found a regression.

It's fine to reintroduce this but it should be done in a less intrusive way
(conceptually that seems straightforward - we're just remembering the cursor
position).

Revert "Fix spurious blank lines when executing scrolled commandline"

This reverts commit 0e512f8033.

Revert "On undo after execute, restore the cursor position "

This reverts commit 610338cc70.
2025-03-05 17:57:48 -08:00
Łukasz Wieczorek
82750bbaec Reformat with black. 2025-02-12 22:59:27 +08:00
Johannes Altmanninger
64859fc242 Blocking wait for responses to startup queries
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
2025-01-26 14:22:52 +01:00
Johannes Altmanninger
06b49b9721 Update pexpect helper output sanitization 2025-01-26 14:19:40 +01:00
Fabian Boehm
e66f6878b5 Make tests usable with path with spaces
This is somewhat subtle:

The #RUN line in a littlecheck file will be run by a posix shell,
which means the substitutions will also be mangled by it.

Now, we *have* shell-quoted them, but unfortunately what we need is to
quote them for inside a pre-existing layer of quotes, e.g.

    # RUN: fish -C 'set -g fish %fish'

here, %fish can't be replaced with `'path with spaces/fish'`, because
that ends up as

    # RUN: fish -C 'set -g fish 'path with spaces/fish''

which is just broken.

So instead, we pass it as a variable to that fish:

    # RUN: fish=%fish fish...

In addition, we need to not mangle the arguments in our test_driver.

For that, because we insist on posix shell, which has only one array,
and we source a file, we *need* to stop having that file use
arguments.

Which is okay - test_env.sh could previously be used to start a test,
and now it no longer can because that is test_*driver*.sh's job.

For the interactive tests, it's slightly different:

pexpect.spawn(foo) is sensitive to shell metacharacters like space.

So we shell-quote it.

But if you pass any args to pexpect.spawn, it no longer uses a shell,
and so we cannot shell-quote it.

There could be a better way to fix this?
2025-01-01 16:45:43 +01:00
Fabian Boehm
1df8de06c1 Move littlecheck/pexpect to tests
This removes the need for a bunch of setup, and makes it easier to
make the tests agnostic to our test root setup.
2025-01-01 16:45:43 +01:00